Skip to content

Commit

Permalink
url: exit early when : delimiter is seen in hostname
Browse files Browse the repository at this point in the history
This aligns with an upstream spec change.

PR-URL: #38742
Fixes: #38710
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
TimothyGu authored and danielleadams committed May 31, 2021
1 parent e9be209 commit 7773d58
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 54 deletions.
6 changes: 3 additions & 3 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 == '?' ||
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/wpt/LICENSE.md
Original file line number Diff line number Diff line change
@@ -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:

Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/wpt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
37 changes: 21 additions & 16 deletions test/fixtures/wpt/url/failure.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
16 changes: 8 additions & 8 deletions test/fixtures/wpt/url/resources/setters_tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
},
Expand Down
41 changes: 31 additions & 10 deletions test/fixtures/wpt/url/resources/urltestdata.json
Original file line number Diff line number Diff line change
Expand Up @@ -3156,7 +3156,8 @@
{
"input": "http:/:@/www.example.com",
"base": "about:blank",
"failure": true
"failure": true,
"inputCanBeRelative": true
},
{
"input": "http://user@/www.example.com",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -3201,7 +3207,8 @@
{
"input": "http::@/www.example.com",
"base": "about:blank",
"failure": true
"failure": true,
"inputCanBeRelative": true
},
{
"input": "http:a:@www.example.com",
Expand Down Expand Up @@ -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://Go.com",
Expand Down Expand Up @@ -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",
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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;
Expand All @@ -45,4 +34,3 @@
}
}
}
</script>
27 changes: 27 additions & 0 deletions test/fixtures/wpt/url/url-setters.any.js
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
2 changes: 1 addition & 1 deletion test/fixtures/wpt/versions.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"path": "resources"
},
"url": {
"commit": "1439087f27135b06deb70ffbf43e65ff64ff1ee6",
"commit": "1fcb39223d3009fbb46c1b254755d6cc75e290f1",
"path": "url"
}
}
6 changes: 6 additions & 0 deletions test/wpt/status/url.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}

0 comments on commit 7773d58

Please sign in to comment.