Web Inspector: Use URL constructor to better handle all kinds of URLs
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Aug 2019 07:04:02 +0000 (07:04 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Aug 2019 07:04:02 +0000 (07:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165155

Reviewed by Joseph Pecoraro.

Source/WebInspectorUI:

* UserInterface/Base/URLUtilities.js:
(parseURL):

LayoutTests:

* inspector/unit-tests/url-utilities.html:
* inspector/unit-tests/url-utilities-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248895 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/unit-tests/url-utilities-expected.txt
LayoutTests/inspector/unit-tests/url-utilities.html
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/URLUtilities.js

index 47db242..6b606c9 100644 (file)
@@ -1,3 +1,13 @@
+2019-08-20  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Use URL constructor to better handle all kinds of URLs
+        https://bugs.webkit.org/show_bug.cgi?id=165155
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/unit-tests/url-utilities.html:
+        * inspector/unit-tests/url-utilities-expected.txt:
+
 2019-08-19  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Debugger: add a global breakpoint for pausing in the next microtask
index 0b3fe88..a26681d 100644 (file)
@@ -16,7 +16,7 @@ PASS: userinfo should be: 'null'
 PASS: host should be: 'example.com'
 PASS: port should be: 'null'
 PASS: origin should be: 'http://example.com'
-PASS: path should be: 'null'
+PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
@@ -36,8 +36,19 @@ Test Valid: http://example.com:80/
 PASS: scheme should be: 'http'
 PASS: userinfo should be: 'null'
 PASS: host should be: 'example.com'
-PASS: port should be: '80'
-PASS: origin should be: 'http://example.com:80'
+PASS: port should be: 'null'
+PASS: origin should be: 'http://example.com'
+PASS: path should be: '/'
+PASS: queryString should be: 'null'
+PASS: fragment should be: 'null'
+PASS: lastPathComponent should be: 'null'
+
+Test Valid: http://example.com:42/
+PASS: scheme should be: 'http'
+PASS: userinfo should be: 'null'
+PASS: host should be: 'example.com'
+PASS: port should be: '42'
+PASS: origin should be: 'http://example.com:42'
 PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
@@ -61,7 +72,7 @@ PASS: host should be: 'example.com'
 PASS: port should be: 'null'
 PASS: origin should be: 'http://example.com'
 PASS: path should be: '/path/to/page.html'
-PASS: queryString should be: ''
+PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'page.html'
 
@@ -126,7 +137,7 @@ PASS: userinfo should be: 'null'
 PASS: host should be: 'example.com'
 PASS: port should be: 'null'
 PASS: origin should be: 'http://example.com'
-PASS: path should be: 'null'
+PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'alpha/beta'
 PASS: lastPathComponent should be: 'null'
@@ -148,7 +159,7 @@ PASS: userinfo should be: 'null'
 PASS: host should be: 'example'
 PASS: port should be: 'null'
 PASS: origin should be: 'http://example'
-PASS: path should be: 'null'
+PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
@@ -159,7 +170,7 @@ PASS: userinfo should be: 'null'
 PASS: host should be: 'my.example.com'
 PASS: port should be: 'null'
 PASS: origin should be: 'http://my.example.com'
-PASS: path should be: 'null'
+PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
@@ -175,54 +186,32 @@ PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
 
--- Known issues <https://webkit.org/b/165155>
-
 Test Invalid: http://
-FAIL: Should not be a complete URL
-    Expected: truthy
-    Actual: false
+PASS: Should not be a complete URL
 PASS: URL constructor thinks this is invalid
 
-Test Invalid: http://example.com:999999999
-FAIL: Should not be a complete URL
-    Expected: truthy
-    Actual: false
+Test Invalid: http://example.com:65537
+PASS: Should not be a complete URL
 PASS: URL constructor thinks this is invalid
 
 Test Valid: http:example.com/
-FAIL: scheme should be: 'http'
-    Expected: "http"
-    Actual: null
+PASS: scheme should be: 'http'
 PASS: userinfo should be: 'null'
-FAIL: host should be: 'example.com'
-    Expected: "example.com"
-    Actual: null
+PASS: host should be: 'example.com'
 PASS: port should be: 'null'
-FAIL: origin should be: 'http://example.com'
-    Expected: "http://example.com"
-    Actual: null
-FAIL: path should be: '/'
-    Expected: "/"
-    Actual: null
+PASS: origin should be: 'http://example.com'
+PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
 
 Test Valid: http:/example.com/
-FAIL: scheme should be: 'http'
-    Expected: "http"
-    Actual: null
+PASS: scheme should be: 'http'
 PASS: userinfo should be: 'null'
-FAIL: host should be: 'example.com'
-    Expected: "example.com"
-    Actual: null
+PASS: host should be: 'example.com'
 PASS: port should be: 'null'
-FAIL: origin should be: 'http://example.com'
-    Expected: "http://example.com"
-    Actual: null
-FAIL: path should be: '/'
-    Expected: "/"
-    Actual: null
+PASS: origin should be: 'http://example.com'
+PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
@@ -238,45 +227,31 @@ PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
 
-Test Valid: http://user@pass:example.com/
-FAIL: scheme should be: 'http'
-    Expected: "http"
-    Actual: null
-PASS: userinfo should be: 'null'
-FAIL: host should be: 'example.com'
-    Expected: "example.com"
-    Actual: null
+Test Valid: http://:pass@example.com/
+PASS: scheme should be: 'http'
+PASS: userinfo should be: ':pass'
+PASS: host should be: 'example.com'
 PASS: port should be: 'null'
-FAIL: origin should be: 'http://example.com'
-    Expected: "http://example.com"
-    Actual: null
-FAIL: path should be: '/'
-    Expected: "/"
-    Actual: null
+PASS: origin should be: 'http://example.com'
+PASS: path should be: '/'
 PASS: queryString should be: 'null'
 PASS: fragment should be: 'null'
 PASS: lastPathComponent should be: 'null'
 
+Test Invalid: http://user@pass:example.com/
+PASS: Should not be a complete URL
+PASS: URL constructor thinks this is invalid
+
 Test Valid: http://example.com?key=alpha/beta
 PASS: scheme should be: 'http'
 PASS: userinfo should be: 'null'
-FAIL: host should be: 'example.com'
-    Expected: "example.com"
-    Actual: "example.com?key=alpha"
+PASS: host should be: 'example.com'
 PASS: port should be: 'null'
-FAIL: origin should be: 'http://example.com'
-    Expected: "http://example.com"
-    Actual: "http://example.com?key=alpha"
-FAIL: path should be: 'null'
-    Expected: null
-    Actual: "/beta"
-FAIL: queryString should be: 'key=alpha/beta'
-    Expected: "key=alpha/beta"
-    Actual: null
+PASS: origin should be: 'http://example.com'
+PASS: path should be: '/'
+PASS: queryString should be: 'key=alpha/beta'
 PASS: fragment should be: 'null'
-FAIL: lastPathComponent should be: 'null'
-    Expected: null
-    Actual: "beta"
+PASS: lastPathComponent should be: 'null'
 
 -- Running test case: parseDataURL
 
@@ -380,13 +355,11 @@ PASS: The query 'a&b=1&c==2=&d&e=3&f==4=' was parsed successfully.
 PASS: The query 'a=foo%20bar&b=123%3A456' was parsed successfully.
 
 -- Running test case: WI.displayNameForURL
-PASS: Display name of 'a' should be 'a'.
-PASS: Display name of 'http://' should be 'http://'.
 PASS: Display name of 'http://example' should be 'example'.
 PASS: Display name of 'http://example.com' should be 'example.com'.
 PASS: Display name of 'http://example.com/' should be 'example.com'.
-PASS: Display name of 'http://example.com:999999999' should be 'example.com'.
 PASS: Display name of 'http://example.com:80/' should be 'example.com'.
+PASS: Display name of 'http://example.com:42/' should be 'example.com'.
 PASS: Display name of 'http://example.com/path' should be 'path'.
 PASS: Display name of 'http://example.com/path/' should be 'path'.
 PASS: Display name of 'http://example.com/path/to' should be 'to'.
@@ -401,16 +374,23 @@ PASS: Display name of 'http://example.com:123/path/to/page.html?a=1&b=foo%2Fbar#
 PASS: Display name of 'http://example.com#foo%20bar' should be 'example.com'.
 PASS: Display name of 'http://example.com/#foo%20bar' should be 'example.com'.
 PASS: Display name of 'http://example.com/foo%20bar' should be 'foo bar'.
+PASS: Display name of 'http://example.com?key=foo%20bar' should be 'example.com'.
 PASS: Display name of 'http://example.com/?key=foo%20bar' should be 'example.com'.
+PASS: Display name of 'http://example.com?key=foo bar' should be 'example.com'.
+PASS: Display name of 'http://example.com/?key=foo bar' should be 'example.com'.
 PASS: Display name of 'http://example.com/foo%20bar' should be 'foo bar'.
 PASS: Display name of 'http://example.com/foo bar' should be 'foo bar'.
 PASS: Display name of 'http://example.com#foo/bar' should be 'example.com'.
 PASS: Display name of 'http://example.com/#foo/bar' should be 'example.com'.
 PASS: Display name of 'http://example.com#foo%2Fbar' should be 'example.com'.
 PASS: Display name of 'http://example.com/#foo%2Fbar' should be 'example.com'.
+PASS: Display name of 'http://example.com?key=foo%2Fbar' should be 'example.com'.
 PASS: Display name of 'http://example.com/?key=foo%2Fbar' should be 'example.com'.
+PASS: Display name of 'http://example.com?key=foo/bar' should be 'example.com'.
+PASS: Display name of 'http://example.com/?key=foo/bar' should be 'example.com'.
 PASS: Display name of 'http://example.com/foo%2Fbar' should be 'foo/bar'.
 PASS: Display name of 'http://user:pass@example.com/' should be 'example.com'.
+PASS: Display name of 'http://:pass@example.com/' should be 'example.com'.
 PASS: Display name of 'http://my.example.com' should be 'my.example.com'.
 PASS: Display name of 'http://my.example.com/' should be 'my.example.com'.
 PASS: Display name of 'file:///foo' should be 'foo'.
@@ -422,15 +402,17 @@ PASS: Display name of 'about:srcdoc' should be 'about:srcdoc'.
 PASS: Display name of 'app-specific://example.com' should be 'example.com'.
 PASS: Display name of 'app-specific://example.com/' should be 'example.com'.
 PASS: Display name of 'app-specific://example.com/path' should be 'path'.
-
-Allowing directory as name...
 PASS: Display name of 'a' should be 'a'.
 PASS: Display name of 'http://' should be 'http://'.
-PASS: Display name of 'http://example' should be 'example'.
-PASS: Display name of 'http://example.com' should be 'example.com'.
+PASS: Display name of 'http://example.com:65537' should be 'http://example.com:65537'.
+PASS: Display name of 'http://user@pass:example.com/' should be 'http://user@pass:example.com/'.
+
+Allowing directory as name...
+PASS: Display name of 'http://example' should be '/'.
+PASS: Display name of 'http://example.com' should be '/'.
 PASS: Display name of 'http://example.com/' should be '/'.
-PASS: Display name of 'http://example.com:999999999' should be 'example.com'.
 PASS: Display name of 'http://example.com:80/' should be '/'.
+PASS: Display name of 'http://example.com:42/' should be '/'.
 PASS: Display name of 'http://example.com/path' should be 'path'.
 PASS: Display name of 'http://example.com/path/' should be '/'.
 PASS: Display name of 'http://example.com/path/to' should be 'to'.
@@ -442,20 +424,27 @@ PASS: Display name of 'http://example.com/path/to/page.html?a=1' should be 'page
 PASS: Display name of 'http://example.com/path/to/page.html?a=1&b=foo%2Fbar' should be 'page.html'.
 PASS: Display name of 'http://example.com/path/to/page.html?a=1&b=foo%2Fbar#test' should be 'page.html'.
 PASS: Display name of 'http://example.com:123/path/to/page.html?a=1&b=foo%2Fbar#test' should be 'page.html'.
-PASS: Display name of 'http://example.com#foo%20bar' should be 'example.com'.
+PASS: Display name of 'http://example.com#foo%20bar' should be '/'.
 PASS: Display name of 'http://example.com/#foo%20bar' should be '/'.
 PASS: Display name of 'http://example.com/foo%20bar' should be 'foo bar'.
+PASS: Display name of 'http://example.com?key=foo%20bar' should be '/'.
 PASS: Display name of 'http://example.com/?key=foo%20bar' should be '/'.
+PASS: Display name of 'http://example.com?key=foo bar' should be '/'.
+PASS: Display name of 'http://example.com/?key=foo bar' should be '/'.
 PASS: Display name of 'http://example.com/foo%20bar' should be 'foo bar'.
 PASS: Display name of 'http://example.com/foo bar' should be 'foo bar'.
-PASS: Display name of 'http://example.com#foo/bar' should be 'example.com'.
+PASS: Display name of 'http://example.com#foo/bar' should be '/'.
 PASS: Display name of 'http://example.com/#foo/bar' should be '/'.
-PASS: Display name of 'http://example.com#foo%2Fbar' should be 'example.com'.
+PASS: Display name of 'http://example.com#foo%2Fbar' should be '/'.
 PASS: Display name of 'http://example.com/#foo%2Fbar' should be '/'.
+PASS: Display name of 'http://example.com?key=foo%2Fbar' should be '/'.
 PASS: Display name of 'http://example.com/?key=foo%2Fbar' should be '/'.
+PASS: Display name of 'http://example.com?key=foo/bar' should be '/'.
+PASS: Display name of 'http://example.com/?key=foo/bar' should be '/'.
 PASS: Display name of 'http://example.com/foo%2Fbar' should be 'foo/bar'.
 PASS: Display name of 'http://user:pass@example.com/' should be '/'.
-PASS: Display name of 'http://my.example.com' should be 'my.example.com'.
+PASS: Display name of 'http://:pass@example.com/' should be '/'.
+PASS: Display name of 'http://my.example.com' should be '/'.
 PASS: Display name of 'http://my.example.com/' should be '/'.
 PASS: Display name of 'file:///foo' should be 'foo'.
 PASS: Display name of 'file:///foo/' should be '/'.
@@ -466,35 +455,10 @@ PASS: Display name of 'about:srcdoc' should be 'about:srcdoc'.
 PASS: Display name of 'app-specific://example.com' should be 'example.com'.
 PASS: Display name of 'app-specific://example.com/' should be '/'.
 PASS: Display name of 'app-specific://example.com/path' should be 'path'.
-
--- Known issues <https://webkit.org/b/165155>
-
-FAIL: Display name of 'http://example.com?key=foo%2Fbar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "example.com?key=foo%2fbar"
-FAIL: Display name of 'http://example.com?key=foo/bar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "bar"
-FAIL: Display name of 'http://example.com?key=foo%20bar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "example.com?key=foo%20bar"
-FAIL: Display name of 'http://example.com?key=foo bar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "example.com?key=foo bar"
-
-Allowing directory as name...
-FAIL: Display name of 'http://example.com?key=foo%2Fbar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "example.com?key=foo%2fbar"
-FAIL: Display name of 'http://example.com?key=foo/bar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "bar"
-FAIL: Display name of 'http://example.com?key=foo%20bar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "example.com?key=foo%20bar"
-FAIL: Display name of 'http://example.com?key=foo bar' should be 'example.com'.
-    Expected: "example.com"
-    Actual: "example.com?key=foo bar"
+PASS: Display name of 'a' should be 'a'.
+PASS: Display name of 'http://' should be 'http://'.
+PASS: Display name of 'http://example.com:65537' should be 'http://example.com:65537'.
+PASS: Display name of 'http://user@pass:example.com/' should be 'http://user@pass:example.com/'.
 
 -- Running test case: WI.h2Authority
 PASS: HTTP/2 :authority of 'http://example.com' should be 'example.com'.
@@ -512,11 +476,17 @@ PASS: HTTP/2 :authority of 'https://user:pass@example.com/foo' should be 'exampl
 PASS: HTTP/2 :authority of 'ftp://user:pass@example.com:123/foo' should be 'user:pass@example.com:123'.
 PASS: HTTP/2 :authority of 'http://user:pass@example.com:123/foo' should be 'example.com:123'.
 PASS: HTTP/2 :authority of 'https://user:pass@example.com:123/foo' should be 'example.com:123'.
+PASS: HTTP/2 :authority of 'ftp://:pass@example.com/foo' should be ':pass@example.com'.
+PASS: HTTP/2 :authority of 'http://:pass@example.com/foo' should be 'example.com'.
+PASS: HTTP/2 :authority of 'https://:pass@example.com/foo' should be 'example.com'.
+PASS: HTTP/2 :authority of 'ftp://:pass@example.com:123/foo' should be ':pass@example.com:123'.
+PASS: HTTP/2 :authority of 'http://:pass@example.com:123/foo' should be 'example.com:123'.
+PASS: HTTP/2 :authority of 'https://:pass@example.com:123/foo' should be 'example.com:123'.
 
 -- Running test case: WI.h2Path
 PASS: HTTP/2 :path of 'http://example.com' should be '/'.
 PASS: HTTP/2 :path of 'https://example.com' should be '/'.
-PASS: HTTP/2 :path of 'ftp://example.com' should be ''.
+PASS: HTTP/2 :path of 'ftp://example.com' should be '/'.
 PASS: HTTP/2 :path of 'http://example.com/foo' should be '/foo'.
 PASS: HTTP/2 :path of 'https://example.com/foo' should be '/foo'.
 PASS: HTTP/2 :path of 'ftp://example.com/foo' should be '/foo'.
index 3795beb..ecbad32 100644 (file)
@@ -13,7 +13,11 @@ function test()
             function testInvalid(url) {
                 InspectorTest.log("");
                 InspectorTest.log("Test Invalid: " + url);
-                InspectorTest.expectThat(parseURL(url).scheme === null, "Should not be a complete URL");
+
+                let urlComponents = parseURL(url);
+                InspectorTest.expectNull(urlComponents.scheme, "Should not be a complete URL");
+                if (urlComponents.scheme)
+                    InspectorTest.json(urlComponents);
 
                 try {
                     new URL(url);
@@ -50,7 +54,7 @@ function test()
                 host: "example.com",
                 port: null,
                 origin: "http://example.com",
-                path: null,
+                path: "/",
                 queryString: null,
                 fragment: null,
                 lastPathComponent: null,
@@ -72,8 +76,20 @@ function test()
                 scheme: "http",
                 userinfo: null,
                 host: "example.com",
-                port: 80,
-                origin: "http://example.com:80",
+                port: null,
+                origin: "http://example.com",
+                path: "/",
+                queryString: null,
+                fragment: null,
+                lastPathComponent: null,
+            });
+
+            testValid("http://example.com:42/", {
+                scheme: "http",
+                userinfo: null,
+                host: "example.com",
+                port: 42,
+                origin: "http://example.com:42",
                 path: "/",
                 queryString: null,
                 fragment: null,
@@ -99,7 +115,7 @@ function test()
                 port: null,
                 origin: "http://example.com",
                 path: "/path/to/page.html",
-                queryString: "",
+                queryString: null,
                 fragment: null,
                 lastPathComponent: "page.html",
             });
@@ -170,7 +186,7 @@ function test()
                 host: "example.com",
                 port: null,
                 origin: "http://example.com",
-                path: null,
+                path: "/",
                 queryString: null,
                 fragment: "alpha/beta",
                 lastPathComponent: null,
@@ -194,7 +210,7 @@ function test()
                 host: "example",
                 port: null,
                 origin: "http://example",
-                path: null,
+                path: "/",
                 queryString: null,
                 fragment: null,
                 lastPathComponent: null,
@@ -206,7 +222,7 @@ function test()
                 host: "my.example.com",
                 port: null,
                 origin: "http://my.example.com",
-                path: null,
+                path: "/",
                 queryString: null,
                 fragment: null,
                 lastPathComponent: null,
@@ -225,12 +241,8 @@ function test()
                 lastPathComponent: null,
             });
 
