FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

child_process: avoid repeated calls to `normalizeSpawnArguments` · nodejs/node@c1893b7 · GitHub

/ node Public

Commit c1893b7

Browse files
zhmushan
authored and
danielleadams
committed
child_process: avoid repeated calls to normalizeSpawnArguments
PR-URL: #43345 Fixes: #43333 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name>
1 parent e8c66f9 commit c1893b7

2 files changed

Lines changed: 71 additions & 28 deletions

File tree

‎lib/child_process.js‎

Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232
ArrayPrototypeSort,
3333
ArrayPrototypeSplice,
3434
ArrayPrototypeUnshift,
35+
ArrayPrototypePushApply,
3536
NumberIsInteger,
3637
ObjectAssign,
3738
ObjectDefineProperty,
@@ -249,6 +250,45 @@ ObjectDefineProperty(exec, promisify.custom, {
249250
value: customPromiseExecFunction(exec)
250251
});
251252

253+
function normalizeExecFileArgs(file, args, options, callback) {
254+
if (ArrayIsArray(args)) {
255+
args = ArrayPrototypeSlice(args);
256+
} else if (args != null && typeof args === 'object') {
257+
callback = options;
258+
options = args;
259+
args = null;
260+
} else if (typeof args === 'function') {
261+
callback = args;
262+
options = null;
263+
args = null;
264+
}
265+
266+
if (args == null) {
267+
args = [];
268+
}
269+
270+
if (typeof options === 'function') {
271+
callback = options;
272+
} else if (options != null) {
273+
validateObject(options, 'options');
274+
}
275+
276+
if (options == null) {
277+
options = kEmptyObject;
278+
}
279+
280+
if (callback != null) {
281+
validateFunction(callback, 'callback');
282+
}
283+
284+
// Validate argv0, if present.
285+
if (options.argv0 != null) {
286+
validateString(options.argv0, 'options.argv0');
287+
}
288+
289+
return { file, args, options, callback };
290+
}
291+
252292
/**
253293
* Spawns the specified file as a shell.
254294
* @param {string} file
@@ -274,27 +314,8 @@ ObjectDefineProperty(exec, promisify.custom, {
274314
* ) => any} [callback]
275315
* @returns {ChildProcess}
276316
*/
277-
function execFile(file, args = [], options, callback) {
278-
if (args != null && typeof args === 'object' && !ArrayIsArray(args)) {
279-
callback = options;
280-
options = args;
281-
args = null;
282-
} else if (typeof args === 'function') {
283-
callback = args;
284-
options = null;
285-
args = null;
286-
}
287-
288-
if (typeof options === 'function') {
289-
callback = options;
290-
options = null;
291-
} else if (options != null) {
292-
validateObject(options, 'options');
293-
}
294-
295-
if (callback != null) {
296-
validateFunction(callback, 'callback');
297-
}
317+
function execFile(file, args, options, callback) {
318+
({ file, args, options, callback } = normalizeExecFileArgs(file, args, options, callback));
298319

299320
options = {
300321
encoding: 'utf8',
@@ -824,7 +845,7 @@ function checkExecSyncError(ret, args, cmd) {
824845

825846
/**
826847
* Spawns a file as a shell synchronously.
827-
* @param {string} command
848+
* @param {string} file
828849
* @param {string[]} [args]
829850
* @param {{
830851
* cwd?: string;
@@ -842,17 +863,18 @@ function checkExecSyncError(ret, args, cmd) {
842863
* }} [options]
843864
* @returns {Buffer | string}
844865
*/
845-
function execFileSync(command, args, options) {
846-
options = normalizeSpawnArguments(command, args, options);
866+
function execFileSync(file, args, options) {
867+
({ file, args, options } = normalizeExecFileArgs(file, args, options));
847868

848869
const inheritStderr = !options.stdio;
849-
const ret = spawnSync(options.file,
850-
ArrayPrototypeSlice(options.args, 1), options);
870+
const ret = spawnSync(file, args, options);
851871

852872
if (inheritStderr && ret.stderr)
853873
process.stderr.write(ret.stderr);
854874

855-
const err = checkExecSyncError(ret, options.args, undefined);
875+
const errArgs = [options.argv0 || file];
876+
ArrayPrototypePushApply(errArgs, args);
877+
const err = checkExecSyncError(ret, errArgs);
856878

857879
if (err)
858880
throw err;

‎test/parallel/test-child-process-execfile.js‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22

33
const common = require('../common');
44
const assert = require('assert');
5-
const execFile = require('child_process').execFile;
5+
const { execFile, execFileSync } = require('child_process');
66
const { getEventListeners } = require('events');
77
const { getSystemErrorName } = require('util');
88
const fixtures = require('../common/fixtures');
9+
const os = require('os');
910

1011
const fixture = fixtures.path('exit.js');
1112
const echoFixture = fixtures.path('echo.js');
@@ -99,3 +100,23 @@ const execOpts = { encoding: 'utf8', shell: true };
99100
});
100101
execFile(process.execPath, [fixture, 0], { signal }, callback);
101102
}
103+
104+
// Verify the execFile() stdout is the same as execFileSync().
105+
{
106+
const file = 'echo';
107+
const args = ['foo', 'bar'];
108+
109+
// Test with and without `{ shell: true }`
110+
[
111+
// Skipping shell-less test on Windows because its echo command is a shell built-in command.
112+
...(common.isWindows ? [] : [{ encoding: 'utf8' }]),
113+
{ shell: true, encoding: 'utf8' },
114+
].forEach((options) => {
115+
const execFileSyncStdout = execFileSync(file, args, options);
116+
assert.strictEqual(execFileSyncStdout, `foo bar${os.EOL}`);
117+
118+
execFile(file, args, options, common.mustCall((_, stdout) => {
119+
assert.strictEqual(stdout, execFileSyncStdout);
120+
}));
121+
});
122+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL