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: #8454 Reviewed-by: Trevor Norris <[email protected]>
nodejs · Nov 19, 2014 · e17c5a7 · e17c5a7
1 parent 2ff29cc
commit e17c5a7
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 10 deletions.
diff --git 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.
+  // 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, {

diff --git 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, {});
+});