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.
nodejs · Dec 23, 2021 · 59a9252 · 59a9252
1 parent d1586f3
commit 59a9252
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 31 deletions.
diff --git a/lib/child_process.js b/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');
@@ -126,13 +128,10 @@ function fork(modulePath, args = [], options) {
     args = [];
   }
 
-  if (options == null) {
-    options = {};
-  } else if (typeof options !== 'object') {
-    throw new ERR_INVALID_ARG_VALUE('options', options);
-  } else {
-    options = { ...options };
+  if (options != null) {
+    validateObject(options, 'options');
   }
+  options = { ...options };
 
   // Prepare arguments for fork:
   execArgv = options.execArgv || process.execArgv;
@@ -286,25 +285,21 @@ function execFile(file, args = [], options, callback) {
     }
   } else if (typeof args === 'function') {
     callback = args;
-    options = {};
+    options = null;
     args = [];
-  } else {
-    throw new ERR_INVALID_ARG_VALUE('args', args);
   }
 
-  if (options == null) {
-    options = {};
-  } else if (typeof options === 'function') {
+  if (typeof options === 'function') {
     callback = options;
-    options = {};
-  } else if (typeof options !== 'object') {
-    throw new ERR_INVALID_ARG_VALUE('options', options);
+    options = null;
+  } else if (options != null) {
+    validateObject(options, 'options');
   }
 
-  if (callback && typeof callback !== 'function') {
-    throw new ERR_INVALID_ARG_VALUE('callback', callback);
+  if (callback != null) {
+    validateFunction(callback, 'callback');
   }
-
+  
   options = {
     encoding: 'utf8',
     timeout: 0,

diff --git a/test/parallel/test-child-process-fork-args.js b/test/parallel/test-child-process-fork-args.js
@@ -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'
       }
     );

diff --git a/test/parallel/test-child-process-spawn-typeerror.js b/test/parallel/test-child-process-spawn-typeerror.js
@@ -158,17 +158,17 @@ 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(function() { execFile(cmd, s, o, c); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, a, s, c); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, a, o, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, a, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, o, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, u, u, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, n, n, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, a, u, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, a, n, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, u, o, s); }, invalidArgTypeError);
+assert.throws(function() { execFile(cmd, n, o, s); }, invalidArgTypeError);
 
 execFile(cmd, c, s); // Should not throw.
 
@@ -191,4 +191,4 @@ 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(function() { fork(empty, a, s); }, invalidArgTypeError);