Skip to content

Commit

Permalink
child_process: improve docs and validate strings in exec and spawn
Browse files Browse the repository at this point in the history
  • Loading branch information
@puskin94
puskin94 committed Dec 5, 2024
1 parent c4aa34a commit 40a0a9f
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 31 deletions.
60 changes: 33 additions & 27 deletions lib/child_process.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,7 @@ function _forkChild(fd, serializationMode) {
}

function normalizeExecArgs(command, options, callback) {
validateString(command, 'command');
validateArgumentNullCheck(command, 'command');
validateStringParam(command, 'command');

if (typeof options === 'function') {
callback = options;
Expand Down Expand Up @@ -260,6 +259,8 @@ ObjectDefineProperty(exec, promisify.custom, {
});

function normalizeExecFileArgs(file, args, options, callback) {
validateStringParam(file, 'file');

if (ArrayIsArray(args)) {
args = ArrayPrototypeSlice(args);
} else if (args != null && typeof args === 'object') {
Expand Down Expand Up @@ -535,12 +536,17 @@ function copyProcessEnvToEnv(env, name, optionEnv) {
}
}

function normalizeSpawnArguments(file, args, options) {
validateString(file, 'file');
validateArgumentNullCheck(file, 'file');
function validateStringParam(param, paramName) {
validateString(param, paramName);
validateArgumentNullCheck(param, paramName);

if (file.length === 0)
throw new ERR_INVALID_ARG_VALUE('file', file, 'cannot be empty');
if (param.length === 0) {
throw new ERR_INVALID_ARG_VALUE(paramName, param, 'cannot be empty');
}
}

function normalizeSpawnArguments(command, args, options) {
validateStringParam(command, 'command');

if (ArrayIsArray(args)) {
args = ArrayPrototypeSlice(args);
Expand Down Expand Up @@ -611,35 +617,35 @@ function normalizeSpawnArguments(file, args, options) {

if (options.shell) {
validateArgumentNullCheck(options.shell, 'options.shell');
const command = ArrayPrototypeJoin([file, ...args], ' ');
const fullCommand = ArrayPrototypeJoin([command, ...args], ' ');
// Set the shell, switches, and commands.
if (process.platform === 'win32') {
if (typeof options.shell === 'string')
file = options.shell;
command = options.shell;
else
file = process.env.comspec || 'cmd.exe';
command = process.env.comspec || 'cmd.exe';
// '/d /s /c' is used only for cmd.exe.
if (RegExpPrototypeExec(/^(?:.*\\)?cmd(?:\.exe)?$/i, file) !== null) {
args = ['/d', '/s', '/c', `"${command}"`];
if (RegExpPrototypeExec(/^(?:.*\\)?cmd(?:\.exe)?$/i, command) !== null) {
args = ['/d', '/s', '/c', `"${fullCommand}"`];
windowsVerbatimArguments = true;
} else {
args = ['-c', command];
args = ['-c', fullCommand];
}
} else {
if (typeof options.shell === 'string')
file = options.shell;
command = options.shell;
else if (process.platform === 'android')
file = '/system/bin/sh';
command = '/system/bin/sh';
else
file = '/bin/sh';
args = ['-c', command];
command = '/bin/sh';
args = ['-c', fullCommand];
}
}

if (typeof options.argv0 === 'string') {
ArrayPrototypeUnshift(args, options.argv0);
} else {
ArrayPrototypeUnshift(args, file);
ArrayPrototypeUnshift(args, command);
}

const env = options.env || process.env;
Expand Down Expand Up @@ -702,7 +708,7 @@ function normalizeSpawnArguments(file, args, options) {
cwd,
detached: !!options.detached,
envPairs,
file,
file: command,
windowsHide: !!options.windowsHide,
windowsVerbatimArguments: !!windowsVerbatimArguments,
};
Expand All @@ -721,8 +727,8 @@ function abortChildProcess(child, killSignal, reason) {
}

/**
* Spawns a new process using the given `file`.
* @param {string} file
* Spawns a new process using the given `command`.
* @param {string} command
* @param {string[]} [args]
* @param {{
* cwd?: string | URL;
Expand All @@ -742,8 +748,8 @@ function abortChildProcess(child, killSignal, reason) {
* }} [options]
* @returns {ChildProcess}
*/
function spawn(file, args, options) {
options = normalizeSpawnArguments(file, args, options);
function spawn(command, args, options) {
options = normalizeSpawnArguments(command, args, options);
validateTimeout(options.timeout);
validateAbortSignal(options.signal, 'options.signal');
const killSignal = sanitizeKillSignal(options.killSignal);
Expand Down Expand Up @@ -791,8 +797,8 @@ function spawn(file, args, options) {
}

/**
* Spawns a new process synchronously using the given `file`.
* @param {string} file
* Spawns a new process synchronously using the given `command`.
* @param {string} command
* @param {string[]} [args]
* @param {{
* cwd?: string | URL;
Expand Down Expand Up @@ -820,11 +826,11 @@ function spawn(file, args, options) {
* error: Error;
* }}
*/
function spawnSync(file, args, options) {
function spawnSync(command, args, options) {
options = {
__proto__: null,
maxBuffer: MAX_BUFFER,
...normalizeSpawnArguments(file, args, options),
...normalizeSpawnArguments(command, args, options),
};

debug('spawnSync', options);
Expand Down
64 changes: 60 additions & 4 deletions test/parallel/test-child-process-exec-error.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const child_process = require('child_process');
const { exec, execSync, execFile } = require('child_process');

function test(fn, code, expectPidType = 'number') {
const child = fn('does-not-exist', common.mustCall(function(err) {
Expand All @@ -35,10 +35,66 @@ function test(fn, code, expectPidType = 'number') {

// With `shell: true`, expect pid (of the shell)
if (common.isWindows) {
test(child_process.exec, 1, 'number'); // Exit code of cmd.exe
test(exec, 1, 'number'); // Exit code of cmd.exe
} else {
test(child_process.exec, 127, 'number'); // Exit code of /bin/sh
test(exec, 127, 'number'); // Exit code of /bin/sh
}

// With `shell: false`, expect no pid
test(child_process.execFile, 'ENOENT', 'undefined');
test(execFile, 'ENOENT', 'undefined');


// Verify that the exec() function throws when command parameter is not a valid string
{
assert.throws(() => {
exec(123, common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "command" argument must be of type string. Received type number (123)'
});

assert.throws(() => {
exec('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' cannot be empty. Received ''"
});

assert.throws(() => {
exec('\u0000', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
});
}


// Verify that the execSync() function throws when command parameter is not a valid string
{
assert.throws(() => {
execSync(123, common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "command" argument must be of type string. Received type number (123)'
});

assert.throws(() => {
execSync('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' cannot be empty. Received ''"
});

assert.throws(() => {
execSync('\u0000', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
});
}
54 changes: 54 additions & 0 deletions test/parallel/test-child-process-execfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,57 @@ const execOpts = { encoding: 'utf8', shell: true, env: { ...process.env, NODE: p
}));
});
}

// Verify that the execFile() function throws when file parameter is not a valid string
{
assert.throws(() => {
execFile(123, common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "file" argument must be of type string. Received type number (123)'
});

assert.throws(() => {
execFile('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'file' cannot be empty. Received ''"
});

assert.throws(() => {
execFile('\u0000', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'file' must be a string without null bytes. Received '\\x00'"
});
}

// Verify that the execFileSync() function throws when file parameter is not a valid string
{
assert.throws(() => {
execFileSync(123, common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "file" argument must be of type string. Received type number (123)'
});

assert.throws(() => {
execFileSync('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'file' cannot be empty. Received ''"
});

assert.throws(() => {
execFileSync('\u0000', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'file' must be a string without null bytes. Received '\\x00'"
});
}
28 changes: 28 additions & 0 deletions test/parallel/test-child-process-spawn-error.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,31 @@ enoentChild.on('error', common.mustCall(function(err) {
assert.strictEqual(err.path, enoentPath);
assert.deepStrictEqual(err.spawnargs, spawnargs);
}));


// Verify that the spawn() function throws when command parameter is not a valid string
{
assert.throws(() => {
spawn(123, common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "command" argument must be of type string. Received type number (123)'
});

assert.throws(() => {
spawn('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' cannot be empty. Received ''"
});

assert.throws(() => {
spawn('\u0000', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
});
}
27 changes: 27 additions & 0 deletions test/parallel/test-child-process-spawnsync.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,30 @@ assert.deepStrictEqual(ret_err.spawnargs, ['bar']);
];
assert.deepStrictEqual(retUTF8.output, stringifiedDefault);
}

// Verify that the spawnSync() function throws when command parameter is not a valid string
{
assert.throws(() => {
spawnSync(123, common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "command" argument must be of type string. Received type number (123)'
});

assert.throws(() => {
spawnSync('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' cannot be empty. Received ''"
});

assert.throws(() => {
spawnSync('\u0000', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'command' must be a string without null bytes. Received '\\x00'"
});
}

0 comments on commit 40a0a9f

Please sign in to comment.