Skip to content

Commit da14246

Browse files
panvapanva
committed
url: align default argument handling for URLPattern with webidl
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent dfe438d commit da14246

File tree

6 files changed

+196
-46
lines changed

6 files changed

+196
-46
lines changed

‎src/node_url_pattern.cc‎

Lines changed: 58 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,11 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& args) {
202202
// - new URLPattern(input, baseURL)
203203
// - new URLPattern(input, options)
204204
// - 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()) {
206210
BufferValue input_buffer(env->isolate(), args[0]);
207211
CHECK_NOT_NULL(*input_buffer);
208212
input = input_buffer.ToString();
@@ -217,41 +221,56 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& args) {
217221
return;
218222
}
219223

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 → overload 1 (baseURL),
227+
// else → 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.
222232
if (args[1]->IsString()) {
223233
BufferValue base_url_buffer(env->isolate(), args[1]);
224234
CHECK_NOT_NULL(*base_url_buffer);
225235
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");
235240
} else {
236241
THROW_ERR_INVALID_ARG_TYPE(env,
237-
"second argument must be a string or object");
242+
"second argument must be a string");
238243
return;
239244
}
240245

241-
// Only remaining argument can be options.
242-
if (args.Length() > 2) {
246+
// arg2 is options. Per WebIDL, null/undefined for a dictionary
247+
// uses the default value (empty dict).
248+
if (!args[2]->IsNullOrUndefined()) {
243249
if (!args[2]->IsObject()) {
244250
THROW_ERR_INVALID_ARG_TYPE(env, "options must be an object");
245251
return;
246252
}
247253
CHECK(!options.has_value());
248254
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-
}
255+
if (!options) return;
256+
}
257+
} else if (args.Length() == 2) {
258+
// Overload resolution: string → overload 1 (baseURL),
259+
// else → overload 2 (options).
260+
if (args[1]->IsString()) {
261+
BufferValue base_url_buffer(env->isolate(), args[1]);
262+
CHECK_NOT_NULL(*base_url_buffer);
263+
base_url = base_url_buffer.ToString();
264+
} else if (args[1]->IsNullOrUndefined()) {
265+
// Overload 2, options uses default → skip
266+
} else if (args[1]->IsObject()) {
267+
CHECK(!options.has_value());
268+
options = URLPatternOptions::FromJsObject(env, args[1].As<Object>());
269+
if (!options) return;
270+
} else {
271+
THROW_ERR_INVALID_ARG_TYPE(env,
272+
"second argument must be a string or object");
273+
return;
255274
}
256275
}
257276

