Skip to content

Commit 975a59e

Browse files
committed
child_process: validate shell option in normalizeExecArgs()
- narrow validation type to string (previously de facto not validated) - ensure empty string is coerced to true - add test cases for options.shell
1 parent da1ca7d commit 975a59e

File tree

3 files changed

+30
-2
lines changed

3 files changed

+30
-2
lines changed

lib/child_process.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,12 @@ function normalizeExecArgs(command, options, callback) {
196196

197197
// Make a shallow copy so we don't clobber the user's options object.
198198
options = { __proto__: null, ...options };
199-
options.shell = typeof options.shell === 'string' ? options.shell : true;
199+
200+
// Validate the shell, if present, and ensure a truthy value.
201+
if (options.shell != null) {
202+
validateString(options.shell, 'options.shell');
203+
}
204+
options.shell ||= true;
200205

201206
return {
202207
file: command,

test/parallel/test-child-process-exec-any-shells-windows.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const testCopy = (shellName, shellPath) => {
3333
const system32 = `${process.env.SystemRoot}\\System32`;
3434

3535
// Test CMD
36-
test(true);
36+
test();
3737
test('cmd');
3838
testCopy('cmd.exe', `${system32}\\cmd.exe`);
3939
test('cmd.exe');
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { exec, execSync } = require('child_process');
5+
6+
const invalidArgTypeError = {
7+
code: 'ERR_INVALID_ARG_TYPE',
8+
name: 'TypeError'
9+
};
10+
11+
exec('echo should-be-passed-as-argument', { shell: '' }, common.mustSucceed((stdout, stderr) => {
12+
assert.match(stdout, /should-be-passed-as-argument/);
13+
assert.ok(!stderr);
14+
}));
15+
16+
{
17+
const ret = execSync('echo should-be-passed-as-argument', { encoding: 'utf-8', shell: '' });
18+
assert.match(ret, /should-be-passed-as-argument/);
19+
}
20+
21+
for (const fn of [exec, execSync]) {
22+
assert.throws(() => fn('should-throw-on-boolean-shell-option', { shell: false }), invalidArgTypeError);
23+
}

0 commit comments

Comments
 (0)