child_process: validate strings in exec and spawn

lib/child_process.js

@@ -186,9 +186,6 @@ function _forkChild(fd, serializationMode) {
 }
 
 function normalizeExecArgs(command, options, callback) {
-  validateString(command, 'command');
-  validateArgumentNullCheck(command, 'command');
-
   if (typeof options === 'function') {
     callback = options;
     options = undefined;
@@ -535,12 +532,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 (param.length === 0) {
+    throw new ERR_INVALID_ARG_VALUE(paramName, param, 'cannot be empty');
+  }
+}
 
-  if (file.length === 0)
-    throw new ERR_INVALID_ARG_VALUE('file', file, 'cannot be empty');
+function normalizeSpawnArguments(file, args, options) {
+  validateStringParam(file, 'file');
 
   if (ArrayIsArray(args)) {
     args = ArrayPrototypeSlice(args);

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,42 @@ 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' });
+
+  assert.throws(() => {
+    exec('', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+
+  assert.throws(() => {
+    exec('\u0000', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+}
+
+
+// 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' });
+
+  assert.throws(() => {
+    execSync('', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+
+  assert.throws(() => {
+    execSync('\u0000', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+}

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

@@ -125,3 +125,33 @@ const execOpts = { encoding: 'utf8', shell: true, env: { ...process.env, NODE: p
     }));
   });
 }
+
+// Verify that the execFile() function throws when the file parameter is not a valid string
+{
+  assert.throws(() => {
+    execFile(123, common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_TYPE' });
+
+  assert.throws(() => {
+    execFile('', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+
+  assert.throws(() => {
+    execFile('\u0000', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+}
+
+// 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' });
+
+  assert.throws(() => {
+    execFileSync('', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+
+  assert.throws(() => {
+    execFileSync('\u0000', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+}

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

@@ -53,3 +53,19 @@ enoentChild.on('error', common.mustCall(function(err) {
   assert.strictEqual(err.path, enoentPath);
   assert.deepStrictEqual(err.spawnargs, spawnargs);
 }));
+
+
+// Verify that the spawn() function throws when the file parameter is not a valid string
+{
+  assert.throws(() => {
+    spawn(123, common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_TYPE' });
+
+  assert.throws(() => {
+    spawn('', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+
+  assert.throws(() => {
+    spawn('\u0000', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+}

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

@@ -65,3 +65,18 @@ assert.deepStrictEqual(ret_err.spawnargs, ['bar']);
   ];
   assert.deepStrictEqual(retUTF8.output, stringifiedDefault);
 }
+
+// Verify that the spawnSync() function throws when the file parameter is not a valid string
+{
+  assert.throws(() => {
+    spawnSync(123, common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_TYPE' });
+
+  assert.throws(() => {
+    spawnSync('', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+
+  assert.throws(() => {
+    spawnSync('\u0000', common.mustNotCall());
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+}