Skip to content

Commit

Permalink
url: preserve null char in WHATWG URL errors
Browse files Browse the repository at this point in the history
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: nodejs#39592

PR-URL: nodejs#42263
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Trott authored and xtx1130 committed Apr 25, 2022
1 parent 5b55d97 commit dd79ab9
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 19 deletions.
8 changes: 5 additions & 3 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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]() {
Expand Down Expand Up @@ -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
Expand Down
18 changes: 2 additions & 16 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T> \
bool name(const T ch1, const T ch2) { \
Expand Down Expand Up @@ -1679,12 +1669,8 @@ void Parse(Environment* env,
SetArgs(env, argv, url);
cb->Call(context, recv, arraysize(argv), argv).FromMaybe(Local<Value>());
} else if (error_cb->IsFunction()) {
Local<Value> 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<Function>()->Call(context, recv, arraysize(argv), argv)
.FromMaybe(Local<Value>());
Local<Value> flags = Integer::NewFromUnsigned(isolate, url.flags);
USE(error_cb.As<Function>()->Call(context, recv, 1, &flags));
}
}

Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-url-null-char.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';
require('../common');
const assert = require('assert');

assert.throws(
() => { new URL('a\0b'); },
{ input: 'a\0b' }
);

0 comments on commit dd79ab9

Please sign in to comment.