Skip to content

Commit 07f53d7

Browse files
bzozjoesepi
authored andcommitted
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: nodejs#35129 PR-URL: nodejs#35210 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com>
1 parent 3fcfea7 commit 07f53d7

File tree

3 files changed

+34
-1
lines changed

3 files changed

+34
-1
lines changed

doc/api/child_process.md

+4
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ the first one case-insensitively matching `PATH` to perform command lookup.
4343
This may lead to issues on Windows when passing objects to `env` option that
4444
have multiple variants of `PATH` variable.
4545

46+
On Windows Node.js will sanitize the `env` by removing case-insensitive
47+
duplicates. Only first (in lexicographic order) entry will be passed to the
48+
child process.
49+
4650
The [`child_process.spawn()`][] method spawns the child process asynchronously,
4751
without blocking the Node.js event loop. The [`child_process.spawnSync()`][]
4852
function provides equivalent functionality in a synchronous manner that blocks

lib/child_process.js

+20
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const {
2929
ObjectDefineProperty,
3030
ObjectPrototypeHasOwnProperty,
3131
Promise,
32+
Set,
3233
} = primordials;
3334

3435
const {
@@ -524,8 +525,27 @@ function normalizeSpawnArguments(file, args, options) {
524525
env.NODE_V8_COVERAGE = process.env.NODE_V8_COVERAGE;
525526
}
526527

528+
let envKeys = [];
527529
// Prototype values are intentionally included.
528530
for (const key in env) {
531+
envKeys.push(key);
532+
}
533+
534+
if (process.platform === 'win32') {
535+
// On Windows env keys are case insensitive. Filter out duplicates,
536+
// keeping only the first one (in lexicographic order)
537+
const sawKey = new Set();
538+
envKeys = envKeys.sort().filter((key) => {
539+
const uppercaseKey = key.toUpperCase();
540+
if (sawKey.has(uppercaseKey)) {
541+
return false;
542+
}
543+
sawKey.add(uppercaseKey);
544+
return true;
545+
});
546+
}
547+
548+
for (const key of envKeys) {
529549
const value = env[key];
530550
if (value !== undefined) {
531551
envPairs.push(`${key}=${value}`);

test/parallel/test-child-process-env.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ const env = {
3636
'HELLO': 'WORLD',
3737
'UNDEFINED': undefined,
3838
'NULL': null,
39-
'EMPTY': ''
39+
'EMPTY': '',
40+
'duplicate': 'lowercase',
41+
'DUPLICATE': 'uppercase',
4042
};
4143
Object.setPrototypeOf(env, {
4244
'FOO': 'BAR'
@@ -65,4 +67,11 @@ child.stdout.on('end', mustCall(() => {
6567
assert.ok(!response.includes('UNDEFINED=undefined'));
6668
assert.ok(response.includes('NULL=null'));
6769
assert.ok(response.includes(`EMPTY=${os.EOL}`));
70+
if (isWindows) {
71+
assert.ok(response.includes('DUPLICATE=uppercase'));
72+
assert.ok(!response.includes('duplicate=lowercase'));
73+
} else {
74+
assert.ok(response.includes('DUPLICATE=uppercase'));
75+
assert.ok(response.includes('duplicate=lowercase'));
76+
}
6877
}));

0 commit comments

Comments
 (0)