child_process: spawn ignores options in case args is undefined

spawn method ignores 3-d argument 'options' in case the second one 'args' equals to 'undefined'. Fixes: #24912 PR-URL: #24913 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

Author: eduardbme

lib/child_process.js

@@ -403,8 +403,9 @@ function normalizeSpawnArguments(file, args, options) {
 
  if (Array.isArray(args)) {
  args = args.slice(0);
- } else if (args !== undefined &&
- (args === null || typeof args !== 'object')) {
+ } else if (args == null) {
+ args = [];
+ } else if (typeof args !== 'object') {
  throw new ERR_INVALID_ARG_TYPE('args', 'object', args);
  } else {
  options = args;

test/parallel/test-child-process-spawn-args.js

@@ -0,0 +1,53 @@
+'use strict';
+
+// This test confirms that `undefined`, `null`, and `[]`
+// can be used as a placeholder for the second argument (`args`) of `spawn()`.
+// Previously, there was a bug where using `undefined` for the second argument
+// caused the third argument (`options`) to be ignored.
+// See https://github.com/nodejs/node/issues/24912.
+
+const assert = require('assert');
+const { spawn } = require('child_process');
+
+const common = require('../common');
+const tmpdir = require('../common/tmpdir');
+
+const command = common.isWindows ? 'cd' : 'pwd';
+const options = { cwd: tmpdir.path };
+
+if (common.isWindows) {
+ // This test is not the case for Windows based systems
+ // unless the `shell` options equals to `true`
+
+ options.shell = true;
+}
+
+const testCases = [
+ undefined,
+ null,
+ [],
+];
+
+const expectedResult = tmpdir.path.trim().toLowerCase();
+
+(async () => {
+ const results = await Promise.all(
+ testCases.map((testCase) => {
+ return new Promise((resolve) => {
+ const subprocess = spawn(command, testCase, options);
+
+ let accumulatedData = Buffer.alloc(0);
+
+ subprocess.stdout.on('data', common.mustCall((data) => {
+ accumulatedData = Buffer.concat([accumulatedData, data]);
+ }));
+
+ subprocess.stdout.on('end', () => {
+ resolve(accumulatedData.toString().trim().toLowerCase());
+ });
+ });
+ })
+ );
+
+ assert.deepStrictEqual([...new Set(results)], [expectedResult]);
+})();

test/parallel/test-child-process-spawn-typeerror.js

@@ -33,7 +33,7 @@ const invalidArgValueError =
  common.expectsError({ code: 'ERR_INVALID_ARG_VALUE', type: TypeError }, 14);
 
 const invalidArgTypeError =
- common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError }, 13);
+ common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError }, 11);
 
 assert.throws(function() {
  spawn(invalidcmd, 'this is not an array');
@@ -59,10 +59,6 @@ assert.throws(function() {
  spawn(file);
 }, invalidArgTypeError);
 
-assert.throws(function() {
- spawn(cmd, null);
-}, invalidArgTypeError);
-
 assert.throws(function() {
  spawn(cmd, true);
 }, invalidArgTypeError);
@@ -103,9 +99,9 @@ spawn(cmd, o);
 
 // Variants of undefined as explicit 'no argument' at a position.
 spawn(cmd, u, o);
+spawn(cmd, n, o);
 spawn(cmd, a, u);
 
-assert.throws(function() { spawn(cmd, n, o); }, invalidArgTypeError);
 assert.throws(function() { spawn(cmd, a, n); }, invalidArgTypeError);
 
 assert.throws(function() { spawn(cmd, s); }, invalidArgTypeError);

test/parallel/test-child-process-spawnsync-args.js

@@ -0,0 +1,45 @@
+'use strict';
+
+// This test confirms that `undefined`, `null`, and `[]` can be used
+// as a placeholder for the second argument (`args`) of `spawnSync()`.
+// Previously, there was a bug where using `undefined` for the second argument
+// caused the third argument (`options`) to be ignored.
+// See https://github.com/nodejs/node/issues/24912.
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const common = require('../common');
+const tmpdir = require('../common/tmpdir');
+
+const command = common.isWindows ? 'cd' : 'pwd';
+const options = { cwd: tmpdir.path };
+
+if (common.isWindows) {
+ // This test is not the case for Windows based systems
+ // unless the `shell` options equals to `true`
+
+ options.shell = true;
+}
+
+const testCases = [
+ undefined,
+ null,
+ [],
+];
+
+const expectedResult = tmpdir.path.trim().toLowerCase();
+
+const results = testCases.map((testCase) => {
+ const { stdout, stderr } = spawnSync(
+ command,
+ testCase,
+ options
+ );
+
+ assert.deepStrictEqual(stderr, Buffer.alloc(0));
+
+ return stdout.toString().trim().toLowerCase();
+});
+
+assert.deepStrictEqual([...new Set(results)], [expectedResult]);