child_process: improve argument validation

For execFile() and fork(), use INVALID_ARG_TYPE as appropriate instead of INVALID_ARG_VALUE. Use validator functions where sensible. PR-URL: #41305 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

Author: Trott

lib/child_process.js

@@ -75,7 +75,9 @@ const{getValidatedPath } = require('internal/fs/utils');
 const{
  isInt32,
  validateAbortSignal,
+ validateArray,
  validateBoolean,
+ validateFunction,
  validateObject,
  validateString,
 }=require('internal/validators');
@@ -119,20 +121,18 @@ function fork(modulePath, args = [], options){
 
 if(args==null){
 args=[];
-}elseif(typeofargs!=='object'){
-thrownewERR_INVALID_ARG_VALUE('args',args);
-}elseif(!ArrayIsArray(args)){
+}elseif(typeofargs==='object'&&!ArrayIsArray(args)){
 options=args;
 args=[];
+}else{
+validateArray(args,'args');
 }
 
-if(options==null){
-options={};
-}elseif(typeofoptions!=='object'){
-thrownewERR_INVALID_ARG_VALUE('options',options);
-}else{
-options={ ...options};
+if(options!=null){
+validateObject(options,'options');
 }
+options={ ...options,shell: false};
+options.execPath=options.execPath||process.execPath;
 
 // Prepare arguments for fork:
 execArgv=options.execArgv||process.execArgv;
@@ -160,9 +160,6 @@ function fork(modulePath, args = [], options){
 thrownewERR_CHILD_PROCESS_IPC_REQUIRED('options.stdio');
 }
 
-options.execPath=options.execPath||process.execPath;
-options.shell=false;
-
 returnspawn(options.execPath,args,options);
 }
 
@@ -276,33 +273,25 @@ ObjectDefineProperty(exec, promisify.custom,{
  * @returns{ChildProcess}
  */
 functionexecFile(file,args=[],options,callback){
-if(args==null){
-args=[];
-}elseif(typeofargs==='object'){
-if(!ArrayIsArray(args)){
-callback=options;
-options=args;
-args=[];
-}
+if(args!=null&&typeofargs==='object'&&!ArrayIsArray(args)){
+callback=options;
+options=args;
+args=null;
 }elseif(typeofargs==='function'){
 callback=args;
-options={};
-args=[];
-}else{
-thrownewERR_INVALID_ARG_VALUE('args',args);
+options=null;
+args=null;
 }
 
-if(options==null){
-options={};
-}elseif(typeofoptions==='function'){
+if(typeofoptions==='function'){
 callback=options;
-options={};
-}elseif(typeofoptions!=='object'){
-thrownewERR_INVALID_ARG_VALUE('options',options);
+options=null;
+}elseif(options!=null){
+validateObject(options,'options');
 }
 
-if(callback&&typeofcallback!=='function'){
-thrownewERR_INVALID_ARG_VALUE('callback',callback);
+if(callback!=null){
+validateFunction(callback,'callback');
 }
 
 options={
@@ -391,7 +380,7 @@ function execFile(file, args = [], options, callback){
 return;
 }
 
-if(args.length!==0)
+if(args?.length)
 cmd+=` ${ArrayPrototypeJoin(args,' ')}`;
 
 if(!ex){

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

@@ -54,7 +54,7 @@ const expectedEnv ={foo: 'bar'};
 fork(fixtures.path('child-process-echo-options.js'),arg);
 },
 {
-code: 'ERR_INVALID_ARG_VALUE',
+code: 'ERR_INVALID_ARG_TYPE',
 name: 'TypeError'
 }
 );
@@ -97,7 +97,7 @@ const expectedEnv ={foo: 'bar'};
 fork(fixtures.path('child-process-echo-options.js'),[],arg);
 },
 {
-code: 'ERR_INVALID_ARG_VALUE',
+code: 'ERR_INVALID_ARG_TYPE',
 name: 'TypeError'
 }
 );

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

@@ -106,10 +106,10 @@ spawn(cmd, u, o);
 spawn(cmd,n,o);
 spawn(cmd,a,u);
 
-assert.throws(function(){spawn(cmd,a,n);},invalidArgTypeError);
-
-assert.throws(function(){spawn(cmd,s);},invalidArgTypeError);
-assert.throws(function(){spawn(cmd,a,s);},invalidArgTypeError);
+assert.throws(()=>{spawn(cmd,a,n);},invalidArgTypeError);
+assert.throws(()=>{spawn(cmd,s);},invalidArgTypeError);
+assert.throws(()=>{spawn(cmd,a,s);},invalidArgTypeError);
+assert.throws(()=>{spawn(cmd,a,a);},invalidArgTypeError);
 
 
 // Verify that execFile has same argument parsing behavior as spawn.
@@ -158,17 +158,18 @@ execFile(cmd, c, n);
 // String is invalid in arg position (this may seem strange, but is
 // consistent across node API, cf. `net.createServer('not options', 'not
 // callback')`.
-assert.throws(function(){execFile(cmd,s,o,c);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,a,s,c);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,a,o,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,a,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,o,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,u,u,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,n,n,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,a,u,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,a,n,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,u,o,s);},invalidArgValueError);
-assert.throws(function(){execFile(cmd,n,o,s);},invalidArgValueError);
+assert.throws(()=>{execFile(cmd,s,o,c);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,a,s,c);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,a,o,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,a,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,o,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,u,u,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,n,n,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,a,u,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,a,n,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,u,o,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,n,o,s);},invalidArgTypeError);
+assert.throws(()=>{execFile(cmd,a,a);},invalidArgTypeError);
 
 execFile(cmd,c,s);// Should not throw.
 
@@ -190,5 +191,6 @@ fork(empty, n, n);
 fork(empty,n,o);
 fork(empty,a,n);
 
-assert.throws(function(){fork(empty,s);},invalidArgValueError);
-assert.throws(function(){fork(empty,a,s);},invalidArgValueError);
+assert.throws(()=>{fork(empty,s);},invalidArgTypeError);
+assert.throws(()=>{fork(empty,a,s);},invalidArgTypeError);
+assert.throws(()=>{fork(empty,a,a);},invalidArgTypeError);