Skip to content

url: fix URLPattern USVString coercion #64786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Archkon wants to merge 1 commit into
base: main
Choose a base branch
from
+163 −91
Open

url: fix URLPattern USVString coercion #64786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 37 additions & 71 deletions src/node_url_pattern.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ using v8::Signature;
using v8::String;
using v8::Value;

static std::optional<std::string> ToUSVString(Environment* env,
Local<Value> value) {
Local<String> string;
if (!value->ToString(env->context()).ToLocal(&string)) {
return std::nullopt;
}
return Utf8Value(env->isolate(), string).ToString();
}

std::optional<URLPatternRegexProvider::regex_type>
URLPatternRegexProvider::create_instance(std::string_view pattern,
bool ignore_case) {
Expand Down Expand Up @@ -206,41 +215,25 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& 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<Object>());
// If init does not have a value here, the implication is that an
// error was thrown. Let's allow that to be handled now by returning
// 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).
Expand All @@ -254,22 +247,15 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& 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<Object>());
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;
}
}

Expand Down Expand Up @@ -394,16 +380,16 @@ std::optional<ada::url_pattern_init> URLPattern::URLPatternInit::FromJsObject(
Local<Value> 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;
}
Expand Down Expand Up @@ -581,30 +567,20 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& 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<String>());
input_base = input_value.ToString();
input = std::string_view(input_base);
} else if (args[0]->IsObject()) {
auto maybeInput = URLPatternInit::FromJsObject(env, args[0].As<Object>());
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<String>());
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<Value> result;
Expand All @@ -627,30 +603,20 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& 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<String>());
input_base = input_value.ToString();
input = std::string_view(input_base);
} else if (args[0]->IsObject()) {
auto maybeInput = URLPatternInit::FromJsObject(env, args[0].As<Object>());
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<String>());
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<std::string_view> baseURL_opt =
Expand Down
146 changes: 126 additions & 20 deletions test/parallel/test-urlpattern-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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',
});

Expand All @@ -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 {}).
Expand Down Expand Up @@ -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();
Expand Down
Loading