-            // FIXME: <https://webkit.org/b/165155> Web Inspector: Use URL constructor to better handle all kinds of URLs
-            InspectorTest.log("");
-            InspectorTest.log("-- Known issues <https://webkit.org/b/165155>");
-
             testInvalid("http://");
-            testInvalid("http://example.com:999999999");
+            testInvalid("http://example.com:65537");
 
             testValid("http:example.com/", {
                 scheme: "http",
@@ -268,9 +280,9 @@ function test()
                 lastPathComponent: null,
             });
 
-            testValid("http://user@pass:example.com/", {
+            testValid("http://:pass@example.com/", {
                 scheme: "http",
-                userinfo: null,
+                userinfo: ":pass",
                 host: "example.com",
                 port: null,
                 origin: "http://example.com",
@@ -280,13 +292,15 @@ function test()
                 lastPathComponent: null,
             });
 
+            testInvalid("http://user@pass:example.com/");
+
             testValid("http://example.com?key=alpha/beta", {
                 scheme: "http",
                 userinfo: null,
                 host: "example.com",
                 port: null,
                 origin: "http://example.com",
-                path: null,
+                path: "/",
                 queryString: "key=alpha/beta",
                 fragment: null,
                 lastPathComponent: null,
@@ -443,25 +457,26 @@ function test()
         name: "WI.displayNameForURL",
         test() {
             function test(tests) {
-                for (let {url, expected} of tests)
+                for (let {url, expected} of tests) {
+                    expected = expected || url;
                     InspectorTest.expectEqual(WI.displayNameForURL(url), expected, `Display name of '${url}' should be '${expected}'.`);
+                }
 
                 InspectorTest.newline();
 
                 InspectorTest.log("Allowing directory as name...");
-                for (let {url, expected, directory} of tests)
-                    InspectorTest.expectEqual(WI.displayNameForURL(url, null, {allowDirectoryAsName: true}), directory || expected, `Display name of '${url}' should be '${directory || expected}'.`);
+                for (let {url, expected, directory} of tests) {
+                    expected = directory || expected || url;
+                    InspectorTest.expectEqual(WI.displayNameForURL(url, null, {allowDirectoryAsName: true}), expected, `Display name of '${url}' should be '${expected}'.`);
+                }
             }
 
             test([
-                {url: "a", expected: "a"},
-
-                {url: "http://", expected: "http://"},
-                {url: "http://example", expected: "example"},
-                {url: "http://example.com", expected: "example.com"},
+                {url: "http://example", expected: "example", directory: "/"},
+                {url: "http://example.com", expected: "example.com", directory: "/"},
                 {url: "http://example.com/", expected: "example.com", directory: "/"},
-                {url: "http://example.com:999999999", expected: "example.com"},
                 {url: "http://example.com:80/", expected: "example.com", directory: "/"},
+                {url: "http://example.com:42/", expected: "example.com", directory: "/"},
                 {url: "http://example.com/path", expected: "path"},
                 {url: "http://example.com/path/", expected: "path", directory: "/"},
                 {url: "http://example.com/path/to", expected: "to"},
@@ -474,23 +489,30 @@ function test()
                 {url: "http://example.com/path/to/page.html?a=1&b=foo%2Fbar#test", expected: "page.html"},
                 {url: "http://example.com:123/path/to/page.html?a=1&b=foo%2Fbar#test", expected: "page.html"},
 
-                {url: "http://example.com#foo%20bar", expected: "example.com"},
+                {url: "http://example.com#foo%20bar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/#foo%20bar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/foo%20bar", expected: "foo bar"},
+                {url: "http://example.com?key=foo%20bar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/?key=foo%20bar", expected: "example.com", directory: "/"},
+                {url: "http://example.com?key=foo bar", expected: "example.com", directory: "/"},
+                {url: "http://example.com/?key=foo bar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/foo%20bar", expected: "foo bar"},
                 {url: "http://example.com/foo bar", expected: "foo bar"},
 
-                {url: "http://example.com#foo/bar", expected: "example.com"},
+                {url: "http://example.com#foo/bar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/#foo/bar", expected: "example.com", directory: "/"},
-                {url: "http://example.com#foo%2Fbar", expected: "example.com"},
+                {url: "http://example.com#foo%2Fbar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/#foo%2Fbar", expected: "example.com", directory: "/"},
+                {url: "http://example.com?key=foo%2Fbar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/?key=foo%2Fbar", expected: "example.com", directory: "/"},
+                {url: "http://example.com?key=foo/bar", expected: "example.com", directory: "/"},
+                {url: "http://example.com/?key=foo/bar", expected: "example.com", directory: "/"},
                 {url: "http://example.com/foo%2Fbar", expected: "foo/bar"},
 
                 {url: "http://user:pass@example.com/", expected: "example.com", directory: "/"},
+                {url: "http://:pass@example.com/", expected: "example.com", directory: "/"},
 
-                {url: "http://my.example.com", expected: "my.example.com"},
+                {url: "http://my.example.com", expected: "my.example.com", directory: "/"},
                 {url: "http://my.example.com/", expected: "my.example.com", directory: "/"},
 
                 {url: "file:///foo", expected: "foo"},
@@ -505,19 +527,12 @@ function test()
                 {url: "app-specific://example.com", expected: "example.com"},
                 {url: "app-specific://example.com/", expected: "example.com", directory: "/"},
                 {url: "app-specific://example.com/path", expected: "path"},
-            ]);
-
-            // FIXME: <https://webkit.org/b/165155> Web Inspector: Use URL constructor to better handle all kinds of URLs
-            InspectorTest.newline();
-            InspectorTest.log("-- Known issues <https://webkit.org/b/165155>");
-            InspectorTest.newline();
 
-            test([
-                {url: "http://example.com?key=foo%2Fbar", expected: "example.com"},
-                {url: "http://example.com?key=foo/bar", expected: "example.com"},
-
-                {url: "http://example.com?key=foo%20bar", expected: "example.com"},
-                {url: "http://example.com?key=foo bar", expected: "example.com"},
+                // Invalid
+                {url: "a"},
+                {url: "http://"},
+                {url: "http://example.com:65537"},
+                {url: "http://user@pass:example.com/"},
             ]);
         },
     });
@@ -550,6 +565,14 @@ function test()
             test("http://user:pass@example.com:123/foo", "example.com:123");
             test("https://user:pass@example.com:123/foo", "example.com:123");
 
+            test("ftp://:pass@example.com/foo", ":pass@example.com");
+            test("http://:pass@example.com/foo", "example.com");
+            test("https://:pass@example.com/foo", "example.com");
+
+            test("ftp://:pass@example.com:123/foo", ":pass@example.com:123");
+            test("http://:pass@example.com:123/foo", "example.com:123");
+            test("https://:pass@example.com:123/foo", "example.com:123");
+
             return true;
         }
     });
