Skip to content

Commit 56bb942

Browse files
committed
child_process: check execFile and fork args
Port of joyent/node commits: * nodejs/node-v0.x-archive@e17c5a7 * nodejs/node-v0.x-archive@70dafa7 Pull over test-child-process-spawn-typeerror.js from v0.12, replacing the existing test in master. The new test includes a broader set of tests on the various arg choices and throws.
1 parent 56d9584 commit 56bb942

File tree

2 files changed

+128
-21
lines changed

2 files changed

+128
-21
lines changed

lib/child_process.js

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ exports.fork = function(modulePath /*, args, options*/) {
2323
if (Array.isArray(arguments[1])) {
2424
args = arguments[1];
2525
options = util._extend({}, arguments[2]);
26+
} else if (arguments[1] && typeof arguments[1] !== 'object') {
27+
throw new TypeError('Incorrect value of args option');
2628
} else {
2729
args = [];
2830
options = util._extend({}, arguments[1]);
@@ -104,7 +106,7 @@ exports.exec = function(command /*, options, callback*/) {
104106

105107

106108
exports.execFile = function(file /*, args, options, callback*/) {
107-
var args, callback;
109+
var args = [], callback;
108110
var options = {
109111
encoding: 'utf8',
110112
timeout: 0,
@@ -114,18 +116,26 @@ exports.execFile = function(file /*, args, options, callback*/) {
114116
env: null
115117
};
116118

117-
// Parse the parameters.
119+
// Parse the optional positional parameters.
120+
var pos = 1;
121+
if (pos < arguments.length && Array.isArray(arguments[pos])) {
122+
args = arguments[pos++];
123+
} else if (pos < arguments.length && arguments[pos] == null) {
124+
pos++;
125+
}
118126

119-
if (typeof arguments[arguments.length - 1] === 'function') {
120-
callback = arguments[arguments.length - 1];
127+
if (pos < arguments.length && typeof arguments[pos] === 'object') {
128+
options = util._extend(options, arguments[pos++]);
129+
} else if (pos < arguments.length && arguments[pos] == null) {
130+
pos++;
121131
}
122132

123-
if (Array.isArray(arguments[1])) {
124-
args = arguments[1];
125-
options = util._extend(options, arguments[2]);
126-
} else {
127-
args = [];
128-
options = util._extend(options, arguments[1]);
133+
if (pos < arguments.length && typeof arguments[pos] === 'function') {
134+
callback = arguments[pos++];
135+
}
136+
137+
if (pos === 1 && arguments.length > 1) {
138+
throw new TypeError('Incorrect value of args option');
129139
}
130140

131141
var child = spawn(file, args, {

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

Lines changed: 108 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,28 @@
11
'use strict';
2-
var assert = require('assert');
3-
var child_process = require('child_process');
4-
var spawn = child_process.spawn;
5-
var cmd = require('../common').isWindows ? 'rundll32' : 'ls';
6-
var invalidArgsMsg = /Incorrect value of args option/;
7-
var invalidOptionsMsg = /options argument must be an object/;
8-
9-
// verify that args argument must be an array
10-
assert.throws(function() {
11-
spawn(cmd, 'this is not an array');
12-
}, TypeError);
2+
var assert = require('assert'),
3+
child_process = require('child_process'),
4+
spawn = child_process.spawn,
5+
fork = child_process.fork,
6+
execFile = child_process.execFile,
7+
windows = (process.platform === 'win32'),
8+
cmd = (windows) ? 'rundll32' : 'ls',
9+
invalidcmd = 'hopefully_you_dont_have_this_on_your_machine',
10+
invalidArgsMsg = /Incorrect value of args option/,
11+
invalidOptionsMsg = /options argument must be an object/,
12+
empty = require('../common').fixturesDir + '/empty.js',
13+
errors = 0;
14+
15+
try {
16+
// Ensure this throws a TypeError
17+
var child = spawn(invalidcmd, 'this is not an array');
18+
19+
child.on('error', function(err) {
20+
errors++;
21+
});
22+
23+
} catch (e) {
24+
assert.equal(e instanceof TypeError, true);
25+
}
1326

1427
// verify that valid argument combinations do not throw
1528
assert.doesNotThrow(function() {
@@ -49,3 +62,87 @@ assert.throws(function() {
4962
spawn(cmd, [], 1);
5063
}, invalidOptionsMsg);
5164

65+
process.on('exit', function() {
66+
assert.equal(errors, 0);
67+
});
68+
69+
// Argument types for combinatorics
70+
var a = [],
71+
o = {},
72+
c = (function callback() {}),
73+
s = 'string',
74+
u = undefined,
75+
n = null;
76+
77+
// function spawn(file=f [,args=a] [, options=o]) has valid combinations:
78+
// (f)
79+
// (f, a)
80+
// (f, a, o)
81+
// (f, o)
82+
assert.doesNotThrow(function() { spawn(cmd); });
83+
assert.doesNotThrow(function() { spawn(cmd, a); });
84+
assert.doesNotThrow(function() { spawn(cmd, a, o); });
85+
assert.doesNotThrow(function() { spawn(cmd, o); });
86+
87+
// Variants of undefined as explicit 'no argument' at a position
88+
assert.doesNotThrow(function() { spawn(cmd, u, o); });
89+
assert.doesNotThrow(function() { spawn(cmd, a, u); });
90+
91+
assert.throws(function() { spawn(cmd, n, o); }, TypeError);
92+
assert.throws(function() { spawn(cmd, a, n); }, TypeError);
93+
94+
assert.throws(function() { spawn(cmd, s); }, TypeError);
95+
assert.throws(function() { spawn(cmd, a, s); }, TypeError);
96+
97+
98+
// verify that execFile has same argument parsing behaviour as spawn
99+
//
100+
// function execFile(file=f [,args=a] [, options=o] [, callback=c]) has valid
101+
// combinations:
102+
// (f)
103+
// (f, a)
104+
// (f, a, o)
105+
// (f, a, o, c)
106+
// (f, a, c)
107+
// (f, o)
108+
// (f, o, c)
109+
// (f, c)
110+
assert.doesNotThrow(function() { execFile(cmd); });
111+
assert.doesNotThrow(function() { execFile(cmd, a); });
112+
assert.doesNotThrow(function() { execFile(cmd, a, o); });
113+
assert.doesNotThrow(function() { execFile(cmd, a, o, c); });
114+
assert.doesNotThrow(function() { execFile(cmd, a, c); });
115+
assert.doesNotThrow(function() { execFile(cmd, o); });
116+
assert.doesNotThrow(function() { execFile(cmd, o, c); });
117+
assert.doesNotThrow(function() { execFile(cmd, c); });
118+
119+
// Variants of undefined as explicit 'no argument' at a position
120+
assert.doesNotThrow(function() { execFile(cmd, u, o, c); });
121+
assert.doesNotThrow(function() { execFile(cmd, a, u, c); });
122+
assert.doesNotThrow(function() { execFile(cmd, a, o, u); });
123+
assert.doesNotThrow(function() { execFile(cmd, n, o, c); });
124+
assert.doesNotThrow(function() { execFile(cmd, a, n, c); });
125+
assert.doesNotThrow(function() { execFile(cmd, a, o, n); });
126+
127+
// string is invalid in arg position (this may seem strange, but is
128+
// consistent across node API, cf. `net.createServer('not options', 'not
129+
// callback')`
130+
assert.throws(function() { execFile(cmd, s, o, c); }, TypeError);
131+
assert.doesNotThrow(function() { execFile(cmd, a, s, c); });
132+
assert.doesNotThrow(function() { execFile(cmd, a, o, s); });
133+
134+
135+
// verify that fork has same argument parsing behaviour as spawn
136+
//
137+
// function fork(file=f [,args=a] [, options=o]) has valid combinations:
138+
// (f)
139+
// (f, a)
140+
// (f, a, o)
141+
// (f, o)
142+
assert.doesNotThrow(function() { fork(empty); });
143+
assert.doesNotThrow(function() { fork(empty, a); });
144+
assert.doesNotThrow(function() { fork(empty, a, o); });
145+
assert.doesNotThrow(function() { fork(empty, o); });
146+
147+
assert.throws(function() { fork(empty, s); }, TypeError);
148+
assert.doesNotThrow(function() { fork(empty, a, s); }, TypeError);

0 commit comments

Comments
 (0)