Skip to content

Commit

Permalink
src: fix console debug output on Windows
Browse files Browse the repository at this point in the history
The FWrite function on Windows assumed that MultiByteToWideChar
automatically null-terminates the resulting string, but it will only do
so if the size of the source was passed as -1 or null character was
explicitly counted in the size. The FWrite uses std::string and its
`.size()` method which doesn't count the null character even though the
`.data()` method adds it to the resulting string.

https://docs.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-multibytetowidechar#remarks
> MultiByteToWideChar does not null-terminate an output string if the
  input string length is explicitly specified without a terminating null
  character. To null-terminate an output string for this function, the
  application should pass in -1 or explicitly count the terminating null
  character for the input string.

PR-URL: #31580
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
lundibundi authored and codebytere committed Feb 17, 2020
1 parent 49be500 commit 9fd1e71
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 9 deletions.
4 changes: 1 addition & 3 deletions src/debug_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,7 @@ void FWrite(FILE* file, const std::string& str) {
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);
WriteConsoleW(handle, wbuf.data(), n, nullptr, nullptr);
return;
#elif defined(__ANDROID__)
if (file == stderr) {
Expand Down
12 changes: 6 additions & 6 deletions test/parallel/test-http2-debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ const { stdout, stderr } = child_process.spawnSync(process.execPath, [
path.resolve(__dirname, 'test-http2-ping.js')
], { encoding: 'utf8' });

assert(stderr.match(/Setting the NODE_DEBUG environment variable to 'http2' can expose sensitive data \(such as passwords, tokens and authentication headers\) in the resulting log\./),
assert(stderr.match(/Setting the NODE_DEBUG environment variable to 'http2' can expose sensitive data \(such as passwords, tokens and authentication headers\) in the resulting log\.\r?\n/),
stderr);
assert(stderr.match(/Http2Session client \(\d+\) handling data frame for stream \d+/),
assert(stderr.match(/Http2Session client \(\d+\) handling data frame for stream \d+\r?\n/),
stderr);
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session client \(\d+\)\] reading starting/),
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session client \(\d+\)\] reading starting\r?\n/),
stderr);
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session client \(\d+\)\] closed with code 0/),
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session client \(\d+\)\] closed with code 0\r?\n/),
stderr);
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session server \(\d+\)\] closed with code 0/),
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session server \(\d+\)\] closed with code 0\r?\n/),
stderr);
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session server \(\d+\)\] tearing down stream/),
assert(stderr.match(/HttpStream \d+ \(\d+\) \[Http2Session server \(\d+\)\] tearing down stream\r?\n/),
stderr);
assert.strictEqual(stdout.trim(), '');

0 comments on commit 9fd1e71

Please sign in to comment.