From bf8a9986e8edf433f215a73af28a2635d05df33f Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 10 Mar 2017 22:40:06 -0800 Subject: [PATCH 1/3] test: add coverage for child_process bounds check Make sure that monkey-patching process.execArgv doesn't cause child_process to incorrectly munge execArgv in fork(). This basically is adding coverage for an `index > 0` check (see Refs). Previously, that condition was never false in any of the tests. Refs: https://github.com/nodejs/node/blob/c67207731f16a78f6cae90e49c53b10728241ecf/lib/child_process.js#L76 --- test/parallel/test-cli-eval.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js index 7795016dc58255..7f17cf0a58183f 100644 --- a/test/parallel/test-cli-eval.js +++ b/test/parallel/test-cli-eval.js @@ -143,6 +143,17 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`, assert.strictEqual(stdout, ''); assert.strictEqual(stderr, ''); })); + + // Make sure that monkey-patching process.execArgv doesn't cause child_process + // to incorrectly munge execArgv. + child.exec( + `${nodejs} -e 'process.execArgv = ["-e", "console.log(42)", "thirdArg"]; + require("child_process").fork("${emptyFile}")'`, + common.mustCall((err, stdout, stderr) => { + assert.ifError(err); + assert.strictEqual(stdout, '42\n'); + assert.strictEqual(stderr, ''); + })); } // Regression test for https://github.com/nodejs/node/issues/8534. From c2a2dc2a1c83f6d7bf877534eff1596c33bbf171 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 13 Mar 2017 21:17:45 -0700 Subject: [PATCH 2/3] squash: hopeful fix for Windows --- test/parallel/test-cli-eval.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js index 7f17cf0a58183f..00c1bbd6a0f8ca 100644 --- a/test/parallel/test-cli-eval.js +++ b/test/parallel/test-cli-eval.js @@ -147,8 +147,8 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`, // Make sure that monkey-patching process.execArgv doesn't cause child_process // to incorrectly munge execArgv. child.exec( - `${nodejs} -e 'process.execArgv = ["-e", "console.log(42)", "thirdArg"]; - require("child_process").fork("${emptyFile}")'`, + `${nodejs} -e "process.execArgv = ['-e', 'console.log(42)', 'thirdArg']; + require('child_process').fork('${emptyFile}')"`, common.mustCall((err, stdout, stderr) => { assert.ifError(err); assert.strictEqual(stdout, '42\n'); From f28a356c40492ce3c6bb035192023eedaed7da0e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 17 Mar 2017 12:57:58 -0700 Subject: [PATCH 3/3] squash: make it one line and see if that fixes it on Windows --- test/parallel/test-cli-eval.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js index 00c1bbd6a0f8ca..6d21b5d50be83c 100644 --- a/test/parallel/test-cli-eval.js +++ b/test/parallel/test-cli-eval.js @@ -147,8 +147,8 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`, // Make sure that monkey-patching process.execArgv doesn't cause child_process // to incorrectly munge execArgv. child.exec( - `${nodejs} -e "process.execArgv = ['-e', 'console.log(42)', 'thirdArg']; - require('child_process').fork('${emptyFile}')"`, + `${nodejs} -e "process.execArgv = ['-e', 'console.log(42)', 'thirdArg'];` + + `require('child_process').fork('${emptyFile}')"`, common.mustCall((err, stdout, stderr) => { assert.ifError(err); assert.strictEqual(stdout, '42\n');