FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

url: improve invalid url performance · nodejs/node@7d91a73 · GitHub

/ node Public

Commit 7d91a73

Browse files
anonrig
authored and
ruyadorno
committed
url: improve invalid url performance
PR-URL: #49692 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 6db0d3d commit 7d91a73

8 files changed

Lines changed: 70 additions & 14 deletions

File tree

‎benchmark/url/whatwg-url-validity.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
const common = require('../common.js');
3+
const url = require('url');
4+
const URL = url.URL;
5+
6+
const bench = common.createBenchmark(main, {
7+
type: ['valid', 'invalid'],
8+
e: [1e5],
9+
});
10+
11+
// This benchmark is used to compare the `Invalid URL` path of the URL parser
12+
function main({ type, e }) {
13+
const url = type === 'valid' ? 'https://www.nodejs.org' : 'www.nodejs.org';
14+
bench.start();
15+
for (let i = 0; i < e; i++) {
16+
try {
17+
new URL(url);
18+
} catch {
19+
// do nothing
20+
}
21+
}
22+
bench.end(e);
23+
}

‎lib/internal/errors.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1370,8 +1370,13 @@ E('ERR_INVALID_SYNC_FORK_INPUT',
13701370
E('ERR_INVALID_THIS', 'Value of "this" must be of type %s', TypeError);
13711371
E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple', TypeError);
13721372
E('ERR_INVALID_URI', 'URI malformed', URIError);
1373-
E('ERR_INVALID_URL', function(input) {
1373+
E('ERR_INVALID_URL', function(input, base = null) {
13741374
this.input = input;
1375+
1376+
if (base != null) {
1377+
this.base = base;
1378+
}
1379+
13751380
// Don't include URL in message.
13761381
// (See https://github.com/nodejs/node/pull/38614)
13771382
return 'Invalid URL';

‎lib/internal/url.js‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -772,13 +772,7 @@ class URL {
772772
base = `${base}`;
773773
}
774774

775-
const href = bindingUrl.parse(input, base);
776-
777-
if (!href) {
778-
throw new ERR_INVALID_URL(input);
779-
}
780-
781-
this.#updateContext(href);
775+
this.#updateContext(bindingUrl.parse(input, base));
782776
}
783777

784778
[inspect.custom](depth, opts) {

‎src/env_properties.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
V(args_string, "args") \
5555
V(asn1curve_string, "asn1Curve") \
5656
V(async_ids_stack_string, "async_ids_stack") \
57+
V(base_string, "base") \
5758
V(bits_string, "bits") \
5859
V(block_list_string, "blockList") \
5960
V(buffer_string, "buffer") \

‎src/node_url.cc‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,35 @@ void BindingData::Format(const FunctionCallbackInfo<Value>& args) {
227227
.ToLocalChecked());
228228
}
229229

230+
void BindingData::ThrowInvalidURL(node::Environment* env,
231+
std::string_view input,
232+
std::optional<std::string> base) {
233+
Local<Value> err = ERR_INVALID_URL(env->isolate(), "Invalid URL");
234+
DCHECK(err->IsObject());
235+
236+
auto err_object = err.As<Object>();
237+
238+
USE(err_object->Set(env->context(),
239+
env->input_string(),
240+
v8::String::NewFromUtf8(env->isolate(),
241+
input.data(),
242+
v8::NewStringType::kNormal,
243+
input.size())
244+
.ToLocalChecked()));
245+
246+
if (base.has_value()) {
247+
USE(err_object->Set(env->context(),
248+
env->base_string(),
249+
v8::String::NewFromUtf8(env->isolate(),
250+
base.value().c_str(),
251+
v8::NewStringType::kNormal,
252+
base.value().size())
253+
.ToLocalChecked()));
254+
}
255+
256+
env->isolate()->ThrowException(err);
257+
}
258+
230259
void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
231260
CHECK_GE(args.Length(), 1);
232261
CHECK(args[0]->IsString()); // input
@@ -235,23 +264,24 @@ void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
235264
Realm* realm = Realm::GetCurrent(args);
236265
BindingData* binding_data = realm->GetBindingData<BindingData>();
237266
Isolate* isolate = realm->isolate();
267+
std::optional<std::string> base_{};
238268

239269
Utf8Value input(isolate, args[0]);
240270
ada::result<ada::url_aggregator> base;
241271
ada::url_aggregator* base_pointer = nullptr;
242272
if (args[1]->IsString()) {
243-
base =
244-
ada::parse<ada::url_aggregator>(Utf8Value(isolate, args[1]).ToString());
273+
base_ = Utf8Value(isolate, args[1]).ToString();
274+
base = ada::parse<ada::url_aggregator>(*base_);
245275
if (!base) {
246-
return args.GetReturnValue().Set(false);
276+
return ThrowInvalidURL(realm->env(), input.ToStringView(), base_);
247277
}
248278
base_pointer = &base.value();
249279
}
250280
auto out =
251281
ada::parse<ada::url_aggregator>(input.ToStringView(), base_pointer);
252282

253283
if (!out) {
254-
return args.GetReturnValue().Set(false);
284+
return ThrowInvalidURL(realm->env(), input.ToStringView(), base_);
255285
}
256286

257287
binding_data->UpdateComponents(out->get_components(), out->type);

‎src/node_url.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ class BindingData : public SnapshotableObject {
7676
const ada::scheme::type type);
7777

7878
static v8::CFunction fast_can_parse_methods_[];
79+
static void ThrowInvalidURL(Environment* env,
80+
std::string_view input,
81+
std::optional<std::string> base);
7982
};
8083

8184
std::string FromFilePath(const std::string_view file_path);

‎test/parallel/test-url-null-char.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ const assert = require('assert');
44

55
assert.throws(
66
() => { new URL('a\0b'); },
7-
{ input: 'a\0b' }
7+
{ code: 'ERR_INVALID_URL', input: 'a\0b' }
88
);

‎test/parallel/test-whatwg-url-custom-parsing.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ for (const test of failureTests) {
5555
() => new URL(test.input, test.base),
5656
(error) => {
5757
assert.throws(() => { throw error; }, expectedError);
58-
assert.strictEqual(`${error}`, 'TypeError [ERR_INVALID_URL]: Invalid URL');
58+
assert.strictEqual(`${error}`, 'TypeError: Invalid URL');
5959
assert.strictEqual(error.message, 'Invalid URL');
6060
return true;
6161
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL