url: exit early when : delimiter is seen in hostname

This aligns with an upstream spec change. PR-URL: #38742 Fixes: #38710 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · May 31, 2021 · 7773d58 · 7773d58
1 parent e9be209
commit 7773d58
Show file tree

Hide file tree

Showing 10 changed files with 101 additions and 54 deletions.
diff --git a/src/node_url.cc b/src/node_url.cc
@@ -1766,16 +1766,16 @@ void URL::Parse(const char* input,
             url->flags |= URL_FLAGS_FAILED;
             return;
           }
+          if (state_override == kHostname) {
+            return;
+          }
           url->flags |= URL_FLAGS_HAS_HOST;
           if (!ParseHost(buffer, &url->host, special)) {
             url->flags |= URL_FLAGS_FAILED;
             return;
           }
           buffer.clear();
           state = kPort;
-          if (state_override == kHostname) {
-            return;
-          }
         } else if (ch == kEOL ||
                    ch == '/' ||
                    ch == '?' ||

diff --git a/test/fixtures/wpt/LICENSE.md b/test/fixtures/wpt/LICENSE.md
@@ -1,6 +1,6 @@
 # The 3-Clause BSD License
 
-Copyright 2019 web-platform-tests contributors
+Copyright © web-platform-tests contributors
 
 Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
 

diff --git a/test/fixtures/wpt/README.md b/test/fixtures/wpt/README.md
@@ -21,7 +21,7 @@ Last update:
 - html/webappapis/timers: https://github.com/web-platform-tests/wpt/tree/5873f2d8f1/html/webappapis/timers
 - interfaces: https://github.com/web-platform-tests/wpt/tree/79fa4cf76e/interfaces
 - resources: https://github.com/web-platform-tests/wpt/tree/972ca5b669/resources
-- url: https://github.com/web-platform-tests/wpt/tree/1439087f27/url
+- url: https://github.com/web-platform-tests/wpt/tree/1fcb39223d/url
 
 [Web Platform Tests]: https://github.com/web-platform-tests/wpt
 [`git node wpt`]: https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md#git-node-wpt
diff --git a/test/fixtures/wpt/url/failure.html b/test/fixtures/wpt/url/failure.html
@@ -28,22 +28,27 @@
       assert_throws_js(TypeError, () => url.href = test.input)
     }, "URL's href: " + name)
 
-    self.test(() => {
-      const client = new XMLHttpRequest()
-      assert_throws_dom("SyntaxError", () => client.open("GET", test.input))
-    }, "XHR: " + name)
-
-    self.test(() => {
-      assert_throws_js(TypeError, () => self.navigator.sendBeacon(test.input))
-    }, "sendBeacon(): " + name)
-
-    self.test(() => {
-      assert_throws_js(self[0].TypeError, () => self[0].location = test.input)
-    }, "Location's href: " + name)
-
-    self.test(() => {
-      assert_throws_dom("SyntaxError", () => self.open(test.input).close())
-    }, "window.open(): " + name)
+    // The following use cases resolve the URL input relative to the current
+    // document's URL. If this test input could be construed as a valid URL
+    // when resolved against a base URL, skip these cases.
+    if (!test.inputCanBeRelative) {
+      self.test(() => {
+        const client = new XMLHttpRequest()
+        assert_throws_dom("SyntaxError", () => client.open("GET", test.input))
+      }, "XHR: " + name)
+
+      self.test(() => {
+        assert_throws_js(TypeError, () => self.navigator.sendBeacon(test.input))
+      }, "sendBeacon(): " + name)
+
+      self.test(() => {
+        assert_throws_js(self[0].TypeError, () => self[0].location = test.input)
+      }, "Location's href: " + name)
+
+      self.test(() => {
+        assert_throws_dom("SyntaxError", () => self.open(test.input).close())
+      }, "window.open(): " + name)
+    }
   }
 }
 </script>
diff --git a/test/fixtures/wpt/url/resources/setters_tests.json b/test/fixtures/wpt/url/resources/setters_tests.json
@@ -1153,24 +1153,24 @@
             }
         },
         {
-            "comment": "Stuff after a : delimiter is ignored",
+            "comment": ": delimiter invalidates entire value",
             "href": "http://example.net/path",
             "new_value": "example.com:8080",
             "expected": {
-                "href": "http://example.com/path",
-                "host": "example.com",
-                "hostname": "example.com",
+                "href": "http://example.net/path",
+                "host": "example.net",
+                "hostname": "example.net",
                 "port": ""
             }
         },
         {
-            "comment": "Stuff after a : delimiter is ignored",
+            "comment": ": delimiter invalidates entire value",
             "href": "http://example.net:8080/path",
             "new_value": "example.com:",
             "expected": {
-                "href": "http://example.com:8080/path",
-                "host": "example.com:8080",
-                "hostname": "example.com",
+                "href": "http://example.net:8080/path",
+                "host": "example.net:8080",
+                "hostname": "example.net",
                 "port": "8080"
             }
         },

