GeezerVison is showing github.com — url: improve invalid url performance · nodejs/node@097b598
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboard +23Lines changed: 23 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 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 | +} |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/errors.js +6-1Lines changed: 6 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -1370,8 +1370,13 @@ E('ERR_INVALID_SYNC_FORK_INPUT', | 1370 | 1370 | E('ERR_INVALID_THIS', 'Value of "this" must be of type %s', TypeError); | 1371 | 1371 | E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple', TypeError); | 1372 | 1372 | E('ERR_INVALID_URI', 'URI malformed', URIError); | 1373 | | -E('ERR_INVALID_URL', function(input) { | | 1373 | +E('ERR_INVALID_URL', function(input, base = null) { | 1374 | 1374 | this.input = input; | | 1375 | + | | 1376 | + if (base != null) { | | 1377 | + this.base = base; | | 1378 | + } | | 1379 | + | 1375 | 1380 | // Don't include URL in message. | 1376 | 1381 | // (See https://github.com/nodejs/node/pull/38614) | 1377 | 1382 | return 'Invalid URL'; | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/url.js +1-7Lines changed: 1 addition & 7 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -772,13 +772,7 @@ class URL { | 772 | 772 | base = `${base}`; | 773 | 773 | } | 774 | 774 |
| 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)); | 782 | 776 | } | 783 | 777 |
| 784 | 778 | [inspect.custom](depth, opts) { | |
Collapse file Copy file name to clipboardExpand all lines: src/env_properties.h +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 54 | 54 | V(args_string, "args") \ | 55 | 55 | V(asn1curve_string, "asn1Curve") \ | 56 | 56 | V(async_ids_stack_string, "async_ids_stack") \ | | 57 | + V(base_string, "base") \ | 57 | 58 | V(bits_string, "bits") \ | 58 | 59 | V(block_list_string, "blockList") \ | 59 | 60 | V(buffer_string, "buffer") \ | |
Collapse file Copy file name to clipboardExpand all lines: src/node_url.cc +34-4Lines changed: 34 additions & 4 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -227,6 +227,35 @@ void BindingData::Format(const FunctionCallbackInfo<Value>& args) { | 227 | 227 | .ToLocalChecked()); | 228 | 228 | } | 229 | 229 |
| | 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 | + | 230 | 259 | void BindingData::Parse(const FunctionCallbackInfo<Value>& args) { | 231 | 260 | CHECK_GE(args.Length(), 1); | 232 | 261 | CHECK(args[0]->IsString()); // input | @@ -235,23 +264,24 @@ void BindingData::Parse(const FunctionCallbackInfo<Value>& args) { | 235 | 264 | Realm* realm = Realm::GetCurrent(args); | 236 | 265 | BindingData* binding_data = realm->GetBindingData<BindingData>(); | 237 | 266 | Isolate* isolate = realm->isolate(); | | 267 | + std::optional<std::string> base_{}; | 238 | 268 |
| 239 | 269 | Utf8Value input(isolate, args[0]); | 240 | 270 | ada::result<ada::url_aggregator> base; | 241 | 271 | ada::url_aggregator* base_pointer = nullptr; | 242 | 272 | 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_); | 245 | 275 | if (!base) { | 246 | | - return args.GetReturnValue().Set(false); | | 276 | + return ThrowInvalidURL(realm->env(), input.ToStringView(), base_); | 247 | 277 | } | 248 | 278 | base_pointer = &base.value(); | 249 | 279 | } | 250 | 280 | auto out = | 251 | 281 | ada::parse<ada::url_aggregator>(input.ToStringView(), base_pointer); | 252 | 282 |
| 253 | 283 | if (!out) { | 254 | | - return args.GetReturnValue().Set(false); | | 284 | + return ThrowInvalidURL(realm->env(), input.ToStringView(), base_); | 255 | 285 | } | 256 | 286 |
| 257 | 287 | binding_data->UpdateComponents(out->get_components(), out->type); | |
Collapse file Copy file name to clipboardExpand all lines: src/node_url.h +3Lines changed: 3 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -76,6 +76,9 @@ class BindingData : public SnapshotableObject { | 76 | 76 | const ada::scheme::type type); | 77 | 77 |
| 78 | 78 | static v8::CFunction fast_can_parse_methods_[]; | | 79 | + static void ThrowInvalidURL(Environment* env, | | 80 | + std::string_view input, | | 81 | + std::optional<std::string> base); | 79 | 82 | }; | 80 | 83 |
| 81 | 84 | std::string FromFilePath(const std::string_view file_path); | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-url-null-char.js +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -4,5 +4,5 @@ const assert = require('assert'); | 4 | 4 |
| 5 | 5 | assert.throws( | 6 | 6 | () => { new URL('a\0b'); }, | 7 | | - { input: 'a\0b' } | | 7 | + { code: 'ERR_INVALID_URL', input: 'a\0b' } | 8 | 8 | ); |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-whatwg-url-custom-parsing.js +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -55,7 +55,7 @@ for (const test of failureTests) { | 55 | 55 | () => new URL(test.input, test.base), | 56 | 56 | (error) => { | 57 | 57 | assert.throws(() => { throw error; }, expectedError); | 58 | | - assert.strictEqual(`${error}`, 'TypeError [ERR_INVALID_URL]: Invalid URL'); | | 58 | + assert.strictEqual(`${error}`, 'TypeError: Invalid URL'); | 59 | 59 | assert.strictEqual(error.message, 'Invalid URL'); | 60 | 60 | return true; | 61 | 61 | }); | |
|