Skip to content

Commit a309af0

Browse files
addaleaxcodebytere
addaleax
authored and
codebytere
committed
src: use custom fprintf alike to write errors to stderr
This allows printing errors that contain nul characters, for example. Fixes: #28761 Fixes: #31218 PR-URL: #31446 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 7bdd29f commit a309af0

File tree

9 files changed

+111
-94
lines changed

9 files changed

+111
-94
lines changed

‎src/debug_utils-inl.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ std::string COLD_NOINLINE SPrintF( // NOLINT(runtime/string)
8383
return SPrintFImpl(format, std::forward<Args>(args)...);
8484
}
8585

86+
template <typename... Args>
87+
void COLD_NOINLINE FPrintF(FILE* file, const char* format, Args&&... args) {
88+
FWrite(file, SPrintF(format, std::forward<Args>(args)...));
89+
}
90+
8691
} // namespace node
8792

8893
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

‎src/debug_utils.cc‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
#include <features.h>
77
#endif
88

9+
#ifdef __ANDROID__
10+
#include <android/log.h>
11+
#endif
12+
913
#if defined(__linux__) && !defined(__GLIBC__) || \
1014
defined(__UCLIBC__) || \
1115
defined(_AIX)
@@ -437,6 +441,45 @@ std::vector<std::string> NativeSymbolDebuggingContext::GetLoadedLibraries() {
437441
return list;
438442
}
439443

444+
void FWrite(FILE* file, const std::string& str) {
445+
auto simple_fwrite = [&]() {
446+
// The return value is ignored because there's no good way to handle it.
447+
fwrite(str.data(), str.size(), 1, file);
448+
};
449+
450+
if (file != stderr && file != stdout) {
451+
simple_fwrite();
452+
return;
453+
}
454+
#ifdef _WIN32
455+
HANDLE handle =
456+
GetStdHandle(file == stdout ? STD_OUTPUT_HANDLE : STD_ERROR_HANDLE);
457+
458+
// Check if stderr is something other than a tty/console
459+
if (handle == INVALID_HANDLE_VALUE || handle == nullptr ||
460+
uv_guess_handle(_fileno(file)) != UV_TTY) {
461+
simple_fwrite();
462+
return;
463+
}
464+
465+
// Get required wide buffer size
466+
int n = MultiByteToWideChar(CP_UTF8, 0, str.data(), str.size(), nullptr, 0);
467+
468+
std::vector<wchar_t> wbuf(n);
469+
MultiByteToWideChar(CP_UTF8, 0, str.data(), str.size(), wbuf.data(), n);
470+
471+
// Don't include the final null character in the output
472+
CHECK_GT(n, 0);
473+
WriteConsoleW(handle, wbuf.data(), n - 1, nullptr, nullptr);
474+
return;
475+
#elif defined(__ANDROID__)
476+
if (file == stderr) {
477+
__android_log_print(ANDROID_LOG_ERROR, "nodejs", "%s", str.data());
478+
return;
479+
}
480+
#endif
481+
simple_fwrite();
482+
}
440483

441484
} // namespace node
442485

