url: show input in parse error message

PR-URL: https://github.com/nodejs/node/pull/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>

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;

src/node_url.cc

@@ -1233,7 +1233,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);
@@ -1254,6 +1255,7 @@ namespace url {
 
     // Define the return value placeholders
     const Local<Value> undef = Undefined(isolate);
+    if (!(url.flags & URL_FLAGS_FAILED)) {
       Local<Value> argv[9] = {
         undef,
         undef,
@@ -1265,9 +1267,7 @@ namespace url {
         undef,
         undef,
       };
-
       argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url.flags);
-    if (!(url.flags & URL_FLAGS_FAILED)) {
       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<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[0]->IsString());  // input
-    CHECK(args[2]->IsUndefined() ||
+    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<Function>());
+          args[4].As<Function>(),
+          args[5]);
   }
 
   static void EncodeAuthSet(const FunctionCallbackInfo<Value>& args) {

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

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