Skip to content

Commit

Permalink
win, child_process: sanitize env variables
Browse files Browse the repository at this point in the history
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: #35129

PR-URL: #35210
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
  • Loading branch information
bzoz committed Sep 24, 2020
1 parent e36ffb7 commit abd8cdf
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
4 changes: 4 additions & 0 deletions doc/api/child_process.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const {
ObjectDefineProperty,
ObjectPrototypeHasOwnProperty,
Promise,
Set,
} = primordials;

const {
Expand Down Expand Up @@ -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}`);
Expand Down
11 changes: 10 additions & 1 deletion test/parallel/test-child-process-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ const env = {
'HELLO': 'WORLD',
'UNDEFINED': undefined,
'NULL': null,
'EMPTY': ''
'EMPTY': '',
'duplicate': 'lowercase',
'DUPLICATE': 'uppercase',
};
Object.setPrototypeOf(env, {
'FOO': 'BAR'
Expand Down Expand Up @@ -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'));
}
}));

0 comments on commit abd8cdf

Please sign in to comment.