‎src/debug_utils.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,16 @@ namespace node {
2525
template <typename T>
2626
inline std::string ToString(const T& value);
2727

28-
// C++-style variant of sprintf() that:
28+
// C++-style variant of sprintf()/fprintf() that:
2929
// - Returns an std::string
3030
// - Handles \0 bytes correctly
3131
// - Supports %p and %s. %d, %i and %u are aliases for %s.
3232
// - Accepts any class that has a ToString() method for stringification.
3333
template <typename... Args>
3434
inline std::string SPrintF(const char* format, Args&&... args);
35+
template <typename... Args>
36+
inline void FPrintF(FILE* file, const char* format, Args&&... args);
37+
void FWrite(FILE* file, const std::string& str);
3538

3639
template <typename... Args>
3740
inline void FORCE_INLINE Debug(Environment* env,
@@ -40,16 +43,15 @@ inline void FORCE_INLINE Debug(Environment* env,
4043
Args&&... args) {
4144
if (!UNLIKELY(env->debug_enabled(cat)))
4245
return;
43-
std::string out = SPrintF(format, std::forward<Args>(args)...);
44-
fwrite(out.data(), out.size(), 1, stderr);
46+
FPrintF(stderr, format, std::forward<Args>(args)...);
4547
}
4648

4749
inline void FORCE_INLINE Debug(Environment* env,
4850
DebugCategory cat,
4951
const char* message) {
5052
if (!UNLIKELY(env->debug_enabled(cat)))
5153
return;
52-
fprintf(stderr, "%s", message);
54+
FPrintF(stderr, "%s", message);
5355
}
5456

5557
template <typename... Args>

‎src/node_errors.cc‎

Lines changed: 40 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include <cerrno>
22
#include <cstdarg>
33

4+
#include "debug_utils-inl.h"
45
#include "node_errors.h"
56
#include "node_internals.h"
67
#ifdef NODE_REPORT
@@ -10,10 +11,6 @@
1011
#include "node_v8_platform-inl.h"
1112
#include "util-inl.h"
1213

13-
#ifdef __ANDROID__
14-
#include <android/log.h>
15-
#endif
16-
1714
namespace node {
1815

1916
using errors::TryCatchScope;
@@ -54,8 +51,6 @@ namespace per_process {
5451
static Mutex tty_mutex;
5552
} // namespace per_process
5653

57-
static const int kMaxErrorSourceLength = 1024;
58-
5954
static std::string GetErrorSource(Isolate* isolate,
6055
Local<Context> context,
6156
Local<Message> message,
@@ -107,41 +102,35 @@ static std::string GetErrorSource(Isolate* isolate,
107102
end -= script_start;
108103
}
109104

110-
int max_off = kMaxErrorSourceLength - 2;
111-
112-
char buf[kMaxErrorSourceLength];
113-
int off = snprintf(buf,
114-
kMaxErrorSourceLength,
115-
"%s:%i\n%s\n",
116-
filename_string,
117-
linenum,
118-
sourceline.c_str());
119-
CHECK_GE(off, 0);
120-
if (off > max_off) {
121-
off = max_off;
122-
}
105+
std::string buf = SPrintF("%s:%i\n%s\n",
106+
filename_string,
107+
linenum,
108+
sourceline.c_str());
109+
CHECK_GT(buf.size(), 0);
123110

111+
constexpr int kUnderlineBufsize = 1020;
112+
char underline_buf[kUnderlineBufsize + 4];
113+
int off = 0;
124114
// Print wavy underline (GetUnderline is deprecated).
125115
for (int i = 0; i < start; i++) {
126-
if (sourceline[i] == '\0' || off >= max_off) {
116+
if (sourceline[i] == '\0' || off >= kUnderlineBufsize) {
127117
break;
128118
}
129-
CHECK_LT(off, max_off);
130-
buf[off++] = (sourceline[i] == '\t') ? '\t' : ' ';
119+
CHECK_LT(off, kUnderlineBufsize);
120+
underline_buf[off++] = (sourceline[i] == '\t') ? '\t' : ' ';
131121
}
132122
for (int i = start; i < end; i++) {
133-
if (sourceline[i] == '\0' || off >= max_off) {
123+
if (sourceline[i] == '\0' || off >= kUnderlineBufsize) {
134124
break;
135125
}
136-
CHECK_LT(off, max_off);
137-
buf[off++] = '^';
126+
CHECK_LT(off, kUnderlineBufsize);
127+
underline_buf[off++] = '^';
138128
}
139-
CHECK_LE(off, max_off);
140-
buf[off] = '\n';
141-
buf[off + 1] = '\0';
129+
CHECK_LE(off, kUnderlineBufsize);
130+
underline_buf[off++] = '\n';
142131

143132
*added_exception_line = true;
144-
return std::string(buf);
133+
return buf + std::string(underline_buf, off);
145134
}
146135

147136
void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
@@ -154,9 +143,9 @@ void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
154143

155144
if (stack_frame->IsEval()) {
156145
if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) {
157-
fprintf(stderr, " at [eval]:%i:%i\n", line_number, column);
146+
FPrintF(stderr, " at [eval]:%i:%i\n", line_number, column);
158147
} else {
159-
fprintf(stderr,
148+
FPrintF(stderr,
160149
" at [eval] (%s:%i:%i)\n",
161150
*script_name,
162151
line_number,
@@ -166,12 +155,12 @@ void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
166155
}
167156

168157
if (fn_name_s.length() == 0) {
169-
fprintf(stderr, " at %s:%i:%i\n", *script_name, line_number, column);
158+
FPrintF(stderr, " at %s:%i:%i\n", script_name, line_number, column);
170159
} else {
171-
fprintf(stderr,
160+
FPrintF(stderr,
172161
" at %s (%s:%i:%i)\n",
173-
*fn_name_s,
174-
*script_name,
162+
fn_name_s,
163+
script_name,
175164
line_number,
176165
column);
177166
}
@@ -189,8 +178,8 @@ void PrintException(Isolate* isolate,
189178
bool added_exception_line = false;
190179
std::string source =
191180
GetErrorSource(isolate, context, message, &added_exception_line);
192-
fprintf(stderr, "%s\n", source.c_str());
193-
fprintf(stderr, "%s\n", *reason);
181+
FPrintF(stderr, "%s\n", source);
182+
FPrintF(stderr, "%s\n", reason);
194183

195184
Local<v8::StackTrace> stack = message->GetStackTrace();
196185
if (!stack.IsEmpty()) PrintStackTrace(isolate, stack);
@@ -235,7 +224,7 @@ void AppendExceptionLine(Environment* env,
235224
env->set_printed_error(true);
236225

237226
ResetStdio();
238-
PrintErrorString("\n%s", source.c_str());
227+
FPrintF(stderr, "\n%s", source);
239228
return;
240229
}
241230

@@ -347,10 +336,10 @@ static void ReportFatalException(Environment* env,
347336
// range errors have a trace member set to undefined
348337
if (trace.length() > 0 && !stack_trace->IsUndefined()) {
349338
if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
350-
PrintErrorString("%s\n", *trace);
339+
FPrintF(stderr, "%s\n", trace);
351340
} else {
352341
node::Utf8Value arrow_string(env->isolate(), arrow);
353-
PrintErrorString("%s\n%s\n", *arrow_string, *trace);
342+
FPrintF(stderr, "%s\n%s\n", arrow_string, trace);
354343
}
355344
} else {
356345
// this really only happens for RangeErrors, since they're the only
@@ -368,76 +357,40 @@ static void ReportFatalException(Environment* env,
368357
if (message.IsEmpty() || message.ToLocalChecked()->IsUndefined() ||
369358
name.IsEmpty() || name.ToLocalChecked()->IsUndefined()) {
370359
// Not an error object. Just print as-is.
371-
String::Utf8Value message(env->isolate(), error);
360+
node::Utf8Value message(env->isolate(), error);
372361

373-
PrintErrorString("%s\n",
374-
*message ? *message : "<toString() threw exception>");
362+
FPrintF(stderr, "%s\n",
363+
*message ? message.ToString() : "<toString() threw exception>");
375364
} else {
376365
node::Utf8Value name_string(env->isolate(), name.ToLocalChecked());
377366
node::Utf8Value message_string(env->isolate(), message.ToLocalChecked());
378367

379368
if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
380-
PrintErrorString("%s: %s\n", *name_string, *message_string);
369+
FPrintF(stderr, "%s: %s\n", name_string, message_string);
381370
} else {
382371
node::Utf8Value arrow_string(env->isolate(), arrow);
383-
PrintErrorString(
384-
"%s\n%s: %s\n", *arrow_string, *name_string, *message_string);
372+
FPrintF(stderr,
373+
"%s\n%s: %s\n", arrow_string, name_string, message_string);
385374
}
386375
}
387376

388377
if (!env->options()->trace_uncaught) {
389-
PrintErrorString("(Use `node --trace-uncaught ...` to show "
390-
"where the exception was thrown)\n");
378+
FPrintF(stderr, "(Use `node --trace-uncaught ...` to show "
379+
"where the exception was thrown)\n");
391380
}
392381
}
393382

394383
if (env->options()->trace_uncaught) {
395384
Local<StackTrace> trace = message->GetStackTrace();
396385
if (!trace.IsEmpty()) {
397-
PrintErrorString("Thrown at:\n");
386+
FPrintF(stderr, "Thrown at:\n");
398387
PrintStackTrace(env->isolate(), trace);
399388
}
400389
}
401390

402391
fflush(stderr);
403392
}
404393

405-
void PrintErrorString(const char* format, ...) {
406-
va_list ap;
407-
va_start(ap, format);
408-
#ifdef _WIN32
409-
HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE);
410-
411-
// Check if stderr is something other than a tty/console
412-
if (stderr_handle == INVALID_HANDLE_VALUE || stderr_handle == nullptr ||
413-
uv_guess_handle(_fileno(stderr)) != UV_TTY) {
414-
vfprintf(stderr, format, ap);
415-
va_end(ap);
416-
return;
417-
}
418-
419-
// Fill in any placeholders
420-
int n = _vscprintf(format, ap);
421-
std::vector<char> out(n + 1);
422-
vsprintf(out.data(), format, ap);
423-
424-
// Get required wide buffer size
425-
n = MultiByteToWideChar(CP_UTF8, 0, out.data(), -1, nullptr, 0);
426-
427-
std::vector<wchar_t> wbuf(n);
428-
MultiByteToWideChar(CP_UTF8, 0, out.data(), -1, wbuf.data(), n);
429-
430-
// Don't include the null character in the output
431-
CHECK_GT(n, 0);
432-
WriteConsoleW(stderr_handle, wbuf.data(), n - 1, nullptr, nullptr);
433-
#elif defined(__ANDROID__)
434-
__android_log_vprint(ANDROID_LOG_ERROR, "nodejs", format, ap);
435-
#else
436-
vfprintf(stderr, format, ap);
437-
#endif
438-
va_end(ap);
439-
}
440-
441394
[[noreturn]] void FatalError(const char* location, const char* message) {
442395
OnFatalError(location, message);
443396
// to suppress compiler warning
@@ -446,9 +399,9 @@ void PrintErrorString(const char* format, ...) {
446399

447400
void OnFatalError(const char* location, const char* message) {
448401
if (location) {
449-
PrintErrorString("FATAL ERROR: %s %s\n", location, message);
402+
FPrintF(stderr, "FATAL ERROR: %s %s\n", location, message);
450403
} else {
451-
PrintErrorString("FATAL ERROR: %s\n", message);
404+
FPrintF(stderr, "FATAL ERROR: %s\n", message);
452405
}
453406
#ifdef NODE_REPORT
454407
Isolate* isolate = Isolate::GetCurrent();

‎src/node_errors.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ void AppendExceptionLine(Environment* env,
2525
[[noreturn]] void FatalError(const char* location, const char* message);
2626
void OnFatalError(const char* location, const char* message);
2727

28-
void PrintErrorString(const char* format, ...);
29-
3028
// Helpers to construct errors similar to the ones provided by
3129
// lib/internal/errors.js.
3230
// Example: with `V(ERR_INVALID_ARG_TYPE, TypeError)`, there will be

‎src/node_process_methods.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "base_object-inl.h"
2+
#include "debug_utils-inl.h"
23
#include "env-inl.h"
34
#include "node.h"
45
#include "node_errors.h"
@@ -209,7 +210,7 @@ void RawDebug(const FunctionCallbackInfo<Value>& args) {
209210
CHECK(args.Length() == 1 && args[0]->IsString() &&
210211
"must be called with a single string");
211212
Utf8Value message(args.GetIsolate(), args[0]);
212-
PrintErrorString("%s\n", *message);
213+
FPrintF(stderr, "%s\n", message);
213214
fflush(stderr);
214215
}
215216

‎src/util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,8 @@ class ArrayBufferViewContents {
475475
class Utf8Value : public MaybeStackBuffer<char> {
476476
public:
477477
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
478+
479+
inline std::string ToString() const { return std::string(out(), length()); }
478480
};
479481

480482
class TwoByteValue : public MaybeStackBuffer<uint16_t> {
@@ -485,6 +487,8 @@ class TwoByteValue : public MaybeStackBuffer<uint16_t> {
485487
class BufferValue : public MaybeStackBuffer<char> {
486488
public:
487489
explicit BufferValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
490+
491+
inline std::string ToString() const { return std::string(out(), length()); }
488492
};
489493

490494
#define SPREAD_BUFFER_ARG(val, name) \

0 commit comments

Comments
 (0)