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();