Skip to content

Commit ee19e29

Browse files
joyeecheungjasnell
joyeecheung
authored and
jasnell
committed
url: show input in parse error message
PR-URL: nodejs#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>
1 parent 2dff3a2 commit ee19e29

4 files changed

Lines changed: 71 additions & 46 deletions

File tree

‎lib/internal/url.js‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ class TupleOrigin {
9999

100100
function onParseComplete(flags, protocol, username, password,
101101
host, port, path, query, fragment) {
102-
if (flags & binding.URL_FLAGS_FAILED)
103-
throw new TypeError('Invalid URL');
104102
var ctx = this[context];
105103
ctx.flags = flags;
106104
ctx.scheme = protocol;
@@ -118,19 +116,23 @@ function onParseComplete(flags, protocol, username, password,
118116
initSearchParams(this[searchParams], query);
119117
}
120118

119+
function onParseError(flags, input) {
120+
const error = new TypeError('Invalid URL: ' + input);
121+
error.input = input;
122+
throw error;
123+
}
124+
121125
// Reused by URL constructor and URL#href setter.
122126
function parse(url, input, base) {
123127
const base_context = base ? base[context] : undefined;
124128
url[context] = new StorageObject();
125129
binding.parse(input.trim(), -1,
126130
base_context, undefined,
127-
onParseComplete.bind(url));
131+
onParseComplete.bind(url), onParseError);
128132
}
129133

130134
function onParseProtocolComplete(flags, protocol, username, password,
131135
host, port, path, query, fragment) {
132-
if (flags & binding.URL_FLAGS_FAILED)
133-
return;
134136
const newIsSpecial = (flags & binding.URL_FLAGS_SPECIAL) !== 0;
135137
const s = this[special];
136138
const ctx = this[context];
@@ -159,8 +161,6 @@ function onParseProtocolComplete(flags, protocol, username, password,
159161

160162
function onParseHostComplete(flags, protocol, username, password,
161163
host, port, path, query, fragment) {
162-
if (flags & binding.URL_FLAGS_FAILED)
163-
return;
164164
const ctx = this[context];
165165
if (host) {
166166
ctx.host = host;
@@ -174,8 +174,6 @@ function onParseHostComplete(flags, protocol, username, password,
174174

175175
function onParseHostnameComplete(flags, protocol, username, password,
176176
host, port, path, query, fragment) {
177-
if (flags & binding.URL_FLAGS_FAILED)
178-
return;
179177
const ctx = this[context];
180178
if (host) {
181179
ctx.host = host;
@@ -187,15 +185,11 @@ function onParseHostnameComplete(flags, protocol, username, password,
187185

188186
function onParsePortComplete(flags, protocol, username, password,
189187
host, port, path, query, fragment) {
190-
if (flags & binding.URL_FLAGS_FAILED)
191-
return;
192188
this[context].port = port;
193189
}
194190

195191
function onParsePathComplete(flags, protocol, username, password,
196192
host, port, path, query, fragment) {
197-
if (flags & binding.URL_FLAGS_FAILED)
198-
return;
199193
const ctx = this[context];
200194
if (path) {
201195
ctx.path = path;
@@ -207,16 +201,12 @@ function onParsePathComplete(flags, protocol, username, password,
207201

208202
function onParseSearchComplete(flags, protocol, username, password,
209203
host, port, path, query, fragment) {
210-
if (flags & binding.URL_FLAGS_FAILED)
211-
return;
212204
const ctx = this[context];
213205
ctx.query = query;
214206
}
215207

216208
function onParseHashComplete(flags, protocol, username, password,
217209
host, port, path, query, fragment) {
218-
if (flags & binding.URL_FLAGS_FAILED)
219-
return;
220210
const ctx = this[context];
221211
if (fragment) {
222212
ctx.fragment = fragment;

‎src/node_url.cc‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,7 +1233,8 @@ namespace url {
12331233
enum url_parse_state state_override,
12341234
Local<Value> base_obj,
12351235
Local<Value> context_obj,
1236-
Local<Function> cb) {
1236+
Local<Function> cb,
1237+
Local<Value> error_cb) {
12371238
Isolate* isolate = env->isolate();
12381239
Local<Context> context = env->context();
12391240
HandleScope handle_scope(isolate);
@@ -1254,20 +1255,19 @@ namespace url {
12541255

12551256
// Define the return value placeholders
12561257
const Local<Value> undef = Undefined(isolate);
1257-
Local<Value> argv[9] = {
1258-
undef,
1259-
undef,
1260-
undef,
1261-
undef,
1262-
undef,
1263-
undef,
1264-
undef,
1265-
undef,
1266-
undef,
1267-
};
1268-
1269-
argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
12701258
if (!(url.flags & URL_FLAGS_FAILED)) {
1259+
Local<Value> argv[9] = {
1260+
undef,
1261+
undef,
1262+
undef,
1263+
undef,
1264+
undef,
1265+
undef,
1266+
undef,
1267+
undef,
1268+
undef,
1269+
};
1270+
argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
12711271
if (url.flags & URL_FLAGS_HAS_SCHEME)
12721272
argv[ARG_PROTOCOL] = OneByteString(isolate, url.scheme.c_str());
12731273
if (url.flags & URL_FLAGS_HAS_USERNAME)
@@ -1284,22 +1284,31 @@ namespace url {
12841284
argv[ARG_PORT] = Integer::New(isolate, url.port);
12851285
if (url.flags & URL_FLAGS_HAS_PATH)
12861286
argv[ARG_PATH] = Copy(env, url.path);
1287+
(void)cb->Call(context, recv, arraysize(argv), argv);
1288+
} else if (error_cb->IsFunction()) {
1289+
Local<Value> argv[2] = { undef, undef };
1290+
argv[ERR_ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
1291+
argv[ERR_ARG_INPUT] =
1292+
String::NewFromUtf8(env->isolate(),
1293+
input,
1294+
v8::NewStringType::kNormal).ToLocalChecked();
1295+
(void)error_cb.As<Function>()->Call(context, recv, arraysize(argv), argv);
12871296
}
1288-
1289-
(void)cb->Call(context, recv, 9, argv);
12901297
}
12911298

12921299
static void Parse(const FunctionCallbackInfo<Value>& args) {
12931300
Environment* env = Environment::GetCurrent(args);
12941301
CHECK_GE(args.Length(), 5);
1295-
CHECK(args[0]->IsString());
1296-
CHECK(args[2]->IsUndefined() ||
1302+
CHECK(args[0]->IsString()); // input
1303+
CHECK(args[2]->IsUndefined() || // base context
12971304
args[2]->IsNull() ||
12981305
args[2]->IsObject());
1299-
CHECK(args[3]->IsUndefined() ||
1306+
CHECK(args[3]->IsUndefined() || // context
13001307
args[3]->IsNull() ||
13011308
args[3]->IsObject());
1302-
CHECK(args[4]->IsFunction());
1309+
CHECK(args[4]->IsFunction()); // complete callback
1310+
CHECK(args[5]->IsUndefined() || args[5]->IsFunction()); // error callback
1311+
13031312
Utf8Value input(env->isolate(), args[0]);
13041313
enum url_parse_state state_override = kUnknownState;
13051314
if (args[1]->IsNumber()) {
@@ -1312,7 +1321,8 @@ namespace url {
13121321
state_override,
13131322
args[2],
13141323
args[3],
1315-
args[4].As<Function>());
1324+
args[4].As<Function>(),
1325+
args[5]);
13161326
}
13171327

13181328
static void EncodeAuthSet(const FunctionCallbackInfo<Value>& args) {

‎src/node_url.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,10 @@ static inline void PercentDecode(const char* input,
463463
XX(ARG_QUERY) \
464464
XX(ARG_FRAGMENT)
465465

466+
#define ERR_ARGS(XX) \
467+
XX(ERR_ARG_FLAGS) \
468+
XX(ERR_ARG_INPUT) \
469+
466470
static const char kEOL = -1;
467471

468472
enum url_parse_state {
@@ -484,6 +488,12 @@ enum url_cb_args {
484488
#undef XX
485489
};
486490

491+
enum url_error_cb_args {
492+
#define XX(name) name,
493+
ERR_ARGS(XX)
494+
#undef XX
495+
} url_error_cb_args;
496+
487497
static inline bool IsSpecial(std::string scheme) {
488498
#define XX(name, _) if (scheme == name) return true;
489499
SPECIALS(XX);

‎test/parallel/test-whatwg-url-parsing.js‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,30 @@ if (!common.hasIntl) {
1313

1414
// Tests below are not from WPT.
1515
const tests = require(path.join(common.fixturesDir, 'url-tests'));
16+
const failureTests = tests.filter((test) => test.failure).concat([
17+
{ input: '' },
18+
{ input: 'test' },
19+
{ input: undefined },
20+
{ input: 0 },
21+
{ input: true },
22+
{ input: false },
23+
{ input: null },
24+
{ input: new Date() },
25+
{ input: new RegExp() },
26+
{ input: () => {} }
27+
]);
1628

17-
for (const test of tests) {
18-
if (typeof test === 'string')
19-
continue;
20-
21-
if (test.failure) {
22-
assert.throws(() => new URL(test.input, test.base),
23-
/^TypeError: Invalid URL$/);
24-
}
29+
for (const test of failureTests) {
30+
assert.throws(
31+
() => new URL(test.input, test.base),
32+
(error) => {
33+
// The input could be processed, so we don't do strict matching here
34+
const match = (error + '').match(/^TypeError: Invalid URL: (.*)$/);
35+
if (!match) {
36+
return false;
37+
}
38+
return error.input === match[1];
39+
});
2540
}
2641

2742
const additional_tests = require(

0 commit comments

Comments
 (0)