Merge pull request #139 from GateNLP/resolve-url-fix

Various fixes to Utils.resolveURL

Author: greenwoodma

src/main/java/gate/Utils.java

@@ -1411,17 +1411,28 @@ public static AnnotationSet intersect(AnnotationSet origSet, Collection<Annotati
     }
     return new ImmutableAnnotationSetImpl(origSet.getDocument(),tmp);    
   }
-  
-  public static URL resolveURL(String url) throws IOException {
-    while (true) {
-      // while we are still following redirects...
 
-      // create an actual URL instance from the string representation
-      URL resourceUrl = new URL(url);
+  public static URL resolveURL(String url) throws IOException {
+    return resolveURL(new URL(url));
+  }
 
-      if (!resourceUrl.getProtocol().startsWith("http"))
-          return resourceUrl;
+  public static URL resolveURL(URL url) throws IOException {
+    // if it's not http or https then there's no notion of redirection, so
+    // stick to the original URL object
+    if (!url.getProtocol().equalsIgnoreCase("http") &&
+            !url.getProtocol().equalsIgnoreCase("https")) {
+      return url;
+    }
 
+    URL resourceUrl = url;
+    Set<String> seenUrls = new HashSet<>();
+    int followedRedirects = 0;
+    // limit to 20 redirects, that's the most any of the major browsers will follow
+    while (followedRedirects++ < 20) {
+      // check for redirection loop
+      if(!seenUrls.add(resourceUrl.toExternalForm())) {
+        throw new IOException("Redirection loop detected for URL " + url);
+      }
       // open a connection to the URL and...
       HttpURLConnection conn = (HttpURLConnection) resourceUrl.openConnection();
 
@@ -1432,19 +1443,27 @@ public static URL resolveURL(String url) throws IOException {
       conn.setInstanceFollowRedirects(false); // Make the logic below easier to detect redirections
 
       switch (conn.getResponseCode()) {
-      case HttpURLConnection.HTTP_MOVED_PERM:
-      case HttpURLConnection.HTTP_MOVED_TEMP:
-        // if we've hit a redirect then get the location from the header
-        String location = conn.getHeaderField("Location");
-        location = URLDecoder.decode(location, "UTF-8");
-        URL next = new URL(resourceUrl, location); // Deal with relative URLs
-        url = next.toExternalForm();
-        continue;
+        case 301: // moved permanently
+        case 302: // moved temporarily
+        case 303: // "see other"
+        case 307: // "temporary redirect"
+          // if we've hit a redirect then get the location from the header
+          String location = conn.getHeaderField("Location");
+          location = URLDecoder.decode(location, "UTF-8");
+          URL newUrl = new URL(resourceUrl, location); // Deal with relative URLs
+          // follow the redirect if (and only if) it goes to another http or https URL
+          if(newUrl.getProtocol().equalsIgnoreCase("http") ||
+                  newUrl.getProtocol().equalsIgnoreCase("https")) {
+            resourceUrl = newUrl;
+            continue;
+          }
       }
 
       // we've found a URL without a redirect so at this point we can stop
       return resourceUrl;
     }
+    
+    throw new IOException("Too many redirects for " + url);
   }
 
 }

src/main/java/gate/corpora/DocumentImpl.java

@@ -252,7 +252,7 @@ public Resource init() throws ResourceInstantiationException {
       getFeatures().put("gate.SourceURL", "created from String");
     } else {
       try {
-        URL resolved = gate.Utils.resolveURL(sourceUrl.toExternalForm());
+        URL resolved = gate.Utils.resolveURL(sourceUrl);
         getFeatures().put("gate.OriginalURL", sourceUrl.toExternalForm());
         sourceUrl = resolved;
       }

src/test/java/gate/util/TestResolveUrl.java

@@ -0,0 +1,97 @@
+package gate.util;
+
+import gate.Utils;
+import junit.framework.TestCase;
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.impl.bootstrap.HttpServer;
+import org.apache.http.impl.bootstrap.ServerBootstrap;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
+
+import java.io.IOException;
+import java.net.*;
+import java.util.concurrent.TimeUnit;
+
+public class TestResolveUrl extends TestCase {
+
+  static class RedirectingHandler implements HttpRequestHandler {
+
+    InetAddress myAddress;
+    int port;
+
+    @Override
+    public void handle(HttpRequest request, HttpResponse response, HttpContext context) throws HttpException, IOException {
+      response.setStatusCode(303);
+      try {
+        String path = request.getRequestLine().getUri();
+        if(path.matches("/loop/[0-3]")) {
+          // simulates a redirection loop
+          int nextNum = (path.charAt(6) - '0' + 1) % 4;
+          String nextPath = "/loop/" + nextNum;
+          System.err.println("Redirecting to " + nextPath);
+          response.addHeader("Location", new URI("http", null, myAddress.getHostAddress(), port, nextPath, null, null).toString());
+        } else if(path.startsWith("/infinite/")) {
+          // simulates a redirection loop
+          String nextPath = path + "/x";
+          System.err.println("Redirecting " + path + " to " + nextPath);
+          response.addHeader("Location", new URI("http", null, myAddress.getHostAddress(), port, nextPath, null, null).toString());
+        } else if(path.equals("/redirect-to-file")) {
+          response.addHeader("Location", new URI("http", null, myAddress.getHostAddress(), port, "/file", null, null).toString());
+        } else if(path.equals("/file")) {
+          System.err.println("Attempting to redirect to a file: URL");
+          response.addHeader("Location", "file:/etc/hosts");
+        }
+      } catch(URISyntaxException e) {
+        throw new IOException(e);
+      }
+    }
+  }
+
+  HttpServer server;
+  URL baseUrl;
+
+  public void setUp() throws Exception {
+    RedirectingHandler handler = new RedirectingHandler();
+    server = ServerBootstrap.bootstrap()
+            .setLocalAddress(InetAddress.getLoopbackAddress())
+            .registerHandler("*", handler).create();
+    server.start();
+    handler.myAddress = server.getInetAddress();
+    handler.port = server.getLocalPort();
+    baseUrl = new URL("http", server.getInetAddress().getHostAddress(), server.getLocalPort(), "");
+    System.err.println("Started server at " + baseUrl);
+  }
+
+  public void testRedirectLoop() throws Exception {
+    URL url = new URL(baseUrl, "/loop/0");
+    try {
+      URL newUrl  = Utils.resolveURL(url);
+      fail("resolveURL should have failed due to redirect loop");
+    } catch(IOException e) {
+      // exception expected
+    }
+  }
+
+  public void testTooManyRedirects() throws Exception {
+    URL url = new URL(baseUrl, "/infinite/x");
+    try {
+      URL newUrl  = Utils.resolveURL(url);
+      fail("resolveURL should have failed due to too many redirects");
+    } catch(IOException e) {
+      // exception expected
+    }
+  }
+
+  public void testRedirectToFile() throws Exception {
+    URL url = new URL(baseUrl, "/redirect-to-file");
+    URL newUrl  = Utils.resolveURL(url);
+    assertEquals("Redirection should have stopped at http://.../file", new URL(baseUrl, "/file"), newUrl);
+  }
+
+  public void tearDown() throws Exception {
+    server.stop();
+    server.awaitTermination(20, TimeUnit.SECONDS);
+  }
+}