Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fd3bf38

Browse filesBrowse files
panvaaduh95
panva
authored and
aduh95
committed
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 c73c8e6 commit fd3bf38
Copy full SHA for fd3bf38

6 files changed

+216-47Lines changed: 216 additions & 47 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/node_url_pattern.cc‎

Copy file name to clipboardExpand all lines: src/node_url_pattern.cc
+58-37Lines 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 =
Collapse file

‎test/fixtures/wpt/README.md‎

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 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
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/wpt/urlpattern/resources/urlpatterntestdata.json
+36Lines 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
]
Collapse file

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

Copy file name to clipboard
+1-5Lines 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>
Collapse file

‎test/fixtures/wpt/versions.json‎

Copy file name to clipboardExpand all lines: test/fixtures/wpt/versions.json
+1-1Lines 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": {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-urlpattern-types.js
+119-3Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.