child_process: Avoid repeated calls to normalizeSpawnArguments

nodejs · Jun 8, 2022 · 8a71cbe · 8a71cbe
1 parent 797e41c
commit 8a71cbe
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 26 deletions.
diff --git a/lib/child_process.js b/lib/child_process.js
@@ -248,6 +248,40 @@ ObjectDefineProperty(exec, promisify.custom, {
   value: customPromiseExecFunction(exec)
 });
 
+function normalizeExecFileArgs(file, args, options, callback) {
+  if (ArrayIsArray(args)) {
+    args = ArrayPrototypeSlice(args)
+  } else if (args != null && typeof args === 'object') {
+    callback = options;
+    options = args;
+    args = null;
+  } else if (typeof args === 'function') {
+    callback = args;
+    options = null;
+    args = null;
+  }
+
+  if (args == null) {
+    args = [];
+  }
+
+  if (typeof options === 'function') {
+    callback = options;
+  } else if (options != null) {
+    validateObject(options, 'options');
+  }
+
+  if (options == null) {
+    options = {};
+  }
+
+  if (callback != null) {
+    validateFunction(callback, 'callback');
+  }
+
+  return [file, args, options, callback];
+}
+
 /**
  * Spawns the specified file as a shell.
  * @param {string} file
@@ -274,26 +308,7 @@ ObjectDefineProperty(exec, promisify.custom, {
  * @returns {ChildProcess}
  */
 function execFile(file, args = [], options, callback) {
-  if (args != null && typeof args === 'object' && !ArrayIsArray(args)) {
-    callback = options;
-    options = args;
-    args = null;
-  } else if (typeof args === 'function') {
-    callback = args;
-    options = null;
-    args = null;
-  }
-
-  if (typeof options === 'function') {
-    callback = options;
-    options = null;
-  } else if (options != null) {
-    validateObject(options, 'options');
-  }
-
-  if (callback != null) {
-    validateFunction(callback, 'callback');
-  }
+  [file, args, options, callback] = normalizeExecFileArgs(file, args, options, callback);
 
   options = {
     encoding: 'utf8',
@@ -841,17 +856,22 @@ function checkExecSyncError(ret, args, cmd) {
  *   }} [options]
  * @returns {Buffer | string}
  */
-function execFileSync(command, args, options) {
-  options = normalizeSpawnArguments(command, args, options);
+function execFileSync(file, args, options) {
+  [file, args, options] = normalizeExecFileArgs(file, args, options);
 
   const inheritStderr = !options.stdio;
-  const ret = spawnSync(options.file,
-                        ArrayPrototypeSlice(options.args, 1), options);
+  const ret = spawnSync(file, args, options);
 
   if (inheritStderr && ret.stderr)
     process.stderr.write(ret.stderr);
 
-  const err = checkExecSyncError(ret, options.args, undefined);
+  if (typeof options.argv0 === 'string') {
+    ArrayPrototypeUnshift(args, options.argv0);
+  } else {
+    ArrayPrototypeUnshift(args, file);
+  }
+
+  const err = checkExecSyncError(ret, args);
 
   if (err)
     throw err;

diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js
@@ -2,7 +2,7 @@
 
 const common = require('../common');
 const assert = require('assert');
-const execFile = require('child_process').execFile;
+const { execFile, execFileSync } = require('child_process');
 const { getEventListeners } = require('events');
 const { getSystemErrorName } = require('util');
 const fixtures = require('../common/fixtures');
@@ -99,3 +99,15 @@ const execOpts = { encoding: 'utf8', shell: true };
   });
   execFile(process.execPath, [fixture, 0], { signal }, callback);
 }
+
+// Verify the execFile() stdout is the same as execFileSync().
+{
+  const execArgs = ['echo', ['foo', 'bar'], { shell: true, encoding: 'utf8' }];
+  const execFileSyncResult = execFileSync(...execArgs);
+  assert.strictEqual(execFileSyncResult.trim(), execArgs[1].join(' '));
+
+  const check = common.mustCall((_, stdout) => {
+    assert.strictEqual(stdout, execFileSyncResult);
+  });
+  execFile(...execArgs, check);
+}