From 06f21c67150e6195d453af860408e09597d527d4 Mon Sep 17 00:00:00 2001 From: ehmicky Date: Wed, 8 May 2019 03:42:32 -0700 Subject: [PATCH] Remove `execa.shell()` and `execa.shellSync()` (#219) --- index.d.ts | 89 +++---------------------------------------------- index.js | 8 ----- index.test-d.ts | 17 ---------- readme.md | 60 ++++++++++++--------------------- test.js | 41 +++++++---------------- 5 files changed, 39 insertions(+), 176 deletions(-) diff --git a/index.d.ts b/index.d.ts index da9dc5af7d..59a9e45e17 100644 --- a/index.d.ts +++ b/index.d.ts @@ -66,6 +66,11 @@ declare namespace execa { /** If `true`, runs `command` inside of a shell. Uses `/bin/sh` on UNIX and `cmd.exe` on Windows. A different shell can be specified as a string. The shell should understand the `-c` switch on UNIX or `/d /s /c` on Windows. + We recommend against using this option since it is: + - not cross-platform, encouraging shell-specific syntax. + - slower, because of the additional shell interpretation. + - unsafe, potentially allowing command injection. + @default false */ readonly shell?: boolean | string; @@ -388,51 +393,6 @@ declare const execa: { stderr(file: string, options?: execa.Options): Promise; stderr(file: string, options?: execa.Options): Promise; - /** - Execute a command through the system shell. - - Prefer `execa()` whenever possible, as it's both faster and safer. - - @param command - The command to execute. - @returns A [`child_process` instance](https://nodejs.org/api/child_process.html#child_process_class_childprocess). - - @example - ``` - import execa from 'execa'; - - (async => { - // Run a shell command - const {stdout} = await execa.shell('echo unicorns'); - //=> 'unicorns' - - // Catching an error - try { - await execa.shell('exit 3'); - } catch (error) { - console.log(error); - //{ - // message: 'Command failed with exit code 3 (ESRCH): exit 3', - // code: 3, - // exitCode: 3, - // exitCodeName: 'ESRCH', - // stdout: '', - // stderr: '', - // all: '', - // failed: true, - // command: 'exit 3', - // timedOut: false, - // killed: false - //} - } - })(); - ``` - */ - shell(command: string, options?: execa.Options): execa.ExecaChildProcess; - shell( - command: string, - options?: execa.Options - ): execa.ExecaChildProcess; - /** Execute a file synchronously. @@ -457,45 +417,6 @@ declare const execa: { file: string, options?: execa.SyncOptions ): execa.ExecaSyncReturnValue; - - /** - Execute a command synchronously through the system shell. - - This method throws an `Error` if the command fails. - - @param command - The command to execute. - @returns The same result object as [`child_process.spawnSync`](https://nodejs.org/api/child_process.html#child_process_child_process_spawnsync_command_args_options). - - @example - ``` - import execa from 'execa'; - - try { - execa.shellSync('exit 3'); - } catch (error) { - console.log(error); - //{ - // message: 'Command failed with exit code 3 (ESRCH): exit 3', - // code: 3, - // exitCode: 3, - // exitCodeName: 'ESRCH', - // stdout: '', - // stderr: '', - // failed: true, - // command: 'exit 3', - // timedOut: false - //} - } - ``` - */ - shellSync( - command: string, - options?: execa.Options - ): execa.ExecaSyncReturnValue; - shellSync( - command: string, - options?: execa.Options - ): execa.ExecaSyncReturnValue; }; export = execa; diff --git a/index.js b/index.js index 8088f76769..72159c41eb 100644 --- a/index.js +++ b/index.js @@ -127,10 +127,6 @@ function handleOutput(options, value) { return value; } -function handleShell(fn, command, options) { - return fn(command, {...options, shell: true}); -} - function makeAllStream(spawned) { if (!spawned.stdout && !spawned.stderr) { return; @@ -446,8 +442,6 @@ module.exports.stderr = async (...args) => { return stderr; }; -module.exports.shell = (command, options) => handleShell(execa, command, options); - module.exports.sync = (command, args, options) => { const parsed = handleArgs(command, args, options); const joinedCommand = joinCommand(command, args); @@ -483,5 +477,3 @@ module.exports.sync = (command, args, options) => { timedOut: false }; }; - -module.exports.shellSync = (command, options) => handleShell(execa.sync, command, options); diff --git a/index.test-d.ts b/index.test-d.ts index e89d9661d7..fbddbf5885 100644 --- a/index.test-d.ts +++ b/index.test-d.ts @@ -128,15 +128,6 @@ expectType(await execa.stderr('unicorns', {encoding: null})); expectType(await execa.stderr('unicorns', ['foo'], {encoding: 'utf8'})); expectType(await execa.stderr('unicorns', ['foo'], {encoding: null})); -expectType>(execa.shell('unicorns')); -expectType>(await execa.shell('unicorns')); -expectType>( - await execa.shell('unicorns', {encoding: 'utf8'}) -); -expectType>( - await execa.shell('unicorns', {encoding: null}) -); - expectType>(execa.sync('unicorns')); expectType>( execa.sync('unicorns', {encoding: 'utf8'}) @@ -150,11 +141,3 @@ expectType>( expectType>( execa.sync('unicorns', ['foo'], {encoding: null}) ); - -expectType>(execa.shellSync('unicorns')); -expectType>( - execa.shellSync('unicorns', {encoding: 'utf8'}) -); -expectType>( - execa.shellSync('unicorns', {encoding: null}) -); diff --git a/readme.md b/readme.md index 971fa6d9ad..f63ebc4df8 100644 --- a/readme.md +++ b/readme.md @@ -52,28 +52,25 @@ const execa = require('execa'); execa('echo', ['unicorns']).stdout.pipe(process.stdout); - // Run a shell command - const {stdout} = await execa.shell('echo unicorns'); - //=> 'unicorns' - // Catching an error try { - await execa.shell('exit 3'); + await execa('wrong command'); } catch (error) { console.log(error); /* { - message: 'Command failed with exit code 3 (ESRCH): exit 3', - code: 3, - exitCode: 3, - exitCodeName: 'ESRCH', + message: 'spawn wrong command ENOENT', + errno: 'ENOENT', + code: 'ENOENT', + syscall: 'spawn wrong command', + path: 'wrong command', + killed: false, stdout: '', stderr: '', - all: '', failed: true, - command: 'exit 3', - timedOut: false, - killed: false + signal: null, + cmd: 'wrong command', + timedOut: false } */ } @@ -93,20 +90,16 @@ const execa = require('execa'); // Catching an error with a sync method try { - execa.shellSync('exit 3'); + execa.sync('wrong command'); } catch (error) { console.log(error); /* { - message: 'Command failed with exit code 3 (ESRCH): exit 3', - code: 3, - exitCode: 3, - exitCodeName: 'ESRCH', - stdout: '', - stderr: '', - failed: true, - command: 'exit 3', - timedOut: false + message: 'spawnSync wrong command ENOENT', + errno: 'ENOENT', + code: 'ENOENT', + syscall: 'spawnSync wrong command', + path: 'wrong command', } */ } @@ -128,7 +121,7 @@ Arguments should not be escaped nor quoted. Exception: inside `command`, spaces Think of this as a mix of `child_process.execFile` and `child_process.spawn`. -As opposed to [`execa.shell()`](#execashellcommand-options), no shell interpreter (Bash, `cmd.exe`, etc.) is used, so shell features such as variables substitution (`echo $PATH`) are not allowed. +Unless the [`shell`](#shell) option is used, no shell interpreter (Bash, `cmd.exe`, etc.) is used, so shell features such as variables substitution (`echo $PATH`) are not allowed. Returns a [`child_process` instance](https://nodejs.org/api/child_process.html#child_process_class_childprocess) which is enhanced to be a `Promise`. @@ -148,14 +141,6 @@ Same as `execa()`, but returns only `stdout`. Same as `execa()`, but returns only `stderr`. -### execa.shell(command, [options]) - -Execute a command through the system shell. Prefer `execa()` whenever possible, as it's faster, safer and more cross-platform. - -Returns a [`child_process` instance](https://nodejs.org/api/child_process.html#child_process_class_childprocess). - -The `child_process` instance is enhanced to also be promise for a result object with `stdout` and `stderr` properties. - ### execa.sync(file, [arguments], [options]) ### execa.sync(command, [options]) @@ -167,12 +152,6 @@ It does not have the `.all` property that `execa()` has because the [underlying This method throws an `Error` if the command fails. -### execa.shellSync(command, [options]) - -Execute a command synchronously through the system shell. - -Returns the same result object as [`child_process.spawnSync`](https://nodejs.org/api/child_process.html#child_process_child_process_spawnsync_command_args_options). - ### options Type: `object` @@ -236,6 +215,11 @@ Default: `false` If `true`, runs `command` inside of a shell. Uses `/bin/sh` on UNIX and `cmd.exe` on Windows. A different shell can be specified as a string. The shell should understand the `-c` switch on UNIX or `/d /s /c` on Windows. +We recommend against using this option since it is: +- not cross-platform, encouraging shell-specific syntax. +- slower, because of the additional shell interpretation. +- unsafe, potentially allowing command injection. + #### stripFinalNewline Type: `boolean`
diff --git a/test.js b/test.js index d4356d94c5..617d95d710 100644 --- a/test.js +++ b/test.js @@ -137,11 +137,6 @@ test('trim string arguments', async t => { t.is(stdout, 'foo\nbar'); }); -test('execa.shell()', async t => { - const {stdout} = await execa.shell('node fixtures/noop foo'); - t.is(stdout, 'foo'); -}); - test('execa.sync()', t => { const {stdout} = execa.sync('noop', ['foo']); t.is(stdout, 'foo'); @@ -165,23 +160,6 @@ test('skip throwing when using reject option in execa.sync()', t => { t.is(typeof error.stderr, 'string'); }); -test('execa.shellSync()', t => { - const {stdout} = execa.shellSync('node fixtures/noop foo'); - t.is(stdout, 'foo'); -}); - -test('execa.shellSync() includes stdout and stderr in errors for improved debugging', t => { - t.throws(() => { - execa.shellSync('node fixtures/error-message.js'); - }, {message: STDERR_STDOUT_REGEXP, code: 1}); -}); - -test('skip throwing when using reject option in execa.shellSync()', t => { - const error = execa.shellSync('node fixtures/error-message.js', {reject: false}); - t.is(typeof error.stdout, 'string'); - t.is(typeof error.stderr, 'string'); -}); - test('stripEof option (legacy)', async t => { const {stdout} = await execa('noop', ['foo'], {stripEof: false}); t.is(stdout, 'foo\n'); @@ -512,13 +490,6 @@ test('cleanup false - SIGTERM', spawnAndKill, 'SIGTERM', 'false', exitIfWindows) test('cleanup true - SIGKILL', spawnAndKill, 'SIGKILL', 'true', exitIfWindows); test('cleanup false - SIGKILL', spawnAndKill, 'SIGKILL', 'false', exitIfWindows); -test('execa.shell() supports the `shell` option', async t => { - const {stdout} = await execa.shell('node fixtures/noop foo', { - shell: process.platform === 'win32' ? 'cmd.exe' : '/bin/bash' - }); - t.is(stdout, 'foo'); -}); - if (process.platform !== 'win32') { test('write to fast-exit process', async t => { // Try-catch here is necessary, because this test is not 100% accurate @@ -559,6 +530,18 @@ test('do not extend environment with `extendEnv: false`', async t => { ]); }); +test('can use `options.shell: true`', async t => { + const {stdout} = await execa('node fixtures/noop foo', {shell: true}); + t.is(stdout, 'foo'); +}); + +test('can use `options.shell: string`', async t => { + const {stdout} = await execa('node fixtures/noop foo', { + shell: process.platform === 'win32' ? 'cmd.exe' : '/bin/bash' + }); + t.is(stdout, 'foo'); +}); + test('use extend environment with `extendEnv: true` and `shell: true`', async t => { process.env.TEST = 'test'; const command = process.platform === 'win32' ? 'echo %TEST%' : 'echo $TEST';