From 717ca330c61f85c787baa01e2ebfe844f3f02110 Mon Sep 17 00:00:00 2001
From: Sam Roberts <sam@strongloop.com>
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 <trev.norris@gmail.com>
---
 lib/child_process.js                          | 30 ++++++++++++-------
 .../test-child-process-spawn-typeerror.js     | 11 +++++++
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index 0c1b4c99c1fb..53e0bb24ff47 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.
+  // 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
index d13d5e62e4d5..b5d1249e5883 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, {});
+});