Skip to content

Commit 589a8d4

Browse files
mcollinajuanarbol
authored andcommitted
url: handle unparsable serialized URLs in setters
Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #64651 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>
1 parent 086b893 commit 589a8d4

2 files changed

Lines changed: 31 additions & 1 deletion

File tree

src/node_url.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,12 @@ void BindingData::Update(const FunctionCallbackInfo<Value>& args) {
409409
Utf8Value new_value(isolate, args[2].As<String>());
410410

411411
std::string_view new_value_view = new_value.ToStringView();
412+
// A serialized URL is not always reparsable: the IDNA encoder can emit a
413+
// host label that the decoder rejects. Fail the update instead of crashing.
412414
auto out = ada::parse<ada::url_aggregator>(input.ToStringView());
413-
CHECK(out);
415+
if (!out) {
416+
return args.GetReturnValue().Set(false);
417+
}
414418

415419
bool result{true};
416420

test/parallel/test-whatwg-url-custom-setters.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,32 @@ const additionalTestCases =
3939
}
4040
}
4141

42+
// The parser can produce a serialization it rejects when parsing it back: a
43+
// Unicode host encodes to an `xn--xn--` label that the punycode decoder turns
44+
// down. Setters reparse `href`, so the failure must not take the process down.
45+
// Implementations backed by ICU accept that label, and ada does too as of
46+
// https://github.com/ada-url/idna/pull/72, so this URL round-trips once that
47+
// lands here and the setters below apply as usual.
48+
test(function() {
49+
const url = new URL('http:\u{1F600}xn-');
50+
const setters = {
51+
hostname: 'example.com',
52+
host: 'example.com:8080',
53+
protocol: 'https:',
54+
pathname: '/path',
55+
search: '?search',
56+
hash: '#hash',
57+
port: '8080',
58+
username: 'username',
59+
password: 'password',
60+
};
61+
62+
for (const [property, value] of Object.entries(setters)) {
63+
url[property] = value;
64+
assert_equals(typeof url.href, 'string', `Setting ${property} does not crash`);
65+
}
66+
}, 'URL: setting properties with an unparsable serialized URL');
67+
4268
{
4369
const url = new URL('http://example.com/');
4470
const obj = {

0 commit comments

Comments
 (0)