child_process: improve spawn() argument handling
Add stricter argument type checking to normalizeSpawnArguments(). Removes a number of extraneous checks in spawn(). Fix regression in handling of the optional args argument. Add more thorough testing of spawn() arguments. Reviewed-by: Trevor Norris <trev.norris@gmail.com>
This commit is contained in:
parent
f3473d7db6
commit
9d95774722
@ -930,28 +930,29 @@ function _validateStdio(stdio, sync) {
|
||||
}
|
||||
|
||||
|
||||
function normalizeSpawnArguments(/*file, args, options*/) {
|
||||
function normalizeSpawnArguments(file /*, args, options*/) {
|
||||
var args, options;
|
||||
|
||||
var file = arguments[0];
|
||||
|
||||
if (Array.isArray(arguments[1])) {
|
||||
args = arguments[1].slice(0);
|
||||
options = arguments[2];
|
||||
} else if (arguments[1] && !Array.isArray(arguments[1])) {
|
||||
} else if (arguments[1] !== undefined && !util.isObject(arguments[1])) {
|
||||
throw new TypeError('Incorrect value of args option');
|
||||
} else {
|
||||
args = [];
|
||||
options = arguments[1];
|
||||
}
|
||||
|
||||
if (!options)
|
||||
if (options === undefined)
|
||||
options = {};
|
||||
else if (!util.isObject(options))
|
||||
throw new TypeError('options argument must be an object');
|
||||
|
||||
args.unshift(file);
|
||||
|
||||
var env = (options && options.env ? options.env : null) || process.env;
|
||||
var env = options.env || process.env;
|
||||
var envPairs = [];
|
||||
|
||||
for (var key in env) {
|
||||
envPairs.push(key + '=' + env[key]);
|
||||
}
|
||||
@ -969,24 +970,19 @@ function normalizeSpawnArguments(/*file, args, options*/) {
|
||||
|
||||
var spawn = exports.spawn = function(/*file, args, options*/) {
|
||||
var opts = normalizeSpawnArguments.apply(null, arguments);
|
||||
|
||||
var file = opts.file;
|
||||
var args = opts.args;
|
||||
var options = opts.options;
|
||||
var envPairs = opts.envPairs;
|
||||
|
||||
var child = new ChildProcess();
|
||||
|
||||
child.spawn({
|
||||
file: file,
|
||||
args: args,
|
||||
cwd: options ? options.cwd : null,
|
||||
windowsVerbatimArguments: !!(options && options.windowsVerbatimArguments),
|
||||
detached: !!(options && options.detached),
|
||||
envPairs: envPairs,
|
||||
stdio: options ? options.stdio : null,
|
||||
uid: options ? options.uid : null,
|
||||
gid: options ? options.gid : null
|
||||
file: opts.file,
|
||||
args: opts.args,
|
||||
cwd: options.cwd,
|
||||
windowsVerbatimArguments: !!options.windowsVerbatimArguments,
|
||||
detached: !!options.detached,
|
||||
envPairs: opts.envPairs,
|
||||
stdio: options.stdio,
|
||||
uid: options.uid,
|
||||
gid: options.gid
|
||||
});
|
||||
|
||||
return child;
|
||||
@ -1340,7 +1336,7 @@ function checkExecSyncError(ret) {
|
||||
|
||||
function execFileSync(/*command, options*/) {
|
||||
var opts = normalizeSpawnArguments.apply(null, arguments);
|
||||
var inheritStderr = !!!opts.options.stdio;
|
||||
var inheritStderr = !opts.options.stdio;
|
||||
|
||||
var ret = spawnSync(opts.file, opts.args.slice(1), opts.options);
|
||||
|
||||
@ -1359,7 +1355,7 @@ exports.execFileSync = execFileSync;
|
||||
|
||||
function execSync(/*comand, options*/) {
|
||||
var opts = normalizeExecArgs.apply(null, arguments);
|
||||
var inheritStderr = opts.options ? !!!opts.options.stdio : true;
|
||||
var inheritStderr = opts.options ? !opts.options.stdio : true;
|
||||
|
||||
var ret = spawnSync(opts.file, opts.args, opts.options);
|
||||
ret.cmd = opts.cmd;
|
||||
|
@ -22,12 +22,15 @@
|
||||
var spawn = require('child_process').spawn,
|
||||
assert = require('assert'),
|
||||
windows = (process.platform === 'win32'),
|
||||
cmd = (windows) ? 'ls' : 'dir',
|
||||
cmd = (windows) ? 'dir' : 'ls',
|
||||
invalidcmd = (windows) ? 'ls' : 'dir',
|
||||
invalidArgsMsg = /Incorrect value of args option/,
|
||||
invalidOptionsMsg = /options argument must be an object/,
|
||||
errors = 0;
|
||||
|
||||
try {
|
||||
// Ensure this throws a TypeError
|
||||
var child = spawn(cmd, 'this is not an array');
|
||||
var child = spawn(invalidcmd, 'this is not an array');
|
||||
|
||||
child.on('error', function (err) {
|
||||
errors++;
|
||||
@ -37,6 +40,44 @@ try {
|
||||
assert.equal(e instanceof TypeError, true);
|
||||
}
|
||||
|
||||
// verify that valid argument combinations do not throw
|
||||
assert.doesNotThrow(function() {
|
||||
spawn(cmd);
|
||||
});
|
||||
|
||||
assert.doesNotThrow(function() {
|
||||
spawn(cmd, []);
|
||||
});
|
||||
|
||||
assert.doesNotThrow(function() {
|
||||
spawn(cmd, {});
|
||||
});
|
||||
|
||||
assert.doesNotThrow(function() {
|
||||
spawn(cmd, [], {});
|
||||
});
|
||||
|
||||
// verify that invalid argument combinations throw
|
||||
assert.throws(function() {
|
||||
spawn();
|
||||
}, /Bad argument/);
|
||||
|
||||
assert.throws(function() {
|
||||
spawn(cmd, null);
|
||||
}, invalidArgsMsg);
|
||||
|
||||
assert.throws(function() {
|
||||
spawn(cmd, true);
|
||||
}, invalidArgsMsg);
|
||||
|
||||
assert.throws(function() {
|
||||
spawn(cmd, [], null);
|
||||
}, invalidOptionsMsg);
|
||||
|
||||
assert.throws(function() {
|
||||
spawn(cmd, [], 1);
|
||||
}, invalidOptionsMsg);
|
||||
|
||||
process.on('exit', function() {
|
||||
assert.equal(errors, 0);
|
||||
});
|
||||
|
Loading…
x
Reference in New Issue
Block a user