Skip to content

Commit 4e612c0

Browse files
panvapanva
authored
url: align default argument handling for URLPattern with webidl
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #62719 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent afb8bcd commit 4e612c0

6 files changed

Lines changed: 216 additions & 47 deletions

File tree

‎src/node_url_pattern.cc‎

Lines changed: 58 additions & 37 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,55 @@ 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, 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.
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 {
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");
238242
return;
239243
}
240244

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()) {
243248
if (!args[2]->IsObject()) {
244249
THROW_ERR_INVALID_ARG_TYPE(env, "options must be an object");
245250
return;
246251
}
247252
CHECK(!options.has_value());
248253
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;
255273
}
256274
}
257275

@@ -493,11 +511,8 @@ URLPattern::URLPatternOptions::FromJsObject(Environment* env,
493511
Local<Value> ignore_case;
494512
if (obj->Get(env->context(), env->ignore_case_string())
495513
.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());
501516
} else {
502517
// If ToLocal returns false, the assumption is that getting the
503518
// ignore_case_string threw an error, let's propagate that now
@@ -564,7 +579,7 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& args) {
564579
ada::url_pattern_input input;
565580
std::optional<std::string> baseURL{};
566581
std::string input_base;
567-
if (args.Length() == 0) {
582+
if (args.Length() == 0 || args[0]->IsNullOrUndefined()) {
568583
input = ada::url_pattern_init{};
569584
} else if (args[0]->IsString()) {
570585
Utf8Value input_value(env->isolate(), args[0].As<String>());
@@ -580,13 +595,16 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& args) {
580595
return;
581596
}
582597

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 {
585605
THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string");
586606
return;
587607
}
588-
Utf8Value base_url_value(env->isolate(), args[1].As<String>());
589-
baseURL = base_url_value.ToStringView();
590608
}
591609

592610
Local<Value> result;
@@ -607,7 +625,7 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& args) {
607625
ada::url_pattern_input input;
608626
std::optional<std::string> baseURL{};
609627
std::string input_base;
610-
if (args.Length() == 0) {
628+
if (args.Length() == 0 || args[0]->IsNullOrUndefined()) {
611629
input = ada::url_pattern_init{};
612630
} else if (args[0]->IsString()) {
613631
Utf8Value input_value(env->isolate(), args[0].As<String>());
@@ -623,13 +641,16 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& args) {
623641
return;
624642
}
625643

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 {
628651
THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string");
629652
return;
630653
}
631-
Utf8Value base_url_value(env->isolate(), args[1].As<String>());
632-
baseURL = base_url_value.ToStringView();
633654
}
634655

635656
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: 119 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,39 +8,155 @@ const assert = require('assert');
88
// Verifies that calling URLPattern with no new keyword throws.
99
assert.throws(() => URLPattern(), {
1010
code: 'ERR_CONSTRUCT_CALL_REQUIRED',
11+
name: 'TypeError',
1112
});
1213

1314
// Verifies that type checks are performed on the arguments.
1415
assert.throws(() => new URLPattern(1), {
1516
code: 'ERR_INVALID_ARG_TYPE',
17+
name: 'TypeError',
1618
});
1719

1820
assert.throws(() => new URLPattern({}, 1), {
1921
code: 'ERR_INVALID_ARG_TYPE',
22+
name: 'TypeError',
2023
});
2124

2225
assert.throws(() => new URLPattern({}, '', 1), {
2326
code: 'ERR_INVALID_ARG_TYPE',
27+
name: 'TypeError',
2428
});
2529

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+
}
2943

3044
const pattern = new URLPattern();
3145

3246
assert.throws(() => pattern.exec(1), {
3347
code: 'ERR_INVALID_ARG_TYPE',
48+
name: 'TypeError',
3449
});
3550

3651
assert.throws(() => pattern.exec('', 1), {
3752
code: 'ERR_INVALID_ARG_TYPE',
53+
name: 'TypeError',
3854
});
3955

4056
assert.throws(() => pattern.test(1), {
4157
code: 'ERR_INVALID_ARG_TYPE',
58+
name: 'TypeError',
4259
});
4360

4461
assert.throws(() => pattern.test('', 1), {
4562
code: 'ERR_INVALID_ARG_TYPE',
63+
name: 'TypeError',
4664
});
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+
}

0 commit comments

Comments
 (0)