From fe01940f355f7e12dbdbdf629a561684404dba4f Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 10 Mar 2022 21:51:41 -0800 Subject: [PATCH] url: preserve null char in WHATWG URL errors A null character in the middle of an invalid URL was resulting in an error message that truncated the input string. This preserves the entire input string in the error message. Refs: https://github.com/nodejs/node/issues/39592 PR-URL: https://github.com/nodejs/node/pull/42263 Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- lib/internal/url.js | 8 +++++--- src/node_url.cc | 18 ++---------------- test/parallel/test-url-null-char.js | 8 ++++++++ 3 files changed, 15 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-url-null-char.js diff --git a/lib/internal/url.js b/lib/internal/url.js index 9e22f608977baf..06dd6cc7e3f024 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -559,7 +559,7 @@ function onParseComplete(flags, protocol, username, password, initSearchParams(this[searchParams], query); } -function onParseError(flags, input) { +function onParseError(input, flags) { throw new ERR_INVALID_URL(input); } @@ -641,7 +641,8 @@ class URL { } this[context] = new URLContext(); parse(input, -1, base_context, undefined, - FunctionPrototypeBind(onParseComplete, this), onParseError); + FunctionPrototypeBind(onParseComplete, this), + FunctionPrototypeBind(onParseError, this, input)); } get [special]() { @@ -760,7 +761,8 @@ class URL { // toUSVString is not needed. input = `${input}`; parse(input, -1, undefined, undefined, - FunctionPrototypeBind(onParseComplete, this), onParseError); + FunctionPrototypeBind(onParseComplete, this), + FunctionPrototypeBind(onParseError, this, input)); } // readonly diff --git a/src/node_url.cc b/src/node_url.cc index 59abbe43f9917d..79476e7a2f6a95 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -142,22 +142,12 @@ URLHost::~URLHost() { XX(ARG_FRAGMENT) \ XX(ARG_COUNT) // This one has to be last. -#define ERR_ARGS(XX) \ - XX(ERR_ARG_FLAGS) \ - XX(ERR_ARG_INPUT) \ - enum url_cb_args { #define XX(name) name, ARGS(XX) #undef XX }; -enum url_error_cb_args { -#define XX(name) name, - ERR_ARGS(XX) -#undef XX -}; - #define TWO_CHAR_STRING_TEST(bits, name, expr) \ template \ bool name(const T ch1, const T ch2) { \ @@ -1679,12 +1669,8 @@ void Parse(Environment* env, SetArgs(env, argv, url); cb->Call(context, recv, arraysize(argv), argv).FromMaybe(Local()); } else if (error_cb->IsFunction()) { - Local argv[2] = { undef, undef }; - argv[ERR_ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags); - argv[ERR_ARG_INPUT] = - String::NewFromUtf8(env->isolate(), input).ToLocalChecked(); - error_cb.As()->Call(context, recv, arraysize(argv), argv) - .FromMaybe(Local()); + Local flags = Integer::NewFromUnsigned(isolate, url.flags); + USE(error_cb.As()->Call(context, recv, 1, &flags)); } } diff --git a/test/parallel/test-url-null-char.js b/test/parallel/test-url-null-char.js new file mode 100644 index 00000000000000..468080844d534b --- /dev/null +++ b/test/parallel/test-url-null-char.js @@ -0,0 +1,8 @@ +'use strict'; +require('../common'); +const assert = require('assert'); + +assert.throws( + () => { new URL('a\0b'); }, + { input: 'a\0b' } +);