@@ -493,11 +512,8 @@ URLPattern::URLPatternOptions::FromJsObject(Environment* env,
493512
Local<Value> ignore_case;
494513
if (obj->Get(env->context(), env->ignore_case_string())
495514
.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();
515+
// Per WebIDL, boolean dictionary members are coerced (not type-checked).
516+
options.ignore_case = ignore_case->BooleanValue(env->isolate());
501517
} else {
502518
// If ToLocal returns false, the assumption is that getting the
503519
// ignore_case_string threw an error, let's propagate that now
@@ -564,7 +580,7 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& args) {
564580
ada::url_pattern_input input;
565581
std::optional<std::string> baseURL{};
566582
std::string input_base;
567-
if (args.Length() == 0) {
583+
if (args.Length() == 0 || args[0]->IsNullOrUndefined()) {
568584
input = ada::url_pattern_init{};
569585
} else if (args[0]->IsString()) {
570586
Utf8Value input_value(env->isolate(), args[0].As<String>());
@@ -580,13 +596,16 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& args) {
580596
return;
581597
}
582598

583-
if (args.Length() > 1) {
584-
if (!args[1]->IsString()) {
599+
if (args.Length() > 1 && !args[1]->IsUndefined()) {
600+
if (args[1]->IsNull()) {
601+
baseURL = std::string("null");
602+
} else if (args[1]->IsString()) {
603+
Utf8Value base_url_value(env->isolate(), args[1].As<String>());
604+
baseURL = base_url_value.ToStringView();
605+
} else {
585606
THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string");
586607
return;
587608
}
588-
Utf8Value base_url_value(env->isolate(), args[1].As<String>());
589-
baseURL = base_url_value.ToStringView();
590609
}
591610

592611
Local<Value> result;
@@ -607,7 +626,7 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& args) {
607626
ada::url_pattern_input input;
608627
std::optional<std::string> baseURL{};
609628
std::string input_base;
610-
if (args.Length() == 0) {
629+
if (args.Length() == 0 || args[0]->IsNullOrUndefined()) {
611630
input = ada::url_pattern_init{};
612631
} else if (args[0]->IsString()) {
613632
Utf8Value input_value(env->isolate(), args[0].As<String>());
@@ -623,13 +642,16 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& args) {
623642
return;
624643
}
625644

626-
if (args.Length() > 1) {
627-
if (!args[1]->IsString()) {
645+
if (args.Length() > 1 && !args[1]->IsUndefined()) {
646+
if (args[1]->IsNull()) {
647+
baseURL = std::string("null");
648+
} else if (args[1]->IsString()) {
649+
Utf8Value base_url_value(env->isolate(), args[1].As<String>());
650+
baseURL = base_url_value.ToStringView();
651+
} else {
628652
THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string");
629653
return;
630654
}
631-
Utf8Value base_url_value(env->isolate(), args[1].As<String>());
632-
baseURL = base_url_value.ToStringView();
633655
}
634656

635657
std::optional<std::string_view> baseURL_opt =

‎test/fixtures/wpt/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Last update:
2929
- resources: https://github.com/web-platform-tests/wpt/tree/6a2f322376/resources
3030
- streams: https://github.com/web-platform-tests/wpt/tree/bc9dcbbf1a/streams
3131
- 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
3333
- user-timing: https://github.com/web-platform-tests/wpt/tree/5ae85bf826/user-timing
3434
- wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/65a2134d50/wasm/jsapi
3535
- wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi

‎test/fixtures/wpt/urlpattern/resources/urlpatterntestdata.json‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3118,5 +3118,41 @@
31183118
"expected_match": {
31193119
"hostname": { "input": "localhost", "groups": { "domain" : "localhost"} }
31203120
}
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+
}
31213157
}
31223158
]

test/fixtures/wpt/urlpattern/urlpattern-constructor.html renamed to test/fixtures/wpt/urlpattern/urlpattern-constructor.any.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
<!DOCTYPE html>
2-
<script src="/resources/testharness.js"></script>
3-
<script src="/resources/testharnessreport.js"></script>
4-
<script>
1+
// META: global=window,worker
52
test(() => {
63
assert_throws_js(TypeError, () => { new URLPattern(new URL('https://example.org/%(')); } );
74
assert_throws_js(TypeError, () => { new URLPattern(new URL('https://example.org/%((')); } );
@@ -11,4 +8,3 @@
118
test(() => {
129
new URLPattern(undefined, undefined);
1310
}, `Test constructor with undefined`);
14-
</script>

‎test/fixtures/wpt/versions.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676
"path": "url"
7777
},
7878
"urlpattern": {
79-
"commit": "a2e15ad40518c30c4e7f649584dbda699a40d531",
79+
"commit": "f07c03cbede41ba677c3d26fd17ff3e02ba26783",
8080
"path": "urlpattern"
8181
},
8282
"user-timing": {

‎test/parallel/test-urlpattern-types.js‎

Lines changed: 99 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,19 @@ assert.throws(() => new URLPattern({}, '', 1), {
2323
code: 'ERR_INVALID_ARG_TYPE',
2424
});
2525

26-
assert.throws(() => new URLPattern({}, { ignoreCase: '' }), {
27-
code: 'ERR_INVALID_ARG_TYPE',
28-
});
26+
// Per WebIDL, ignoreCase is coerced to boolean (not type-checked).
27+
{
28+
const p = new URLPattern({}, { ignoreCase: '' });
29+
assert.strictEqual(p.protocol, '*');
30+
}
31+
{
32+
const p = new URLPattern({}, { ignoreCase: undefined });
33+
assert.strictEqual(p.protocol, '*');
34+
}
35+
{
36+
const p = new URLPattern({}, {});
37+
assert.strictEqual(p.protocol, '*');
38+
}
2939

3040
const pattern = new URLPattern();
3141

@@ -44,3 +54,89 @@ assert.throws(() => pattern.test(1), {
4454
assert.throws(() => pattern.test('', 1), {
4555
code: 'ERR_INVALID_ARG_TYPE',
4656
});
57+
58+
// Per WebIDL, undefined/null for a URLPatternInput (union including dictionary)
59+
// uses the default value (empty URLPatternInit {}).
60+
61+
// Constructor: undefined input should be treated as empty init.
62+
{
63+
const p = new URLPattern(undefined);
64+
assert.strictEqual(p.protocol, '*');
65+
assert.strictEqual(p.hostname, '*');
66+
}
67+
68+
// Constructor: null input should be treated as empty init (union, dict branch).
69+
{
70+
const p = new URLPattern(null);
71+
assert.strictEqual(p.protocol, '*');
72+
assert.strictEqual(p.hostname, '*');
73+
}
74+
75+
// Constructor: 2-arg with undefined/null uses overload 2 (options defaults).
76+
{
77+
const p1 = new URLPattern(undefined, undefined);
78+
assert.strictEqual(p1.protocol, '*');
79+
const p2 = new URLPattern(null, null);
80+
assert.strictEqual(p2.protocol, '*');
81+
const p3 = new URLPattern({}, null);
82+
assert.strictEqual(p3.protocol, '*');
83+
const p4 = new URLPattern('https://example.com', null);
84+
assert.strictEqual(p4.hostname, 'example.com');
85+
const p5 = new URLPattern('https://example.com', undefined);
86+
assert.strictEqual(p5.hostname, 'example.com');
87+
}
88+
89+
// Constructor: valid input with undefined/null options.
90+
{
91+
const p = new URLPattern({ pathname: '/foo' }, undefined);
92+
assert.strictEqual(p.pathname, '/foo');
93+
}
94+
95+
// Constructor: 3-arg with null/undefined baseURL is stringified per WebIDL,
96+
// rejected as invalid URL by the parser.
97+
assert.throws(() => new URLPattern('https://example.com', null, null));
98+
assert.throws(() => new URLPattern('https://example.com', undefined, undefined));
99+
100+
// Constructor: 3-arg with valid baseURL and null options uses defaults.
101+
{
102+
const p = new URLPattern('https://example.com', 'https://example.com', null);
103+
assert.strictEqual(p.hostname, 'example.com');
104+
const p2 = new URLPattern('https://example.com', 'https://example.com', undefined);
105+
assert.strictEqual(p2.hostname, 'example.com');
106+
}
107+
108+
// exec() and test(): undefined input should be treated as empty init.
109+
{
110+
const p = new URLPattern();
111+
assert.strictEqual(p.test(undefined), true);
112+
assert.strictEqual(p.test(undefined, undefined), true);
113+
assert.notStrictEqual(p.exec(undefined), null);
114+
assert.notStrictEqual(p.exec(undefined, undefined), null);
115+
}
116+
117+
// exec() and test(): null input should be treated as empty init.
118+
{
119+
const p = new URLPattern();
120+
assert.strictEqual(p.test(null), true);
121+
assert.notStrictEqual(p.exec(null), null);
122+
}
123+
124+
// exec() and test(): null for baseURL is stringified to "null" per WebIDL.
125+
// With string input, "null" is not a valid base URL so match fails silently.
126+
// With dict input, providing baseURL with a dict throws per spec.
127+
{
128+
const p = new URLPattern();
129+
// String input + null baseURL: no throw, match returns null (false).
130+
assert.strictEqual(p.test('https://example.com', null), false);
131+
assert.strictEqual(p.exec('https://example.com', null), null);
132+
// Dict input + null baseURL: throws (baseURL not allowed with dict input).
133+
assert.throws(() => p.test(null, null));
134+
assert.throws(() => p.exec(null, null));
135+
}
136+
137+
// exec() and test(): valid input with undefined baseURL.
138+
{
139+
const p = new URLPattern({ protocol: 'https' });
140+
assert.strictEqual(p.test('https://example.com', undefined), true);
141+
assert.notStrictEqual(p.exec('https://example.com', undefined), null);
142+
}

0 commit comments

Comments
 (0)