Skip to content

Commit f675518

Browse files
joyeecheungMylesBorins
joyeecheung
authored and
MylesBorins
committed
url: show input in parse error message
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>
1 parent bb2de4a commit f675518

File tree

4 files changed

+71
-46
lines changed

4 files changed

+71
-46
lines changed

‎lib/internal/url.js‎

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

8484
function onParseComplete(flags, protocol, username, password,
8585
host, port, path, query, fragment) {
86-
if (flags & binding.URL_FLAGS_FAILED)
87-
throw new TypeError('Invalid URL');
8886
var ctx = this[context];
8987
ctx.flags = flags;
9088
ctx.scheme = protocol;
@@ -102,20 +100,24 @@ function onParseComplete(flags, protocol, username, password,
102100
initSearchParams(this[searchParams], query);
103101
}
104102

103+
function onParseError(flags, input) {
104+
const error = new TypeError('Invalid URL: ' + input);
105+
error.input = input;
106+
throw error;
107+
}
108+
105109
// Reused by URL constructor and URL#href setter.
106110
function parse(url, input, base) {
107111
input = String(input);
108112
const base_context = base ? base[context] : undefined;
109113
url[context] = new StorageObject();
110114
binding.parse(input.trim(), -1,
111115
base_context, undefined,
112-
onParseComplete.bind(url));
116+
onParseComplete.bind(url), onParseError);
113117
}
114118

115119
function onParseProtocolComplete(flags, protocol, username, password,
116120
host, port, path, query, fragment) {
117-
if (flags & binding.URL_FLAGS_FAILED)
118-
return;
119121
const newIsSpecial = (flags & binding.URL_FLAGS_SPECIAL) !== 0;
120122
const s = this[special];
121123
const ctx = this[context];
@@ -144,8 +146,6 @@ function onParseProtocolComplete(flags, protocol, username, password,
144146

145147
function onParseHostComplete(flags, protocol, username, password,
146148
host, port, path, query, fragment) {
147-
if (flags & binding.URL_FLAGS_FAILED)
148-
return;
149149
const ctx = this[context];
150150
if (host) {
151151
ctx.host = host;
@@ -159,8 +159,6 @@ function onParseHostComplete(flags, protocol, username, password,
159159

160160
function onParseHostnameComplete(flags, protocol, username, password,
161161
host, port, path, query, fragment) {
162-
if (flags & binding.URL_FLAGS_FAILED)
163-
return;
164162
const ctx = this[context];
165163
if (host) {
166164
ctx.host = host;
@@ -172,15 +170,11 @@ function onParseHostnameComplete(flags, protocol, username, password,
172170

173171
function onParsePortComplete(flags, protocol, username, password,
174172
host, port, path, query, fragment) {
175-
if (flags & binding.URL_FLAGS_FAILED)
176-
return;
177173
this[context].port = port;
178174
}
179175

180176
function onParsePathComplete(flags, protocol, username, password,
181177
host, port, path, query, fragment) {
182-
if (flags & binding.URL_FLAGS_FAILED)
183-
return;
184178
const ctx = this[context];
185179
if (path) {
186180
ctx.path = path;
@@ -192,16 +186,12 @@ function onParsePathComplete(flags, protocol, username, password,
192186

193187
function onParseSearchComplete(flags, protocol, username, password,
194188
host, port, path, query, fragment) {
195-
if (flags & binding.URL_FLAGS_FAILED)
196-
return;
197189
const ctx = this[context];
198190
ctx.query = query;
199191
}
200192

201193
function onParseHashComplete(flags, protocol, username, password,
202194
host, port, path, query, fragment) {
203-
if (flags & binding.URL_FLAGS_FAILED)
204-
return;
205195
const ctx = this[context];
206196
if (fragment) {
207197
ctx.fragment = fragment;

‎src/node_url.cc‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,7 +1246,8 @@ namespace url {
12461246
enum url_parse_state state_override,
12471247
Local<Value> base_obj,
12481248
Local<Value> context_obj,
1249-
Local<Function> cb) {
1249+
Local<Function> cb,
1250+
Local<Value> error_cb) {
12501251
Isolate* isolate = env->isolate();
12511252
Local<Context> context = env->context();
12521253
HandleScope handle_scope(isolate);
@@ -1267,20 +1268,19 @@ namespace url {
12671268

12681269
// Define the return value placeholders
12691270
const Local<Value> undef = Undefined(isolate);
1270-
Local<Value> argv[9] = {
1271-
undef,
1272-
undef,
1273-
undef,
1274-
undef,
1275-
undef,
1276-
undef,
1277-
undef,
1278-
undef,
1279-
undef,
1280-
};
1281-
1282-
argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
12831271
if (!(url.flags & URL_FLAGS_FAILED)) {
1272+
Local<Value> argv[9] = {
1273+
undef,
1274+
undef,
1275+
undef,
1276+
undef,
1277+
undef,
1278+
undef,
1279+
undef,
1280+
undef,
1281+
undef,
1282+
};
1283+
argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
12841284
if (url.flags & URL_FLAGS_HAS_SCHEME)
12851285
argv[ARG_PROTOCOL] = OneByteString(isolate, url.scheme.c_str());
12861286
if (url.flags & URL_FLAGS_HAS_USERNAME)
@@ -1297,22 +1297,31 @@ namespace url {
12971297
argv[ARG_PORT] = Integer::New(isolate, url.port);
12981298
if (url.flags & URL_FLAGS_HAS_PATH)
12991299
argv[ARG_PATH] = Copy(env, url.path);
1300+
(void)cb->Call(context, recv, arraysize(argv), argv);
1301+
} else if (error_cb->IsFunction()) {
1302+
Local<Value> argv[2] = { undef, undef };
1303+
argv[ERR_ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
1304+
argv[ERR_ARG_INPUT] =
1305+
String::NewFromUtf8(env->isolate(),
1306+
input,
1307+
v8::NewStringType::kNormal).ToLocalChecked();
1308+
(void)error_cb.As<Function>()->Call(context, recv, arraysize(argv), argv);
13001309
}
1301-
1302-
(void)cb->Call(context, recv, 9, argv);
13031310
}
13041311

13051312
static void Parse(const FunctionCallbackInfo<Value>& args) {
13061313
Environment* env = Environment::GetCurrent(args);
13071314
CHECK_GE(args.Length(), 5);
1308-
CHECK(args[0]->IsString());
1309-
CHECK(args[2]->IsUndefined() ||
1315+
CHECK(args[0]->IsString()); // input
1316+
CHECK(args[2]->IsUndefined() || // base context
13101317
args[2]->IsNull() ||
13111318
args[2]->IsObject());
1312-
CHECK(args[3]->IsUndefined() ||
1319+
CHECK(args[3]->IsUndefined() || // context
13131320
args[3]->IsNull() ||
13141321
args[3]->IsObject());
1315-
CHECK(args[4]->IsFunction());
1322+
CHECK(args[4]->IsFunction()); // complete callback
1323+
CHECK(args[5]->IsUndefined() || args[5]->IsFunction()); // error callback
1324+
13161325
Utf8Value input(env->isolate(), args[0]);
13171326
enum url_parse_state state_override = kUnknownState;
13181327
if (args[1]->IsNumber()) {
@@ -1325,7 +1334,8 @@ namespace url {
13251334
state_override,
13261335
args[2],
13271336
args[3],
1328-
args[4].As<Function>());
1337+
args[4].As<Function>(),
1338+
args[5]);
13291339
}
13301340

13311341
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)