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 committed Dec 9, 2024
1 parent c4aa34a commit 18ddc47
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 11 deletions.
20 changes: 13 additions & 7 deletions lib/child_process.js
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 (param.length === 0) {
throw new ERR_INVALID_ARG_VALUE(paramName, param, 'cannot be empty');
}
}

if (file.length === 0)
throw new ERR_INVALID_ARG_VALUE('file', file, 'cannot be empty');
function normalizeSpawnArguments(file, args, options) {
validateStringParam(file, 'file');

if (ArrayIsArray(args)) {
args = ArrayPrototypeSlice(args);
Expand Down
64 changes: 60 additions & 4 deletions test/parallel/test-child-process-exec-error.js
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
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 the 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
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 the file parameter is not a valid string
{
assert.throws(() => {
spawn(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(() => {
spawn('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'file' cannot be empty. Received ''"
});

assert.throws(() => {
spawn('\u0000', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'file' must be a string without null bytes. Received '\\x00'"
});
}
27 changes: 27 additions & 0 deletions test/parallel/test-child-process-spawnsync.js
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 the file parameter is not a valid string
{
assert.throws(() => {
spawnSync(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(() => {
spawnSync('', common.mustNotCall());
}, {
code: 'ERR_INVALID_ARG_VALUE',
name: 'TypeError',
message: "The argument 'file' cannot be empty. Received ''"
});

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

0 comments on commit 18ddc47

Please sign in to comment.