Skip to content

child-process: support any shells on Windows #21943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tkamenoko wants to merge 16 commits into from
Closed

child-process: support any shells on Windows #21943

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b42be28
child-process: fix switches for alternative shells on Windows
tkamenoko Jul 21, 2018
e9863e6
child-process: fix lint errors
tkamenoko Jul 21, 2018
6f3bccc
test: add exec-any-shells test
tkamenoko Jul 21, 2018
6b9b090
test: fix exec-any-shell test for powershell
tkamenoko Jul 23, 2018
1dcea34
child-process: enable to accept full path that contains spaces
tkamenoko Jul 23, 2018
c7163f8
child-process: undo unrelated style changes
tkamenoko Jul 24, 2018
a314340
test: remove hard coding path
tkamenoko Jul 26, 2018
ce7999a
test: fix spawnsync-shell to check correct options on Windows
tkamenoko Jul 26, 2018
097c905
child-process: set windowsVerbatimArguments true for any shell
tkamenoko Aug 2, 2018
a4122b7
child-process: use case-insensitive comparison to check shell name
tkamenoko Aug 2, 2018
27ba48f
test: add testcases to ensure uppercase shell name and complex commands
tkamenoko Aug 2, 2018
f93c796
test: fix to accept multiple lines output of `where bash`
tkamenoko Aug 2, 2018
ade01d9
test: revert windowsVerbatim flag, fix `isCmd` check in spawnsync-shell
tkamenoko Aug 2, 2018
5a2c130
doc: update shell requirements in child_process
tkamenoko Aug 7, 2018
e2192a3
fixup: windowsVerbatimArguments
joaocgreis Aug 14, 2018
7a10600
fixup: don't exhaust exec buffer
joaocgreis Sep 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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: #21905
  • Loading branch information
@tkamenoko @joaocgreis
tkamenoko authored and joaocgreis committed Sep 1, 2018
commit b42be2815c1416567e9017576326ec32da77fd51
51 changes: 28 additions & 23 deletions lib/child_process.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ function normalizeExecArgs(command, options, callback) {
exports.exec = function exec(/* command , options, callback */) {
const opts = normalizeExecArgs.apply(null, arguments);
return exports.execFile(opts.file,
opts.options,
opts.callback);
opts.options,
opts.callback);
};

const customPromiseExecFunction = (orig) => {
Expand Down Expand Up @@ -383,7 +383,7 @@ const _deprecatedCustomFds = deprecate(
return fd === -1 ? 'pipe' : fd;
});
}, 'child_process: options.customFds option is deprecated. ' +
'Use options.stdio instead.', 'DEP0006');
'Use options.stdio instead.', 'DEP0006');

function _convertCustomFds(options) {
if (options.customFds && !options.stdio) {
Expand All @@ -400,7 +400,7 @@ function normalizeSpawnArguments(file, args, options) {
if (Array.isArray(args)) {
args = args.slice(0);
} else if (args !== undefined &&
(args === null || typeof args !== 'object')) {
(args === null || typeof args !== 'object')) {
Copy link
Copy Markdown
Member

@addaleax addaleax Jul 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you undo unrelated style changes? They make review harder, break git blame, and tend to induce merge conflicts.

throw new ERR_INVALID_ARG_TYPE('args', 'object', args);
} else {
options = args;
Expand All @@ -414,15 +414,15 @@ function normalizeSpawnArguments(file, args, options) {

// Validate the cwd, if present.
if (options.cwd != null &&
typeof options.cwd !== 'string') {
typeof options.cwd !== 'string') {
throw new ERR_INVALID_ARG_TYPE('options.cwd', 'string', options.cwd);
}

// Validate detached, if present.
if (options.detached != null &&
typeof options.detached !== 'boolean') {
typeof options.detached !== 'boolean') {
throw new ERR_INVALID_ARG_TYPE('options.detached',
'boolean', options.detached);
'boolean', options.detached);
}

// Validate the uid, if present.
Expand All @@ -437,45 +437,50 @@ function normalizeSpawnArguments(file, args, options) {

// Validate the shell, if present.
if (options.shell != null &&
typeof options.shell !== 'boolean' &&
typeof options.shell !== 'string') {
typeof options.shell !== 'boolean' &&
typeof options.shell !== 'string') {
throw new ERR_INVALID_ARG_TYPE('options.shell',
['boolean', 'string'], options.shell);
['boolean', 'string'], options.shell);
}

// Validate argv0, if present.
if (options.argv0 != null &&
typeof options.argv0 !== 'string') {
typeof options.argv0 !== 'string') {
throw new ERR_INVALID_ARG_TYPE('options.argv0', 'string', options.argv0);
}

// Validate windowsHide, if present.
if (options.windowsHide != null &&
typeof options.windowsHide !== 'boolean') {
typeof options.windowsHide !== 'boolean') {
throw new ERR_INVALID_ARG_TYPE('options.windowsHide',
'boolean', options.windowsHide);
'boolean', options.windowsHide);
}

// Validate windowsVerbatimArguments, if present.
if (options.windowsVerbatimArguments != null &&
typeof options.windowsVerbatimArguments !== 'boolean') {
typeof options.windowsVerbatimArguments !== 'boolean') {
throw new ERR_INVALID_ARG_TYPE('options.windowsVerbatimArguments',
'boolean',
options.windowsVerbatimArguments);
'boolean',
options.windowsVerbatimArguments);
}

// 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(' ');

// 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}"`];
// "/d /s /c" is used only for cmd.exe.
if (file.endsWith("cmd.exe") || file.endsWith("cmd")) {
args = ['/d', '/s', '/c', `"${command}"`];
} else {
args = ['-c', `"${command}"`];
}
options.windowsVerbatimArguments = true;
} else {
if (typeof options.shell === 'string')
Expand Down Expand Up @@ -583,7 +588,7 @@ function spawnSync(/* file, args, options */) {
'TypedArray',
'DataView',
'string'],
input);
input);
}
}
}
Expand Down Expand Up @@ -660,8 +665,8 @@ function validateTimeout(timeout) {
function validateMaxBuffer(maxBuffer) {
if (maxBuffer != null && !(typeof maxBuffer === 'number' && maxBuffer >= 0)) {
throw new ERR_OUT_OF_RANGE('options.maxBuffer',
'a positive number',
maxBuffer);
'a positive number',
maxBuffer);
}
}

Expand All @@ -671,7 +676,7 @@ function sanitizeKillSignal(killSignal) {
return convertToValidSignal(killSignal);
} else if (killSignal != null) {
throw new ERR_INVALID_ARG_TYPE('options.killSignal',
['string', 'number'],
killSignal);
['string', 'number'],
killSignal);
}
}