FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

child_process: improve argument validation · nodejs/node@26c973d · GitHub

/ node Public

Commit 26c973d

Browse files
Trott
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. PR-URL: #41305 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent db02f6f commit 26c973d

3 files changed

Lines changed: 43 additions & 52 deletions

File tree

‎lib/child_process.js‎

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ const { getValidatedPath } = require('internal/fs/utils');
7575
const {
7676
isInt32,
7777
validateAbortSignal,
78+
validateArray,
7879
validateBoolean,
80+
validateFunction,
7981
validateObject,
8082
validateString,
8183
} = require('internal/validators');
@@ -119,20 +121,18 @@ function fork(modulePath, args = [], options) {
119121

120122
if (args == null) {
121123
args = [];
122-
} else if (typeof args !== 'object') {
123-
throw new ERR_INVALID_ARG_VALUE('args', args);
124-
} else if (!ArrayIsArray(args)) {
124+
} else if (typeof args === 'object' && !ArrayIsArray(args)) {
125125
options = args;
126126
args = [];
127+
} else {
128+
validateArray(args, 'args');
127129
}
128130

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 };
131+
if (options != null) {
132+
validateObject(options, 'options');
135133
}
134+
options = { ...options, shell: false };
135+
options.execPath = options.execPath || process.execPath;
136136

137137
// Prepare arguments for fork:
138138
execArgv = options.execArgv || process.execArgv;
@@ -160,9 +160,6 @@ function fork(modulePath, args = [], options) {
160160
throw new ERR_CHILD_PROCESS_IPC_REQUIRED('options.stdio');
161161
}
162162

163-
options.execPath = options.execPath || process.execPath;
164-
options.shell = false;
165-
166163
return spawn(options.execPath, args, options);
167164
}
168165

@@ -276,33 +273,25 @@ ObjectDefineProperty(exec, promisify.custom, {
276273
* @returns {ChildProcess}
277274
*/
278275
function execFile(file, args = [], options, callback) {
279-
if (args == null) {
280-
args = [];
281-
} else if (typeof args === 'object') {
282-
if (!ArrayIsArray(args)) {
283-
callback = options;
284-
options = args;
285-
args = [];
286-
}
276+
if (args != null && typeof args === 'object' && !ArrayIsArray(args)) {
277+
callback = options;
278+
options = args;
279+
args = null;
287280
} else if (typeof args === 'function') {
288281
callback = args;
289-
options = {};
290-
args = [];
291-
} else {
292-
throw new ERR_INVALID_ARG_VALUE('args', args);
282+
options = null;
283+
args = null;
293284
}
294285

295-
if (options == null) {
296-
options = {};
297-
} else if (typeof options === 'function') {
286+
if (typeof options === 'function') {
298287
callback = options;
299-
options = {};
300-
} else if (typeof options !== 'object') {
301-
throw new ERR_INVALID_ARG_VALUE('options', options);
288+
options = null;
289+
} else if (options != null) {
290+
validateObject(options, 'options');
302291
}
303292

304-
if (callback && typeof callback !== 'function') {
305-
throw new ERR_INVALID_ARG_VALUE('callback', callback);
293+
if (callback != null) {
294+
validateFunction(callback, 'callback');
306295
}
307296

308297
options = {
@@ -391,7 +380,7 @@ function execFile(file, args = [], options, callback) {
391380
return;
392381
}
393382

394-
if (args.length !== 0)
383+
if (args?.length)
395384
cmd += ` ${ArrayPrototypeJoin(args, ' ')}`;
396385

397386
if (!ex) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const expectedEnv = { foo: 'bar' };
5454
fork(fixtures.path('child-process-echo-options.js'), arg);
5555
},
5656
{
57-
code: 'ERR_INVALID_ARG_VALUE',
57+
code: 'ERR_INVALID_ARG_TYPE',
5858
name: 'TypeError'
5959
}
6060
);
@@ -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: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,10 @@ spawn(cmd, u, o);
106106
spawn(cmd, n, o);
107107
spawn(cmd, a, u);
108108

109-
assert.throws(function() { spawn(cmd, a, n); }, invalidArgTypeError);
110-
111-
assert.throws(function() { spawn(cmd, s); }, invalidArgTypeError);
112-
assert.throws(function() { spawn(cmd, a, s); }, invalidArgTypeError);
109+
assert.throws(() => { spawn(cmd, a, n); }, invalidArgTypeError);
110+
assert.throws(() => { spawn(cmd, s); }, invalidArgTypeError);
111+
assert.throws(() => { spawn(cmd, a, s); }, invalidArgTypeError);
112+
assert.throws(() => { spawn(cmd, a, a); }, invalidArgTypeError);
113113

114114

115115
// Verify that execFile has same argument parsing behavior as spawn.
@@ -158,17 +158,18 @@ 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(() => { execFile(cmd, s, o, c); }, invalidArgTypeError);
162+
assert.throws(() => { execFile(cmd, a, s, c); }, invalidArgTypeError);
163+
assert.throws(() => { execFile(cmd, a, o, s); }, invalidArgTypeError);
164+
assert.throws(() => { execFile(cmd, a, s); }, invalidArgTypeError);
165+
assert.throws(() => { execFile(cmd, o, s); }, invalidArgTypeError);
166+
assert.throws(() => { execFile(cmd, u, u, s); }, invalidArgTypeError);
167+
assert.throws(() => { execFile(cmd, n, n, s); }, invalidArgTypeError);
168+
assert.throws(() => { execFile(cmd, a, u, s); }, invalidArgTypeError);
169+
assert.throws(() => { execFile(cmd, a, n, s); }, invalidArgTypeError);
170+
assert.throws(() => { execFile(cmd, u, o, s); }, invalidArgTypeError);
171+
assert.throws(() => { execFile(cmd, n, o, s); }, invalidArgTypeError);
172+
assert.throws(() => { execFile(cmd, a, a); }, invalidArgTypeError);
172173

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

@@ -190,5 +191,6 @@ fork(empty, n, n);
190191
fork(empty, n, o);
191192
fork(empty, a, n);
192193

193-
assert.throws(function() { fork(empty, s); }, invalidArgValueError);
194-
assert.throws(function() { fork(empty, a, s); }, invalidArgValueError);
194+
assert.throws(() => { fork(empty, s); }, invalidArgTypeError);
195+
assert.throws(() => { fork(empty, a, s); }, invalidArgTypeError);
196+
assert.throws(() => { fork(empty, a, a); }, invalidArgTypeError);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL