From aec9ad8f6c8a75518d81279ec5e7188348e508de Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Thu, 30 Jan 2020 16:37:02 +0200 Subject: [PATCH] src: fix console debug output on Windows 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: https://github.com/nodejs/node/pull/31580 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Rich Trott --- src/debug_utils.cc | 4 +--- test/parallel/test-http2-debug.js | 12 ++++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/debug_utils.cc b/src/debug_utils.cc index 8f1e6aa3e6f77c..984c04eb2d381d 100644 --- a/src/debug_utils.cc +++ b/src/debug_utils.cc @@ -468,9 +468,7 @@ void FWrite(FILE* file, const std::string& str) { std::vector 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) { diff --git a/test/parallel/test-http2-debug.js b/test/parallel/test-http2-debug.js index ce1873234e76ec..0d6b5f677c234b 100644 --- a/test/parallel/test-http2-debug.js +++ b/test/parallel/test-http2-debug.js @@ -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(), '');