1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

child_process: simplify spawn argument parsing

Browse Source 
This commit simplifies the object returned by
normalizeSpawnArguments(). This does impact monkey patching,
as illustrated by the changes in tests.

PR-URL: https://github.com/nodejs/node/pull/27854
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This commit is contained in:
cjihrig 2019-05-24 11:00:45 -04:00
parent 476c3ec750
commit 4f9cd2770a
No known key found for this signature in database
GPG Key ID: 7434390BDBE9B9C5
5 changed files with 38 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
lib/child_process.js
View File

@ -460,16 +460,14 @@ function normalizeSpawnArguments(file, args, options) {
}
// Validate windowsVerbatimArguments, if present.
if (options.windowsVerbatimArguments != null &&
typeof options.windowsVerbatimArguments !== 'boolean') {
let { windowsVerbatimArguments } = options;
if (windowsVerbatimArguments != null &&
typeof windowsVerbatimArguments !== 'boolean') {
throw new ERR_INVALID_ARG_TYPE('options.windowsVerbatimArguments',
'boolean',
options.windowsVerbatimArguments);
windowsVerbatimArguments);
}
// Make a shallow copy so we don't clobber the user's options object.
options = { ...options };
if (options.shell) {
const command = [file].concat(args).join(' ');
// Set the shell, switches, and commands.
@ -481,7 +479,7 @@ function normalizeSpawnArguments(file, args, options) {
// '/d /s /c' is used only for cmd.exe.
if (/^(?:.*\\)?cmd(?:\.exe)?$/i.test(file)) {
args = ['/d', '/s', '/c', `"${command}"`];
options.windowsVerbatimArguments = true;
windowsVerbatimArguments = true;
} else {
args = ['-c', command];
}
@ -521,47 +519,35 @@ function normalizeSpawnArguments(file, args, options) {
}
return {
file: file,
args: args,
options: options,
envPairs: envPairs
// Make a shallow copy so we don't clobber the user's options object.
...options,
args,
detached: !!options.detached,
envPairs,
file,
windowsHide: !!options.windowsHide,
windowsVerbatimArguments: !!windowsVerbatimArguments
};
}
var spawn = exports.spawn = function spawn(file, args, options) {
const opts = normalizeSpawnArguments(file, args, options);
const child = new ChildProcess();
options = opts.options;
debug('spawn', opts.args, options);
child.spawn({
file: opts.file,
args: opts.args,
cwd: options.cwd,
windowsHide: !!options.windowsHide,
windowsVerbatimArguments: !!options.windowsVerbatimArguments,
detached: !!options.detached,
envPairs: opts.envPairs,
stdio: options.stdio,
uid: options.uid,
gid: options.gid
});
options = normalizeSpawnArguments(file, args, options);
debug('spawn', options);
child.spawn(options);
return child;
};
function spawnSync(file, args, options) {
const opts = normalizeSpawnArguments(file, args, options);
const defaults = {
options = {
maxBuffer: MAX_BUFFER,
...opts.options
...normalizeSpawnArguments(file, args, options)
};
options = opts.options = defaults;
debug('spawnSync', opts.args, options);
debug('spawnSync', options);
// Validate the timeout, if present.
validateTimeout(options.timeout);
@ -569,10 +555,6 @@ function spawnSync(file, args, options) {
// Validate maxBuffer, if present.
validateMaxBuffer(options.maxBuffer);
options.file = opts.file;
options.args = opts.args;
options.envPairs = opts.envPairs;
// Validate and translate the kill signal, if present.
options.killSignal = sanitizeKillSignal(options.killSignal);
@ -603,7 +585,7 @@ function spawnSync(file, args, options) {
}
}
return child_process.spawnSync(opts);
return child_process.spawnSync(options);
}
exports.spawnSync = spawnSync;
@ -628,15 +610,15 @@ function checkExecSyncError(ret, args, cmd) {
function execFileSync(command, args, options) {
const opts = normalizeSpawnArguments(command, args, options);
const inheritStderr = !opts.options.stdio;
options = normalizeSpawnArguments(command, args, options);
const ret = spawnSync(opts.file, opts.args.slice(1), opts.options);
const inheritStderr = !options.stdio;
const ret = spawnSync(options.file, options.args.slice(1), options);
if (inheritStderr && ret.stderr)
process.stderr.write(ret.stderr);
const err = checkExecSyncError(ret, opts.args, undefined);
const err = checkExecSyncError(ret, options.args, undefined);
if (err)
throw err;

9
lib/internal/child_process.js
View File

@ -1022,8 +1022,7 @@ function maybeClose(subprocess) {
}
}
function spawnSync(opts) {
const options = opts.options;
function spawnSync(options) {
const result = spawn_sync.spawn(options);
if (result.output && options.encoding && options.encoding !== 'buffer') {
@ -1038,9 +1037,9 @@ function spawnSync(opts) {
result.stderr = result.output && result.output[2];
if (result.error) {
result.error = errnoException(result.error, 'spawnSync ' + opts.file);
result.error.path = opts.file;
result.error.spawnargs = opts.args.slice(1);
result.error = errnoException(result.error, 'spawnSync ' + options.file);
result.error.path = options.file;
result.error.spawnargs = options.args.slice(1);
}
return result;

6
test/parallel/test-child-process-spawnsync-kill-signal.js
View File

@ -36,7 +36,7 @@ if (process.argv[2] === 'child') {
// Verify that the default kill signal is SIGTERM.
{
const child = spawn(undefined, (opts) => {
assert.strictEqual(opts.options.killSignal, undefined);
assert.strictEqual(opts.killSignal, undefined);
});
assert.strictEqual(child.signal, 'SIGTERM');
@ -45,7 +45,7 @@ if (process.argv[2] === 'child') {
// Verify that a string signal name is handled properly.
{
const child = spawn('SIGKILL', (opts) => {
assert.strictEqual(opts.options.killSignal, SIGKILL);
assert.strictEqual(opts.killSignal, SIGKILL);
});
assert.strictEqual(child.signal, 'SIGKILL');
@ -56,7 +56,7 @@ if (process.argv[2] === 'child') {
assert.strictEqual(typeof SIGKILL, 'number');
const child = spawn(SIGKILL, (opts) => {
assert.strictEqual(opts.options.killSignal, SIGKILL);
assert.strictEqual(opts.killSignal, SIGKILL);
});
assert.strictEqual(child.signal, 'SIGKILL');

12
test/parallel/test-child-process-spawnsync-shell.js
View File

@ -64,12 +64,12 @@ assert.strictEqual(env.stdout.toString().trim(), 'buzz');
assert.strictEqual(opts.file, shellOutput);
assert.deepStrictEqual(opts.args,
[shellOutput, ...shellFlags, outputCmd]);
assert.strictEqual(opts.options.shell, shell);
assert.strictEqual(opts.options.file, opts.file);
assert.deepStrictEqual(opts.options.args, opts.args);
assert.strictEqual(opts.options.windowsHide, undefined);
assert.strictEqual(opts.options.windowsVerbatimArguments,
windowsVerbatim);
assert.strictEqual(opts.shell, shell);
assert.strictEqual(opts.file, opts.file);
assert.deepStrictEqual(opts.args, opts.args);
assert.strictEqual(opts.windowsHide, false);
assert.strictEqual(opts.windowsVerbatimArguments,
!!windowsVerbatim);
});
cp.spawnSync(cmd, { shell });
internalCp.spawnSync = oldSpawnSync;

2
test/parallel/test-child-process-windows-hide.js
View File

@ -19,7 +19,7 @@ internalCp.ChildProcess.prototype.spawn = common.mustCall(function(options) {
}, 2);
internalCp.spawnSync = common.mustCall(function(options) {
assert.strictEqual(options.options.windowsHide, true);
assert.strictEqual(options.windowsHide, true);
return originalSpawnSync.apply(this, arguments);
});