Unicode characters are not properly parsed from NODE_OPTIONS on Windows

[connor4312]

• Version: verified on 14.5.0 / 12.14.1 / 10.15.2
• Platform: Windows 1909
• Subsystem:

What steps will reproduce the bug?

1. Create a file foó.js containing console.log("in foó");, and an bar.js with console.log("in bar");
2. Create a file index.js containing

require('child_process').spawn('node', ['bar.js'], {
  cwd: __dirname,
  env: { HELLO: 'cónnór', NODE_OPTIONS: '--require "./foó.js"' },
  stdio: 'inherit'
})

3. Run index.js

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior?

Output like:

> node index.js
in foo
in bar

What do you see instead?

> node index.js
internal/modules/cjs/loader.js:796
    throw err;
    ^

Error: Cannot find module './fo�.js'
Require stack:
- internal/preload
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:793:17)
    at Function.Module._load (internal/modules/cjs/loader.js:686:27)
    at Module.require (internal/modules/cjs/loader.js:848:19)
    at Module._preloadModules (internal/modules/cjs/loader.js:1133:12)
    at loadPreloadModules (internal/bootstrap/pre_execution.js:443:5)
    at prepareMainThreadExecution (internal/bootstrap/pre_execution.js:62:3)
    at internal/main/run_main_module.js:7:1 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'internal/preload' ]
}

Additional information

• This does work Linux and OSX
• This happens irrespective of setting shell: true/false in spawn()
• I seem to be able to pass unicode characters through other variables and read them, e.g. console.log(process.env.HELLO) in the above example prints out fine
• For kicks I tried replacing the accented character with \\\\xF3, but it doesn't look like Node tries to parse these from the string.

References: microsoft/vscode-js-debug#563

[richardlau]

cc @nodejs/platform-windows

[bnoordhuis]

NODE_OPTIONS is parsed by credentials::SafeGetenv(), which calls getenv() from libc/msvcrt, and that function may or may not decode multi-byte sequences properly because... well, it's complicated.

Switching to uv_os_getenv() should fix it because that function always performs proper decoding. Untested, but the fix would look like this:

diff --git a/src/node_credentials.cc b/src/node_credentials.cc
index d552a50172..e77d0378b2 100644
--- a/src/node_credentials.cc
+++ b/src/node_credentials.cc
@@ -57,9 +57,23 @@ 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;
+    MaybeStackBuffer<char, 256> value;
+    size_t size = 256;  // TODO(bnoordhuis) DRY, re-use kStackStorageSize here.
+    int rc;
+
+    {
+      Mutex::ScopedLock lock(per_process::env_var_mutex);
+      rc = uv_os_getenv(key, *value, &size);
+    }
+
+    if (rc == UV_ENOBUFS) {
+      value.AllocateSufficientStorage(size);
+      Mutex::ScopedLock lock(per_process::env_var_mutex);
+      rc = uv_os_getenv(key, *value, &size);
+    }
+
+    if (rc >= 0) {
+      *text = *value;
       return true;
     }
   }