8147462: URI.toURL could be more efficient for most non-opaque URIs
authorredestad
Fri, 29 Jan 2016 11:35:56 +0100
changeset 35393 12d55e1947f7
parent 35392 725d738b0ead
child 35394 282c3cb6a0c1
8147462: URI.toURL could be more efficient for most non-opaque URIs Reviewed-by: alanb, chegar
jdk/src/java.base/share/classes/java/net/URI.java
jdk/src/java.base/share/classes/java/net/URL.java
jdk/test/java/net/URI/URItoURLTest.java
--- a/jdk/src/java.base/share/classes/java/net/URI.java	Thu Jan 28 09:49:00 2016 -0800
+++ b/jdk/src/java.base/share/classes/java/net/URI.java	Fri Jan 29 11:35:56 2016 +0100
@@ -1080,11 +1080,8 @@
      *          If a protocol handler for the URL could not be found,
      *          or if some other error occurred while constructing the URL
      */
-    public URL toURL()
-        throws MalformedURLException {
-        if (!isAbsolute())
-            throw new IllegalArgumentException("URI is not absolute");
-        return new URL(toString());
+    public URL toURL() throws MalformedURLException {
+        return URL.fromURI(this);
     }
 
     // -- Component access methods --
--- a/jdk/src/java.base/share/classes/java/net/URL.java	Thu Jan 28 09:49:00 2016 -0800
+++ b/jdk/src/java.base/share/classes/java/net/URL.java	Fri Jan 29 11:35:56 2016 +0100
@@ -659,6 +659,42 @@
         }
     }
 
+    /**
+     * Creates a URL from a URI, as if by invoking {@code uri.toURL()}.
+     *
+     * @see java.net.URI#toURL()
+     */
+    static URL fromURI(URI uri) throws MalformedURLException {
+        if (!uri.isAbsolute()) {
+            throw new IllegalArgumentException("URI is not absolute");
+        }
+        String protocol = uri.getScheme();
+        if (!uri.isOpaque() && uri.getRawFragment() == null &&
+                !isOverrideable(protocol)) {
+            // non-opaque URIs will have already validated the components,
+            // so using the component-based URL constructor here is safe.
+            //
+            // All URL constructors will properly check if the scheme
+            // maps to a valid protocol handler
+
+            String query = uri.getRawQuery();
+            String path = uri.getRawPath();
+            String file = (query == null) ? path : path + "?" + query;
+
+            // URL represent undefined host as empty string while URI use null
+            String host = uri.getHost();
+            if (host == null) {
+                host = "";
+            }
+
+            int port = uri.getPort();
+
+            return new URL(protocol, host, port, file, null);
+        } else {
+            return new URL((URL)null, uri.toString(), null);
+        }
+    }
+
     /*
      * Returns true if specified string is a valid protocol name.
      */
@@ -1275,11 +1311,28 @@
         }
     }
 
