Permalink
Browse files

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.

Reviewed-By: trevnorris - Trevor Norris <trevnorris@nodejs.org>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: thefourtheye - Sakthipriyan Vairamani
PR-URL: #2667
Fixes: #2515
  • Loading branch information...
jasnell authored and rvagg committed Sep 2, 2015
1 parent fe4b309 commit 609db5a1ddcd93ecbdc99bdc8bb32e79532071dd
Showing with 114 additions and 19 deletions.
  1. +20 −10 lib/child_process.js
  2. +94 −9 test/parallel/test-child-process-spawn-typeerror.js
View
@@ -23,6 +23,8 @@ exports.fork = function(modulePath /*, args, options*/) {
if (Array.isArray(arguments[1])) {
args = arguments[1];
options = util._extend({}, arguments[2]);
+ } else if (arguments[1] && typeof arguments[1] !== 'object') {
+ throw new TypeError('Incorrect value of args option');
} else {
args = [];
options = util._extend({}, arguments[1]);
@@ -104,7 +106,7 @@ exports.exec = function(command /*, options, callback*/) {
exports.execFile = function(file /*, args, options, callback*/) {
- var args, callback;
+ var args = [], callback;
var options = {
encoding: 'utf8',
timeout: 0,
@@ -114,18 +116,26 @@ exports.execFile = function(file /*, args, options, callback*/) {
env: null
};
- // Parse the parameters.
+ // Parse the optional positional parameters.
+ var pos = 1;
+ if (pos < arguments.length && Array.isArray(arguments[pos])) {
+ args = arguments[pos++];
+ } else if (pos < arguments.length && arguments[pos] == null) {
+ pos++;
+ }
- if (typeof arguments[arguments.length - 1] === 'function') {
- callback = arguments[arguments.length - 1];
+ if (pos < arguments.length && typeof arguments[pos] === 'object') {
+ options = util._extend(options, arguments[pos++]);
+ } else if (pos < arguments.length && arguments[pos] == null) {
+ pos++;
}
- if (Array.isArray(arguments[1])) {
- args = arguments[1];
- options = util._extend(options, arguments[2]);
- } else {
- args = [];
- options = util._extend(options, arguments[1]);
+ if (pos < arguments.length && typeof arguments[pos] === 'function') {
+ callback = arguments[pos++];
+ }
+
+ if (pos === 1 && arguments.length > 1) {
+ throw new TypeError('Incorrect value of args option');
}
var child = spawn(file, args, {
@@ -1,14 +1,19 @@
'use strict';
-var assert = require('assert');
-var child_process = require('child_process');
-var spawn = child_process.spawn;
-var cmd = require('../common').isWindows ? 'rundll32' : 'ls';
-var invalidArgsMsg = /Incorrect value of args option/;
-var invalidOptionsMsg = /options argument must be an object/;
-
-// verify that args argument must be an array
+const assert = require('assert');
+const child_process = require('child_process');
+const spawn = child_process.spawn;
+const fork = child_process.fork;
+const execFile = child_process.execFile;
+const common = require('../common');
+const cmd = common.isWindows ? 'rundll32' : 'ls';
+const invalidcmd = 'hopefully_you_dont_have_this_on_your_machine';
+const invalidArgsMsg = /Incorrect value of args option/;
+const invalidOptionsMsg = /options argument must be an object/;
+const empty = common.fixturesDir + '/empty.js';
+
assert.throws(function() {
- spawn(cmd, 'this is not an array');
+ var child = spawn(invalidcmd, 'this is not an array');
+ child.on('error', assert.fail);
}, TypeError);
// verify that valid argument combinations do not throw
@@ -49,3 +54,83 @@ assert.throws(function() {
spawn(cmd, [], 1);
}, invalidOptionsMsg);
+// Argument types for combinatorics
+const a = [];
+const o = {};
+const c = function callback() {};
+const s = 'string';
+const u = undefined;
+const n = null;
+
+// function spawn(file=f [,args=a] [, options=o]) has valid combinations:
+// (f)
+// (f, a)
+// (f, a, o)
+// (f, o)
+assert.doesNotThrow(function() { spawn(cmd); });
+assert.doesNotThrow(function() { spawn(cmd, a); });
+assert.doesNotThrow(function() { spawn(cmd, a, o); });
+assert.doesNotThrow(function() { spawn(cmd, o); });
+
+// Variants of undefined as explicit 'no argument' at a position
+assert.doesNotThrow(function() { spawn(cmd, u, o); });
+assert.doesNotThrow(function() { spawn(cmd, a, u); });
+
+assert.throws(function() { spawn(cmd, n, o); }, TypeError);
+assert.throws(function() { spawn(cmd, a, n); }, TypeError);
+
+assert.throws(function() { spawn(cmd, s); }, TypeError);
+assert.throws(function() { spawn(cmd, a, s); }, TypeError);
+
+
+// verify that execFile has same argument parsing behaviour as spawn
+//
+// function execFile(file=f [,args=a] [, options=o] [, callback=c]) has valid
+// combinations:
+// (f)
+// (f, a)
+// (f, a, o)
+// (f, a, o, c)
+// (f, a, c)
+// (f, o)
+// (f, o, c)
+// (f, c)
+assert.doesNotThrow(function() { execFile(cmd); });
+assert.doesNotThrow(function() { execFile(cmd, a); });
+assert.doesNotThrow(function() { execFile(cmd, a, o); });
+assert.doesNotThrow(function() { execFile(cmd, a, o, c); });
+assert.doesNotThrow(function() { execFile(cmd, a, c); });
+assert.doesNotThrow(function() { execFile(cmd, o); });
+assert.doesNotThrow(function() { execFile(cmd, o, c); });
+assert.doesNotThrow(function() { execFile(cmd, c); });
+
+// Variants of undefined as explicit 'no argument' at a position
+assert.doesNotThrow(function() { execFile(cmd, u, o, c); });
+assert.doesNotThrow(function() { execFile(cmd, a, u, c); });
+assert.doesNotThrow(function() { execFile(cmd, a, o, u); });
+assert.doesNotThrow(function() { execFile(cmd, n, o, c); });
+assert.doesNotThrow(function() { execFile(cmd, a, n, c); });
+assert.doesNotThrow(function() { execFile(cmd, a, o, 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); }, TypeError);
+assert.doesNotThrow(function() { execFile(cmd, a, s, c); });
+assert.doesNotThrow(function() { execFile(cmd, a, o, s); });
+
+
+// verify that fork has same argument parsing behaviour as spawn
+//
+// function fork(file=f [,args=a] [, options=o]) has valid combinations:
+// (f)
+// (f, a)
+// (f, a, o)
+// (f, o)
+assert.doesNotThrow(function() { fork(empty); });
+assert.doesNotThrow(function() { fork(empty, a); });
+assert.doesNotThrow(function() { fork(empty, a, o); });
+assert.doesNotThrow(function() { fork(empty, o); });
+
+assert.throws(function() { fork(empty, s); }, TypeError);
+assert.doesNotThrow(function() { fork(empty, a, s); }, TypeError);

0 comments on commit 609db5a

Please sign in to comment.