diff --git a/test/fixtures/wpt/url/resources/urltestdata.json b/test/fixtures/wpt/url/resources/urltestdata.json
@@ -3156,7 +3156,8 @@
   {
     "input": "http:/:@/www.example.com",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "http://user@/www.example.com",
@@ -3166,12 +3167,14 @@
   {
     "input": "http:@/www.example.com",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "http:/@/www.example.com",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "http://@/www.example.com",
@@ -3181,17 +3184,20 @@
   {
     "input": "https:@/www.example.com",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "http:a:b@/www.example.com",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "http:/a:b@/www.example.com",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "http://a:b@/www.example.com",
@@ -3201,7 +3207,8 @@
   {
     "input": "http::@/www.example.com",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "http:a:@www.example.com",
@@ -3645,6 +3652,17 @@
     "search": "?%EF%BF%BD",
     "hash": "#%EF%BF%BD"
   },
+  "Domain is ASCII, but a label is invalid IDNA",
+  {
+    "input": "http://a.b.c.xn--pokxncvks",
+    "base": "about:blank",
+    "failure": true
+  },
+  {
+    "input": "http://10.0.0.xn--pokxncvks",
+    "base": "about:blank",
+    "failure": true
+  },
   "Test name prepping, fullwidth input should be converted to ASCII and NOT IDN-ized. This is 'Go' in fullwidth UTF-8/UTF-16.",
   {
     "input": "http://Ｇｏ.com",
@@ -7320,17 +7338,20 @@
   {
     "input": "a",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "a/",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   {
     "input": "a//",
     "base": "about:blank",
-    "failure": true
+    "failure": true,
+    "inputCanBeRelative": true
   },
   "Bases that don't fail to parse but fail to be bases",
   {

diff --git a/test/fixtures/wpt/url/url-setters.html → ...ures/wpt/url/url-setters-a-area.window.js b/test/fixtures/wpt/url/url-setters.html → ...ures/wpt/url/url-setters-a-area.window.js
@@ -1,9 +1,5 @@
-<!doctype html>
-<meta charset=utf-8>
-<script src=/resources/testharness.js></script>
-<script src=/resources/testharnessreport.js></script>
-<div id=log></div>
-<script>
+// Keep this file in sync with url-setters.any.js.
+
 promise_test(() => fetch("resources/setters_tests.json").then(res => res.json()).then(runURLSettersTests), "Loading data…");
 
 function runURLSettersTests(all_test_cases) {
@@ -19,13 +15,6 @@
       if ("comment" in test_case) {
         name += " " + test_case.comment;
       }
-      test(function() {
-        var url = new URL(test_case.href);
-        url[attribute_to_be_set] = test_case.new_value;
-        for (var attribute in test_case.expected) {
-          assert_equals(url[attribute], test_case.expected[attribute])
-        }
-      }, "URL: " + name)
       test(function() {
         var url = document.createElement("a");
         url.href = test_case.href;
@@ -45,4 +34,3 @@
     }
   }
 }
-</script>
diff --git a/test/fixtures/wpt/url/url-setters.any.js b/test/fixtures/wpt/url/url-setters.any.js
@@ -0,0 +1,27 @@
+// Keep this file in sync with url-setters-a-area.window.js.
+
+promise_test(() => fetch("resources/setters_tests.json").then(res => res.json()).then(runURLSettersTests), "Loading data…");
+
+function runURLSettersTests(all_test_cases) {
+  for (var attribute_to_be_set in all_test_cases) {
+    if (attribute_to_be_set == "comment") {
+      continue;
+    }
+    var test_cases = all_test_cases[attribute_to_be_set];
+    for(var i = 0, l = test_cases.length; i < l; i++) {
+      var test_case = test_cases[i];
+      var name = "Setting <" + test_case.href + ">." + attribute_to_be_set +
+                 " = '" + test_case.new_value + "'";
+      if ("comment" in test_case) {
+        name += " " + test_case.comment;
+      }
+      test(function() {
+        var url = new URL(test_case.href);
+        url[attribute_to_be_set] = test_case.new_value;
+        for (var attribute in test_case.expected) {
+          assert_equals(url[attribute], test_case.expected[attribute])
+        }
+      }, "URL: " + name)
+    }
+  }
+}
diff --git a/test/fixtures/wpt/versions.json b/test/fixtures/wpt/versions.json
@@ -44,7 +44,7 @@
     "path": "resources"
   },
   "url": {
-    "commit": "1439087f27135b06deb70ffbf43e65ff64ff1ee6",
+    "commit": "1fcb39223d3009fbb46c1b254755d6cc75e290f1",
     "path": "url"
   }
 }
diff --git a/test/wpt/status/url.json b/test/wpt/status/url.json
@@ -24,5 +24,11 @@
   },
   "url-origin.any.js": {
     "requires": ["small-icu"]
+  },
+  "url-setters.any.js": {
+    "requires": ["small-icu"]
+  },
+  "url-setters-a-area.window.js": {
+    "skip": "already tested in url-setters.any.js"
   }
 }