Skip to content

Commit 0cdb1ce

Browse files
committed
child_process: improve argument validation
For execFile() and fork(), use INVALID_ARG_TYPE as appropriate instead of INVALID_ARG_VALUE. Use validator functions where sensible.
1 parent 4069e7e commit 0cdb1ce

File tree

3 files changed

+25
-30
lines changed

3 files changed

+25
-30
lines changed

lib/child_process.js

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ const {
7676
isInt32,
7777
validateAbortSignal,
7878
validateBoolean,
79+
validateFunction,
7980
validateObject,
8081
validateString,
8182
} = require('internal/validators');
@@ -126,13 +127,10 @@ function fork(modulePath, args = [], options) {
126127
args = [];
127128
}
128129

129-
if (options == null) {
130-
options = {};
131-
} else if (typeof options !== 'object') {
132-
throw new ERR_INVALID_ARG_VALUE('options', options);
133-
} else {
134-
options = { ...options };
130+
if (options != null) {
131+
validateObject(options, 'options', { allowArray: true });
135132
}
133+
options = { ...options };
136134

137135
// Prepare arguments for fork:
138136
execArgv = options.execArgv || process.execArgv;
@@ -286,23 +284,20 @@ function execFile(file, args = [], options, callback) {
286284
}
287285
} else if (typeof args === 'function') {
288286
callback = args;
289-
options = {};
287+
options = null;
290288
args = [];
291-
} else {
292-
throw new ERR_INVALID_ARG_VALUE('args', args);
293289
}
294290

295-
if (options == null) {
296-
options = {};
297-
} else if (typeof options === 'function') {
291+
if (typeof options === 'function') {
298292
callback = options;
299-
options = {};
300-
} else if (typeof options !== 'object') {
301-
throw new ERR_INVALID_ARG_VALUE('options', options);
293+
options = null;
294+
} else if (options != null) {
295+
// TODO: Should this have allowArray: true set?
296+
validateObject(options, 'options');
302297
}
303298

304-
if (callback && typeof callback !== 'function') {
305-
throw new ERR_INVALID_ARG_VALUE('callback', callback);
299+
if (callback != null) {
300+
validateFunction(callback, 'callback');
306301
}
307302

308303
options = {

test/parallel/test-child-process-fork-args.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ const expectedEnv = { foo: 'bar' };
9797
fork(fixtures.path('child-process-echo-options.js'), [], arg);
9898
},
9999
{
100-
code: 'ERR_INVALID_ARG_VALUE',
100+
code: 'ERR_INVALID_ARG_TYPE',
101101
name: 'TypeError'
102102
}
103103
);

test/parallel/test-child-process-spawn-typeerror.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -158,17 +158,17 @@ execFile(cmd, c, n);
158158
// String is invalid in arg position (this may seem strange, but is
159159
// consistent across node API, cf. `net.createServer('not options', 'not
160160
// callback')`.
161-
assert.throws(function() { execFile(cmd, s, o, c); }, invalidArgValueError);
162-
assert.throws(function() { execFile(cmd, a, s, c); }, invalidArgValueError);
163-
assert.throws(function() { execFile(cmd, a, o, s); }, invalidArgValueError);
164-
assert.throws(function() { execFile(cmd, a, s); }, invalidArgValueError);
165-
assert.throws(function() { execFile(cmd, o, s); }, invalidArgValueError);
166-
assert.throws(function() { execFile(cmd, u, u, s); }, invalidArgValueError);
167-
assert.throws(function() { execFile(cmd, n, n, s); }, invalidArgValueError);
168-
assert.throws(function() { execFile(cmd, a, u, s); }, invalidArgValueError);
169-
assert.throws(function() { execFile(cmd, a, n, s); }, invalidArgValueError);
170-
assert.throws(function() { execFile(cmd, u, o, s); }, invalidArgValueError);
171-
assert.throws(function() { execFile(cmd, n, o, s); }, invalidArgValueError);
161+
assert.throws(function() { execFile(cmd, s, o, c); }, invalidArgTypeError);
162+
assert.throws(function() { execFile(cmd, a, s, c); }, invalidArgTypeError);
163+
assert.throws(function() { execFile(cmd, a, o, s); }, invalidArgTypeError);
164+
assert.throws(function() { execFile(cmd, a, s); }, invalidArgTypeError);
165+
assert.throws(function() { execFile(cmd, o, s); }, invalidArgTypeError);
166+
assert.throws(function() { execFile(cmd, u, u, s); }, invalidArgTypeError);
167+
assert.throws(function() { execFile(cmd, n, n, s); }, invalidArgTypeError);
168+
assert.throws(function() { execFile(cmd, a, u, s); }, invalidArgTypeError);
169+
assert.throws(function() { execFile(cmd, a, n, s); }, invalidArgTypeError);
170+
assert.throws(function() { execFile(cmd, u, o, s); }, invalidArgTypeError);
171+
assert.throws(function() { execFile(cmd, n, o, s); }, invalidArgTypeError);
172172

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

@@ -191,4 +191,4 @@ fork(empty, n, o);
191191
fork(empty, a, n);
192192

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

0 commit comments

Comments
 (0)