Skip to content

Commit

Permalink
child_process: avoid repeated calls to normalizeSpawnArguments
Browse files Browse the repository at this point in the history
PR-URL: #43345
Fixes: #43333
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
  • Loading branch information
zhmushan authored and danielleadams committed Jul 26, 2022
1 parent e8c66f9 commit c1893b7
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 28 deletions.
76 changes: 49 additions & 27 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const {
ArrayPrototypeSort,
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
ArrayPrototypePushApply,
NumberIsInteger,
ObjectAssign,
ObjectDefineProperty,
Expand Down Expand Up @@ -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
Expand All @@ -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',
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
23 changes: 22 additions & 1 deletion test/parallel/test-child-process-execfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
}));
});
}

0 comments on commit c1893b7

Please sign in to comment.