From 5e376478f806f32a099c323d380c561fa24be2a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=B0=E5=85=85?= Date: Wed, 30 Aug 2017 17:57:12 +0800 Subject: [PATCH 1/2] child_process: ignore undef/proto values of env 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: https://github.com/nodejs/node/issues/15087 --- doc/api/child_process.md | 2 ++ lib/child_process.js | 7 +++++-- test/parallel/test-child-process-env.js | 10 ++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/doc/api/child_process.md b/doc/api/child_process.md index 801c4251942e66..25ac180cb79767 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -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: diff --git a/lib/child_process.js b/lib/child_process.js index c4512ab2f47c08..cd67aba283296d 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -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); diff --git a/test/parallel/test-child-process-env.js b/test/parallel/test-child-process-env.js index 4582c48fda5db1..b77e453655ed95 100644 --- a/test/parallel/test-child-process-env.js +++ b/test/parallel/test-child-process-env.js @@ -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' @@ -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')); }); From e7c8d07e0b9c256be4efad8a65ff1b1d294e39e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Mon, 15 Jan 2018 12:30:12 +0100 Subject: [PATCH 2/2] fixup: fix test for Windows --- test/parallel/test-child-process-env.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-child-process-env.js b/test/parallel/test-child-process-env.js index b77e453655ed95..5bd02c24badef5 100644 --- a/test/parallel/test-child-process-env.js +++ b/test/parallel/test-child-process-env.js @@ -22,6 +22,7 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); +const os = require('os'); const spawn = require('child_process').spawn; @@ -59,5 +60,5 @@ process.on('exit', function() { assert.ok(!response.includes('FOO=')); assert.ok(!response.includes('UNDEFINED=undefined')); assert.ok(response.includes('NULL=null')); - assert.ok(response.includes('EMPTY=\n')); + assert.ok(response.includes(`EMPTY=${os.EOL}`)); });