child_process: fix switches for alternative shells on Windows

On Windows, normalizeSpawnArguments set "/d /s /c" for any shells.
It cause exec and other methods are limited to cmd.exe as a shell.

Powershell and git-bash are often used instead of cmd.exe,
and they can recieve "-c" switch like unix shells.
So normalizeSpawnArguments is changed to set "/d /s /c" for cmd.exe,
and "-c" for others.

Fixes: https://github.com/nodejs/node/issues/21905
PR-URL: https://github.com/nodejs/node/pull/21943
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

4 files changed, 86 insertions, 11 deletions

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 5b2b16f253..9cee76c02e 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -415,7 +415,7 @@ changes:
     [Default Windows Shell][]. **Default:** `false` (no shell).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
-    when `shell` is specified. **Default:** `false`.
+    when `shell` is specified and is CMD. **Default:** `false`.
   * `windowsHide` {boolean} Hide the subprocess console window that would
     normally be created on Windows systems. **Default:** `true`.
 * Returns: {ChildProcess}
@@ -867,7 +867,7 @@ changes:
     [Default Windows Shell][]. **Default:** `false` (no shell).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
-    when `shell` is specified. **Default:** `false`.
+    when `shell` is specified and is CMD. **Default:** `false`.
   * `windowsHide` {boolean} Hide the subprocess console window that would
     normally be created on Windows systems. **Default:** `true`.
 * Returns: {Object}
@@ -1432,8 +1432,9 @@ to `stdout` although there are only 4 characters.
 
 ## Shell Requirements
 
-The shell should understand the `-c` switch on UNIX or `/d /s /c` on Windows.
-On Windows, command line parsing should be compatible with `'cmd.exe'`.
+The shell should understand the `-c` switch. If the shell is `'cmd.exe'`, it
+should understand the `/d /s /c` switches and command line parsing should be
+compatible.
 
 ## Default Windows Shell
 
diff --git a/lib/child_process.js b/lib/child_process.js
index 2e9c71c1ef..2962f5cf05 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -469,14 +469,19 @@ function normalizeSpawnArguments(file, args, options) {
 
   if (options.shell) {
     const command = [file].concat(args).join(' ');
-
+    // Set the shell, switches, and commands.
     if (process.platform === 'win32') {
       if (typeof options.shell === 'string')
         file = options.shell;
       else
         file = process.env.comspec || 'cmd.exe';
-      args = ['/d', '/s', '/c', `"${command}"`];
-      options.windowsVerbatimArguments = true;
+      // '/d /s /c' is used only for cmd.exe.
+      if (/^(?:.*\\)?cmd(?:\.exe)?$/i.test(file)) {
+        args = ['/d', '/s', '/c', `"${command}"`];
+        options.windowsVerbatimArguments = true;
+      } else {
+        args = ['-c', command];
+      }
     } else {
       if (typeof options.shell === 'string')
         file = options.shell;
diff --git a/test/parallel/test-child-process-exec-any-shells-windows.js b/test/parallel/test-child-process-exec-any-shells-windows.js
new file mode 100644
index 0000000000..8cdd03d7e5
--- /dev/null
+++ b/test/parallel/test-child-process-exec-any-shells-windows.js
@@ -0,0 +1,68 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+const fs = require('fs');
+const tmpdir = require('../common/tmpdir');
+
+// This test is only relevant on Windows.
+if (!common.isWindows)
+  common.skip('Windows specific test.');
+
+// This test ensures that child_process.exec can work with any shells.
+
+tmpdir.refresh();
+const tmpPath = `${tmpdir.path}\\path with spaces`;
+fs.mkdirSync(tmpPath);
+
+const test = (shell) => {
+  cp.exec('echo foo bar', { shell: shell },
+          common.mustCall((error, stdout, stderror) => {
+            assert.ok(!error && !stderror);
+            assert.ok(stdout.includes('foo') && stdout.includes('bar'));
+          }));
+};
+const testCopy = (shellName, shellPath) => {
+  // Copy the executable to a path with spaces, to ensure there are no issues
+  // related to quoting of argv0
+  const copyPath = `${tmpPath}\\${shellName}`;
+  fs.copyFileSync(shellPath, copyPath);
+  test(copyPath);
+};
+
+const system32 = `${process.env.SystemRoot}\\System32`;
+
+// Test CMD
+test(true);
+test('cmd');
+testCopy('cmd.exe', `${system32}\\cmd.exe`);
+test('cmd.exe');
+test('CMD');
+
+// Test PowerShell
+test('powershell');
+testCopy('powershell.exe',
+         `${system32}\\WindowsPowerShell\\v1.0\\powershell.exe`);
+fs.writeFile(`${tmpPath}\\test file`, 'Test', common.mustCall((err) => {
+  assert.ifError(err);
+  cp.exec(`Get-ChildItem "${tmpPath}" | Select-Object -Property Name`,
+          { shell: 'PowerShell' },
+          common.mustCall((error, stdout, stderror) => {
+            assert.ok(!error && !stderror);
+            assert.ok(stdout.includes(
+              'test file'));
+          }));
+}));
+
+// Test Bash (from WSL and Git), if available
+cp.exec('where bash', common.mustCall((error, stdout) => {
+  if (error) {
+    return;
+  }
+  const lines = stdout.trim().split(/[\r\n]+/g);
+  for (let i = 0; i < lines.length; ++i) {
+    const bashPath = lines[i].trim();
+    test(bashPath);
+    testCopy(`bash_${i}.exe`, bashPath);
+  }
+}));
diff --git a/test/parallel/test-child-process-spawnsync-shell.js b/test/parallel/test-child-process-spawnsync-shell.js
index 637cb12a23..6f7c0ec0f1 100644
--- a/test/parallel/test-child-process-spawnsync-shell.js
+++ b/test/parallel/test-child-process-spawnsync-shell.js
@@ -54,11 +54,12 @@ assert.strictEqual(env.stdout.toString().trim(), 'buzz');
 
   function test(testPlatform, shell, shellOutput) {
     platform = testPlatform;
-
+    const isCmd = /^(?:.*\\)?cmd(?:\.exe)?$/i.test(shellOutput);
     const cmd = 'not_a_real_command';
-    const shellFlags = platform === 'win32' ? ['/d', '/s', '/c'] : ['-c'];
-    const outputCmd = platform === 'win32' ? `"${cmd}"` : cmd;
-    const windowsVerbatim = platform === 'win32' ? true : undefined;
+
+    const shellFlags = isCmd ? ['/d', '/s', '/c'] : ['-c'];
+    const outputCmd = isCmd ? `"${cmd}"` : cmd;
+    const windowsVerbatim = isCmd ? true : undefined;
     internalCp.spawnSync = common.mustCall(function(opts) {
       assert.strictEqual(opts.file, shellOutput);
       assert.deepStrictEqual(opts.args,