GeezerVison is showing github.com — url: handle unparsable serialized URLs in setters · nodejs/node@9ffc0da
File tree
Expand file treeCollapse file tree
Open diff view settings
Expand file treeCollapse file tree
Open diff view settings
Collapse file
Copy file name to clipboardExpand all lines: src/node_url.cc +5-1Lines changed: 5 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -443,8 +443,12 @@ void BindingData::Update(const FunctionCallbackInfo<Value>& args) { |
443 | 443 | Utf8Value new_value(isolate, args[2].As<String>()); |
444 | 444 |
|
445 | 445 | std::string_view new_value_view = new_value.ToStringView(); |
| 446 | + // A serialized URL is not always reparsable: the IDNA encoder can emit a |
| 447 | + // host label that the decoder rejects. Fail the update instead of crashing. |
446 | 448 | auto out = ada::parse<ada::url_aggregator>(input. ToStringView()); |
447 | | - CHECK(out); |
| 449 | + if (!out) { |
| 450 | + return args.GetReturnValue().Set(false); |
| 451 | + } |
448 | 452 |
|
449 | 453 | bool result{true}; |
450 | 454 |
|
|
Collapse file
Copy file name to clipboardExpand all lines: test/parallel/test-whatwg-url-custom-setters.js +26Lines changed: 26 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -39,6 +39,32 @@ const additionalTestCases = |
39 | 39 | } |
40 | 40 | } |
41 | 41 |
|
| 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 | + |
42 | 68 | { |
43 | 69 | const url = new URL('http://example.com/'); |
44 | 70 | const obj = { |
|