From 2df82581162337f9e402d55f2d2d87585cb81dea Mon Sep 17 00:00:00 2001
From: Bartosz Sosnowski <bartosz@janeasystems.com>
Date: Tue, 15 Sep 2020 16:41:29 +0200
Subject: [PATCH] win, child_process: sanitize env variables

On Windows environment variables are case-insensitive. When spawning
child process certain apps can get confused if some of the variables are
duplicated.

This adds a step on Windows to normalizeSpawnArguments that removes such
duplicates, keeping only the first (in lexicographic order) entry in the
env key of options. This is partly already done for the PATH entry.

Fixes: https://github.com/nodejs/node/issues/35129
---
 doc/api/child_process.md                |  4 ++++
 lib/child_process.js                    | 20 ++++++++++++++++++++
 test/parallel/test-child-process-env.js | 11 ++++++++++-
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 3a5f64d622885a..235a2a80e71e2a 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -43,6 +43,10 @@ the first one case-insensitively matching `PATH` to perform command lookup.
 This may lead to issues on Windows when passing objects to `env` option that
 have multiple variants of `PATH` variable.
 
+On Windows Node.js will sanitize the `env` by removing case-insensitive
+duplicates. Only first (in lexicographic order) entry will be passed to the
+child process.
+
 The [`child_process.spawn()`][] method spawns the child process asynchronously,
 without blocking the Node.js event loop. The [`child_process.spawnSync()`][]
 function provides equivalent functionality in a synchronous manner that blocks
diff --git a/lib/child_process.js b/lib/child_process.js
index 9e1c37af8f169f..24c5e809ad62c1 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -29,6 +29,7 @@ const {
   ObjectDefineProperty,
   ObjectPrototypeHasOwnProperty,
   Promise,
+  Set,
 } = primordials;
 
 const {
@@ -524,8 +525,27 @@ function normalizeSpawnArguments(file, args, options) {
     env.NODE_V8_COVERAGE = process.env.NODE_V8_COVERAGE;
   }
 
+  let envKeys = [];
   // Prototype values are intentionally included.
   for (const key in env) {
+    envKeys.push(key);
+  }
+
+  if (process.platform === 'win32') {
+    // On Windows env keys are case insensitive. Filter out duplicates,
+    // keeping only the first one (in lexicographic order)
+    const sawKey = new Set();
+    envKeys = envKeys.sort().filter((key) => {
+      const uppercaseKey = key.toUpperCase();
+      if (sawKey.has(uppercaseKey)) {
+        return false;
+      }
+      sawKey.add(uppercaseKey);
+      return true;
+    });
+  }
+
+  for (const key of envKeys) {
     const value = env[key];
     if (value !== undefined) {
       envPairs.push(`${key}=${value}`);
diff --git a/test/parallel/test-child-process-env.js b/test/parallel/test-child-process-env.js
index 783e392c3e42a8..f9815ff0154f7d 100644
--- a/test/parallel/test-child-process-env.js
+++ b/test/parallel/test-child-process-env.js
@@ -36,7 +36,9 @@ const env = {
   'HELLO': 'WORLD',
   'UNDEFINED': undefined,
   'NULL': null,
-  'EMPTY': ''
+  'EMPTY': '',
+  'duplicate': 'lowercase',
+  'DUPLICATE': 'uppercase',
 };
 Object.setPrototypeOf(env, {
   'FOO': 'BAR'
@@ -65,4 +67,11 @@ child.stdout.on('end', mustCall(() => {
   assert.ok(!response.includes('UNDEFINED=undefined'));
   assert.ok(response.includes('NULL=null'));
   assert.ok(response.includes(`EMPTY=${os.EOL}`));
+  if (isWindows) {
+    assert.ok(response.includes('DUPLICATE=uppercase'));
+    assert.ok(!response.includes('duplicate=lowercase'));
+  } else {
+    assert.ok(response.includes('DUPLICATE=uppercase'));
+    assert.ok(response.includes('duplicate=lowercase'));
+  }
 }));