Skip to content

Commit

Permalink
child_process: ignore undef/proto values of env
Browse files Browse the repository at this point in the history
At present, undefined values of env option will be transferred as
a "undefined" string value, and values in prototype will also be
included, which are not usual behaviors.

Since non-string env values & prototype values are undocumented,
this change may be treated as a bugfix or a breaking change.

Tested on Mac, Windows not yet.

Fixes: #15087
  • Loading branch information
Gerhut committed Dec 6, 2017
1 parent abd5d95 commit 5e37647
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
2 changes: 2 additions & 0 deletions doc/api/child_process.md
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,8 @@ If not given, the default is to inherit the current working directory.
Use `env` to specify environment variables that will be visible to the new
process, the default is [`process.env`][].

`undefined` values in `env` will be ignored.

Example of running `ls -lh /usr`, capturing `stdout`, `stderr`, and the
exit code:

Expand Down
7 changes: 5 additions & 2 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,11 @@ function normalizeSpawnArguments(file, args, options) {
var env = options.env || process.env;
var envPairs = [];

for (var key in env) {
envPairs.push(`${key}=${env[key]}`);
for (const key of Object.keys(env)) {
const value = env[key];
if (value !== undefined) {
envPairs.push(`${key}=${value}`);
}
}

_convertCustomFds(options);
Expand Down
10 changes: 8 additions & 2 deletions test/parallel/test-child-process-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ const assert = require('assert');
const spawn = require('child_process').spawn;

const env = {
'HELLO': 'WORLD'
'HELLO': 'WORLD',
'UNDEFINED': undefined,
'NULL': null,
'EMPTY': ''
};
Object.setPrototypeOf(env, {
'FOO': 'BAR'
Expand All @@ -53,5 +56,8 @@ child.stdout.on('data', function(chunk) {

process.on('exit', function() {
assert.ok(response.includes('HELLO=WORLD'));
assert.ok(response.includes('FOO=BAR'));
assert.ok(!response.includes('FOO='));
assert.ok(!response.includes('UNDEFINED=undefined'));
assert.ok(response.includes('NULL=null'));
assert.ok(response.includes('EMPTY=\n'));
});

0 comments on commit 5e37647

Please sign in to comment.