From 12d0405b8578409023e7fed5770787419156b66a Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 22 Jul 2020 12:55:11 +0200 Subject: [PATCH 1/5] process: correctly parse Unicode in NODE_OPTIONS Fixes an issue on Windows, where Unicode in NODE_OPTIONS was not parsed correctly. Fixes: https://github.com/nodejs/node/issues/34399 --- src/node_credentials.cc | 16 +++++++++++++-- test/parallel/test-unicode-node-options.js | 24 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-unicode-node-options.js diff --git a/src/node_credentials.cc b/src/node_credentials.cc index acc48cac3c90ef..dcd0d037f12fd4 100644 --- a/src/node_credentials.cc +++ b/src/node_credentials.cc @@ -57,8 +57,20 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) { { Mutex::ScopedLock lock(per_process::env_var_mutex); - if (const char* value = getenv(key)) { - *text = value; + + size_t init_sz = 256; + MaybeStackBuffer val; + int ret = uv_os_getenv(key, *val, &init_sz); + + if (ret == UV_ENOBUFS) { + // Buffer is not large enough, reallocate to the updated init_sz + // and fetch env value again. + val.AllocateSufficientStorage(init_sz); + ret = uv_os_getenv(key, *val, &init_sz); + } + + if (ret >= 0) { // Env key value fetch success. + *text = *val; return true; } } diff --git a/test/parallel/test-unicode-node-options.js b/test/parallel/test-unicode-node-options.js new file mode 100644 index 00000000000000..0f2f2958b299e8 --- /dev/null +++ b/test/parallel/test-unicode-node-options.js @@ -0,0 +1,24 @@ +'use strict'; +// Flags: --expose-internals +require('../common'); +const { getOptionValue } = require('internal/options'); +const assert = require('assert'); +const cp = require('child_process'); + +const expected_redirect_value = 'foó'; + +if (process.argv.length === 2) { + const NODE_OPTIONS = `--redirect-warnings=${expected_redirect_value}`; + const result = cp.spawnSync(process.argv0, + ['--expose-internals', __filename, 'test'], + { + env: { + NODE_OPTIONS + } + }); + assert.strictEqual(result.status, 0); + process.exit(0); +} else { + const redirect_value = getOptionValue('--redirect-warnings'); + assert.strictEqual(redirect_value, expected_redirect_value); +} From 7b15fa7724171ebc7ffecf86e7c6239e35998689 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 22 Jul 2020 14:48:49 +0200 Subject: [PATCH 2/5] fixup: linter --- src/node_credentials.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_credentials.cc b/src/node_credentials.cc index dcd0d037f12fd4..fa3dfa48a3ceb2 100644 --- a/src/node_credentials.cc +++ b/src/node_credentials.cc @@ -57,7 +57,7 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) { { Mutex::ScopedLock lock(per_process::env_var_mutex); - + size_t init_sz = 256; MaybeStackBuffer val; int ret = uv_os_getenv(key, *val, &init_sz); From c88bb3981be99f798ce4d5e2e11a2c512e323482 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 12 Aug 2020 15:38:31 +0200 Subject: [PATCH 3/5] fixup: nits --- test/parallel/test-unicode-node-options.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/parallel/test-unicode-node-options.js b/test/parallel/test-unicode-node-options.js index 0f2f2958b299e8..b22ff2141efaae 100644 --- a/test/parallel/test-unicode-node-options.js +++ b/test/parallel/test-unicode-node-options.js @@ -11,13 +11,8 @@ if (process.argv.length === 2) { const NODE_OPTIONS = `--redirect-warnings=${expected_redirect_value}`; const result = cp.spawnSync(process.argv0, ['--expose-internals', __filename, 'test'], - { - env: { - NODE_OPTIONS - } - }); + { env: { NODE_OPTIONS } }); assert.strictEqual(result.status, 0); - process.exit(0); } else { const redirect_value = getOptionValue('--redirect-warnings'); assert.strictEqual(redirect_value, expected_redirect_value); From aa20c6fb2636fb5fa5e99a5f2a101d702ae87037 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Thu, 13 Aug 2020 10:31:15 +0200 Subject: [PATCH 4/5] fixup: add logging --- test/parallel/test-unicode-node-options.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-unicode-node-options.js b/test/parallel/test-unicode-node-options.js index b22ff2141efaae..fb43daedc273c1 100644 --- a/test/parallel/test-unicode-node-options.js +++ b/test/parallel/test-unicode-node-options.js @@ -11,9 +11,13 @@ if (process.argv.length === 2) { const NODE_OPTIONS = `--redirect-warnings=${expected_redirect_value}`; const result = cp.spawnSync(process.argv0, ['--expose-internals', __filename, 'test'], - { env: { NODE_OPTIONS } }); + { + env: { NODE_OPTIONS }, + stdio: 'inherit' + }); assert.strictEqual(result.status, 0); } else { const redirect_value = getOptionValue('--redirect-warnings'); + console.log(`--redirect-warings=${redirect_value}`); assert.strictEqual(redirect_value, expected_redirect_value); } From 15ac48ad2725ba2cedc98364296a7a096d4e6c5b Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Thu, 20 Aug 2020 03:30:24 +0200 Subject: [PATCH 5/5] fixup: also pass env --- test/parallel/test-unicode-node-options.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-unicode-node-options.js b/test/parallel/test-unicode-node-options.js index fb43daedc273c1..e5a40d118791d3 100644 --- a/test/parallel/test-unicode-node-options.js +++ b/test/parallel/test-unicode-node-options.js @@ -12,7 +12,10 @@ if (process.argv.length === 2) { const result = cp.spawnSync(process.argv0, ['--expose-internals', __filename, 'test'], { - env: { NODE_OPTIONS }, + env: { + ...process.env, + NODE_OPTIONS + }, stdio: 'inherit' }); assert.strictEqual(result.status, 0);