Fix URI to behave more like the spec and browsers and less like the RI.

When computing relative paths from URLs like foo.com/a to foo.com/a/b,
this now uses "a/b" as the relative path and not just "b". That was an
RI bug that we were foolishly compatible with.

Change-Id: I6e3f10d84602c08bbf6bd7c7e43c2ba3387e4c3b
http://b/2753295

4 files changed, 85 insertions, 65 deletions

diff --git a/luni/src/main/java/java/net/URI.java b/luni/src/main/java/java/net/URI.java
index d1e50f2..f260b2e 100644
--- a/luni/src/main/java/java/net/URI.java
+++ b/luni/src/main/java/java/net/URI.java
@@ -1131,11 +1131,11 @@ public final class URI implements Comparable<URI>, Serializable {
         return opaque;
     }
 
-    /*
-     * normalize path, and return the resulting string
+    /**
+     * Returns the normalized path.
      */
-    private String normalize(String path) {
-        path = UrlUtils.canonicalizePath(path, false);
+    private String normalize(String path, boolean discardRelativePrefix) {
+        path = UrlUtils.canonicalizePath(path, discardRelativePrefix);
 
         /*
          * If the path contains a colon before the first colon, prepend
@@ -1162,7 +1162,7 @@ public final class URI implements Comparable<URI>, Serializable {
         if (opaque) {
             return this;
         }
-        String normalizedPath = normalize(path);
+        String normalizedPath = normalize(path, false);
         // if the path is already normalized, return this
         if (path.equals(normalizedPath)) {
             return this;
@@ -1216,18 +1216,17 @@ public final class URI implements Comparable<URI>, Serializable {
         }
 
         // normalize both paths
-        String thisPath = normalize(path);
-        String relativePath = normalize(relative.path);
+        String thisPath = normalize(path, false);
+        String relativePath = normalize(relative.path, false);
 
         /*
          * if the paths aren't equal, then we need to determine if this URI's
          * path is a parent path (begins with) the relative URI's path
          */
         if (!thisPath.equals(relativePath)) {
-            // if this URI's path doesn't end in a '/', add one
-            if (!thisPath.endsWith("/")) {
-                thisPath = thisPath + '/';
-            }
+            // drop everything after the last slash in this path
+            thisPath = thisPath.substring(0, thisPath.lastIndexOf('/') + 1);
+
             /*
              * if the relative URI's path doesn't start with this URI's path,
              * then just return the relative URI; the URIs have nothing in
@@ -1260,46 +1259,39 @@ public final class URI implements Comparable<URI>, Serializable {
             return relative;
         }
 
-        URI result;
-        if (relative.path.isEmpty() && relative.scheme == null
-                && relative.authority == null && relative.query == null
-                && relative.fragment != null) {
-            // if the relative URI only consists of fragment,
-            // the resolved URI is very similar to this URI,
-            // except that it has the fragment from the relative URI.
-            result = duplicate();
+        if (relative.authority != null) {
+            // If the relative URI has an authority, the result is the relative
+            // with this URI's scheme.
+            URI result = relative.duplicate();
+            result.scheme = scheme;
+            result.absolute = absolute;
+            return result;
+        }
+
+        if (relative.path.isEmpty() && relative.scheme == null && relative.query == null) {
+            // if the relative URI only consists of at most a fragment,
+            URI result = duplicate();
             result.fragment = relative.fragment;
-            // no need to re-calculate the scheme specific part,
-            // since fragment is not part of scheme specific part.
             return result;
         }
 
-        if (relative.authority != null) {
-            // if the relative URI has authority,
-            // the resolved URI is almost the same as the relative URI,
-            // except that it has the scheme of this URI.
-            result = relative.duplicate();
-            result.scheme = scheme;
-            result.absolute = absolute;
+        URI result = duplicate();
+        result.fragment = relative.fragment;
+        result.query = relative.query;
+        String resolvedPath;
+        if (relative.path.startsWith("/")) {
+            // The relative URI has an absolute path; use it.
+            resolvedPath = relative.path;
+        } else if (relative.path.isEmpty()) {
+            // The relative URI has no path; use the base path.
+            resolvedPath = path;
         } else {
-            // since relative URI has no authority,
-            // the resolved URI is very similar to this URI,
-            // except that it has the query and fragment of the relative URI,
-            // and the path is different.
-            result = duplicate();
-            result.fragment = relative.fragment;
-            result.query = relative.query;
-            if (relative.path.startsWith("/")) {
-                result.path = relative.path;
-            } else {
-                // resolve a relative reference
-                int endIndex = path.lastIndexOf('/') + 1;
-                result.path = normalize(path.substring(0, endIndex) + relative.path);
-            }
-            // re-calculate the scheme specific part since
-            // query and path of the resolved URI is different from this URI.
-            result.setSchemeSpecificPart();
+            // The relative URI has a relative path; combine the paths.
+            int endIndex = path.lastIndexOf('/') + 1;
+            resolvedPath = path.substring(0, endIndex) + relative.path;
         }
+        result.path = UrlUtils.authoritySafePath(result.authority, normalize(resolvedPath, true));
+        result.setSchemeSpecificPart();
         return result;
     }
 
diff --git a/luni/src/main/java/java/net/URLStreamHandler.java b/luni/src/main/java/java/net/URLStreamHandler.java
index 8674d12..d5c922e 100644
--- a/luni/src/main/java/java/net/URLStreamHandler.java
+++ b/luni/src/main/java/java/net/URLStreamHandler.java
@@ -180,13 +180,7 @@ public abstract class URLStreamHandler {
             path = "";
         }
 
-        /*
-         * Force the path to start with a '/' if this URL has an authority.
-         * Otherwise they run together like http://android.comindex.html.
-         */
-        if (authority != null && !authority.isEmpty() && !path.startsWith("/") && !path.isEmpty()) {
-            path = "/" + path;
-        }
+        path = UrlUtils.authoritySafePath(authority, path);
 
         setURL(url, url.getProtocol(), host, port, authority, userInfo, path, query, ref);
     }
diff --git a/luni/src/main/java/libcore/net/url/UrlUtils.java b/luni/src/main/java/libcore/net/url/UrlUtils.java
index 59131b8..81704fe 100644
--- a/luni/src/main/java/libcore/net/url/UrlUtils.java
+++ b/luni/src/main/java/libcore/net/url/UrlUtils.java
@@ -82,6 +82,18 @@ public final class UrlUtils {
     }
 
     /**
+     * Returns a path that can be safely concatenated with {@code authority}. If
+     * the authority is null or empty, this can be any path. Otherwise the paths
+     * run together like {@code http://android.comindex.html}.
+     */
+    public static String authoritySafePath(String authority, String path) {
+        if (authority != null && !authority.isEmpty() && !path.isEmpty() && !path.startsWith("/")) {
+            return "/" + path;
+        }
+        return path;
+    }
+
+    /**
      * Returns the scheme prefix like "http" from the URL spec, or null if the
      * spec doesn't start with a scheme. Scheme prefixes match this pattern:
      * {@code alpha ( alpha | digit | '+' | '-' | '.' )* ':'}
diff --git a/luni/src/test/java/libcore/java/net/URITest.java b/luni/src/test/java/libcore/java/net/URITest.java
index a11e49a..57ec713 100644
--- a/luni/src/test/java/libcore/java/net/URITest.java
+++ b/luni/src/test/java/libcore/java/net/URITest.java
@@ -366,7 +366,7 @@ public final class URITest extends TestCase {
         assertEquals("http://a/b/c/g/", base.resolve("g/").toString());
         assertEquals("http://a/g", base.resolve("/g").toString());
         assertEquals("http://g", base.resolve("//g").toString());
-//        assertEquals("http://a/b/c/d;p?y", base.resolve("?y").toString()); // fails on RI
+        assertEquals("http://a/b/c/d;p?y", base.resolve("?y").toString()); // fails on RI
         assertEquals("http://a/b/c/g?y", base.resolve("g?y").toString());
         assertEquals("http://a/b/c/d;p?q#s", base.resolve("#s").toString());
         assertEquals("http://a/b/c/g#s", base.resolve("g#s").toString());
@@ -374,7 +374,7 @@ public final class URITest extends TestCase {
         assertEquals("http://a/b/c/;x", base.resolve(";x").toString());
         assertEquals("http://a/b/c/g;x", base.resolve("g;x").toString());
         assertEquals("http://a/b/c/g;x?y#s", base.resolve("g;x?y#s").toString());
-//        assertEquals("http://a/b/c/d;p?q", base.resolve("").toString()); RI returns http://a/b/c/
+        assertEquals("http://a/b/c/d;p?q", base.resolve("").toString()); // RI returns http://a/b/c/
         assertEquals("http://a/b/c/", base.resolve(".").toString());
         assertEquals("http://a/b/c/", base.resolve("./").toString());
         assertEquals("http://a/b/", base.resolve("..").toString());
@@ -387,14 +387,14 @@ public final class URITest extends TestCase {
 
     public void testRfc1808AbnormalExampleTooManyDotDotSequences() throws Exception {
         URI base = new URI("http://a/b/c/d;p?q");
-//        assertEquals("http://a/g", base.resolve("../../../g").toString()); // fails on RI
-//        assertEquals("http://a/g", base.resolve("../../../../g").toString()); // fails on RI
+        assertEquals("http://a/g", base.resolve("../../../g").toString()); // fails on RI
+        assertEquals("http://a/g", base.resolve("../../../../g").toString()); // fails on RI
     }
 
     public void testRfc1808AbnormalExampleRemoveDotSegments() throws Exception {
         URI base = new URI("http://a/b/c/d;p?q");
-//        assertEquals("http://a/g", base.resolve("/./g").toString()); // fails on RI
-//        assertEquals("http://a/g", base.resolve("/../g").toString()); // fails on RI
+        assertEquals("http://a/g", base.resolve("/./g").toString()); // fails on RI
+        assertEquals("http://a/g", base.resolve("/../g").toString()); // fails on RI
         assertEquals("http://a/b/c/g.", base.resolve("g.").toString());
         assertEquals("http://a/b/c/.g", base.resolve(".g").toString());
         assertEquals("http://a/b/c/g..", base.resolve("g..").toString());
@@ -413,8 +413,10 @@ public final class URITest extends TestCase {
 
     public void testRfc1808AbnormalExampleRelativeScheme() throws Exception {
         URI base = new URI("http://a/b/c/d;p?q");
-        // this result is permitted; strict parsers prefer "http:g"
-//        assertEquals("http://a/b/c/g", base.resolve("http:g").toString()); // RI yields strict result
+        URI uri = base.resolve("http:g");
+        assertEquals("http:g", uri.toString()); // this is an opaque URI
+        assertEquals(true, uri.isOpaque());
+        assertEquals(true, uri.isAbsolute());
     }
 
     public void testRfc1808AbnormalExampleQueryOrFragmentDots() throws Exception {
@@ -462,14 +464,14 @@ public final class URITest extends TestCase {
     }
 
     public void testFileUrlRelativePath() throws Exception {
-        URI base = new URI("file:a/b/c");
-//        assertEquals("file:a/b/d", base.resolve("d").toString()); // fails on RI
+        URI base = new URI("file:/a/b/c");
+        assertEquals("file:/a/b/d", base.resolve("d").toString());
     }
 
     public void testFileUrlDottedPath() throws Exception {
         URI url = new URI("file:../a/b");
-//        assertEquals("../a/b", url.getPath()); // fails on RI
-        assertEquals("file:../a/b", url.toString());
+        assertTrue(url.isOpaque());
+        assertNull(url.getPath());
     }
 
     /**
@@ -506,7 +508,27 @@ public final class URITest extends TestCase {
     public void testRelativize() throws Exception {
         URI a = new URI("http://host/a/b");
         URI b = new URI("http://host/a/b/c");
-//        assertEquals("b/c", a.relativize(b).toString()); // fails on both the RI and libcore
+        assertEquals("b/c", a.relativize(b).toString()); // fails on both the RI and libcore
+    }
+
+    public void testParseServerAuthorityInvalidAuthority() throws Exception {
+        URI uri = new URI("http://host:-2/");
+        assertEquals("host:-2", uri.getAuthority());
+        assertNull(uri.getHost());
+        assertEquals(-1, uri.getPort());
+        try {
+            uri.parseServerAuthority();
+            fail();
+        } catch (URISyntaxException expected) {
+        }
+    }
+
+    public void testParseServerAuthorityOmittedAuthority() throws Exception {
+        URI uri = new URI("http:file");
+        uri.parseServerAuthority(); // does nothing!
+        assertNull(uri.getAuthority());
+        assertNull(uri.getHost());
+        assertEquals(-1, uri.getPort());
     }
 
     // Adding a new test? Consider adding an equivalent test to URLTest.java