Skip to content

Commit

Permalink
url: improve invalid url performance
Browse files Browse the repository at this point in the history
  • Loading branch information
anonrig committed Sep 19, 2023
1 parent 346abdd commit 29267d9
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 14 deletions.
23 changes: 23 additions & 0 deletions benchmark/url/whatwg-url-validity.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common.js');
const url = require('url');
const URL = url.URL;

const bench = common.createBenchmark(main, {
type: ['valid', 'invalid'],
e: [1e5],
});

// This benchmark is used to compare the `Invalid URL` path of the URL parser
function main({ type, e }) {
const url = type === 'valid' ? 'https://www.nodejs.org' : 'www.nodejs.org';
bench.start();
for (let i = 0; i < e; i++) {
try {
new URL(url);
} catch {
// do nothing
}
}
bench.end(e);
}
7 changes: 6 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1370,8 +1370,13 @@ E('ERR_INVALID_SYNC_FORK_INPUT',
E('ERR_INVALID_THIS', 'Value of "this" must be of type %s', TypeError);
E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple', TypeError);
E('ERR_INVALID_URI', 'URI malformed', URIError);
E('ERR_INVALID_URL', function(input) {
E('ERR_INVALID_URL', function(input, base) {
this.input = input;

if (base != null) {
this.base = base;
}

// Don't include URL in message.
// (See https://github.com/nodejs/node/pull/38614)
return 'Invalid URL';
Expand Down
8 changes: 1 addition & 7 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -780,13 +780,7 @@ class URL {
base = `${base}`;
}

const href = bindingUrl.parse(input, base);

if (!href) {
throw new ERR_INVALID_URL(input);
}

this.#updateContext(href);
this.#updateContext(bindingUrl.parse(input, base));
}

[inspect.custom](depth, opts) {
Expand Down
1 change: 1 addition & 0 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
V(args_string, "args") \
V(asn1curve_string, "asn1Curve") \
V(async_ids_stack_string, "async_ids_stack") \
V(base_string, "base") \
V(bits_string, "bits") \
V(block_list_string, "blockList") \
V(buffer_string, "buffer") \
Expand Down
38 changes: 34 additions & 4 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,35 @@ void BindingData::Format(const FunctionCallbackInfo<Value>& args) {
.ToLocalChecked());
}

void BindingData::ThrowInvalidURL(node::Environment* env,
std::string_view input,
std::optional<std::string> base) {
Local<Value> err = ERR_INVALID_URL(env->isolate(), "Invalid URL");
DCHECK(err->IsObject());

auto err_object = err.As<Object>();

USE(err_object->Set(env->context(),
env->input_string(),
v8::String::NewFromUtf8(env->isolate(),
input.data(),
v8::NewStringType::kNormal,
input.size())
.ToLocalChecked()));

if (base.has_value()) {
USE(err_object->Set(env->context(),
env->base_string(),
v8::String::NewFromUtf8(env->isolate(),
base.value().c_str(),
v8::NewStringType::kNormal,
base.value().size())
.ToLocalChecked()));
}

env->isolate()->ThrowException(err);
}

void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
CHECK_GE(args.Length(), 1);
CHECK(args[0]->IsString()); // input
Expand All @@ -235,23 +264,24 @@ void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
Realm* realm = Realm::GetCurrent(args);
BindingData* binding_data = realm->GetBindingData<BindingData>();
Isolate* isolate = realm->isolate();
std::optional<std::string> base_{};

Utf8Value input(isolate, args[0]);
ada::result<ada::url_aggregator> base;
ada::url_aggregator* base_pointer = nullptr;
if (args[1]->IsString()) {
base =
ada::parse<ada::url_aggregator>(Utf8Value(isolate, args[1]).ToString());
base_ = Utf8Value(isolate, args[1]).ToString();
base = ada::parse<ada::url_aggregator>(*base_);
if (!base) {
return args.GetReturnValue().Set(false);
return ThrowInvalidURL(realm->env(), input.ToStringView(), base_);
}
base_pointer = &base.value();
}
auto out =
ada::parse<ada::url_aggregator>(input.ToStringView(), base_pointer);

if (!out) {
return args.GetReturnValue().Set(false);
return ThrowInvalidURL(realm->env(), input.ToStringView(), base_);
}

binding_data->UpdateComponents(out->get_components(), out->type);
Expand Down
3 changes: 3 additions & 0 deletions src/node_url.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ class BindingData : public SnapshotableObject {
const ada::scheme::type type);

static v8::CFunction fast_can_parse_methods_[];
static void ThrowInvalidURL(Environment* env,
std::string_view input,
std::optional<std::string> base);
};

std::string FromFilePath(const std::string_view file_path);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-url-null-char.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ const assert = require('assert');

assert.throws(
() => { new URL('a\0b'); },
{ input: 'a\0b' }
{ code: 'ERR_INVALID_URL', input: 'a\0b' }
);
2 changes: 1 addition & 1 deletion test/parallel/test-whatwg-url-custom-parsing.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ for (const test of failureTests) {
() => new URL(test.input, test.base),
(error) => {
assert.throws(() => { throw error; }, expectedError);
assert.strictEqual(`${error}`, 'TypeError [ERR_INVALID_URL]: Invalid URL');
assert.strictEqual(`${error}`, 'TypeError: Invalid URL');
assert.strictEqual(error.message, 'Invalid URL');
return true;
});
Expand Down

0 comments on commit 29267d9

Please sign in to comment.