From e17c5a72b23f920f291d61f2780068c18768cb92 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Thu, 25 Sep 2014 22:28:33 -0700 Subject: [PATCH] child_process: check execFile args is an array execFile and spawn have same API signature with respect to optional arg array and optional options object, they should have same behaviour with respect to argument validation. PR-URL: https://github.com/joyent/node/pull/8454 Reviewed-by: Trevor Norris --- lib/child_process.js | 32 ++++++++++++------- .../test-child-process-spawn-typeerror.js | 11 +++++++ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 0c1b4c99c1f..53e0bb24ff4 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -591,7 +591,7 @@ exports.exec = function(command /*, options, callback */) { exports.execFile = function(file /* args, options, callback */) { - var args, optionArg, callback; + var args = [], optionArg, callback; var options = { encoding: 'utf8', timeout: 0, @@ -601,18 +601,28 @@ exports.execFile = function(file /* args, options, callback */) { env: null }; - // Parse the parameters. - - if (typeof arguments[arguments.length - 1] === 'function') { - callback = arguments[arguments.length - 1]; + // 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 (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] === 'object') { + options = util._extend(options, arguments[pos++]); + } + else if(pos < arguments.length && arguments[pos] == null) { + pos++; + } + + 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, { diff --git a/test/simple/test-child-process-spawn-typeerror.js b/test/simple/test-child-process-spawn-typeerror.js index d13d5e62e4d..b5d1249e588 100644 --- a/test/simple/test-child-process-spawn-typeerror.js +++ b/test/simple/test-child-process-spawn-typeerror.js @@ -22,6 +22,7 @@ var assert = require('assert'); var child_process = require('child_process'); var spawn = child_process.spawn; +var execFile = child_process.execFile; var cmd = (process.platform === 'win32') ? 'dir' : 'ls'; @@ -34,3 +35,13 @@ assert.throws(function() { assert.doesNotThrow(function() { spawn(cmd, {}); }); + + +// verify that execFile has same argument parsing behaviour as spawn +assert.throws(function() { + execFile(cmd, 'this is not an array'); +}, TypeError); + +assert.doesNotThrow(function() { + execFile(cmd, {}); +});