From a35bf6bd24939e0ba3f4d7b2d3bafbdf3dcdf2df Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 18:30:20 +0100
Subject: [PATCH 01/11] src: add C++-style sprintf utility

Add an utility that handles C++-style strings and objects well.
---
 node.gyp                  |  1 +
 src/debug_utils-inl.h     | 86 +++++++++++++++++++++++++++++++++++++++
 src/debug_utils.h         | 14 ++++++-
 src/inspector_profiler.cc |  2 +-
 src/node_http2.cc         |  2 +-
 src/node_wasi.cc          |  4 +-
 src/tls_wrap.cc           |  2 +-
 test/cctest/test_util.cc  | 42 +++++++++++++++++++
 8 files changed, 147 insertions(+), 6 deletions(-)
 create mode 100644 src/debug_utils-inl.h

diff --git a/node.gyp b/node.gyp
index c46becb586e7ce..1495b0dab2d80b 100644
--- a/node.gyp
+++ b/node.gyp
@@ -608,6 +608,7 @@
         'src/connect_wrap.h',
         'src/connection_wrap.h',
         'src/debug_utils.h',
+        'src/debug_utils-inl.h',
         'src/env.h',
         'src/env-inl.h',
         'src/handle_wrap.h',
diff --git a/src/debug_utils-inl.h b/src/debug_utils-inl.h
new file mode 100644
index 00000000000000..8e4570a37a5195
--- /dev/null
+++ b/src/debug_utils-inl.h
@@ -0,0 +1,86 @@
+#ifndef SRC_DEBUG_UTILS_INL_H_
+#define SRC_DEBUG_UTILS_INL_H_
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#include "debug_utils.h"
+
+#include <type_traits>
+
+namespace node {
+
+struct ToStringHelper {
+  template <typename T>
+  static std::string Convert(
+      const T& value,
+      std::string(T::* to_string)() const = &T::ToString) {
+    return (value.*to_string)();
+  }
+  template <typename T,
+            typename test_for_number = typename std::
+                enable_if<std::is_arithmetic<T>::value, bool>::type,
+            typename dummy = bool>
+  static std::string Convert(const T& value) { return std::to_string(value); }
+  static std::string Convert(const char* value) { return value; }
+  static std::string Convert(const std::string& value) { return value; }
+  static std::string Convert(bool value) { return value ? "true" : "false"; }
+};
+
+template <typename T>
+std::string ToString(const T& value) {
+  return ToStringHelper::Convert(value);
+}
+
+inline std::string SPrintFImpl(const char* format) {
+  return format;
+}
+
+template <typename Arg, typename... Args>
+std::string COLD_NOINLINE SPrintFImpl(  // NOLINT(runtime/string)
+    const char* format, Arg&& arg, Args&&... args) {
+  const char* p = strchr(format, '%');
+  if (p == nullptr) return format;
+  std::string ret(format, p);
+  // Ignore long / size_t modifiers
+  while (strchr("lz", *++p) != nullptr) {}
+  switch (*p) {
+    case '%': {
+      return ret + '%' + SPrintFImpl(p + 1,
+                                     std::forward<Arg>(arg),
+                                     std::forward<Args>(args)...);
+    }
+    default: {
+      return ret + '%' + SPrintFImpl(p,
+                                     std::forward<Arg>(arg),
+                                     std::forward<Args>(args)...);
+    }
+    case 'd':
+    case 'i':
+    case 'u':
+    case 's': ret += ToString(arg); break;
+    case 'p': {
+      CHECK(std::is_pointer<typename std::remove_reference<Arg>::type>::value);
+      char out[20];
+      int n = snprintf(out,
+                       sizeof(out),
+                       "%p",
+                       *reinterpret_cast<const void* const*>(&arg));
+      CHECK_GE(n, 0);
+      ret += out;
+      break;
+    }
+  }
+  return ret + SPrintFImpl(p + 1, std::forward<Args>(args)...);
+}
+
+template <typename... Args>
+std::string COLD_NOINLINE SPrintF(  // NOLINT(runtime/string)
+    const char* format, Args&&... args) {
+  return SPrintFImpl(format, std::forward<Args>(args)...);
+}
+
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_DEBUG_UTILS_INL_H_
diff --git a/src/debug_utils.h b/src/debug_utils.h
index db01cacba6a1b6..08d23bb7703fe6 100644
--- a/src/debug_utils.h
+++ b/src/debug_utils.h
@@ -22,6 +22,17 @@
 
 namespace node {
 
+template <typename T>
+inline std::string ToString(const T& value);
+
+// C++-style variant of sprintf() that:
+// - Returns an std::string
+// - Handles \0 bytes correctly
+// - Supports %p and %s. %d, %i and %u are aliases for %s.
+// - Accepts any class that has a ToString() method for stringification.
+template <typename... Args>
+inline std::string SPrintF(const char* format, Args&&... args);
+
 template <typename... Args>
 inline void FORCE_INLINE Debug(Environment* env,
                                DebugCategory cat,
@@ -29,7 +40,8 @@ inline void FORCE_INLINE Debug(Environment* env,
                                Args&&... args) {
   if (!UNLIKELY(env->debug_enabled(cat)))
     return;
-  fprintf(stderr, format, std::forward<Args>(args)...);
+  std::string out = SPrintF(format, std::forward<Args>(args)...);
+  fwrite(out.data(), out.size(), 1, stderr);
 }
 
 inline void FORCE_INLINE Debug(Environment* env,
diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc
index e0d02d6952a3f9..f9d3c2b512f1c1 100644
--- a/src/inspector_profiler.cc
+++ b/src/inspector_profiler.cc
@@ -1,6 +1,6 @@
 #include "inspector_profiler.h"
 #include "base_object-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "diagnosticfilename-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_file.h"
diff --git a/src/node_http2.cc b/src/node_http2.cc
index e0d6398f2a44a9..f8d9363902b829 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -1,5 +1,5 @@
 #include "aliased_buffer.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node.h"
 #include "node_buffer.h"
diff --git a/src/node_wasi.cc b/src/node_wasi.cc
index 277b171224c290..20872c58d60fb7 100644
--- a/src/node_wasi.cc
+++ b/src/node_wasi.cc
@@ -1,6 +1,6 @@
 #include "env-inl.h"
 #include "base_object-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_mem-inl.h"
 #include "util-inl.h"
@@ -1063,7 +1063,7 @@ void WASI::PathFilestatGet(const FunctionCallbackInfo<Value>& args) {
   CHECK_TO_TYPE_OR_RETURN(args, args[4], Uint32, buf_ptr);
   ASSIGN_OR_RETURN_UNWRAP(&wasi, args.This());
   Debug(wasi,
-        "path_filestat_get(%d, %d, %d, %d, %d)\n",
+        "path_filestat_get(%d, %d, %d)\n",
         fd,
         path_ptr,
         path_len);
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 10ebc4ccd9a8de..82274fde6db0c1 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -21,7 +21,7 @@
 
 #include "tls_wrap.h"
 #include "async_wrap-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_buffer.h"  // Buffer
 #include "node_crypto.h"  // SecureContext
diff --git a/test/cctest/test_util.cc b/test/cctest/test_util.cc
index 7a0da1e6185d51..de9c5368bddfda 100644
--- a/test/cctest/test_util.cc
+++ b/test/cctest/test_util.cc
@@ -1,4 +1,6 @@
 #include "util-inl.h"
+#include "debug_utils-inl.h"
+#include "env-inl.h"
 #include "gtest/gtest.h"
 
 TEST(UtilTest, ListHead) {
@@ -250,3 +252,43 @@ TEST(UtilTest, MaybeStackBuffer) {
     EXPECT_TRUE(buf.IsInvalidated());
   }
 }
+
+TEST(UtilTest, SPrintF) {
+  using node::SPrintF;
+
+  // %d, %u and %s all do the same thing. The actual C++ type is used to infer
+  // the right representation.
+  EXPECT_EQ(SPrintF("%s", false), "false");
+  EXPECT_EQ(SPrintF("%s", true), "true");
+  EXPECT_EQ(SPrintF("%d", true), "true");
+  EXPECT_EQ(SPrintF("%u", true), "true");
+  EXPECT_EQ(SPrintF("%d", 10000000000LL), "10000000000");
+  EXPECT_EQ(SPrintF("%d", -10000000000LL), "-10000000000");
+  EXPECT_EQ(SPrintF("%u", 10000000000LL), "10000000000");
+  EXPECT_EQ(SPrintF("%u", -10000000000LL), "-10000000000");
+  EXPECT_EQ(SPrintF("%i", 10), "10");
+  EXPECT_EQ(SPrintF("%d", 10), "10");
+
+  EXPECT_EQ(atof(SPrintF("%s", 0.5).c_str()), 0.5);
+  EXPECT_EQ(atof(SPrintF("%s", -0.5).c_str()), -0.5);
+
+  void (*fn)() = []() {};
+  void* p = reinterpret_cast<void*>(&fn);
+  EXPECT_GE(SPrintF("%p", fn).size(), 8u);
+  EXPECT_GE(SPrintF("%p", p).size(), 8u);
+
+  const std::string foo = "foo";
+  const char* bar = "bar";
+  EXPECT_EQ(SPrintF("%s %s", foo, "bar"), "foo bar");
+  EXPECT_EQ(SPrintF("%s %s", foo, bar), "foo bar");
+
+  struct HasToString {
+    std::string ToString() const {
+      return "meow";
+    }
+  };
+  EXPECT_EQ(SPrintF("%s", HasToString{}), "meow");
+
+  const std::string with_zero = std::string("a") + '\0' + 'b';
+  EXPECT_EQ(SPrintF("%s", with_zero), with_zero);
+}

From ec00fdabfb0006baf87da39c38c817620dafbed1 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 20:30:35 +0100
Subject: [PATCH 02/11] src: use custom fprintf alike to write errors to stderr

This allows printing errors that contain nul characters, for example.

Fixes: https://github.com/nodejs/node/issues/28761
Fixes: https://github.com/nodejs/node/issues/31218
---
 src/debug_utils-inl.h           |   5 ++
 src/debug_utils.cc              |  35 +++++++++
 src/debug_utils.h               |  10 ++-
 src/node_errors.cc              | 127 ++++++++++----------------------
 src/node_errors.h               |   2 -
 src/node_process_methods.cc     |   3 +-
 src/util.h                      |   4 +
 test/message/error_with_nul.js  |  11 +++
 test/message/error_with_nul.out | Bin 0 -> 552 bytes
 9 files changed, 103 insertions(+), 94 deletions(-)
 create mode 100644 test/message/error_with_nul.js
 create mode 100644 test/message/error_with_nul.out

diff --git a/src/debug_utils-inl.h b/src/debug_utils-inl.h
index 8e4570a37a5195..03b49490622d49 100644
--- a/src/debug_utils-inl.h
+++ b/src/debug_utils-inl.h
@@ -79,6 +79,11 @@ std::string COLD_NOINLINE SPrintF(  // NOLINT(runtime/string)
   return SPrintFImpl(format, std::forward<Args>(args)...);
 }
 
+template <typename... Args>
+void COLD_NOINLINE FPrintF(FILE* file, const char* format, Args&&... args) {
+  FWrite(file, SPrintF(format, std::forward<Args>(args)...));
+}
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
diff --git a/src/debug_utils.cc b/src/debug_utils.cc
index 8dd51b3931959b..c71be6ffc98157 100644
--- a/src/debug_utils.cc
+++ b/src/debug_utils.cc
@@ -6,6 +6,10 @@
 #include <features.h>
 #endif
 
+#ifdef __ANDROID__
+#include <android/log.h>
+#endif
+
 #if defined(__linux__) && !defined(__GLIBC__) || \
     defined(__UCLIBC__) || \
     defined(_AIX)
@@ -437,6 +441,37 @@ std::vector<std::string> NativeSymbolDebuggingContext::GetLoadedLibraries() {
   return list;
 }
 
+void FWrite(FILE* file, const std::string& str) {
+  if (file != stderr && file != stdout) goto simple_fwrite;
+#ifdef _WIN32
+  HANDLE handle =
+      GetStdHandle(file == stdout ? STD_OUTPUT_HANDLE : STD_ERROR_HANDLE);
+
+  // Check if stderr is something other than a tty/console
+  if (handle == INVALID_HANDLE_VALUE || handle == nullptr ||
+      uv_guess_handle(_fileno(file)) != UV_TTY) {
+    goto simple_fwrite;
+  }
+
+  // Get required wide buffer size
+  int n = MultiByteToWideChar(CP_UTF8, 0, str.data(), str.size(), nullptr, 0);
+
+  std::vector<wchar_t> wbuf(n);
+  MultiByteToWideChar(CP_UTF8, 0, str.data(), str.size(), wbuf.data(), n);
+
+  // Don't include the final null character in the output
+  CHECK_GT(n, 0);
+  WriteConsoleW(handle, wbuf.data(), n - 1, nullptr, nullptr);
+  return;
+#elif defined(__ANDROID__)
+  if (file == stderr) {
+    __android_log_print(ANDROID_LOG_ERROR, "nodejs", "%s", str.data());
+    return;
+  }
+#endif
+simple_fwrite:
+  fwrite(str.data(), str.size(), 1, file);
+}
 
 }  // namespace node
 
diff --git a/src/debug_utils.h b/src/debug_utils.h
index 08d23bb7703fe6..c745cbe0a1a74b 100644
--- a/src/debug_utils.h
+++ b/src/debug_utils.h
@@ -25,13 +25,16 @@ namespace node {
 template <typename T>
 inline std::string ToString(const T& value);
 
-// C++-style variant of sprintf() that:
+// C++-style variant of sprintf()/fprintf() that:
 // - Returns an std::string
 // - Handles \0 bytes correctly
 // - Supports %p and %s. %d, %i and %u are aliases for %s.
 // - Accepts any class that has a ToString() method for stringification.
 template <typename... Args>
 inline std::string SPrintF(const char* format, Args&&... args);
+template <typename... Args>
+inline void FPrintF(FILE* file, const char* format, Args&&... args);
+void FWrite(FILE* file, const std::string& str);
 
 template <typename... Args>
 inline void FORCE_INLINE Debug(Environment* env,
@@ -40,8 +43,7 @@ inline void FORCE_INLINE Debug(Environment* env,
                                Args&&... args) {
   if (!UNLIKELY(env->debug_enabled(cat)))
     return;
-  std::string out = SPrintF(format, std::forward<Args>(args)...);
-  fwrite(out.data(), out.size(), 1, stderr);
+  FPrintF(stderr, format, std::forward<Args>(args)...);
 }
 
 inline void FORCE_INLINE Debug(Environment* env,
@@ -49,7 +51,7 @@ inline void FORCE_INLINE Debug(Environment* env,
                                const char* message) {
   if (!UNLIKELY(env->debug_enabled(cat)))
     return;
-  fprintf(stderr, "%s", message);
+  FPrintF(stderr, "%s", message);
 }
 
 template <typename... Args>
diff --git a/src/node_errors.cc b/src/node_errors.cc
index 7e23fca28dfc0e..352d86a26783ec 100644
--- a/src/node_errors.cc
+++ b/src/node_errors.cc
@@ -1,6 +1,7 @@
 #include <cerrno>
 #include <cstdarg>
 
+#include "debug_utils-inl.h"
 #include "node_errors.h"
 #include "node_internals.h"
 #ifdef NODE_REPORT
@@ -10,10 +11,6 @@
 #include "node_v8_platform-inl.h"
 #include "util-inl.h"
 
-#ifdef __ANDROID__
-#include <android/log.h>
-#endif
-
 namespace node {
 
 using errors::TryCatchScope;
@@ -54,8 +51,6 @@ namespace per_process {
 static Mutex tty_mutex;
 }  // namespace per_process
 
-static const int kMaxErrorSourceLength = 1024;
-
 static std::string GetErrorSource(Isolate* isolate,
                                   Local<Context> context,
                                   Local<Message> message,
@@ -107,41 +102,35 @@ static std::string GetErrorSource(Isolate* isolate,
     end -= script_start;
   }
 
-  int max_off = kMaxErrorSourceLength - 2;
-
-  char buf[kMaxErrorSourceLength];
-  int off = snprintf(buf,
-                     kMaxErrorSourceLength,
-                     "%s:%i\n%s\n",
-                     filename_string,
-                     linenum,
-                     sourceline.c_str());
-  CHECK_GE(off, 0);
-  if (off > max_off) {
-    off = max_off;
-  }
+  std::string buf = SPrintF("%s:%i\n%s\n",
+                            filename_string,
+                            linenum,
+                            sourceline.c_str());
+  CHECK_GE(buf.size(), 0);
 
+  constexpr int kUnderlineBufsize = 1020;
+  char underline_buf[kUnderlineBufsize + 4];
+  int off = 0;
   // Print wavy underline (GetUnderline is deprecated).
   for (int i = 0; i < start; i++) {
-    if (sourceline[i] == '\0' || off >= max_off) {
+    if (sourceline[i] == '\0' || off >= kUnderlineBufsize) {
       break;
     }
-    CHECK_LT(off, max_off);
-    buf[off++] = (sourceline[i] == '\t') ? '\t' : ' ';
+    CHECK_LT(off, kUnderlineBufsize);
+    underline_buf[off++] = (sourceline[i] == '\t') ? '\t' : ' ';
   }
   for (int i = start; i < end; i++) {
-    if (sourceline[i] == '\0' || off >= max_off) {
+    if (sourceline[i] == '\0' || off >= kUnderlineBufsize) {
       break;
     }
-    CHECK_LT(off, max_off);
-    buf[off++] = '^';
+    CHECK_LT(off, kUnderlineBufsize);
+    underline_buf[off++] = '^';
   }
-  CHECK_LE(off, max_off);
-  buf[off] = '\n';
-  buf[off + 1] = '\0';
+  CHECK_LE(off, kUnderlineBufsize);
+  underline_buf[off++] = '\n';
 
   *added_exception_line = true;
-  return std::string(buf);
+  return buf + std::string(underline_buf, off);
 }
 
 void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
@@ -154,9 +143,9 @@ void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
 
     if (stack_frame->IsEval()) {
       if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) {
-        fprintf(stderr, "    at [eval]:%i:%i\n", line_number, column);
+        FPrintF(stderr, "    at [eval]:%i:%i\n", line_number, column);
       } else {
-        fprintf(stderr,
+        FPrintF(stderr,
                 "    at [eval] (%s:%i:%i)\n",
                 *script_name,
                 line_number,
@@ -166,12 +155,12 @@ void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
     }
 
     if (fn_name_s.length() == 0) {
-      fprintf(stderr, "    at %s:%i:%i\n", *script_name, line_number, column);
+      FPrintF(stderr, "    at %s:%i:%i\n", script_name, line_number, column);
     } else {
-      fprintf(stderr,
+      FPrintF(stderr,
               "    at %s (%s:%i:%i)\n",
-              *fn_name_s,
-              *script_name,
+              fn_name_s,
+              script_name,
               line_number,
               column);
     }
@@ -189,8 +178,8 @@ void PrintException(Isolate* isolate,
   bool added_exception_line = false;
   std::string source =
       GetErrorSource(isolate, context, message, &added_exception_line);
-  fprintf(stderr, "%s\n", source.c_str());
-  fprintf(stderr, "%s\n", *reason);
+  FPrintF(stderr, "%s\n", source);
+  FPrintF(stderr, "%s\n", reason);
 
   Local<v8::StackTrace> stack = message->GetStackTrace();
   if (!stack.IsEmpty()) PrintStackTrace(isolate, stack);
@@ -235,7 +224,7 @@ void AppendExceptionLine(Environment* env,
     env->set_printed_error(true);
 
     ResetStdio();
-    PrintErrorString("\n%s", source.c_str());
+    FPrintF(stderr, "\n%s", source);
     return;
   }
 
@@ -350,10 +339,10 @@ static void ReportFatalException(Environment* env,
   // range errors have a trace member set to undefined
   if (trace.length() > 0 && !stack_trace->IsUndefined()) {
     if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
-      PrintErrorString("%s\n", *trace);
+      FPrintF(stderr, "%s\n", trace);
     } else {
       node::Utf8Value arrow_string(env->isolate(), arrow);
-      PrintErrorString("%s\n%s\n", *arrow_string, *trace);
+      FPrintF(stderr, "%s\n%s\n", arrow_string, trace);
     }
   } else {
     // this really only happens for RangeErrors, since they're the only
@@ -371,33 +360,33 @@ static void ReportFatalException(Environment* env,
     if (message.IsEmpty() || message.ToLocalChecked()->IsUndefined() ||
         name.IsEmpty() || name.ToLocalChecked()->IsUndefined()) {
       // Not an error object. Just print as-is.
-      String::Utf8Value message(env->isolate(), error);
+      node::Utf8Value message(env->isolate(), error);
 
-      PrintErrorString("%s\n",
-                       *message ? *message : "<toString() threw exception>");
+      FPrintF(stderr, "%s\n",
+              *message ? message.ToString() : "<toString() threw exception>");
     } else {
       node::Utf8Value name_string(env->isolate(), name.ToLocalChecked());
       node::Utf8Value message_string(env->isolate(), message.ToLocalChecked());
 
       if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
-        PrintErrorString("%s: %s\n", *name_string, *message_string);
+        FPrintF(stderr, "%s: %s\n", name_string, message_string);
       } else {
         node::Utf8Value arrow_string(env->isolate(), arrow);
-        PrintErrorString(
-            "%s\n%s: %s\n", *arrow_string, *name_string, *message_string);
+        FPrintF(stderr,
+            "%s\n%s: %s\n", arrow_string, name_string, message_string);
       }
     }
 
     if (!env->options()->trace_uncaught) {
-      PrintErrorString("(Use `node --trace-uncaught ...` to show "
-                       "where the exception was thrown)\n");
+      FPrintF(stderr, "(Use `node --trace-uncaught ...` to show "
+                      "where the exception was thrown)\n");
     }
   }
 
   if (env->options()->trace_uncaught) {
     Local<StackTrace> trace = message->GetStackTrace();
     if (!trace.IsEmpty()) {
-      PrintErrorString("Thrown at:\n");
+      FPrintF(stderr, "Thrown at:\n");
       PrintStackTrace(env->isolate(), trace);
     }
   }
@@ -405,42 +394,6 @@ static void ReportFatalException(Environment* env,
   fflush(stderr);
 }
 
-void PrintErrorString(const char* format, ...) {
-  va_list ap;
-  va_start(ap, format);
-#ifdef _WIN32
-  HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE);
-
-  // Check if stderr is something other than a tty/console
-  if (stderr_handle == INVALID_HANDLE_VALUE || stderr_handle == nullptr ||
-      uv_guess_handle(_fileno(stderr)) != UV_TTY) {
-    vfprintf(stderr, format, ap);
-    va_end(ap);
-    return;
-  }
-
-  // Fill in any placeholders
-  int n = _vscprintf(format, ap);
-  std::vector<char> out(n + 1);
-  vsprintf(out.data(), format, ap);
-
-  // Get required wide buffer size
-  n = MultiByteToWideChar(CP_UTF8, 0, out.data(), -1, nullptr, 0);
-
-  std::vector<wchar_t> wbuf(n);
-  MultiByteToWideChar(CP_UTF8, 0, out.data(), -1, wbuf.data(), n);
-
-  // Don't include the null character in the output
-  CHECK_GT(n, 0);
-  WriteConsoleW(stderr_handle, wbuf.data(), n - 1, nullptr, nullptr);
-#elif defined(__ANDROID__)
-  __android_log_vprint(ANDROID_LOG_ERROR, "nodejs", format, ap);
-#else
-  vfprintf(stderr, format, ap);
-#endif
-  va_end(ap);
-}
-
 [[noreturn]] void FatalError(const char* location, const char* message) {
   OnFatalError(location, message);
   // to suppress compiler warning
@@ -449,9 +402,9 @@ void PrintErrorString(const char* format, ...) {
 
 void OnFatalError(const char* location, const char* message) {
   if (location) {
-    PrintErrorString("FATAL ERROR: %s %s\n", location, message);
+    FPrintF(stderr, "FATAL ERROR: %s %s\n", location, message);
   } else {
-    PrintErrorString("FATAL ERROR: %s\n", message);
+    FPrintF(stderr, "FATAL ERROR: %s\n", message);
   }
 #ifdef NODE_REPORT
   Isolate* isolate = Isolate::GetCurrent();
diff --git a/src/node_errors.h b/src/node_errors.h
index e554d5fd9250a3..81062ecb874e77 100644
--- a/src/node_errors.h
+++ b/src/node_errors.h
@@ -25,8 +25,6 @@ void AppendExceptionLine(Environment* env,
 [[noreturn]] void FatalError(const char* location, const char* message);
 void OnFatalError(const char* location, const char* message);
 
-void PrintErrorString(const char* format, ...);
-
 // Helpers to construct errors similar to the ones provided by
 // lib/internal/errors.js.
 // Example: with `V(ERR_INVALID_ARG_TYPE, TypeError)`, there will be
diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc
index 7efe8efb9b9e6d..7b91f89e79159a 100644
--- a/src/node_process_methods.cc
+++ b/src/node_process_methods.cc
@@ -1,4 +1,5 @@
 #include "base_object-inl.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "node.h"
 #include "node_errors.h"
@@ -216,7 +217,7 @@ void RawDebug(const FunctionCallbackInfo<Value>& args) {
   CHECK(args.Length() == 1 && args[0]->IsString() &&
         "must be called with a single string");
   Utf8Value message(args.GetIsolate(), args[0]);
-  PrintErrorString("%s\n", *message);
+  FPrintF(stderr, "%s\n", message);
   fflush(stderr);
 }
 
diff --git a/src/util.h b/src/util.h
index 2f6c17fc321a67..c020f356219df5 100644
--- a/src/util.h
+++ b/src/util.h
@@ -473,6 +473,8 @@ class ArrayBufferViewContents {
 class Utf8Value : public MaybeStackBuffer<char> {
  public:
   explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
+
+  inline std::string ToString() const { return std::string(out(), length()); }
 };
 
 class TwoByteValue : public MaybeStackBuffer<uint16_t> {
@@ -483,6 +485,8 @@ class TwoByteValue : public MaybeStackBuffer<uint16_t> {
 class BufferValue : public MaybeStackBuffer<char> {
  public:
   explicit BufferValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
+
+  inline std::string ToString() const { return std::string(out(), length()); }
 };
 
 #define SPREAD_BUFFER_ARG(val, name)                                          \
diff --git a/test/message/error_with_nul.js b/test/message/error_with_nul.js
new file mode 100644
index 00000000000000..2849e9d21c878c
--- /dev/null
+++ b/test/message/error_with_nul.js
@@ -0,0 +1,11 @@
+'use strict';
+require('../common');
+
+function test() {
+  const a = 'abc\0def';
+  console.error(a);
+  throw new Error(a);
+}
+Object.defineProperty(test, 'name', { value: 'fun\0name' });
+
+test();
diff --git a/test/message/error_with_nul.out b/test/message/error_with_nul.out
new file mode 100644
index 0000000000000000000000000000000000000000..e1f4ea9f0175d49b3f169d23f8ff184e3b29d807
GIT binary patch
literal 552
zcmbVIJ5R$f5bo?>abugo-UDJQTLlXf0_*Br)kO9g`JuG`o@+>i1eH*^&G)^|AMV|%
zql0P;r4*AL`*O5gy&iJgD1dTIaRjCjwikSjn>*RQD>Xe^z*!G6Sm023#TW?IR}I@{
zBkGCyv>(V7eUBXZ9AeE6e`|KJtDdxVF?{r1LV7Ng7`+J-l8qa(@ew;p(+7U%Ef#bt
z#JP{0wJ>jTnk_CdH|ZWLyCR8tx#9l~bRt&^y<|#TF6sOnr3rY&45{)Cdw#oenLsuO
S_OqGq5(P91%YIcllhr4qJh+_z

literal 0
HcmV?d00001


From 6db1bb74574dd54e440024ba5908acc2321d812e Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 21:15:52 +0100
Subject: [PATCH 03/11] fixup! src: add C++-style sprintf utility

---
 src/debug_utils.cc    | 2 +-
 src/inspector_io.cc   | 2 +-
 src/node.cc           | 2 +-
 src/node_messaging.cc | 2 +-
 src/node_platform.cc  | 2 +-
 src/node_report.cc    | 2 +-
 src/node_watchdog.cc  | 2 +-
 src/node_worker.cc    | 2 +-
 src/spawn_sync.cc     | 2 +-
 src/tracing/agent.cc  | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/debug_utils.cc b/src/debug_utils.cc
index c71be6ffc98157..05ab13da3f54b9 100644
--- a/src/debug_utils.cc
+++ b/src/debug_utils.cc
@@ -1,4 +1,4 @@
-#include "debug_utils.h"
+#include "debug_utils-inl.h"  // NOLINT(build/include)
 #include "env-inl.h"
 
 #ifdef __POSIX__
diff --git a/src/inspector_io.cc b/src/inspector_io.cc
index 76e481c9530d95..ed9035136c51db 100644
--- a/src/inspector_io.cc
+++ b/src/inspector_io.cc
@@ -4,7 +4,7 @@
 #include "inspector/main_thread_interface.h"
 #include "inspector/node_string.h"
 #include "base_object-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "node.h"
 #include "node_crypto.h"
 #include "node_internals.h"
diff --git a/src/node.cc b/src/node.cc
index d46c78a927b39c..0cb89044e58f74 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -23,7 +23,7 @@
 
 // ========== local headers ==========
 
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_binding.h"
diff --git a/src/node_messaging.cc b/src/node_messaging.cc
index 1028da69b44961..58b2a3c6b5af7e 100644
--- a/src/node_messaging.cc
+++ b/src/node_messaging.cc
@@ -1,7 +1,7 @@
 #include "node_messaging.h"
 
 #include "async_wrap-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_contextify.h"
 #include "node_buffer.h"
diff --git a/src/node_platform.cc b/src/node_platform.cc
index 4be929c7ee3a6d..b30f907a02d159 100644
--- a/src/node_platform.cc
+++ b/src/node_platform.cc
@@ -2,7 +2,7 @@
 #include "node_internals.h"
 
 #include "env-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include <algorithm>  // find_if(), find(), move()
 #include <cmath>  // llround()
 #include <memory>  // unique_ptr(), shared_ptr(), make_shared()
diff --git a/src/node_report.cc b/src/node_report.cc
index ddeb216c82d6bc..3a6cb42556461d 100644
--- a/src/node_report.cc
+++ b/src/node_report.cc
@@ -1,6 +1,6 @@
 #include "env-inl.h"
 #include "node_report.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "diagnosticfilename-inl.h"
 #include "node_internals.h"
 #include "node_metadata.h"
diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc
index a0f73e973f6b67..4ab625ce65b598 100644
--- a/src/node_watchdog.cc
+++ b/src/node_watchdog.cc
@@ -21,7 +21,7 @@
 
 #include <algorithm>
 
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "node_errors.h"
 #include "node_internals.h"
diff --git a/src/node_worker.cc b/src/node_worker.cc
index 2f43780a951ad4..1cb8091c258aa3 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -1,5 +1,5 @@
 #include "node_worker.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_errors.h"
 #include "node_buffer.h"
diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc
index 0751bc21a7eb3d..3b277ad70adb66 100644
--- a/src/spawn_sync.cc
+++ b/src/spawn_sync.cc
@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "spawn_sync.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "node_internals.h"
 #include "string_bytes.h"
diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc
index 3d19c6c75844fc..7d265dcb0c4c3b 100644
--- a/src/tracing/agent.cc
+++ b/src/tracing/agent.cc
@@ -3,7 +3,7 @@
 #include <string>
 #include "trace_event.h"
 #include "tracing/node_trace_buffer.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 
 namespace node {

From 3d905534f2af713725a39781efcc0cf4a6e6a140 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 21:15:55 +0100
Subject: [PATCH 04/11] fixup! src: use custom fprintf alike to write errors to
 stderr

---
 src/debug_utils.cc | 15 +++++++++++----
 src/node_errors.cc |  2 +-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/debug_utils.cc b/src/debug_utils.cc
index 05ab13da3f54b9..664da3d646be75 100644
--- a/src/debug_utils.cc
+++ b/src/debug_utils.cc
@@ -442,7 +442,14 @@ std::vector<std::string> NativeSymbolDebuggingContext::GetLoadedLibraries() {
 }
 
 void FWrite(FILE* file, const std::string& str) {
-  if (file != stderr && file != stdout) goto simple_fwrite;
+  auto simple_fwrite = [&]() {
+    fwrite(str.data(), str.size(), 1, file);
+  };
+
+  if (file != stderr && file != stdout) {
+    simple_fwrite();
+    return;
+  }
 #ifdef _WIN32
   HANDLE handle =
       GetStdHandle(file == stdout ? STD_OUTPUT_HANDLE : STD_ERROR_HANDLE);
@@ -450,7 +457,8 @@ void FWrite(FILE* file, const std::string& str) {
   // Check if stderr is something other than a tty/console
   if (handle == INVALID_HANDLE_VALUE || handle == nullptr ||
       uv_guess_handle(_fileno(file)) != UV_TTY) {
-    goto simple_fwrite;
+    simple_fwrite();
+    return;
   }
 
   // Get required wide buffer size
@@ -469,8 +477,7 @@ void FWrite(FILE* file, const std::string& str) {
     return;
   }
 #endif
-simple_fwrite:
-  fwrite(str.data(), str.size(), 1, file);
+  simple_fwrite();
 }
 
 }  // namespace node
diff --git a/src/node_errors.cc b/src/node_errors.cc
index 352d86a26783ec..f82054c339b493 100644
--- a/src/node_errors.cc
+++ b/src/node_errors.cc
@@ -106,7 +106,7 @@ static std::string GetErrorSource(Isolate* isolate,
                             filename_string,
                             linenum,
                             sourceline.c_str());
-  CHECK_GE(buf.size(), 0);
+  CHECK_GT(buf.size(), 0);
 
   constexpr int kUnderlineBufsize = 1020;
   char underline_buf[kUnderlineBufsize + 4];

From 8824f8bc9ee4c11c3ab44430f4e6a672ebd4e3fe Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 22:59:02 +0100
Subject: [PATCH 05/11] fixup! src: add C++-style sprintf utility

---
 src/debug_utils-inl.h    | 8 ++++++--
 test/cctest/test_util.cc | 2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/debug_utils-inl.h b/src/debug_utils-inl.h
index 03b49490622d49..d5303bf41e50fe 100644
--- a/src/debug_utils-inl.h
+++ b/src/debug_utils-inl.h
@@ -32,14 +32,18 @@ std::string ToString(const T& value) {
 }
 
 inline std::string SPrintFImpl(const char* format) {
-  return format;
+  const char* p = strchr(format, '%');
+  if (LIKELY(p == nullptr)) return format;
+  CHECK(p[1] == '%');  // Only '%%' allowed when there are no arguments.
+
+  return std::string(format, p + 1) + SPrintFImpl(p + 2);
 }
 
 template <typename Arg, typename... Args>
 std::string COLD_NOINLINE SPrintFImpl(  // NOLINT(runtime/string)
     const char* format, Arg&& arg, Args&&... args) {
   const char* p = strchr(format, '%');
-  if (p == nullptr) return format;
+  CHECK_NOT_NULL(p);
   std::string ret(format, p);
   // Ignore long / size_t modifiers
   while (strchr("lz", *++p) != nullptr) {}
diff --git a/test/cctest/test_util.cc b/test/cctest/test_util.cc
index de9c5368bddfda..e3a4a700e96583 100644
--- a/test/cctest/test_util.cc
+++ b/test/cctest/test_util.cc
@@ -282,6 +282,8 @@ TEST(UtilTest, SPrintF) {
   EXPECT_EQ(SPrintF("%s %s", foo, "bar"), "foo bar");
   EXPECT_EQ(SPrintF("%s %s", foo, bar), "foo bar");
 
+  EXPECT_EQ(SPrintF("[%% %s %%]", foo), "[% foo %]");
+
   struct HasToString {
     std::string ToString() const {
       return "meow";

From bed1750a2f331942a1276e3207c8c37f5c848783 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 23:07:44 +0100
Subject: [PATCH 06/11] fixup! src: use custom fprintf alike to write errors to
 stderr

---
 test/message/error_with_nul.out | Bin 552 -> 549 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)

diff --git a/test/message/error_with_nul.out b/test/message/error_with_nul.out
index e1f4ea9f0175d49b3f169d23f8ff184e3b29d807..396d94debf0058b49d9be19dcc6519486b9f8a6d 100644
GIT binary patch
delta 27
jcmZ3%vXq50F)5iLB{hvpYa&;`#D2Mnm7*K_XEFi+c47$%

delta 32
ocmZ3=vVw&xF)5iLB{hvpOMfC~0HglI9=VC-qKx_*duB2M0GP}PkpKVy


From cb717aa229ab9b3e749b9b4497182be5c7a54fb0 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 23:08:07 +0100
Subject: [PATCH 07/11] fixup! fixup! src: add C++-style sprintf utility

---
 test/cctest/test_util.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/cctest/test_util.cc b/test/cctest/test_util.cc
index e3a4a700e96583..843d16d9f527c6 100644
--- a/test/cctest/test_util.cc
+++ b/test/cctest/test_util.cc
@@ -274,8 +274,8 @@ TEST(UtilTest, SPrintF) {
 
   void (*fn)() = []() {};
   void* p = reinterpret_cast<void*>(&fn);
-  EXPECT_GE(SPrintF("%p", fn).size(), 8u);
-  EXPECT_GE(SPrintF("%p", p).size(), 8u);
+  EXPECT_GE(SPrintF("%p", fn).size(), 4u);
+  EXPECT_GE(SPrintF("%p", p).size(), 4u);
 
   const std::string foo = "foo";
   const char* bar = "bar";

From 3a1c8013975c82cc1847d79111ccd77beef99e75 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 21 Jan 2020 23:18:08 +0100
Subject: [PATCH 08/11] fixup! fixup! fixup! src: add C++-style sprintf utility

---
 src/debug_utils-inl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/debug_utils-inl.h b/src/debug_utils-inl.h
index d5303bf41e50fe..a066c9be6d19d8 100644
--- a/src/debug_utils-inl.h
+++ b/src/debug_utils-inl.h
@@ -34,7 +34,7 @@ std::string ToString(const T& value) {
 inline std::string SPrintFImpl(const char* format) {
   const char* p = strchr(format, '%');
   if (LIKELY(p == nullptr)) return format;
-  CHECK(p[1] == '%');  // Only '%%' allowed when there are no arguments.
+  CHECK_EQ(p[1], '%');  // Only '%%' allowed when there are no arguments.
 
   return std::string(format, p + 1) + SPrintFImpl(p + 2);
 }

From a851023758af8a8b11d90784df2681e48d37a751 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 22 Jan 2020 17:48:14 +0100
Subject: [PATCH 09/11] fixup! src: add C++-style sprintf utility

Co-Authored-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 src/debug_utils-inl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/debug_utils-inl.h b/src/debug_utils-inl.h
index a066c9be6d19d8..f24643fbae3ad8 100644
--- a/src/debug_utils-inl.h
+++ b/src/debug_utils-inl.h
@@ -43,7 +43,7 @@ template <typename Arg, typename... Args>
 std::string COLD_NOINLINE SPrintFImpl(  // NOLINT(runtime/string)
     const char* format, Arg&& arg, Args&&... args) {
   const char* p = strchr(format, '%');
-  CHECK_NOT_NULL(p);
+  CHECK_NOT_NULL(p);  // If you hit this, you passed in too many arguments.
   std::string ret(format, p);
   // Ignore long / size_t modifiers
   while (strchr("lz", *++p) != nullptr) {}

From bd8c9d68783ebcfeca52b1e2acf94835e9ded661 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 22 Jan 2020 17:49:16 +0100
Subject: [PATCH 10/11] fixup! src: use custom fprintf alike to write errors to
 stderr

---
 src/debug_utils.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/debug_utils.cc b/src/debug_utils.cc
index 664da3d646be75..8f1e6aa3e6f77c 100644
--- a/src/debug_utils.cc
+++ b/src/debug_utils.cc
@@ -443,6 +443,7 @@ std::vector<std::string> NativeSymbolDebuggingContext::GetLoadedLibraries() {
 
 void FWrite(FILE* file, const std::string& str) {
   auto simple_fwrite = [&]() {
+    // The return value is ignored because there's no good way to handle it.
     fwrite(str.data(), str.size(), 1, file);
   };
 

From 13b9da7c4cb15baa68db74317b63ac19023f01ab Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 22 Jan 2020 17:57:09 +0100
Subject: [PATCH 11/11] fixup! src: add C++-style sprintf utility

---
 src/node_http2.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/node_http2.cc b/src/node_http2.cc
index f8d9363902b829..e48bf091408000 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -1959,7 +1959,7 @@ std::string Http2Stream::diagnostic_name() const {
 
 // Notify the Http2Stream that a new block of HEADERS is being processed.
 void Http2Stream::StartHeaders(nghttp2_headers_category category) {
-  Debug(this, "starting headers, category: %d", id_, category);
+  Debug(this, "starting headers, category: %d", category);
   CHECK(!this->IsDestroyed());
   session_->DecrementCurrentSessionMemory(current_headers_length_);
   current_headers_length_ = 0;
@@ -2217,7 +2217,7 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap,
     req_wrap->Done(UV_EOF);
     return 0;
   }
-  Debug(this, "queuing %d buffers to send", id_, nbufs);
+  Debug(this, "queuing %d buffers to send", nbufs);
   for (size_t i = 0; i < nbufs; ++i) {
     // Store the req_wrap on the last write info in the queue, so that it is
     // only marked as finished once all buffers associated with it are finished.