-    private static final String[] NON_OVERRIDEABLE_PROTOCOLS = {"file", "jrt"};
-    private static boolean isOverrideable(String protocol) {
-        for (String p : NON_OVERRIDEABLE_PROTOCOLS)
-            if (protocol.equalsIgnoreCase(p))
+
+    /**
+     * Non-overrideable protocols: "jrt" and "file"
+     *
+     * Character-based comparison for performance reasons; also ensures
+     * case-insensitive comparison in a locale-independent fashion.
+     */
+    static boolean isOverrideable(String protocol) {
+        if (protocol.length() == 3) {
+            if ((Character.toLowerCase(protocol.charAt(0)) == 'j') &&
+                    (Character.toLowerCase(protocol.charAt(1)) == 'r') &&
+                    (Character.toLowerCase(protocol.charAt(2)) == 't')) {
                 return false;
+            }
+        } else if (protocol.length() == 4) {
+            if ((Character.toLowerCase(protocol.charAt(0)) == 'f') &&
+                    (Character.toLowerCase(protocol.charAt(1)) == 'i') &&
+                    (Character.toLowerCase(protocol.charAt(2)) == 'l') &&
+                    (Character.toLowerCase(protocol.charAt(3)) == 'e')) {
+                return false;
+            }
+        }
         return true;
     }
 
--- a/jdk/test/java/net/URI/URItoURLTest.java	Thu Jan 28 09:49:00 2016 -0800
+++ b/jdk/test/java/net/URI/URItoURLTest.java	Fri Jan 29 11:35:56 2016 +0100
@@ -23,13 +23,16 @@
 
 /**
  * @test
- * @bug  4768755 4677045
- * @summary URL.equal(URL) is inconsistant for opaque URI.toURL()
- *                      and new URL(URI.toString)
+ * @bug  4768755 4677045 8147462
+ * @summary URL.equal(URL) is inconsistent for opaque URI.toURL()
+ *              and new URL(URI.toString)
  *          URI.toURL() does not always work as specified
+ *          Ensure URIs representing invalid/malformed URLs throw similar
+ *              exception with new URL(URI.toString()) and URI.toURL()
  */
 
 import java.net.*;
+import java.util.Objects;
 
 public class URItoURLTest {
 
@@ -39,19 +42,43 @@
         URL classUrl = testClass.getClass().
                                     getResource("/java/lang/Object.class");
 
-        String[] uris = { "mailto:xyz@abc.de",
+        String[] uris = {
+                        "mailto:xyz@abc.de",
                         "file:xyz#ab",
                         "http:abc/xyz/pqr",
+                        "http:abc/xyz/pqr?id=x%0a&ca=true",
                         "file:/C:/v700/dev/unitTesting/tests/apiUtil/uri",
                         "http:///p",
+                        "file:/C:/v700/dev/unitTesting/tests/apiUtil/uri",
+                        "file:/C:/v700/dev%20src/unitTesting/tests/apiUtil/uri",
+                        "file:/C:/v700/dev%20src/./unitTesting/./tests/apiUtil/uri",
+                        "http://localhost:80/abc/./xyz/../pqr?id=x%0a&ca=true",
+                        "file:./test/./x",
+                        "file:./././%20#i=3",
+                        "file:?hmm",
+                        "file:.#hmm",
                         classUrl.toExternalForm(),
                         };
 
+        // Strings that represent valid URIs but invalid URLs that should throw
+        // MalformedURLException both when calling toURL and new URL(String)
+        String[] malformedUrls = {
+                        "test:/test",
+                        "fiel:test",
+                        };
+
+        // Non-absolute URIs should throw IAE when calling toURL but will throw
+        // MalformedURLException when calling new URL
+        String[] illegalUris = {
+                        "./test",
+                        "/test",
+                        };
+
         boolean isTestFailed = false;
         boolean isURLFailed = false;
 
-        for (int i = 0; i < uris.length; i++) {
-            URI uri = URI.create(uris[i]);
+        for (String uriString : uris) {
+            URI uri = URI.create(uriString);
 
             URL url1 = new URL(uri.toString());
             URL url2 = uri.toURL();
@@ -107,6 +134,42 @@
             System.out.println();
             isURLFailed = false;
         }
+        for (String malformedUrl : malformedUrls) {
+            Exception toURLEx = null;
+            Exception newURLEx = null;
+            try {
+                new URI(malformedUrl).toURL();
+            } catch (Exception e) {
+                // expected
+                toURLEx = e;
+            }
+            try {
+                new URL(new URI(malformedUrl).toString());
+            } catch (Exception e) {
+                // expected
+                newURLEx = e;
+            }
+            if (!(toURLEx instanceof MalformedURLException) ||
+                    !(newURLEx instanceof MalformedURLException) ||
+                    !toURLEx.getMessage().equals(newURLEx.getMessage())) {
+                isTestFailed = true;
+                System.out.println("Expected the same MalformedURLException: " +
+                    newURLEx + " vs " + toURLEx);
+            }
+        }
+        for (String illegalUri : illegalUris) {
+             try {
+                 new URI(illegalUri).toURL();
+             } catch (IllegalArgumentException e) {
+                 // pass
+             }
+
+             try {
+                 new URL(illegalUri);
+             } catch (MalformedURLException e) {
+                 // pass
+             }
+        }
         if (isTestFailed) {
             throw new Exception("URI.toURL() test failed");
         }