child_process: improve docs and validate strings in exec and spawn

lib/child_process.js

@@ -186,8 +186,7 @@ function _forkChild(fd, serializationMode) {
 }
 
 function normalizeExecArgs(command, options, callback) {
-  validateString(command, 'command');
-  validateArgumentNullCheck(command, 'command');
+  validateStringParam(command, 'command');
 
   if (typeof options === 'function') {
     callback = options;
@@ -260,6 +259,8 @@ ObjectDefineProperty(exec, promisify.custom, {
 });
 
 function normalizeExecFileArgs(file, args, options, callback) {
+  validateStringParam(file, 'file');
+
   if (ArrayIsArray(args)) {
     args = ArrayPrototypeSlice(args);
   } else if (args != null && typeof args === 'object') {
@@ -535,12 +536,17 @@ function copyProcessEnvToEnv(env, name, optionEnv) {
   }
 }
 
-function normalizeSpawnArguments(file, args, options) {
-  validateString(file, 'file');
-  validateArgumentNullCheck(file, 'file');
+function validateStringParam(param, paramName) {
+  validateString(param, paramName);
+  validateArgumentNullCheck(param, paramName);
 
-  if (file.length === 0)
-    throw new ERR_INVALID_ARG_VALUE('file', file, 'cannot be empty');
+  if (param.length === 0) {
+    throw new ERR_INVALID_ARG_VALUE(paramName, param, 'cannot be empty');
+  }
+}
+
+function normalizeSpawnArguments(command, args, options) {
+  validateStringParam(command, 'command');
 
   if (ArrayIsArray(args)) {
     args = ArrayPrototypeSlice(args);
@@ -611,35 +617,35 @@ function normalizeSpawnArguments(file, args, options) {
 
   if (options.shell) {
     validateArgumentNullCheck(options.shell, 'options.shell');
-    const command = ArrayPrototypeJoin([file, ...args], ' ');
+    const fullCommand = ArrayPrototypeJoin([command, ...args], ' ');
     // Set the shell, switches, and commands.
     if (process.platform === 'win32') {
       if (typeof options.shell === 'string')
-        file = options.shell;
+        command = options.shell;
       else
-        file = process.env.comspec || 'cmd.exe';
+        command = process.env.comspec || 'cmd.exe';
       // '/d /s /c' is used only for cmd.exe.
-      if (RegExpPrototypeExec(/^(?:.*\\)?cmd(?:\.exe)?$/i, file) !== null) {
-        args = ['/d', '/s', '/c', `"${command}"`];
+      if (RegExpPrototypeExec(/^(?:.*\\)?cmd(?:\.exe)?$/i, command) !== null) {
+        args = ['/d', '/s', '/c', `"${fullCommand}"`];
         windowsVerbatimArguments = true;
       } else {
-        args = ['-c', command];
+        args = ['-c', fullCommand];
       }
     } else {
       if (typeof options.shell === 'string')
-        file = options.shell;
+        command = options.shell;
       else if (process.platform === 'android')
-        file = '/system/bin/sh';
+        command = '/system/bin/sh';
       else
-        file = '/bin/sh';
-      args = ['-c', command];
+        command = '/bin/sh';
+      args = ['-c', fullCommand];
     }
   }
 
   if (typeof options.argv0 === 'string') {
     ArrayPrototypeUnshift(args, options.argv0);
   } else {
-    ArrayPrototypeUnshift(args, file);
+    ArrayPrototypeUnshift(args, command);
   }
 
   const env = options.env || process.env;
@@ -702,7 +708,7 @@ function normalizeSpawnArguments(file, args, options) {
     cwd,
     detached: !!options.detached,
     envPairs,
-    file,
+    file: command,
     windowsHide: !!options.windowsHide,
     windowsVerbatimArguments: !!windowsVerbatimArguments,
   };
@@ -721,8 +727,8 @@ function abortChildProcess(child, killSignal, reason) {
 }
 
 /**
- * Spawns a new process using the given `file`.
- * @param {string} file
+ * Spawns a new process using the given `command`.
+ * @param {string} command
  * @param {string[]} [args]
  * @param {{
  *   cwd?: string | URL;
@@ -742,8 +748,8 @@ function abortChildProcess(child, killSignal, reason) {
  *   }} [options]
  * @returns {ChildProcess}
  */
-function spawn(file, args, options) {
-  options = normalizeSpawnArguments(file, args, options);
+function spawn(command, args, options) {
+  options = normalizeSpawnArguments(command, args, options);
   validateTimeout(options.timeout);
   validateAbortSignal(options.signal, 'options.signal');
   const killSignal = sanitizeKillSignal(options.killSignal);
@@ -791,8 +797,8 @@ function spawn(file, args, options) {
 }
 
 /**
- * Spawns a new process synchronously using the given `file`.
- * @param {string} file
+ * Spawns a new process synchronously using the given `command`.
+ * @param {string} command
  * @param {string[]} [args]
  * @param {{
  *   cwd?: string | URL;
@@ -820,11 +826,11 @@ function spawn(file, args, options) {
  *   error: Error;
  *   }}
  */
-function spawnSync(file, args, options) {
+function spawnSync(command, args, options) {
   options = {
     __proto__: null,
     maxBuffer: MAX_BUFFER,
-    ...normalizeSpawnArguments(file, args, options),
+    ...normalizeSpawnArguments(command, args, options),
   };
 
   debug('spawnSync', options);

test/parallel/test-child-process-exec-error.js

@@ -22,7 +22,7 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const child_process = require('child_process');
+const {exec, execSync, execFile} = require('child_process');
 
 function test(fn, code, expectPidType = 'number') {
   const child = fn('does-not-exist', common.mustCall(function(err) {
@@ -35,10 +35,66 @@ function test(fn, code, expectPidType = 'number') {
 
 // With `shell: true`, expect pid (of the shell)
 if (common.isWindows) {
-  test(child_process.exec, 1, 'number'); // Exit code of cmd.exe
+  test(exec, 1, 'number'); // Exit code of cmd.exe
 } else {
-  test(child_process.exec, 127, 'number'); // Exit code of /bin/sh
+  test(exec, 127, 'number'); // Exit code of /bin/sh
 }
 
 // With `shell: false`, expect no pid
-test(child_process.execFile, 'ENOENT', 'undefined');
+test(execFile, 'ENOENT', 'undefined');
+
+
+// Verify that the exec() function throws when command parameter is not a valid string
+{
+  assert.throws(() => {
+    exec(123, common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "command" argument must be of type string. Received type number (123)'
+  });
+
+  assert.throws(() => {
+    exec('', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' cannot be empty. Received ''"
+  });
+
+  assert.throws(() => {
+    exec('\u0000', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
+  });
+}
+
+
+// Verify that the execSync() function throws when command parameter is not a valid string
+{
+  assert.throws(() => {
+    execSync(123, common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "command" argument must be of type string. Received type number (123)'
+  });
+
+  assert.throws(() => {
+    execSync('', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' cannot be empty. Received ''"
+  });
+
+  assert.throws(() => {
+    execSync('\u0000', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
+  });
+}

test/parallel/test-child-process-execfile.js

@@ -125,3 +125,57 @@ const execOpts = { encoding: 'utf8', shell: true, env: { ...process.env, NODE: p
     }));
   });
 }
+
+// Verify that the execFile() function throws when file parameter is not a valid string
+{
+  assert.throws(() => {
+    execFile(123, common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "file" argument must be of type string. Received type number (123)'
+  });
+
+  assert.throws(() => {
+    execFile('', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'file' cannot be empty. Received ''"
+  });
+
+  assert.throws(() => {
+    execFile('\u0000', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'file' must be a string without null bytes. Received '\\x00'"
+  });
+}
+
+// Verify that the execFileSync() function throws when file parameter is not a valid string
+{
+  assert.throws(() => {
+    execFileSync(123, common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "file" argument must be of type string. Received type number (123)'
+  });
+
+  assert.throws(() => {
+    execFileSync('', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'file' cannot be empty. Received ''"
+  });
+
+  assert.throws(() => {
+    execFileSync('\u0000', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'file' must be a string without null bytes. Received '\\x00'"
+  });
+}

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

@@ -53,3 +53,31 @@ enoentChild.on('error', common.mustCall(function(err) {
   assert.strictEqual(err.path, enoentPath);
   assert.deepStrictEqual(err.spawnargs, spawnargs);
 }));
+
+
+// Verify that the spawn() function throws when command parameter is not a valid string
+{
+  assert.throws(() => {
+    spawn(123, common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "command" argument must be of type string. Received type number (123)'
+  });
+
+  assert.throws(() => {
+    spawn('', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' cannot be empty. Received ''"
+  });
+
+  assert.throws(() => {
+    spawn('\u0000', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
+  });
+}

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

@@ -65,3 +65,30 @@ assert.deepStrictEqual(ret_err.spawnargs, ['bar']);
   ];
   assert.deepStrictEqual(retUTF8.output, stringifiedDefault);
 }
+
+// Verify that the spawnSync() function throws when command parameter is not a valid string
+{
+  assert.throws(() => {
+    spawnSync(123, common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "command" argument must be of type string. Received type number (123)'
+  });
+
+  assert.throws(() => {
+    spawnSync('', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' cannot be empty. Received ''"
+  });
+
+  assert.throws(() => {
+    spawnSync('\u0000', common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_VALUE',
+    name: 'TypeError',
+    message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
+  });
+}