From 7773d58f1a2bf9dd4a741109d4f698196137deff Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 20 May 2021 00:42:07 -0700 Subject: [PATCH] url: exit early when : delimiter is seen in hostname This aligns with an upstream spec change. PR-URL: https://github.com/nodejs/node/pull/38742 Fixes: https://github.com/nodejs/node/issues/38710 Reviewed-By: Colin Ihrig Reviewed-By: Darshan Sen Reviewed-By: James M Snell --- src/node_url.cc | 6 +-- test/fixtures/wpt/LICENSE.md | 2 +- test/fixtures/wpt/README.md | 2 +- test/fixtures/wpt/url/failure.html | 37 +++++++++-------- .../wpt/url/resources/setters_tests.json | 16 ++++---- .../wpt/url/resources/urltestdata.json | 41 ++++++++++++++----- ...ters.html => url-setters-a-area.window.js} | 16 +------- test/fixtures/wpt/url/url-setters.any.js | 27 ++++++++++++ test/fixtures/wpt/versions.json | 2 +- test/wpt/status/url.json | 6 +++ 10 files changed, 101 insertions(+), 54 deletions(-) rename test/fixtures/wpt/url/{url-setters.html => url-setters-a-area.window.js} (74%) create mode 100644 test/fixtures/wpt/url/url-setters.any.js diff --git a/src/node_url.cc b/src/node_url.cc index 554ee855848cc7..3b4c2e690ee713 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -1766,6 +1766,9 @@ 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; @@ -1773,9 +1776,6 @@ void URL::Parse(const char* input, } 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 index ad4858c8745cfa..39c46d03ac2988 100644 --- 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 index fef6bb0fd8c30d..b68466264ebdf8 100644 --- 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 index 8f3d0299a40fdb..c22357b6c10749 100644 --- 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) + } } } diff --git a/test/fixtures/wpt/url/resources/setters_tests.json b/test/fixtures/wpt/url/resources/setters_tests.json index 8aa74d6b8a28d9..56bcae464a6f54 100644 --- 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 index bb156f126bf9b6..96c42d2284ebf7 100644 --- 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 b/test/fixtures/wpt/url/url-setters-a-area.window.js similarity index 74% rename from test/fixtures/wpt/url/url-setters.html rename to test/fixtures/wpt/url/url-setters-a-area.window.js index db30cf0774da44..8c66f2883d3c2e 100644 --- a/test/fixtures/wpt/url/url-setters.html +++ b/test/fixtures/wpt/url/url-setters-a-area.window.js @@ -1,9 +1,5 @@ - - - - -
- diff --git a/test/fixtures/wpt/url/url-setters.any.js b/test/fixtures/wpt/url/url-setters.any.js new file mode 100644 index 00000000000000..1cddf94a8ecca9 --- /dev/null +++ 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 index 67df1aaa931761..b610d0ab47ec94 100644 --- 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" } } \ No newline at end of file diff --git a/test/wpt/status/url.json b/test/wpt/status/url.json index f70bdd5c5cf459..ab89356f82c805 100644 --- 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" } }