child_process: add shell option to spawn()

This commit adds a shell option, to spawn() and spawnSync(). This
option allows child processes to be spawned with or without a
shell. The option also allows a custom shell to be defined, for
compatibility with exec()'s shell option.

Fixes: https://github.com/nodejs/node/issues/1009
PR-URL: https://github.com/nodejs/node/pull/4598
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

doc/api/child_process.markdown

@@ -77,11 +77,12 @@ The importance of the distinction between `child_process.exec()` and
 `child_process.execFile()` can vary based on platform. On Unix-type operating
 systems (Unix, Linux, OSX) `child_process.execFile()` can be more efficient
 because it does not spawn a shell. On Windows, however, `.bat` and `.cmd`
-files are not executable on their own without a terminal and therefore cannot
-be launched using `child_process.execFile()` (or even `child_process.spawn()`).
-When running on Windows, `.bat` and `.cmd` files can only be invoked using
-either `child_process.exec()` or by spawning `cmd.exe` and passing the `.bat`
-or `.cmd` file as an argument (which is what `child_process.exec()` does).
+files are not executable on their own without a terminal, and therefore cannot
+be launched using `child_process.execFile()`. When running on Windows, `.bat`
+and `.cmd` files can be invoked using `child_process.spawn()` with the `shell`
+option set, with `child_process.exec()`, or by spawning `cmd.exe` and passing
+the `.bat` or `.cmd` file as an argument (which is what the `shell` option and
+`child_process.exec()` do).
 
 ```js
 // On Windows Only ...
@@ -277,6 +278,10 @@ not clone the current process.*
     [`options.detached`][])
   * `uid` {Number} Sets the user identity of the process. (See setuid(2).)
   * `gid` {Number} Sets the group identity of the process. (See setgid(2).)
+  * `shell` {Boolean|String} If `true`, runs `command` inside of a shell. Uses
+    '/bin/sh' on UNIX, and 'cmd.exe' on Windows. A different shell can be
+    specified as a string. The shell should understand the `-c` switch on UNIX,
+    or `/s /c` on Windows. Defaults to `false` (no shell).
 * return: {ChildProcess object}
 
 The `child_process.spawn()` method spawns a new process using the given
@@ -581,6 +586,10 @@ throw.  The [`Error`][] object will contain the entire result from
   * `maxBuffer` {Number} largest amount of data (in bytes) allowed on stdout or
     stderr - if exceeded child process is killed
   * `encoding` {String} The encoding used for all stdio inputs and outputs. (Default: 'buffer')
+  * `shell` {Boolean|String} If `true`, runs `command` inside of a shell. Uses
+    '/bin/sh' on UNIX, and 'cmd.exe' on Windows. A different shell can be
+    specified as a string. The shell should understand the `-c` switch on UNIX,
+    or `/s /c` on Windows. Defaults to `false` (no shell).
 * return: {Object}
   * `pid` {Number} Pid of the child process
   * `output` {Array} Array of results from stdio output

lib/child_process.js

@@ -71,7 +71,8 @@ exports._forkChild = function(fd) {
 
 
 function normalizeExecArgs(command /*, options, callback*/) {
-  var file, args, options, callback;
+  let options;
+  let callback;
 
   if (typeof arguments[1] === 'function') {
     options = undefined;
@@ -81,25 +82,12 @@ function normalizeExecArgs(command /*, options, callback*/) {
     callback = arguments[2];
   }
 
-  if (process.platform === 'win32') {
-    file = process.env.comspec || 'cmd.exe';
-    args = ['/s', '/c', '"' + command + '"'];
-    // Make a shallow copy before patching so we don't clobber the user's
-    // options object.
-    options = util._extend({}, options);
-    options.windowsVerbatimArguments = true;
-  } else {
-    file = '/bin/sh';
-    args = ['-c', command];
-  }
-
-  if (options && options.shell)
-    file = options.shell;
+  // Make a shallow copy so we don't clobber the user's options object.
+  options = Object.assign({}, options);
+  options.shell = typeof options.shell === 'string' ? options.shell : true;
 
   return {
-    cmd: command,
-    file: file,
-    args: args,
+    file: command,
     options: options,
     callback: callback
   };
@@ -109,7 +97,6 @@ function normalizeExecArgs(command /*, options, callback*/) {
 exports.exec = function(command /*, options, callback*/) {
   var opts = normalizeExecArgs.apply(null, arguments);
   return exports.execFile(opts.file,
-                          opts.args,
                           opts.options,
                           opts.callback);
 };
@@ -123,7 +110,8 @@ exports.execFile = function(file /*, args, options, callback*/) {
     maxBuffer: 200 * 1024,
     killSignal: 'SIGTERM',
     cwd: null,
-    env: null
+    env: null,
+    shell: false
   };
 
   // Parse the optional positional parameters.
@@ -153,6 +141,7 @@ exports.execFile = function(file /*, args, options, callback*/) {
     env: options.env,
     gid: options.gid,
     uid: options.uid,
+    shell: options.shell,
     windowsVerbatimArguments: !!options.windowsVerbatimArguments
   });
 
@@ -331,7 +320,23 @@ function normalizeSpawnArguments(file /*, args, options*/) {
   else if (options === null || typeof options !== 'object')
     throw new TypeError('"options" argument must be an object');
 
-  options = util._extend({}, options);
+  // Make a shallow copy so we don't clobber the user's options object.
+  options = Object.assign({}, options);
+
+  if (options.shell) {
+    const command = [file].concat(args).join(' ');
+
+    if (process.platform === 'win32') {
+      file = typeof options.shell === 'string' ? options.shell :
+              process.env.comspec || 'cmd.exe';
+      args = ['/s', '/c', '"' + command + '"'];
+      options.windowsVerbatimArguments = true;
+    } else {
+      file = typeof options.shell === 'string' ? options.shell : '/bin/sh';
+      args = ['-c', command];
+    }
+  }
+
   args.unshift(file);
 
   var env = options.env || process.env;
@@ -491,12 +496,12 @@ function execFileSync(/*command, args, options*/) {
 exports.execFileSync = execFileSync;
 
 
-function execSync(/*command, options*/) {
+function execSync(command /*, options*/) {
   var opts = normalizeExecArgs.apply(null, arguments);
   var inheritStderr = opts.options ? !opts.options.stdio : true;
 
-  var ret = spawnSync(opts.file, opts.args, opts.options);
-  ret.cmd = opts.cmd;
+  var ret = spawnSync(opts.file, opts.options);
+  ret.cmd = command;
 
   if (inheritStderr)
     process.stderr.write(ret.stderr);

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

@@ -0,0 +1,64 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+
+// Verify that a shell is, in fact, executed
+const doesNotExist = cp.spawn('does-not-exist', {shell: true});
+
+assert.notEqual(doesNotExist.spawnfile, 'does-not-exist');
+doesNotExist.on('error', common.fail);
+doesNotExist.on('exit', common.mustCall((code, signal) => {
+  assert.strictEqual(signal, null);
+
+  if (common.isWindows)
+    assert.strictEqual(code, 1);    // Exit code of cmd.exe
+  else
+    assert.strictEqual(code, 127);  // Exit code of /bin/sh
+}));
+
+// Verify that passing arguments works
+const echo = cp.spawn('echo', ['foo'], {
+  encoding: 'utf8',
+  shell: true
+});
+let echoOutput = '';
+
+assert.strictEqual(echo.spawnargs[echo.spawnargs.length - 1].replace(/"/g, ''),
+                   'echo foo');
+echo.stdout.on('data', data => {
+  echoOutput += data;
+});
+echo.on('close', common.mustCall((code, signal) => {
+  assert.strictEqual(echoOutput.trim(), 'foo');
+}));
+
+// Verify that shell features can be used
+const cmd = common.isWindows ? 'echo bar | more' : 'echo bar | cat';
+const command = cp.spawn(cmd, {
+  encoding: 'utf8',
+  shell: true
+});
+let commandOutput = '';
+
+command.stdout.on('data', data => {
+  commandOutput += data;
+});
+command.on('close', common.mustCall((code, signal) => {
+  assert.strictEqual(commandOutput.trim(), 'bar');
+}));
+
+// Verify that the environment is properly inherited
+const env = cp.spawn(`"${process.execPath}" -pe process.env.BAZ`, {
+  env: Object.assign({}, process.env, {BAZ: 'buzz'}),
+  encoding: 'utf8',
+  shell: true
+});
+let envOutput = '';
+
+env.stdout.on('data', data => {
+  envOutput += data;
+});
+env.on('close', common.mustCall((code, signal) => {
+  assert.strictEqual(envOutput.trim(), 'buzz');
+}));

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

@@ -0,0 +1,37 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+
+// Verify that a shell is, in fact, executed
+const doesNotExist = cp.spawnSync('does-not-exist', {shell: true});
+
+assert.notEqual(doesNotExist.file, 'does-not-exist');
+assert.strictEqual(doesNotExist.error, undefined);
+assert.strictEqual(doesNotExist.signal, null);
+
+if (common.isWindows)
+  assert.strictEqual(doesNotExist.status, 1);    // Exit code of cmd.exe
+else
+  assert.strictEqual(doesNotExist.status, 127);  // Exit code of /bin/sh
+
+// Verify that passing arguments works
+const echo = cp.spawnSync('echo', ['foo'], {shell: true});
+
+assert.strictEqual(echo.args[echo.args.length - 1].replace(/"/g, ''),
+                   'echo foo');
+assert.strictEqual(echo.stdout.toString().trim(), 'foo');
+
+// Verify that shell features can be used
+const cmd = common.isWindows ? 'echo bar | more' : 'echo bar | cat';
+const command = cp.spawnSync(cmd, {shell: true});
+
+assert.strictEqual(command.stdout.toString().trim(), 'bar');
+
+// Verify that the environment is properly inherited
+const env = cp.spawnSync(`"${process.execPath}" -pe process.env.BAZ`, {
+  env: Object.assign({}, process.env, {BAZ: 'buzz'}),
+  shell: true
+});
+
+assert.strictEqual(env.stdout.toString().trim(), 'buzz');