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: #39592

PR-URL: #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 danielleadams committed Apr 24, 2022
1 parent 245577e commit e918570
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
Expand Up @@ -549,7 +549,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 @@ -627,7 +627,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 @@ -740,7 +741,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
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
@@ -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 e918570

Please sign in to comment.