Skip to content

Commit

Permalink
child_process: improve argument validation
Browse files Browse the repository at this point in the history
For execFile() and fork(), use INVALID_ARG_TYPE as appropriate instead
of INVALID_ARG_VALUE. Use validator functions where sensible.
  • Loading branch information
Trott committed Dec 23, 2021
1 parent 602b6b1 commit 64b0467
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 30 deletions.
28 changes: 11 additions & 17 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const {
isInt32,
validateAbortSignal,
validateBoolean,
validateFunction,
validateObject,
validateString,
} = require('internal/validators');
Expand Down Expand Up @@ -126,13 +127,10 @@ function fork(modulePath, args = [], options) {
args = [];
}

if (options == null) {
options = {};
} else if (typeof options !== 'object') {
throw new ERR_INVALID_ARG_VALUE('options', options);
} else {
options = { ...options };
if (options != null) {
validateObject(options, 'options');
}
options = { ...options };

// Prepare arguments for fork:
execArgv = options.execArgv || process.execArgv;
Expand Down Expand Up @@ -286,23 +284,19 @@ function execFile(file, args = [], options, callback) {
}
} else if (typeof args === 'function') {
callback = args;
options = {};
options = null;
args = [];
} else {
throw new ERR_INVALID_ARG_VALUE('args', args);
}

if (options == null) {
options = {};
} else if (typeof options === 'function') {
if (typeof options === 'function') {
callback = options;
options = {};
} else if (typeof options !== 'object') {
throw new ERR_INVALID_ARG_VALUE('options', options);
options = null;
} else if (options != null) {
validateObject(options, 'options');
}

if (callback && typeof callback !== 'function') {
throw new ERR_INVALID_ARG_VALUE('callback', callback);
if (callback != null) {
validateFunction(callback, 'callback');
}

options = {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-fork-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const expectedEnv = { foo: 'bar' };
fork(fixtures.path('child-process-echo-options.js'), [], arg);
},
{
code: 'ERR_INVALID_ARG_VALUE',
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError'
}
);
Expand Down
24 changes: 12 additions & 12 deletions test/parallel/test-child-process-spawn-typeerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,17 @@ execFile(cmd, c, n);
// String is invalid in arg position (this may seem strange, but is
// consistent across node API, cf. `net.createServer('not options', 'not
// callback')`.
assert.throws(function() { execFile(cmd, s, o, c); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, a, s, c); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, a, o, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, a, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, o, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, u, u, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, n, n, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, a, u, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, a, n, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, u, o, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, n, o, s); }, invalidArgValueError);
assert.throws(function() { execFile(cmd, s, o, c); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, a, s, c); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, a, o, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, a, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, o, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, u, u, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, n, n, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, a, u, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, a, n, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, u, o, s); }, invalidArgTypeError);
assert.throws(function() { execFile(cmd, n, o, s); }, invalidArgTypeError);

execFile(cmd, c, s); // Should not throw.

Expand All @@ -191,4 +191,4 @@ fork(empty, n, o);
fork(empty, a, n);

assert.throws(function() { fork(empty, s); }, invalidArgValueError);
assert.throws(function() { fork(empty, a, s); }, invalidArgValueError);
assert.throws(function() { fork(empty, a, s); }, invalidArgTypeError);

0 comments on commit 64b0467

Please sign in to comment.