From 58d07376184a730ff0a4edc7c4d8990b8659429a Mon Sep 17 00:00:00 2001 From: Archkon <180910180+Archkon@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:27:01 +0800 Subject: [PATCH] url: fix URLPattern USVString coercion Apply WebIDL USVString conversion after resolving URLPattern overloads and union branches. Coerce primitive URLPatternInput values through the string branch and coerce baseURL values in the constructor, exec(), and test(). Keep objects on the URLPatternInit or options dictionary branches where required, and propagate conversion exceptions unchanged. Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com> --- src/node_url_pattern.cc | 108 +++++++----------- test/parallel/test-urlpattern-types.js | 146 +++++++++++++++++++++---- 2 files changed, 163 insertions(+), 91 deletions(-) diff --git a/src/node_url_pattern.cc b/src/node_url_pattern.cc index 571413b47da12d..9c8090759e00b3 100644 --- a/src/node_url_pattern.cc +++ b/src/node_url_pattern.cc @@ -73,6 +73,15 @@ using v8::Signature; using v8::String; using v8::Value; +static std::optional ToUSVString(Environment* env, + Local value) { + Local string; + if (!value->ToString(env->context()).ToLocal(&string)) { + return std::nullopt; + } + return Utf8Value(env->isolate(), string).ToString(); +} + std::optional URLPatternRegexProvider::create_instance(std::string_view pattern, bool ignore_case) { @@ -206,10 +215,6 @@ void URLPattern::New(const FunctionCallbackInfo& args) { // uses the default value (empty init). if (args[0]->IsNullOrUndefined()) { init = ada::url_pattern_init{}; - } else if (args[0]->IsString()) { - BufferValue input_buffer(env->isolate(), args[0]); - CHECK_NOT_NULL(*input_buffer); - input = input_buffer.ToString(); } else if (args[0]->IsObject()) { init = URLPatternInit::FromJsObject(env, args[0].As()); // If init does not have a value here, the implication is that an @@ -217,30 +222,18 @@ void URLPattern::New(const FunctionCallbackInfo& args) { // early. If we don't, the error thrown will be swallowed. if (!init) return; } else { - THROW_ERR_INVALID_ARG_TYPE(env, "Input must be an object or a string"); - return; + input = ToUSVString(env, args[0]); + if (!input) return; } // Per WebIDL overload resolution: // With 3+ args, it's always overload 1: (input, baseURL, options) - // With 2 args, if arg1 is string, it is overload 1 (baseURL), - // otherwise overload 2 (options) + // With 2 args, primitive values other than null and undefined select + // overload 1 (baseURL); objects, null, and undefined select overload 2 + // (options). if (args.Length() >= 3) { - // arg1 is baseURL. Per WebIDL, null/undefined are stringified for - // USVString ("null"/"undefined"), which will be rejected as invalid - // URLs by ada downstream. - if (args[1]->IsString()) { - BufferValue base_url_buffer(env->isolate(), args[1]); - CHECK_NOT_NULL(*base_url_buffer); - base_url = base_url_buffer.ToString(); - } else if (args[1]->IsNull()) { - base_url = std::string("null"); - } else if (args[1]->IsUndefined()) { - base_url = std::string("undefined"); - } else { - THROW_ERR_INVALID_ARG_TYPE(env, "second argument must be a string"); - return; - } + base_url = ToUSVString(env, args[1]); + if (!base_url) return; // arg2 is options. Per WebIDL, null/undefined for a dictionary // uses the default value (empty dict). @@ -254,22 +247,15 @@ void URLPattern::New(const FunctionCallbackInfo& args) { if (!options) return; } } else if (args.Length() == 2) { - // Overload resolution: string is overload 1 (baseURL), - // otherwise overload 2 (options). - if (args[1]->IsString()) { - BufferValue base_url_buffer(env->isolate(), args[1]); - CHECK_NOT_NULL(*base_url_buffer); - base_url = base_url_buffer.ToString(); - } else if (args[1]->IsNullOrUndefined()) { + if (args[1]->IsNullOrUndefined()) { // Overload 2, options uses default. } else if (args[1]->IsObject()) { CHECK(!options.has_value()); options = URLPatternOptions::FromJsObject(env, args[1].As()); if (!options) return; } else { - THROW_ERR_INVALID_ARG_TYPE(env, - "second argument must be a string or object"); - return; + base_url = ToUSVString(env, args[1]); + if (!base_url) return; } } @@ -394,16 +380,16 @@ std::optional URLPattern::URLPatternInit::FromJsObject( Local value; for (const auto& component : components) { Utf8Value key(isolate, component); - if (obj->Get(env->context(), component).ToLocal(&value)) { - if (value->IsString()) { - Utf8Value utf8_value(isolate, value); - set_parameter(key.ToStringView(), utf8_value.ToStringView()); - } - } else { + if (!obj->Get(env->context(), component).ToLocal(&value)) { // If ToLocal failed then we assume an error occurred, // bail out early to propagate the error. return std::nullopt; } + if (value->IsUndefined()) continue; + + auto converted = ToUSVString(env, value); + if (!converted) return std::nullopt; + set_parameter(key.ToStringView(), *converted); } return init; } @@ -581,30 +567,20 @@ void URLPattern::Exec(const FunctionCallbackInfo& args) { std::string input_base; if (args.Length() == 0 || args[0]->IsNullOrUndefined()) { input = ada::url_pattern_init{}; - } else if (args[0]->IsString()) { - Utf8Value input_value(env->isolate(), args[0].As()); - input_base = input_value.ToString(); - input = std::string_view(input_base); } else if (args[0]->IsObject()) { auto maybeInput = URLPatternInit::FromJsObject(env, args[0].As()); if (!maybeInput.has_value()) return; input = std::move(*maybeInput); } else { - THROW_ERR_INVALID_ARG_TYPE( - env, "URLPattern input needs to be a string or an object"); - return; + auto input_value = ToUSVString(env, args[0]); + if (!input_value) return; + input_base = std::move(*input_value); + input = std::string_view(input_base); } if (args.Length() > 1 && !args[1]->IsUndefined()) { - if (args[1]->IsNull()) { - baseURL = std::string("null"); - } else if (args[1]->IsString()) { - Utf8Value base_url_value(env->isolate(), args[1].As()); - baseURL = base_url_value.ToStringView(); - } else { - THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string"); - return; - } + baseURL = ToUSVString(env, args[1]); + if (!baseURL) return; } Local result; @@ -627,30 +603,20 @@ void URLPattern::Test(const FunctionCallbackInfo& args) { std::string input_base; if (args.Length() == 0 || args[0]->IsNullOrUndefined()) { input = ada::url_pattern_init{}; - } else if (args[0]->IsString()) { - Utf8Value input_value(env->isolate(), args[0].As()); - input_base = input_value.ToString(); - input = std::string_view(input_base); } else if (args[0]->IsObject()) { auto maybeInput = URLPatternInit::FromJsObject(env, args[0].As()); if (!maybeInput.has_value()) return; input = std::move(*maybeInput); } else { - THROW_ERR_INVALID_ARG_TYPE( - env, "URLPattern input needs to be a string or an object"); - return; + auto input_value = ToUSVString(env, args[0]); + if (!input_value) return; + input_base = std::move(*input_value); + input = std::string_view(input_base); } if (args.Length() > 1 && !args[1]->IsUndefined()) { - if (args[1]->IsNull()) { - baseURL = std::string("null"); - } else if (args[1]->IsString()) { - Utf8Value base_url_value(env->isolate(), args[1].As()); - baseURL = base_url_value.ToStringView(); - } else { - THROW_ERR_INVALID_ARG_TYPE(env, "baseURL must be a string"); - return; - } + baseURL = ToUSVString(env, args[1]); + if (!baseURL) return; } std::optional baseURL_opt = diff --git a/test/parallel/test-urlpattern-types.js b/test/parallel/test-urlpattern-types.js index 095d1bfb3ec467..a9e25f108e69b2 100644 --- a/test/parallel/test-urlpattern-types.js +++ b/test/parallel/test-urlpattern-types.js @@ -2,7 +2,7 @@ require('../common'); -const { URLPattern } = require('url'); +const { URL, URLPattern } = require('url'); const assert = require('assert'); // Verifies that calling URLPattern with no new keyword throws. @@ -11,14 +11,14 @@ assert.throws(() => URLPattern(), { name: 'TypeError', }); -// Verifies that type checks are performed on the arguments. +// A primitive URLPatternInput is converted to USVString before parsing. assert.throws(() => new URLPattern(1), { - code: 'ERR_INVALID_ARG_TYPE', + code: 'ERR_INVALID_URL_PATTERN', name: 'TypeError', }); assert.throws(() => new URLPattern({}, 1), { - code: 'ERR_INVALID_ARG_TYPE', + code: 'ERR_INVALID_URL_PATTERN', name: 'TypeError', }); @@ -43,25 +43,62 @@ assert.throws(() => new URLPattern({}, '', 1), { const pattern = new URLPattern(); -assert.throws(() => pattern.exec(1), { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError', -}); +// Primitive input and baseURL values behave like their USVString conversions. +assert.deepStrictEqual(pattern.exec(1), pattern.exec('1')); +assert.deepStrictEqual(pattern.exec('', 1), pattern.exec('', '1')); +assert.strictEqual(pattern.test(1), pattern.test('1')); +assert.strictEqual(pattern.test('', 1), pattern.test('', '1')); -assert.throws(() => pattern.exec('', 1), { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError', -}); +// Primitive URLPatternInput values select the USVString union branch. +{ + const baseURL = 'https://example/'; + const p = new URLPattern(123, baseURL); + assert.strictEqual(p.pathname, '/123'); -assert.throws(() => pattern.test(1), { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError', -}); + const result = p.exec(123, baseURL); + assert.notStrictEqual(result, null); + assert.strictEqual(result.inputs[0], '123'); + assert.strictEqual(p.test(123, baseURL), true); +} -assert.throws(() => pattern.test('', 1), { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError', -}); +// Present URLPatternInit members are converted to USVString. +{ + const p = new URLPattern({ pathname: 123 }); + assert.strictEqual(p.pathname, '123'); + assert.strictEqual(p.test({ pathname: 123 }), true); + assert.strictEqual(p.test({ pathname: 456 }), false); + + const result = p.exec({ pathname: 123 }); + assert.notStrictEqual(result, null); + assert.strictEqual(result.inputs[0].pathname, '123'); +} + +// Only undefined URLPatternInit members are treated as absent. +{ + const undefinedPathname = new URLPattern({ pathname: undefined }); + assert.strictEqual(undefinedPathname.pathname, '*'); + + const nullPathname = new URLPattern({ pathname: null }); + assert.strictEqual(nullPathname.pathname, 'null'); +} + +// URLPatternInit member conversion exceptions are propagated unchanged. +{ + const error = new Error('boom'); + const input = { + pathname: { + toString() { + throw error; + }, + }, + }; + const p = new URLPattern({ pathname: '123' }); + const isExpectedError = (actual) => actual === error; + + assert.throws(() => new URLPattern(input), isExpectedError); + assert.throws(() => p.exec(input), isExpectedError); + assert.throws(() => p.test(input), isExpectedError); +} // Per WebIDL, undefined/null for a URLPatternInput (union including dictionary) // uses the default value (empty URLPatternInit {}). @@ -119,6 +156,75 @@ assert.throws( assert.strictEqual(p2.hostname, 'example.com'); } +// Constructor: baseURL is converted to USVString after overload resolution. +{ + let calls = 0; + const baseURL = { + toString() { + calls++; + return 'https://example.com/'; + }, + }; + const p = new URLPattern('foo', baseURL, {}); + assert.strictEqual(calls, 1); + assert.strictEqual(p.protocol, 'https'); + assert.strictEqual(p.hostname, 'example.com'); + assert.strictEqual(p.pathname, '/foo'); +} + +// exec() and test(): baseURL accepts string-convertible objects. +{ + const p = new URLPattern('https://example.com/foo'); + const baseURL = new URL('https://example.com/'); + assert.notStrictEqual(p.exec('foo', baseURL), null); + assert.strictEqual(p.test('foo', baseURL), true); +} + +// Exceptions thrown while converting baseURL are propagated unchanged. +{ + const error = new Error('boom'); + const baseURL = { + toString() { + throw error; + }, + }; + const isExpectedError = (actual) => actual === error; + + assert.throws( + () => new URLPattern('foo', baseURL, {}), + isExpectedError, + ); + assert.throws(() => pattern.exec('foo', baseURL), isExpectedError); + assert.throws(() => pattern.test('foo', baseURL), isExpectedError); +} + +// Symbol conversion throws the native TypeError required by USVString. +{ + const symbol = Symbol(); + const isUncodedTypeError = (error) => + error instanceof TypeError && error.code === undefined; + + assert.throws( + () => new URLPattern(symbol, 'https://example/'), + isUncodedTypeError, + ); + assert.throws( + () => pattern.exec(symbol, 'https://example/'), + isUncodedTypeError, + ); + assert.throws( + () => pattern.test(symbol, 'https://example/'), + isUncodedTypeError, + ); + + assert.throws( + () => new URLPattern('foo', symbol, {}), + isUncodedTypeError, + ); + assert.throws(() => pattern.exec('foo', symbol), isUncodedTypeError); + assert.throws(() => pattern.test('foo', symbol), isUncodedTypeError); +} + // exec() and test(): undefined input should be treated as empty init. { const p = new URLPattern();