From 63b5a89e18693b5dfd2fff1c3bfc8ca34d129a42 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 20 Mar 2017 14:18:37 +0800 Subject: [PATCH] url: show input in parse error message --- lib/internal/url.js | 24 ++++------- src/node_url.cc | 52 ++++++++++++++---------- src/node_url.h | 10 +++++ test/parallel/test-whatwg-url-parsing.js | 31 ++++++++++---- 4 files changed, 71 insertions(+), 46 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 040771c228067e..7a6ff227ed4191 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -99,8 +99,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; @@ -118,19 +116,23 @@ 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) { 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]; @@ -159,8 +161,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; @@ -174,8 +174,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; @@ -187,15 +185,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; @@ -207,16 +201,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; diff --git a/src/node_url.cc b/src/node_url.cc index ba3ceec6aca070..6cd78c2c6c04c8 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -1233,7 +1233,8 @@ namespace url { enum url_parse_state state_override, Local base_obj, Local context_obj, - Local cb) { + Local cb, + Local error_cb) { Isolate* isolate = env->isolate(); Local context = env->context(); HandleScope handle_scope(isolate); @@ -1254,20 +1255,19 @@ namespace url { // Define the return value placeholders const Local undef = Undefined(isolate); - Local 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 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) @@ -1284,22 +1284,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 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()->Call(context, recv, arraysize(argv), argv); } - - (void)cb->Call(context, recv, 9, argv); } static void Parse(const FunctionCallbackInfo& 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()) { @@ -1312,7 +1321,8 @@ namespace url { state_override, args[2], args[3], - args[4].As()); + args[4].As(), + args[5]); } static void EncodeAuthSet(const FunctionCallbackInfo& args) { diff --git a/src/node_url.h b/src/node_url.h index 49f6de866da501..b9d91782be9e59 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -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 { @@ -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); diff --git a/test/parallel/test-whatwg-url-parsing.js b/test/parallel/test-whatwg-url-parsing.js index e4f8306ead8d87..4d04c3194e1f1a 100644 --- a/test/parallel/test-whatwg-url-parsing.js +++ b/test/parallel/test-whatwg-url-parsing.js @@ -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(