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);
-}elseif(args!==undefined&&
-(args===null||typeofargs!=='object')){
+}elseif(args==null){
+args=[];
+}elseif(typeofargs!=='object'){
 thrownewERR_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.
+
+constassert=require('assert');
+const{ spawn }=require('child_process');
+
+constcommon=require('../common');
+consttmpdir=require('../common/tmpdir');
+
+constcommand=common.isWindows ? 'cd' : 'pwd';
+constoptions={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;
+}
+
+consttestCases=[
+undefined,
+null,
+[],
+];
+
+constexpectedResult=tmpdir.path.trim().toLowerCase();
+
+(async()=>{
+constresults=awaitPromise.all(
+testCases.map((testCase)=>{
+returnnewPromise((resolve)=>{
+constsubprocess=spawn(command,testCase,options);
+
+letaccumulatedData=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([...newSet(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);
 
 constinvalidArgTypeError=
-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.
+
+constassert=require('assert');
+const{ spawnSync }=require('child_process');
+
+constcommon=require('../common');
+consttmpdir=require('../common/tmpdir');
+
+constcommand=common.isWindows ? 'cd' : 'pwd';
+constoptions={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;
+}
+
+consttestCases=[
+undefined,
+null,
+[],
+];
+
+constexpectedResult=tmpdir.path.trim().toLowerCase();
+
+constresults=testCases.map((testCase)=>{
+const{ stdout, stderr }=spawnSync(
+command,
+testCase,
+options
+);
+
+assert.deepStrictEqual(stderr,Buffer.alloc(0));
+
+returnstdout.toString().trim().toLowerCase();
+});
+
+assert.deepStrictEqual([...newSet(results)],[expectedResult]);