Skip to content

Commit

Permalink
url: show input in parse error message
Browse files Browse the repository at this point in the history 
PR-URL: #11934
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
  • Loading branch information
@joyeecheung @MylesBorins
joyeecheung authored and MylesBorins committed Mar 28, 2017
1 parent bb2de4a commit f675518
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 46 deletions.
24 changes: 7 additions & 17 deletions lib/internal/url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ class TupleOrigin {

function onParseComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
throw new TypeError('Invalid URL');
var ctx = this[context];
ctx.flags = flags;
ctx.scheme = protocol;
Expand All @@ -102,20 +100,24 @@ function onParseComplete(flags, protocol, username, password,
initSearchParams(this[searchParams], query);
}

function onParseError(flags, input) {
const error = new TypeError('Invalid URL: ' + input);
error.input = input;
throw error;
}

// Reused by URL constructor and URL#href setter.
function parse(url, input, base) {
input = String(input);
const base_context = base ? base[context] : undefined;
url[context] = new StorageObject();
binding.parse(input.trim(), -1,
base_context, undefined,
onParseComplete.bind(url));
onParseComplete.bind(url), onParseError);
}

function onParseProtocolComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
return;
const newIsSpecial = (flags & binding.URL_FLAGS_SPECIAL) !== 0;
const s = this[special];
const ctx = this[context];
Expand Down Expand Up @@ -144,8 +146,6 @@ function onParseProtocolComplete(flags, protocol, username, password,

function onParseHostComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
return;
const ctx = this[context];
if (host) {
ctx.host = host;
Expand All @@ -159,8 +159,6 @@ function onParseHostComplete(flags, protocol, username, password,

function onParseHostnameComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
return;
const ctx = this[context];
if (host) {
ctx.host = host;
Expand All @@ -172,15 +170,11 @@ function onParseHostnameComplete(flags, protocol, username, password,

function onParsePortComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
return;
this[context].port = port;
}

function onParsePathComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
return;
const ctx = this[context];
if (path) {
ctx.path = path;
Expand All @@ -192,16 +186,12 @@ function onParsePathComplete(flags, protocol, username, password,

function onParseSearchComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
return;
const ctx = this[context];
ctx.query = query;
}

function onParseHashComplete(flags, protocol, username, password,
host, port, path, query, fragment) {
if (flags & binding.URL_FLAGS_FAILED)
return;
const ctx = this[context];
if (fragment) {
ctx.fragment = fragment;
Expand Down
52 changes: 31 additions & 21 deletions src/node_url.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,8 @@ namespace url {
enum url_parse_state state_override,
Local<Value> base_obj,
Local<Value> context_obj,
Local<Function> cb) {
Local<Function> cb,
Local<Value> error_cb) {
Isolate* isolate = env->isolate();
Local<Context> context = env->context();
HandleScope handle_scope(isolate);
Expand All @@ -1267,20 +1268,19 @@ namespace url {

// Define the return value placeholders
const Local<Value> undef = Undefined(isolate);
Local<Value> argv[9] = {
undef,
undef,
undef,
undef,
undef,
undef,
undef,
undef,
undef,
};

argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
if (!(url.flags & URL_FLAGS_FAILED)) {
Local<Value> argv[9] = {
undef,
undef,
undef,
undef,
undef,
undef,
undef,
undef,
undef,
};
argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
if (url.flags & URL_FLAGS_HAS_SCHEME)
argv[ARG_PROTOCOL] = OneByteString(isolate, url.scheme.c_str());
if (url.flags & URL_FLAGS_HAS_USERNAME)
Expand All @@ -1297,22 +1297,31 @@ namespace url {
argv[ARG_PORT] = Integer::New(isolate, url.port);
if (url.flags & URL_FLAGS_HAS_PATH)
argv[ARG_PATH] = Copy(env, url.path);
(void)cb->Call(context, recv, arraysize(argv), argv);
} 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,
v8::NewStringType::kNormal).ToLocalChecked();
(void)error_cb.As<Function>()->Call(context, recv, arraysize(argv), argv);
}

(void)cb->Call(context, recv, 9, argv);
}

static void Parse(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK_GE(args.Length(), 5);
CHECK(args[0]->IsString());
CHECK(args[2]->IsUndefined() ||
CHECK(args[0]->IsString()); // input
CHECK(args[2]->IsUndefined() || // base context
args[2]->IsNull() ||
args[2]->IsObject());
CHECK(args[3]->IsUndefined() ||
CHECK(args[3]->IsUndefined() || // context
args[3]->IsNull() ||
args[3]->IsObject());
CHECK(args[4]->IsFunction());
CHECK(args[4]->IsFunction()); // complete callback
CHECK(args[5]->IsUndefined() || args[5]->IsFunction()); // error callback

Utf8Value input(env->isolate(), args[0]);
enum url_parse_state state_override = kUnknownState;
if (args[1]->IsNumber()) {
Expand All @@ -1325,7 +1334,8 @@ namespace url {
state_override,
args[2],
args[3],
args[4].As<Function>());
args[4].As<Function>(),
args[5]);
}

static void EncodeAuthSet(const FunctionCallbackInfo<Value>& args) {
Expand Down
10 changes: 10 additions & 0 deletions src/node_url.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,10 @@ static inline void PercentDecode(const char* input,
XX(ARG_QUERY) \
XX(ARG_FRAGMENT)

#define ERR_ARGS(XX) \
XX(ERR_ARG_FLAGS) \
XX(ERR_ARG_INPUT) \

static const char kEOL = -1;

enum url_parse_state {
Expand All @@ -484,6 +488,12 @@ enum url_cb_args {
#undef XX
};

enum url_error_cb_args {
#define XX(name) name,
ERR_ARGS(XX)
#undef XX
} url_error_cb_args;

static inline bool IsSpecial(std::string scheme) {
#define XX(name, _) if (scheme == name) return true;
SPECIALS(XX);
Expand Down
31 changes: 23 additions & 8 deletions test/parallel/test-whatwg-url-parsing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,30 @@ if (!common.hasIntl) {

// Tests below are not from WPT.
const tests = require(path.join(common.fixturesDir, 'url-tests'));
const failureTests = tests.filter((test) => test.failure).concat([
{ input: '' },
{ input: 'test' },
{ input: undefined },
{ input: 0 },
{ input: true },
{ input: false },
{ input: null },
{ input: new Date() },
{ input: new RegExp() },
{ input: () => {} }
]);

for (const test of tests) {
if (typeof test === 'string')
continue;

if (test.failure) {
assert.throws(() => new URL(test.input, test.base),
/^TypeError: Invalid URL$/);
}
for (const test of failureTests) {
assert.throws(
() => new URL(test.input, test.base),
(error) => {
// The input could be processed, so we don't do strict matching here
const match = (error + '').match(/^TypeError: Invalid URL: (.*)$/);
if (!match) {
return false;
}
return error.input === match[1];
});
}

const additional_tests = require(
Expand Down

0 comments on commit f675518

Please sign in to comment.