GeezerVison is showing github.com — url: align default argument handling for URLPattern with webidl · nodejs/node@fd3bf38
File treeExpand 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_pattern.cc +58-37Lines changed: 58 additions & 37 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -202,7 +202,11 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& args) { | 202 | 202 | // - new URLPattern(input, baseURL) | 203 | 203 | // - new URLPattern(input, options) | 204 | 204 | // - new URLPattern(input, baseURL, options) | 205 | | - if (args[0]->IsString()) { | | 205 | + // Per WebIDL, null/undefined for a union type including a dictionary | | 206 | + // uses the default value (empty init). | | 207 | + if (args[0]->IsNullOrUndefined()) { | | 208 | + init = ada::url_pattern_init{}; | | 209 | + } else if (args[0]->IsString()) { | 206 | 210 | BufferValue input_buffer(env->isolate(), args[0]); | 207 | 211 | CHECK_NOT_NULL(*input_buffer); | 208 | 212 | input = input_buffer.ToString(); | @@ -217,41 +221,55 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& args) { | 217 | 221 | return; | 218 | 222 | } | 219 | 223 |
| 220 | | - // The next argument can be baseURL or options. | 221 | | - if (args.Length() > 1) { | | 224 | + // Per WebIDL overload resolution: | | 225 | + // With 3+ args, it's always overload 1: (input, baseURL, options) | | 226 | + // With 2 args, if arg1 is string, it is overload 1 (baseURL), | | 227 | + // otherwise overload 2 (options) | | 228 | + if (args.Length() >= 3) { | | 229 | + // arg1 is baseURL. Per WebIDL, null/undefined are stringified for | | 230 | + // USVString ("null"/"undefined"), which will be rejected as invalid | | 231 | + // URLs by ada downstream. | 222 | 232 | if (args[1]->IsString()) { | 223 | 233 | BufferValue base_url_buffer(env->isolate(), args[1]); | 224 | 234 | CHECK_NOT_NULL(*base_url_buffer); | 225 | 235 | base_url = base_url_buffer.ToString(); | 226 | | - } else if (args[1]->IsObject()) { | 227 | | - CHECK(!options.has_value()); | 228 | | - options = URLPatternOptions::FromJsObject(env, args[1].As<Object>()); | 229 | | - if (!options) { | 230 | | - // If options does not have a value, we assume an error was | 231 | | - // thrown and scheduled on the isolate. Return early to | 232 | | - // propagate it. | 233 | | - return; | 234 | | - } | | 236 | + } else if (args[1]->IsNull()) { | | 237 | + base_url = std::string("null"); | | 238 | + } else if (args[1]->IsUndefined()) { | | 239 | + base_url = std::string("undefined"); | 235 | 240 | } else { | 236 | | - THROW_ERR_INVALID_ARG_TYPE(env, | 237 | | - "second argument must be a string or object"); | | 241 | + THROW_ERR_INVALID_ARG_TYPE(env, "second argument must be a string"); | 238 | 242 | return; | 239 | 243 | } | 240 | 244 |
| 241 | | - // Only remaining argument can be options. | 242 | | - if (args.Length() > 2) { | | 245 | + // arg2 is options. Per WebIDL, null/undefined for a dictionary | | 246 | + // uses the default value (empty dict). | | 247 | + if (!args[2]->IsNullOrUndefined()) { | 243 | 248 | if (!args[2]->IsObject()) { | 244 | 249 | THROW_ERR_INVALID_ARG_TYPE(env, "options must be an object"); | 245 | 250 | return; | 246 | 251 | } | 247 | 252 | CHECK(!options.has_value()); | 248 | 253 | options = URLPatternOptions::FromJsObject(env, args[2].As<Object>()); | 249 | | - if (!options) { | 250 | | - // If options does not have a value, we assume an error was | 251 | | - // thrown and scheduled on the isolate. Return early to | 252 | | - // propagate it. | 253 | | - return; | 254 | | - } | | 254 | + if (!options) return; | | 255 | + } | | 256 | + } else if (args.Length() == 2) { | | 257 | + // Overload resolution: string is overload 1 (baseURL), | | 258 | + // otherwise overload 2 (options). | | 259 | + if (args[1]->IsString()) { | | 260 | + BufferValue base_url_buffer(env->isolate(), args[1]); | | 261 | + CHECK_NOT_NULL(*base_url_buffer); | | 262 | + base_url = base_url_buffer.ToString(); | | 263 | + } else if (args[1]->IsNullOrUndefined()) { | | 264 | + // Overload 2, options uses default. | | 265 | + } else if (args[1]->IsObject()) { | | 266 | + CHECK(!options.has_value()); | | 267 | + options = URLPatternOptions::FromJsObject(env, args[1].As<Object>()); | | 268 | + if (!options) return; | | 269 | + } else { | | 270 | + THROW_ERR_INVALID_ARG_TYPE(env, | | 271 | + "second argument must be a string or object"); | | 272 | + return; | 255 | 273 | } | 256 | 274 | } | 257 | 275 |
| @@ -493,11 +511,8 @@ URLPattern::URLPatternOptions::FromJsObject(Environment* env, | 493 | 511 | Local<Value> ignore_case; | 494 | 512 | if (obj->Get(env->context(), env->ignore_case_string()) | 495 | 513 | .ToLocal(&ignore_case)) { | 496 | | - if (!ignore_case->IsBoolean()) { | 497 | | - THROW_ERR_INVALID_ARG_TYPE(env, "options.ignoreCase must be a boolean"); | 498 | | - return std::nullopt; | 499 | | - } | 500 | | - options.ignore_case = ignore_case->IsTrue(); | | 514 | + // Per WebIDL, boolean dictionary members are coerced (not type-checked). | | 515 | + options.ignore_case = ignore_case->BooleanValue(env->isolate()); | 501 | 516 | } else { | 502 | 517 | // If ToLocal returns false, the assumption is that getting the | 503 | 518 | // ignore_case_string threw an error, let's propagate that now | @@ -564,7 +579,7 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& args) { | 564 | 579 | ada::url_pattern_input input; | 565 | 580 | std::optional<std::string> baseURL{}; | 566 | 581 | std::string input_base; | 567 | | - if (args.Length() == 0) { | | 582 | + if (args.Length() == 0 || args[0]->IsNullOrUndefined()) { | 568 | 583 | input = ada::url_pattern_init{}; | 569 | 584 | } else if (args[0]->IsString()) { | 570 | 585 | Utf8Value input_value(env->isolate(), args[0].As<String>()); | @@ -580,13 +595,16 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& args) { | 580 | 595 | return; | 581 | 596 | } | 582 | 597 |
| 583 | | - if (args.Length() > 1) { | 584 | | - if (!args[1]->IsString()) { | | 598 | + if (args.Length() > 1 && !args[1]->IsUndefined()) { | | 599 | + if (args[1]->IsNull()) { | | 600 | + baseURL = std::string("null"); | | 601 | + } else if (args[1]->IsString()) { | | 602 | + Utf8Value base_url_value(env->isolate(), args[1].As<String>()); | | 603 | + baseURL = base_url_value.ToStringView(); | | 604 | + } else { | 585 | 605 | THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string"); | 586 | 606 | return; | 587 | 607 | } | 588 | | - Utf8Value base_url_value(env->isolate(), args[1].As<String>()); | 589 | | - baseURL = base_url_value.ToStringView(); | 590 | 608 | } | 591 | 609 |
| 592 | 610 | Local<Value> result; | @@ -607,7 +625,7 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& args) { | 607 | 625 | ada::url_pattern_input input; | 608 | 626 | std::optional<std::string> baseURL{}; | 609 | 627 | std::string input_base; | 610 | | - if (args.Length() == 0) { | | 628 | + if (args.Length() == 0 || args[0]->IsNullOrUndefined()) { | 611 | 629 | input = ada::url_pattern_init{}; | 612 | 630 | } else if (args[0]->IsString()) { | 613 | 631 | Utf8Value input_value(env->isolate(), args[0].As<String>()); | @@ -623,13 +641,16 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& args) { | 623 | 641 | return; | 624 | 642 | } | 625 | 643 |
| 626 | | - if (args.Length() > 1) { | 627 | | - if (!args[1]->IsString()) { | | 644 | + if (args.Length() > 1 && !args[1]->IsUndefined()) { | | 645 | + if (args[1]->IsNull()) { | | 646 | + baseURL = std::string("null"); | | 647 | + } else if (args[1]->IsString()) { | | 648 | + Utf8Value base_url_value(env->isolate(), args[1].As<String>()); | | 649 | + baseURL = base_url_value.ToStringView(); | | 650 | + } else { | 628 | 651 | THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string"); | 629 | 652 | return; | 630 | 653 | } | 631 | | - Utf8Value base_url_value(env->isolate(), args[1].As<String>()); | 632 | | - baseURL = base_url_value.ToStringView(); | 633 | 654 | } | 634 | 655 |
| 635 | 656 | std::optional<std::string_view> baseURL_opt = | |
Collapse file Copy file name to clipboardExpand all lines: test/fixtures/wpt/README.md +1-1Lines changed: 1 addition & 1 deletion - Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|
@@ -29,7 +29,7 @@ Last update: | 29 | 29 | - resources: https://github.com/web-platform-tests/wpt/tree/6a2f322376/resources | 30 | 30 | - streams: https://github.com/web-platform-tests/wpt/tree/bc9dcbbf1a/streams | 31 | 31 | - url: https://github.com/web-platform-tests/wpt/tree/7a3645b79a/url | 32 | | -- urlpattern: https://github.com/web-platform-tests/wpt/tree/a2e15ad405/urlpattern | | 32 | +- urlpattern: https://github.com/web-platform-tests/wpt/tree/f07c03cbed/urlpattern | 33 | 33 | - user-timing: https://github.com/web-platform-tests/wpt/tree/5ae85bf826/user-timing | 34 | 34 | - wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/65a2134d50/wasm/jsapi | 35 | 35 | - wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi | |
Collapse file Copy file name to clipboardExpand all lines: test/fixtures/wpt/urlpattern/resources/urlpatterntestdata.json +36Lines changed: 36 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 3118 | 3118 | "expected_match": { | 3119 | 3119 | "hostname": { "input": "localhost", "groups": { "domain" : "localhost"} } | 3120 | 3120 | } | | 3121 | + }, | | 3122 | + { | | 3123 | + "pattern": ["((?R)):"], | | 3124 | + "expected_obj": "error" | | 3125 | + }, | | 3126 | + { | | 3127 | + "pattern": ["(\\H):"], | | 3128 | + "expected_obj": "error" | | 3129 | + }, | | 3130 | + { | | 3131 | + "pattern": [ | | 3132 | + {"pathname": "/:foo((?<x>a))"} | | 3133 | + ], | | 3134 | + "inputs": [ | | 3135 | + {"pathname": "/a"} | | 3136 | + ], | | 3137 | + "expected_match": { | | 3138 | + "pathname": { | | 3139 | + "input": "/a", | | 3140 | + "groups": {"foo": "a"} | | 3141 | + } | | 3142 | + } | | 3143 | + }, | | 3144 | + { | | 3145 | + "pattern": [ | | 3146 | + {"pathname": "/foo/(bar(?<x>baz))"} | | 3147 | + ], | | 3148 | + "inputs": [ | | 3149 | + {"pathname": "/foo/barbaz"} | | 3150 | + ], | | 3151 | + "expected_match": { | | 3152 | + "pathname": { | | 3153 | + "input": "/foo/barbaz", | | 3154 | + "groups": {"0": "barbaz"} | | 3155 | + } | | 3156 | + } | 3121 | 3157 | } | 3122 | 3158 | ] |
Collapse file Copy file name to clipboard +1-5Lines changed: 1 addition & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | | -<!DOCTYPE html> | 2 | | -<script src="/resources/testharness.js"></script> | 3 | | -<script src="/resources/testharnessreport.js"></script> | 4 | | -<script> | | 1 | +// META: global=window,worker | 5 | 2 | test(() => { | 6 | 3 | assert_throws_js(TypeError, () => { new URLPattern(new URL('https://example.org/%(')); } ); | 7 | 4 | assert_throws_js(TypeError, () => { new URLPattern(new URL('https://example.org/%((')); } ); | | 11 | 8 | test(() => { | 12 | 9 | new URLPattern(undefined, undefined); | 13 | 10 | }, `Test constructor with undefined`); | 14 | | -</script> |
Collapse file Copy file name to clipboardExpand all lines: test/fixtures/wpt/versions.json +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
| 76 | 76 | "path": "url" | 77 | 77 | }, | 78 | 78 | "urlpattern": { | 79 | | - "commit": "a2e15ad40518c30c4e7f649584dbda699a40d531", | | 79 | + "commit": "f07c03cbede41ba677c3d26fd17ff3e02ba26783", | 80 | 80 | "path": "urlpattern" | 81 | 81 | }, | 82 | 82 | "user-timing": { | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-urlpattern-types.js +119-3Lines changed: 119 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -8,39 +8,155 @@ const assert = require('assert'); | 8 | 8 | // Verifies that calling URLPattern with no new keyword throws. | 9 | 9 | assert.throws(() => URLPattern(), { | 10 | 10 | code: 'ERR_CONSTRUCT_CALL_REQUIRED', | | 11 | + name: 'TypeError', | 11 | 12 | }); | 12 | 13 |
| 13 | 14 | // Verifies that type checks are performed on the arguments. | 14 | 15 | assert.throws(() => new URLPattern(1), { | 15 | 16 | code: 'ERR_INVALID_ARG_TYPE', | | 17 | + name: 'TypeError', | 16 | 18 | }); | 17 | 19 |
| 18 | 20 | assert.throws(() => new URLPattern({}, 1), { | 19 | 21 | code: 'ERR_INVALID_ARG_TYPE', | | 22 | + name: 'TypeError', | 20 | 23 | }); | 21 | 24 |
| 22 | 25 | assert.throws(() => new URLPattern({}, '', 1), { | 23 | 26 | code: 'ERR_INVALID_ARG_TYPE', | | 27 | + name: 'TypeError', | 24 | 28 | }); | 25 | 29 |
| 26 | | -assert.throws(() => new URLPattern({}, { ignoreCase: '' }), { | 27 | | - code: 'ERR_INVALID_ARG_TYPE', | 28 | | -}); | | 30 | +// Per WebIDL, ignoreCase is coerced to boolean (not type-checked). | | 31 | +{ | | 32 | + const p = new URLPattern({}, { ignoreCase: '' }); | | 33 | + assert.strictEqual(p.protocol, '*'); | | 34 | +} | | 35 | +{ | | 36 | + const p = new URLPattern({}, { ignoreCase: undefined }); | | 37 | + assert.strictEqual(p.protocol, '*'); | | 38 | +} | | 39 | +{ | | 40 | + const p = new URLPattern({}, {}); | | 41 | + assert.strictEqual(p.protocol, '*'); | | 42 | +} | 29 | 43 |
| 30 | 44 | const pattern = new URLPattern(); | 31 | 45 |
| 32 | 46 | assert.throws(() => pattern.exec(1), { | 33 | 47 | code: 'ERR_INVALID_ARG_TYPE', | | 48 | + name: 'TypeError', | 34 | 49 | }); | 35 | 50 |
| 36 | 51 | assert.throws(() => pattern.exec('', 1), { | 37 | 52 | code: 'ERR_INVALID_ARG_TYPE', | | 53 | + name: 'TypeError', | 38 | 54 | }); | 39 | 55 |
| 40 | 56 | assert.throws(() => pattern.test(1), { | 41 | 57 | code: 'ERR_INVALID_ARG_TYPE', | | 58 | + name: 'TypeError', | 42 | 59 | }); | 43 | 60 |
| 44 | 61 | assert.throws(() => pattern.test('', 1), { | 45 | 62 | code: 'ERR_INVALID_ARG_TYPE', | | 63 | + name: 'TypeError', | 46 | 64 | }); | | 65 | + | | 66 | +// Per WebIDL, undefined/null for a URLPatternInput (union including dictionary) | | 67 | +// uses the default value (empty URLPatternInit {}). | | 68 | + | | 69 | +// Constructor: undefined input should be treated as empty init. | | 70 | +{ | | 71 | + const p = new URLPattern(undefined); | | 72 | + assert.strictEqual(p.protocol, '*'); | | 73 | + assert.strictEqual(p.hostname, '*'); | | 74 | +} | | 75 | + | | 76 | +// Constructor: null input should be treated as empty init (union, dict branch). | | 77 | +{ | | 78 | + const p = new URLPattern(null); | | 79 | + assert.strictEqual(p.protocol, '*'); | | 80 | + assert.strictEqual(p.hostname, '*'); | | 81 | +} | | 82 | + | | 83 | +// Constructor: 2-arg with undefined/null uses overload 2 (options defaults). | | 84 | +{ | | 85 | + const p1 = new URLPattern(undefined, undefined); | | 86 | + assert.strictEqual(p1.protocol, '*'); | | 87 | + const p2 = new URLPattern(null, null); | | 88 | + assert.strictEqual(p2.protocol, '*'); | | 89 | + const p3 = new URLPattern({}, null); | | 90 | + assert.strictEqual(p3.protocol, '*'); | | 91 | + const p4 = new URLPattern('https://example.com', null); | | 92 | + assert.strictEqual(p4.hostname, 'example.com'); | | 93 | + const p5 = new URLPattern('https://example.com', undefined); | | 94 | + assert.strictEqual(p5.hostname, 'example.com'); | | 95 | +} | | 96 | + | | 97 | +// Constructor: valid input with undefined/null options. | | 98 | +{ | | 99 | + const p = new URLPattern({ pathname: '/foo' }, undefined); | | 100 | + assert.strictEqual(p.pathname, '/foo'); | | 101 | +} | | 102 | + | | 103 | +// Constructor: 3-arg with null/undefined baseURL is stringified per WebIDL, | | 104 | +// rejected as invalid URL by the parser. | | 105 | +assert.throws( | | 106 | + () => new URLPattern('https://example.com', null, null), | | 107 | + { code: 'ERR_INVALID_URL_PATTERN', name: 'TypeError' }, | | 108 | +); | | 109 | +assert.throws( | | 110 | + () => new URLPattern('https://example.com', undefined, undefined), | | 111 | + { code: 'ERR_INVALID_URL_PATTERN', name: 'TypeError' }, | | 112 | +); | | 113 | + | | 114 | +// Constructor: 3-arg with valid baseURL and null options uses defaults. | | 115 | +{ | | 116 | + const p = new URLPattern('https://example.com', 'https://example.com', null); | | 117 | + assert.strictEqual(p.hostname, 'example.com'); | | 118 | + const p2 = new URLPattern('https://example.com', 'https://example.com', undefined); | | 119 | + assert.strictEqual(p2.hostname, 'example.com'); | | 120 | +} | | 121 | + | | 122 | +// exec() and test(): undefined input should be treated as empty init. | | 123 | +{ | | 124 | + const p = new URLPattern(); | | 125 | + assert.strictEqual(p.test(undefined), true); | | 126 | + assert.strictEqual(p.test(undefined, undefined), true); | | 127 | + assert.notStrictEqual(p.exec(undefined), null); | | 128 | + assert.notStrictEqual(p.exec(undefined, undefined), null); | | 129 | +} | | 130 | + | | 131 | +// exec() and test(): null input should be treated as empty init. | | 132 | +{ | | 133 | + const p = new URLPattern(); | | 134 | + assert.strictEqual(p.test(null), true); | | 135 | + assert.notStrictEqual(p.exec(null), null); | | 136 | +} | | 137 | + | | 138 | +// exec() and test(): null for baseURL is stringified to "null" per WebIDL. | | 139 | +// With string input, "null" is not a valid base URL so match fails silently. | | 140 | +// With dict input, providing baseURL with a dict throws per spec. | | 141 | +{ | | 142 | + const p = new URLPattern(); | | 143 | + // String input + null baseURL: no throw, match returns null (false). | | 144 | + assert.strictEqual(p.test('https://example.com', null), false); | | 145 | + assert.strictEqual(p.exec('https://example.com', null), null); | | 146 | + // Dict input + null baseURL: throws (baseURL not allowed with dict input). | | 147 | + assert.throws(() => p.test(null, null), { | | 148 | + code: 'ERR_OPERATION_FAILED', | | 149 | + name: 'TypeError', | | 150 | + }); | | 151 | + assert.throws(() => p.exec(null, null), { | | 152 | + code: 'ERR_OPERATION_FAILED', | | 153 | + name: 'TypeError', | | 154 | + }); | | 155 | +} | | 156 | + | | 157 | +// exec() and test(): valid input with undefined baseURL. | | 158 | +{ | | 159 | + const p = new URLPattern({ protocol: 'https' }); | | 160 | + assert.strictEqual(p.test('https://example.com', undefined), true); | | 161 | + assert.notStrictEqual(p.exec('https://example.com', undefined), null); | | 162 | +} |
|