Skip to content

Commit 7a04435

Browse files
committed
fixup! child_process: deprecate passing args to spawn and execFile
1 parent 281d896 commit 7a04435

File tree

2 files changed

+13
-8
lines changed

2 files changed

+13
-8
lines changed

lib/child_process.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ function copyProcessEnvToEnv(env, name, optionEnv) {
535535
}
536536
}
537537

538+
let emittedDEP0190Already = false;
538539
function normalizeSpawnArguments(file, args, options) {
539540
validateString(file, 'file');
540541
validateArgumentNullCheck(file, 'file');
@@ -611,12 +612,13 @@ function normalizeSpawnArguments(file, args, options) {
611612

612613
if (options.shell) {
613614
validateArgumentNullCheck(options.shell, 'options.shell');
614-
if (args.length > 0) {
615+
if (args.length > 0 && !emittedDEP0190Already) {
615616
process.emitWarning(
616617
'Passing args to a child process with shell option true can lead to security ' +
617618
'vulnerabilities, as the arguments are not escaped, only concatenated.',
618619
'DeprecationWarning',
619620
'DEP0190');
621+
emittedDEP0190Already = true;
620622
}
621623
const command = ArrayPrototypeJoin([file, ...args], ' ');
622624
// Set the shell, switches, and commands.

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ const fixture = fixtures.path('exit.js');
1212
const echoFixture = fixtures.path('echo.js');
1313
const execOpts = { encoding: 'utf8', shell: true, env: { ...process.env, NODE: process.execPath, FIXTURE: fixture } };
1414

15+
common.expectWarning(
16+
'DeprecationWarning',
17+
'Passing args to a child process with shell option true can lead to security ' +
18+
'vulnerabilities, as the arguments are not escaped, only concatenated.',
19+
'DEP0190');
20+
1521
{
1622
execFile(
1723
process.execPath,
@@ -47,7 +53,8 @@ const execOpts = { encoding: 'utf8', shell: true, env: { ...process.env, NODE: p
4753
{
4854
// Verify the shell option works properly
4955
execFile(
50-
common.isWindows ? `"${execOpts.env.NODE}" "${execOpts.env.FIXTURE} 0` : `"$NODE" "$FIXTURE" 0`,
56+
`"${common.isWindows ? execOpts.env.NODE : '$NODE'}"`,
57+
[`"${common.isWindows ? execOpts.env.FIXTURE : '$FIXTURE'}"`, 0],
5158
execOpts,
5259
common.mustSucceed(),
5360
);
@@ -116,14 +123,10 @@ const execOpts = { encoding: 'utf8', shell: true, env: { ...process.env, NODE: p
116123
...(common.isWindows ? [] : [{ encoding: 'utf8' }]),
117124
{ shell: true, encoding: 'utf8' },
118125
].forEach((options) => {
119-
const command = options.shell ?
120-
[[file, ...args].join(' ')] :
121-
[file, args];
122-
123-
const execFileSyncStdout = execFileSync(...command, options);
126+
const execFileSyncStdout = execFileSync(file, args, options);
124127
assert.strictEqual(execFileSyncStdout, `foo bar${os.EOL}`);
125128

126-
execFile(...command, options, common.mustCall((_, stdout) => {
129+
execFile(file, args, options, common.mustCall((_, stdout) => {
127130
assert.strictEqual(stdout, execFileSyncStdout);
128131
}));
129132
});

0 commit comments

Comments
 (0)