child_process: avoid repeated calls to normalizeSpawnArguments

PR-URL: https://github.com/nodejs/node/pull/43345
Fixes: https://github.com/nodejs/node/issues/43333
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
This commit is contained in:
木杉 2022-07-18 17:20:57 +08:00 committed by GitHub
parent 0e660ce09f
commit c030f88fcf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 71 additions and 28 deletions

View File

@ -32,6 +32,7 @@ const {
ArrayPrototypeSort,
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
ArrayPrototypePushApply,
NumberIsInteger,
ObjectAssign,
ObjectDefineProperty,
@ -249,6 +250,45 @@ 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 = kEmptyObject;
}
if (callback != null) {
validateFunction(callback, 'callback');
}
// Validate argv0, if present.
if (options.argv0 != null) {
validateString(options.argv0, 'options.argv0');
}
return { file, args, options, callback };
}
/**
* Spawns the specified file as a shell.
* @param {string} file
@ -274,27 +314,8 @@ ObjectDefineProperty(exec, promisify.custom, {
* ) => any} [callback]
* @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');
}
function execFile(file, args, options, callback) {
({ file, args, options, callback } = normalizeExecFileArgs(file, args, options, callback));
options = {
encoding: 'utf8',
@ -824,7 +845,7 @@ function checkExecSyncError(ret, args, cmd) {
/**
* Spawns a file as a shell synchronously.
* @param {string} command
* @param {string} file
* @param {string[]} [args]
* @param {{
* cwd?: string;
@ -842,17 +863,18 @@ 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);
const errArgs = [options.argv0 || file];
ArrayPrototypePushApply(errArgs, args);
const err = checkExecSyncError(ret, errArgs);
if (err)
throw err;

View File

@ -2,10 +2,11 @@
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');
const os = require('os');
const fixture = fixtures.path('exit.js');
const echoFixture = fixtures.path('echo.js');
@ -99,3 +100,23 @@ const execOpts = { encoding: 'utf8', shell: true };
});
execFile(process.execPath, [fixture, 0], { signal }, callback);
}
// Verify the execFile() stdout is the same as execFileSync().
{
const file = 'echo';
const args = ['foo', 'bar'];
// Test with and without `{ shell: true }`
[
// Skipping shell-less test on Windows because its echo command is a shell built-in command.
...(common.isWindows ? [] : [{ encoding: 'utf8' }]),
{ shell: true, encoding: 'utf8' },
].forEach((options) => {
const execFileSyncStdout = execFileSync(file, args, options);
assert.strictEqual(execFileSyncStdout, `foo bar${os.EOL}`);
execFile(file, args, options, common.mustCall((_, stdout) => {
assert.strictEqual(stdout, execFileSyncStdout);
}));
});
}