From a560f731e85e700395e4594ee4b70108925570e1 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Thu, 10 Mar 2022 21:51:41 -0800
Subject: [PATCH] url: preserve null char in WHATWG URL errors

A null character in the middle of an invalid URL was resulting in an
error message that truncated the input string. This preserves the entire
input string in the error message.

Refs: https://github.com/nodejs/node/issues/39592

PR-URL: https://github.com/nodejs/node/pull/42263
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/internal/url.js                 |  8 +++++---
 src/node_url.cc                     | 18 ++----------------
 test/parallel/test-url-null-char.js |  8 ++++++++
 3 files changed, 15 insertions(+), 19 deletions(-)
 create mode 100644 test/parallel/test-url-null-char.js

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 279519004f9207..939374a495856c 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -549,7 +549,7 @@ function onParseComplete(flags, protocol, username, password,
   initSearchParams(this[searchParams], query);
 }
 
-function onParseError(flags, input) {
+function onParseError(input, flags) {
   throw new ERR_INVALID_URL(input);
 }
 
@@ -627,7 +627,8 @@ class URL {
     }
     this[context] = new URLContext();
     parse(input, -1, base_context, undefined,
-          FunctionPrototypeBind(onParseComplete, this), onParseError);
+          FunctionPrototypeBind(onParseComplete, this),
+          FunctionPrototypeBind(onParseError, this, input));
   }
 
   get [special]() {
@@ -740,7 +741,8 @@ class URL {
     // toUSVString is not needed.
     input = `${input}`;
     parse(input, -1, undefined, undefined,
-          FunctionPrototypeBind(onParseComplete, this), onParseError);
+          FunctionPrototypeBind(onParseComplete, this),
+          FunctionPrototypeBind(onParseError, this, input));
   }
 
   // readonly
diff --git a/src/node_url.cc b/src/node_url.cc
index 59abbe43f9917d..79476e7a2f6a95 100644
--- a/src/node_url.cc
+++ b/src/node_url.cc
@@ -142,22 +142,12 @@ URLHost::~URLHost() {
   XX(ARG_FRAGMENT)                                                            \
   XX(ARG_COUNT)  // This one has to be last.
 
-#define ERR_ARGS(XX)                                                          \
-  XX(ERR_ARG_FLAGS)                                                           \
-  XX(ERR_ARG_INPUT)                                                           \
-
 enum url_cb_args {
 #define XX(name) name,
   ARGS(XX)
 #undef XX
 };
 
-enum url_error_cb_args {
-#define XX(name) name,
-  ERR_ARGS(XX)
-#undef XX
-};
-
 #define TWO_CHAR_STRING_TEST(bits, name, expr)                                \
   template <typename T>                                                       \
   bool name(const T ch1, const T ch2) {                                \
@@ -1679,12 +1669,8 @@ void Parse(Environment* env,
     SetArgs(env, argv, url);
     cb->Call(context, recv, arraysize(argv), argv).FromMaybe(Local<Value>());
   } 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).ToLocalChecked();
-    error_cb.As<Function>()->Call(context, recv, arraysize(argv), argv)
-        .FromMaybe(Local<Value>());
+    Local<Value> flags = Integer::NewFromUnsigned(isolate, url.flags);
+    USE(error_cb.As<Function>()->Call(context, recv, 1, &flags));
   }
 }
 
diff --git a/test/parallel/test-url-null-char.js b/test/parallel/test-url-null-char.js
new file mode 100644
index 00000000000000..468080844d534b
--- /dev/null
+++ b/test/parallel/test-url-null-char.js
@@ -0,0 +1,8 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+
+assert.throws(
+  () => { new URL('a\0b'); },
+  { input: 'a\0b' }
+);