@@ -564,7 +587,7 @@ function test()
 
             test("http://example.com", "/");
             test("https://example.com", "/");
-            test("ftp://example.com", "");
+            test("ftp://example.com", "/");
 
             test("http://example.com/foo", "/foo");
             test("https://example.com/foo", "/foo");
index 4948698..80f44ce 100644 (file)
@@ -1,3 +1,13 @@
+2019-08-20  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Use URL constructor to better handle all kinds of URLs
+        https://bugs.webkit.org/show_bug.cgi?id=165155
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Base/URLUtilities.js:
+        (parseURL):
+
 2019-08-19  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Debugger: add a global breakpoint for pausing in the next microtask
index 483c8ac..816ead6 100644 (file)
@@ -23,8 +23,6 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-// FIXME: <https://webkit.org/b/165155> Web Inspector: Use URL constructor to better handle all kinds of URLs
-
 function removeURLFragment(url)
 {
     var hashIndex = url.indexOf("#");
@@ -93,52 +91,71 @@ function parseDataURL(url)
 
 function parseURL(url)
 {
-    url = url ? url.trim() : "";
+    let result = {
+        scheme: null,
+        userinfo: null,
+        host: null,
+        port: null,
+        origin: null,
+        path: null,
+        queryString: null,
+        fragment: null,
+        lastPathComponent: null,
+    };
+
+    // dataURLs should be handled by `parseDataURL`.
+    if (url && url.startsWith("data:")) {
+        result.scheme = "data";
+        return result;
+    }
 
-    if (url.startsWith("data:"))
-        return {scheme: "data", userinfo: null, host: null, port: null, origin: null, path: null, queryString: null, fragment: null, lastPathComponent: null};
+    let parsed = null;
+    try {
+        parsed = new URL(url);
+    } catch {
+        return result;
+    }
 
-    let match = url.match(/^(?<scheme>[^\/:]+):\/\/(?:(?<userinfo>[^#@\/]+)@)?(?<host>[^\/#:]*)(?::(?<port>[\d]+))?(?:(?<path>\/[^#]*)?(?:#(?<fragment>.*))?)?$/i);
-    if (!match)
-        return {scheme: null, userinfo: null, host: null, port: null, origin: null, path: null, queryString: null, fragment: null, lastPathComponent: null};
+    result.scheme = parsed.protocol.slice(0, -1); // remove trailing ":"
 
-    let scheme = match.groups.scheme.toLowerCase();
-    let userinfo = match.groups.userinfo || null;
-    let host = match.groups.host.toLowerCase();
-    let port = Number(match.groups.port) || null;
-    let wholePath = match.groups.path || null;
-    let fragment = match.groups.fragment || null;
-    let path = wholePath;
-    let queryString = null;
-
-    // Split the path and the query string.
-    if (wholePath) {
-        let indexOfQuery = wholePath.indexOf("?");
-        if (indexOfQuery !== -1) {
-            path = wholePath.substring(0, indexOfQuery);
-            queryString = wholePath.substring(indexOfQuery + 1);
-        }
-        path = resolveDotsInPath(path);
+    if (parsed.username)
+        result.userinfo = parsed.username;
+    if (parsed.password)
+        result.userinfo = (result.userinfo || "") + ":" + parsed.password;
+
+    if (parsed.hostname)
+        result.host = parsed.hostname;
+
+    if (parsed.port)
+        result.port = Number(parsed.port);
+
+    if (parsed.origin)
+        result.origin = parsed.origin;
+    else if (result.scheme && result.host) {
+        result.origin = result.scheme + "://" + result.host;
+        if (result.port)
+            result.origin += ":" + result.port;
     }
 
+    if (parsed.pathname)
+        result.path = parsed.pathname;
+
+    if (parsed.search)
+        result.queryString = parsed.search.substring(1); // remove leading "?"
+
+    if (parsed.hash)
+        result.fragment = parsed.hash.substring(1); // remove leading "#"
+
     // Find last path component.
-    let lastPathComponent = null;
-    if (path && path !== "/") {
+    if (result.path && result.path !== "/") {
         // Skip the trailing slash if there is one.
-        let endOffset = path[path.length - 1] === "/" ? 1 : 0;
-        let lastSlashIndex = path.lastIndexOf("/", path.length - 1 - endOffset);
+        let endOffset = result.path.endsWith("/") ? 1 : 0;
+        let lastSlashIndex = result.path.lastIndexOf("/", result.path.length - 1 - endOffset);
         if (lastSlashIndex !== -1)
-            lastPathComponent = path.substring(lastSlashIndex + 1, path.length - endOffset);
+            result.lastPathComponent = result.path.substring(lastSlashIndex + 1, result.path.length - endOffset);
     }
 
-    let origin = null;
-    if (scheme && host) {
-        origin = scheme + "://" + host;
-        if (port)
-            origin += ":" + port;
-    }
-
-    return {scheme, userinfo, host, port, origin, path, queryString, fragment, lastPathComponent};
+    return result;
 }
 
 function absoluteURL(partialURL, baseURL)