From ee68ad88d97aaf500ead4b2f770207c3edabab3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Fri, 6 Dec 2019 17:53:40 -0500 Subject: [PATCH] process/task/terminal: refactor escaping/quoting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add utility functions in `@theia/process/lib/common` to escape common shells' arguments. Refactored terminal processes to not handle shell escaping anymore, it is the caller's responsability to provide the escaped spawn options. Escaping is now done for DAP's `runInTerminal` requests. Changelog: - Moved quoting types and functions from `process/lib/node/termina-process.ts` to `process/lib/common/shell-quoting.ts`. - Added a `ShellCommandBuilder` component, used to build commands for evaluation in a shell (as if someone was typing manually). - `TerminalProcess` no longer supports running things in a shell as part of its options. Execution in a shell must be encoded in the spawn options by the caller. You can use `createShellCommandLine` to process arguments. Signed-off-by: Paul Maréchal --- CHANGELOG.md | 8 +- package.json | 1 + .../src/browser/debug-session-contribution.ts | 7 +- packages/debug/src/browser/debug-session.tsx | 5 +- packages/process/package.json | 4 + .../src/common/process-common-module.ts | 22 + .../common/shell-command-builder.slow-spec.ts | 492 ++++++++++++++++++ .../src/common/shell-command-builder.ts | 157 ++++++ .../process/src/common/shell-quoting.spec.ts | 176 +++++++ packages/process/src/common/shell-quoting.ts | 232 +++++++++ .../src/common/tests/$weird(),file=name.js | 1 + .../process/src/common/tests/white space.js | 1 + packages/process/src/node/process.ts | 4 +- .../process/src/node/terminal-process.spec.ts | 146 +++--- packages/process/src/node/terminal-process.ts | 167 +----- .../src/node/process/process-task-runner.ts | 189 +++++-- .../task/src/node/task-server.slow-spec.ts | 178 ++++--- .../task/src/node/test/task-test-container.ts | 15 + packages/task/test-resources/compare.js | 14 + .../task/test-resources/test-arguments-0.js | 13 + .../task/test-resources/test-arguments-1.js | 13 + .../task/test-resources/test-arguments-2.js | 13 + .../task/test-resources/test-arguments-3.js | 13 + .../src/browser/base/terminal-widget.ts | 15 + .../src/browser/terminal-widget-impl.ts | 15 +- .../src/common/base-terminal-protocol.ts | 6 + .../terminal/src/node/base-terminal-server.ts | 13 +- scripts/prepare-travis.js | 1 + yarn.lock | 2 +- 29 files changed, 1568 insertions(+), 355 deletions(-) create mode 100644 packages/process/src/common/process-common-module.ts create mode 100644 packages/process/src/common/shell-command-builder.slow-spec.ts create mode 100644 packages/process/src/common/shell-command-builder.ts create mode 100644 packages/process/src/common/shell-quoting.spec.ts create mode 100644 packages/process/src/common/shell-quoting.ts create mode 100644 packages/process/src/common/tests/$weird(),file=name.js create mode 100644 packages/process/src/common/tests/white space.js create mode 100644 packages/task/test-resources/compare.js create mode 100644 packages/task/test-resources/test-arguments-0.js create mode 100644 packages/task/test-resources/test-arguments-1.js create mode 100644 packages/task/test-resources/test-arguments-2.js create mode 100644 packages/task/test-resources/test-arguments-3.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a121be14d657..9a8f34dade849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,13 +3,15 @@ ## v0.17.0 - [preferences] add a new preference to silence notifications [#7195](https://github.com/eclipse-theia/theia/pull/7195) -- [core] fixed keybindings for special Numpad keys in editors [#7315] +- [core] fixed keybindings for special Numpad keys in editors [#7315] Breaking changes: - [scm][git] the History view (GitHistoryWidget) has moved from the git package to a new package, scm-extra, and renamed to ScmHistoryWidget. GitNavigableListWidget has also moved. CSS classes have been moved renamed accordingly. [6381](https://github.com/eclipse-theia/theia/pull/6381) +- [process] `TerminalProcess` doesn't handle shell quoting, the shell process arguments must be prepared from the caller. Removed all methods related to shell escaping inside this class. You should use functions located in `@theia/process/lib/common/shell-quoting.ts` in order to process arguments for shells. +- [process/terminal] Moved shell escaping utilities into `@theia/process/lib/common/shell-quoting` and `@theia/process/lib/common/shell-command-builder` for creating shell inputs. ## v0.16.0 @@ -231,6 +233,7 @@ Breaking changes: - One can resolve a current color value programmatically with `ColorRegistry.getCurrentColor`. - One can load a new color theme: - in the frontend module to enable it on startup + ```ts MonacoThemingService.register({ id: 'myDarkTheme', @@ -242,7 +245,9 @@ Breaking changes: } }); ``` + - later from a file: + ```ts @inject(MonacoThemingService) protected readonly monacoThemeService: MonacoThemingService; @@ -254,6 +259,7 @@ Breaking changes: uri: 'file:///absolute/path/to/my_theme.json' }); ``` + - or install from a VS Code extension. - One should not introduce css color variables anymore or hardcode colors in css. - One can contribute new colors by implementing `ColorContribution` contribution point and calling `ColorRegistry.register`. diff --git a/package.json b/package.json index 6877c93424489..717425862860a 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "@typescript-eslint/eslint-plugin-tslint": "^2.16.0", "@typescript-eslint/parser": "^2.16.0", "chai-string": "^1.4.0", + "colors": "^1.4.0", "concurrently": "^3.5.0", "electron-mocha": "^8.2.0", "eslint": "^6.8.0", diff --git a/packages/debug/src/browser/debug-session-contribution.ts b/packages/debug/src/browser/debug-session-contribution.ts index 80629676978d0..5a756456d0c44 100644 --- a/packages/debug/src/browser/debug-session-contribution.ts +++ b/packages/debug/src/browser/debug-session-contribution.ts @@ -123,8 +123,8 @@ export class DefaultDebugSessionFactory implements DebugSessionFactory { resolve(channel); }, { reconnecting: false }) ), - this.getTraceOutputChannel()); - + this.getTraceOutputChannel() + ); return new DebugSession( sessionId, options, @@ -134,7 +134,8 @@ export class DefaultDebugSessionFactory implements DebugSessionFactory { this.breakpoints, this.labelProvider, this.messages, - this.fileSystem); + this.fileSystem, + ); } protected getTraceOutputChannel(): OutputChannel | undefined { diff --git a/packages/debug/src/browser/debug-session.tsx b/packages/debug/src/browser/debug-session.tsx index d04990c67bbce..695be24359c31 100644 --- a/packages/debug/src/browser/debug-session.tsx +++ b/packages/debug/src/browser/debug-session.tsx @@ -385,8 +385,9 @@ export class DebugSession implements CompositeTreeElement { protected async runInTerminal({ arguments: { title, cwd, args, env } }: DebugProtocol.RunInTerminalRequest): Promise { const terminal = await this.doCreateTerminal({ title, cwd, env, useServerTitle: false }); - terminal.sendText(args.join(' ') + '\n'); - return { processId: await terminal.processId }; + const { processId } = terminal; + await terminal.executeCommand({ cwd, args, env }); + return { processId: await processId }; } protected async doCreateTerminal(options: TerminalWidgetOptions): Promise { diff --git a/packages/process/package.json b/packages/process/package.json index c420f08dd509f..03c49dbae2730 100644 --- a/packages/process/package.json +++ b/packages/process/package.json @@ -11,6 +11,10 @@ "access": "public" }, "theiaExtensions": [ + { + "backend": "lib/common/process-common-module", + "frontend": "lib/common/process-common-module" + }, { "backend": "lib/node/process-backend-module" } diff --git a/packages/process/src/common/process-common-module.ts b/packages/process/src/common/process-common-module.ts new file mode 100644 index 0000000000000..3a93751c95277 --- /dev/null +++ b/packages/process/src/common/process-common-module.ts @@ -0,0 +1,22 @@ +/******************************************************************************** + * Copyright (C) 2020 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +import { ContainerModule } from 'inversify'; +import { ShellCommandBuilder } from './shell-command-builder'; + +export default new ContainerModule((bind, unbind, isBound, rebind) => { + bind(ShellCommandBuilder).toSelf().inSingletonScope(); +}); diff --git a/packages/process/src/common/shell-command-builder.slow-spec.ts b/packages/process/src/common/shell-command-builder.slow-spec.ts new file mode 100644 index 0000000000000..943dbaff8aaeb --- /dev/null +++ b/packages/process/src/common/shell-command-builder.slow-spec.ts @@ -0,0 +1,492 @@ +/******************************************************************************** + * Copyright (C) 2020 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +/** + * This test suite assumes that we run in a NodeJS environment! + */ + +import { spawn, execSync, SpawnOptions, ChildProcess, spawnSync } from 'child_process'; +import { Readable } from 'stream'; +import { join } from 'path'; + +import { ShellCommandBuilder, CommandLineOptions, ProcessInfo } from './shell-command-builder'; + +import { + bgRed, bgWhite, bgYellow, + black, green, magenta, red, white, yellow, + bold, +} from 'colors/safe'; // tslint:disable-line:no-implicit-dependencies + +export interface TestProcessInfo extends ProcessInfo { + shell: ChildProcess +} + +const isWindows = process.platform === 'win32'; +/** + * Extra debugging info (very verbose). + */ +const _debug: boolean = Boolean(process.env['THEIA_PROCESS_TEST_DEBUG']); +/** + * On Windows, some shells simply mess up the terminal's output. + * Enable if you still want to test those. + */ +const _runWeirdShell: true | undefined = Boolean(process.env['THEIA_PROCESS_TEST_WEIRD_SHELL']) || undefined; +/** + * You might only have issues with a specific shell (`cmd.exe` I am looking at you). + */ +const _onlyTestShell: string | undefined = process.env['THEIA_PROCESS_TEST_ONLY'] || undefined; +/** + * Only log if environment variable is set. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function debug(...parts: any[]): void { + if (_debug) { + console.debug(...parts); + } +} + +const testResources = join(__dirname, '../../src/common/tests'); +const spawnOptions: SpawnOptions = { + // We do our own quoting, don't rely on the one done by NodeJS: + windowsVerbatimArguments: true, + stdio: ['pipe', 'pipe', 'pipe'], +}; + +// Formatting options, used with `scanLines` for debugging. +const stdoutFormat = (prefix: string) => (data: string) => + `${bold(yellow(`${prefix} STDOUT:`))} ${bgYellow(black(data))}`; +const stderrFormat = (prefix: string) => (data: string) => + `${bold(red(`${prefix} STDERR:`))} ${bgRed(white(data))}`; + +// Default error scanner +const errorScanner = (handle: ScanLineHandle) => { + if ( + /^\s*\w+Error:/.test(handle.line) || + /^\s*Cannot find /.test(handle.line) + ) { + throw new Error(handle.text); + } +}; + +// Yarn mangles the PATH and creates some proxy script around node(.exe), +// which messes up our environment, failing the tests. +const hostNodePath = + process.env['npm_node_execpath'] || + process.env['NODE']; +if (!hostNodePath) { + throw new Error('Could not determine the real node path.'); +} + +const shellCommandBuilder = new ShellCommandBuilder(); +const shellConfigs = [{ + name: 'bash', + path: isWindows + ? _runWeirdShell && execShellCommand('where bash.exe') + : execShellCommand('command -v bash'), + nodePath: + isWindows && 'node' // Good enough +}, { + name: 'wsl', + path: isWindows + ? _runWeirdShell && execShellCommand('where wsl.exe') + : undefined, + nodePath: + isWindows && 'node' // Good enough +}, { + name: 'cmd', + path: isWindows + ? execShellCommand('where cmd.exe') + : undefined, +}, { + name: 'powershell', + path: execShellCommand(isWindows + ? 'where powershell' + : 'command -v pwsh'), +}]; + +/* eslint-disable max-len */ + +// 18d/12m/19y - Ubuntu 16.04: +// Powershell sometimes fails when running as part of an npm lifecycle script. +// See following error: +// +// +// FailFast: +// The type initializer for 'Microsoft.PowerShell.ApplicationInsightsTelemetry' threw an exception. +// +// at System.Environment.FailFast(System.String, System.Exception) +// at System.Environment.FailFast(System.String, System.Exception) +// at Microsoft.PowerShell.UnmanagedPSEntry.Start(System.String, System.String[], Int32) +// at Microsoft.PowerShell.ManagedPSEntry.Main(System.String[]) +// +// Exception details: +// System.TypeInitializationException: The type initializer for 'Microsoft.PowerShell.ApplicationInsightsTelemetry' threw an exception. ---> System.ArgumentException: Item has already been added. Key in dictionary: 'SPAWN_WRAP_SHIM_ROOT' Key being added: 'SPAWN_WRAP_SHIM_ROOT' +// at System.Collections.Hashtable.Insert(Object key, Object nvalue, Boolean add) +// at System.Environment.ToHashtable(IEnumerable`1 pairs) +// at System.Environment.GetEnvironmentVariables() +// at Microsoft.ApplicationInsights.Extensibility.Implementation.Platform.PlatformImplementation..ctor() +// at Microsoft.ApplicationInsights.Extensibility.Implementation.Platform.PlatformSingleton.get_Current() +// at Microsoft.ApplicationInsights.Extensibility.Implementation.TelemetryConfigurationFactory.Initialize(TelemetryConfiguration configuration, TelemetryModules modules) +// at Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration.get_Active() +// at Microsoft.PowerShell.ApplicationInsightsTelemetry..cctor() +// --- End of inner exception stack trace --- +// at Microsoft.PowerShell.ApplicationInsightsTelemetry.SendPSCoreStartupTelemetry() +// at Microsoft.PowerShell.ConsoleHost.Start(String bannerText, String helpText, String[] args) +// at Microsoft.PowerShell.ConsoleShell.Start(String bannerText, String helpText, String[] args) +// at Microsoft.PowerShell.UnmanagedPSEntry.Start(String consoleFilePath, String[] args, Int32 argc) + +/* eslint-enable max-len */ + +let id = 0; +for (const shellConfig of shellConfigs) { + + let skipMessage: string | undefined; + + if (typeof _onlyTestShell === 'string' && shellConfig.name !== _onlyTestShell) { + skipMessage = `only testing ${_onlyTestShell}`; + + } else if (!shellConfig.path) { + // For each shell, skip if we could not find the executable path. + skipMessage = 'cannot find shell'; + + } else { + // Run a test in the shell to catch runtime issues. + // CI seems to have issues with some shells depending on the environment... + try { + const debugName = `${shellConfig.name}/test`; + const shellTest = spawnSync(shellConfig.path, { + input: 'echo abcdefghijkl\n\n', + timeout: 5_000, + }); + debug(stdoutFormat(debugName)(shellTest.stdout.toString())); + debug(stderrFormat(debugName)(shellTest.stderr.toString())); + if (!/abcdefghijkl/m.test(shellTest.output.toString())) { + skipMessage = 'wrong test output'; + } + } catch (error) { + console.error(error); + skipMessage = 'error occured'; + } + } + + /** + * If skipMessage is set, we should skip the test and explain why. + */ + const describeOrSkip = (callback: (this: Mocha.Suite) => void) => { + const describeMessage = `test ${shellConfig.name} commands`; + if (typeof skipMessage === 'undefined') { + describe(describeMessage, callback); + } else { + describe.skip(`${describeMessage} - skip: ${skipMessage}`, callback); + } + }; + + describeOrSkip(function (): void { + this.timeout(10_000); + + let nodePath: string; + let cwd: string; + let submit: string | undefined; + let processInfo: TestProcessInfo; + let context: TestCaseContext; + + beforeEach(() => { + // In WSL, the node path is different than the host one (Windows vs Linux). + nodePath = shellConfig.nodePath || hostNodePath; + + // On windows, when running bash we need to convert paths from Windows + // to their mounting point, assuming bash is running within WSL. + if (isWindows && /bash|wsl/.test(shellConfig.name)) { + cwd = convertWindowsPath(testResources); + } else { + cwd = testResources; + } + + // When running powershell, it seems like good measure to send `\n` twice... + if (shellConfig.name === 'powershell') { + submit = '\n\n'; + } + + // TestContext holds all state for a given test. + const testContextName = `${shellConfig.name}/${++id}`; + context = new TestCaseContext(testContextName, submit); + processInfo = createShell(context, shellConfig.path!); + }); + + afterEach(() => { + processInfo.shell.kill(); + context.finalize(); + }); + + it('use simple environment variables', async () => { + const envName = 'SIMPLE_NAME'; + const envValue = 'SIMPLE_VALUE'; + await testCommandLine( + context, processInfo, + { + cwd, args: [nodePath, '-p', `\`[\${process.env['${envName}']}]\``], + env: { + [envName]: envValue, + } + }, [ + // stderr + scanLines(context, processInfo.shell.stderr, errorScanner, stderrFormat(context.name)), + // stdout + scanLines(context, processInfo.shell.stdout, handle => { + errorScanner(handle); + if (handle.line.includes(`[${envValue}]`)) { + handle.resolve(); + } + }, stdoutFormat(context.name)), + ]); + }); + + it('use problematic environment variables', async () => { + const envName = 'A?B_C | D $PATH'; + const envValue = 'SUCCESS'; + await testCommandLine( + context, processInfo, + { + cwd, args: [nodePath, '-p', `\`[\${process.env['${envName}']}]\``], + env: { + [envName]: envValue, + } + }, [ + // stderr + scanLines(context, processInfo.shell.stderr, errorScanner, stderrFormat(context.name)), + // stdout + scanLines(context, processInfo.shell.stdout, handle => { + errorScanner(handle); + if (handle.line.includes(`[${envValue}]`)) { + handle.resolve(); + } + if (handle.line.includes('[undefined]')) { + handle.reject(new Error(handle.text)); + } + }, stdoutFormat(context.name)), + ]); + }); + + it('command with complex arguments', async () => { + const left = 'ABC'; + const right = 'DEF'; + await testCommandLine( + context, processInfo, + { + cwd, args: [nodePath, '-e', `{ + const left = '${left}'; + const right = '${right}'; + console.log(\`[\${left}|\${right}]\`); + }`], + }, [ + // stderr + scanLines(context, processInfo.shell.stderr, errorScanner, stderrFormat(context.name)), + // stdout + scanLines(context, processInfo.shell.stdout, handle => { + errorScanner(handle); + if (handle.line.includes(`[${left}|${right}]`)) { + handle.resolve(); + } + }, stdoutFormat(context.name)), + ]); + }); + + }); + +} + +/** + * Allow `command` to fail and return undefined instead. + */ +function execShellCommand(command: string): string | undefined { + try { + // If trimmed output is an empty string, return `undefined` instead: + return execSync(command).toString().trim() || undefined; + } catch (error) { + console.error(command, error); + return undefined; + } +} + +/** + * When executing `bash.exe` on Windows, the `C:`, `D:`, etc drives are mounted under `/mnt//...` + */ +function convertWindowsPath(windowsPath: string): string { + return windowsPath + // Convert back-slashes to forward-slashes + .replace(/\\/g, '/') + // Convert drive-letter to usual mounting point in WSL + .replace(/^[A-Za-z]:\//, s => `/mnt/${s[0].toLowerCase()}/`); +} + +/** + * Display trailing whitespace in a string, such as \r and \n. + */ +function displayWhitespaces(line: string): string { + return line + .replace(/\r?\n/, s => s.length === 2 ? '<\\r\\n>\r\n' : '<\\n>\n'); +} + +/** + * Actually run `prepareCommandLine`. + */ +async function testCommandLine( + context: TestCaseContext, + processInfo: TestProcessInfo, + options: CommandLineOptions, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + firstOf: Array>, + // eslint-disable-next-line @typescript-eslint/no-explicit-any +): Promise { + const commandLine = shellCommandBuilder.buildCommand(processInfo, options); + debug(`${bold(white(`${context.name} STDIN:`))} ${bgWhite(black(displayWhitespaces(commandLine)))}`); + processInfo.shell.stdin.write(commandLine + context.submit); + return Promise.race(firstOf); +} + +/** + * Creates a `(Test)ProcessInfo` object by spawning the specified shell. + */ +function createShell( + context: TestCaseContext, + shellExecutable: string, + shellArguments: string[] = [] +): TestProcessInfo { + const shell = spawn(shellExecutable, shellArguments, spawnOptions); + debug(magenta(`${bold(`${context.name} SPAWN:`)} ${shellExecutable} ${shellArguments.join(' ')}`)); + shell.on('close', (code, signal) => debug(magenta( + `${bold(`${context.name} CLOSE:`)} ${shellExecutable} code(${code}) signal(${signal})` + ))); + return { + executable: shellExecutable, + arguments: [], + shell, + }; +} + +/** + * Fire `callback` once per new detected line. + */ +async function scanLines( + context: TestCaseContext, + stream: Readable, + callback: (handle: ScanLineHandle) => void, + debugFormat = (s: string) => s, +): Promise { + return new Promise((resolve, reject) => { + let line = ''; + let text = ''; + stream.on('close', () => { + debug(debugFormat('')); + }); + // The `data` listener will be collected on 'close', which will happen + // once we kill the process. + stream.on('data', data => { + if (context.resolved) { + return; + } + const split = data.toString().split('\n'); + while (!context.resolved && split.length > 1) { + line += split.shift()! + '\n'; + text += line; + debug(debugFormat(displayWhitespaces(line))); + try { + callback({ + resolve: (value: T) => { + if (!context.resolved) { + context.resolve(); + resolve(value); + debug(bold(green(`${context.name} SCANLINES RESOLVED`))); + } + }, + reject: (reason?: Error) => { + if (!context.resolved) { + context.resolve(); + reject(reason); + debug(bold(red(`${context.name} SCANLINES REJECTED`))); + } + }, + line, + text, + }); + } catch (error) { + debug(bold(red(`${context.name} SCANLINES THROWED`))); + context.resolve(); + reject(error); + break; + } + line = ''; + } + line += split[0]; + }); + }); + +} +interface ScanLineHandle { + + /** + * Finish listening to new events with a return value. + */ + resolve: (value: T) => void + /** + * Finish listening to new events with an error. + */ + reject: (reason?: Error) => void + /** + * Currently parsed line. + */ + line: string + /** + * The whole output buffer, containing all lines. + */ + text: string + +} +/** + * We need a test case context to help with catching listeners that timed-out, + * and synchronize multiple listeners so that when one resolves the test case, + * the others can be put in "sleep mode" until destruction. + */ +class TestCaseContext { + + constructor( + /** + * A name associated with this context, to help with debugging. + */ + readonly name: string, + /** + * The characters to send in order to submit a command (mostly + * powershell is causing issues). + */ + public submit = '\n', + /** + * @internal Current state of the test case, if it is finished or not. + */ + public resolved = false + ) { } + + resolve(): void { + this.resolved = true; + } + + finalize(): void { + if (!this.resolved) { + this.resolve(); + debug(red(`${bold(`${this.name} CONTEXT:`)} context wasn't resolved when finalizing, resolving!`)); + } + } + +} diff --git a/packages/process/src/common/shell-command-builder.ts b/packages/process/src/common/shell-command-builder.ts new file mode 100644 index 0000000000000..d2e6fda881ec2 --- /dev/null +++ b/packages/process/src/common/shell-command-builder.ts @@ -0,0 +1,157 @@ +/******************************************************************************** + * Copyright (C) 2020 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { injectable } from 'inversify'; +import { createShellCommandLine, BashQuotingFunctions, PowershellQuotingFunctions, CmdQuotingFunctions, ShellQuoting, ShellQuotedString } from '../common/shell-quoting'; + +export interface ProcessInfo { + executable: string + arguments: string[] +} + +export interface CommandLineOptions { + cwd: string + args: string[] + env?: { + [key: string]: string | null + } +} + +/** + * Create command lines ready to be sent to a shell's stdin for evaluation. + */ +@injectable() +export class ShellCommandBuilder { + + /** + * Constructs a command line to run in a shell. The shell could be + * re-used/long-lived, this means we cannot spawn a new process with a nice + * and fresh environment, we need to encode environment modifications into + * the returned command. + * + * Inspired by VS Code implementation, see: + * https://github.com/microsoft/vscode/blob/f395cac4fff0721a8099126172c01411812bcb4a/src/vs/workbench/contrib/debug/node/terminals.ts#L79 + * + * @param hostProcessInfo the host terminal process infos + * @param commandOptions program to execute in the host terminal + */ + buildCommand(hostProcessInfo: ProcessInfo | undefined, commandOptions: CommandLineOptions): string { + + const host = hostProcessInfo && hostProcessInfo.executable; + const cwd = commandOptions.cwd; + + const args = commandOptions.args.map(value => ({ + value, quoting: ShellQuoting.Strong, + } as ShellQuotedString)); + + const env: Array<[string, string | null]> = []; + if (commandOptions.env) { + for (const key of Object.keys(commandOptions.env)) { + env.push([key, commandOptions.env[key]]); + } + } + if (host) { + if (/(bash|wsl)(.exe)?$/.test(host)) { + return this.buildForBash(args, cwd, env); + } else if (/(ps|pwsh|powershell)(.exe)?$/i.test(host)) { + return this.buildForPowershell(args, cwd, env); + } else if (/cmd(.exe)?$/i.test(host)) { + return this.buildForCmd(args, cwd, env); + } + } + // If we cannot detect which shell is being used, don't escape. + console.warn(`Unknown shell, could not escape arguments: ${host || 'undefined'}`); + return this.buildForDefault(args, cwd, env); + } + + protected buildForBash(args: Array, cwd?: string, env?: Array<[string, string | null]>): string { + let command = ''; + if (cwd) { + command += `cd ${BashQuotingFunctions.strong(cwd)} && `; + } + if (env) { + command += 'env'; + for (const [key, value] of env) { + // eslint-disable-next-line no-null/no-null + if (value === null) { + command += ` -u ${BashQuotingFunctions.strong(key)}`; + } else { + command += ` ${BashQuotingFunctions.strong(`${key}=${value}`)}`; + } + } + command += ' '; + } + command += createShellCommandLine(args, BashQuotingFunctions); + return command; + } + + protected buildForPowershell(args: Array, cwd?: string, env?: Array<[string, string | null]>): string { + let command = ''; + if (cwd) { + command += `cd ${PowershellQuotingFunctions.strong(cwd)}; `; + } + if (env) { + for (const [key, value] of env) { + // Powershell requires special quoting when dealing with + // environment variable names. + const quotedKey = key + .replace(/`/g, '````') + .replace(/\?/g, '``?'); + // eslint-disable-next-line no-null/no-null + if (value === null) { + command += `Remove-Item \${env:${quotedKey}}; `; + } else { + command += `\${env:${quotedKey}}=${PowershellQuotingFunctions.strong(value)}; `; + } + } + } + command += '& ' + createShellCommandLine(args, PowershellQuotingFunctions); + return command; + } + + protected buildForCmd(args: Array, cwd?: string, env?: Array<[string, string | null]>): string { + let command = ''; + if (cwd) { + command += `cd ${CmdQuotingFunctions.strong(cwd)} && `; + } + if (env) { + command += 'cmd /C "'; + for (const [key, value] of env) { + // eslint-disable-next-line no-null/no-null + if (value === null) { + command += `set ${CmdQuotingFunctions.strong(key)}="" && `; + } else { + command += `set ${CmdQuotingFunctions.strong(`${key}=${value}`)} && `; + } + } + } + command += createShellCommandLine(args, CmdQuotingFunctions); + if (env) { + command += '"'; + } + return command; + } + + protected buildForDefault(args: Array, cwd?: string, env?: Array<[string, string | null]>): string { + return args.join(' '); + } + +} diff --git a/packages/process/src/common/shell-quoting.spec.ts b/packages/process/src/common/shell-quoting.spec.ts new file mode 100644 index 0000000000000..0548a7d4c5d91 --- /dev/null +++ b/packages/process/src/common/shell-quoting.spec.ts @@ -0,0 +1,176 @@ +/******************************************************************************** + * Copyright (C) 2020 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +import { expect } from 'chai'; + +import { escapeForShell, BashQuotingFunctions, ShellQuoting, CmdQuotingFunctions, PowershellQuotingFunctions } from './shell-quoting'; + +describe('Shell arguments escaping:', () => { + + // Procedurally execute tests from this list of data. + const testData = { + bash: { + // https://www.gnu.org/software/bash/manual/html_node/Quoting.html + quotingFunctions: BashQuotingFunctions, + data: { + [ShellQuoting.Escape]: [ + { input: 'abc', expected: 'abc' }, + { input: 'ab c', expected: 'ab\\ c' }, + { input: 'ab"c', expected: 'ab\\"c' }, + { input: 'ab\'c', expected: 'ab\\\'c' }, + { input: 'ab\\ c\\', expected: 'ab\\\\\\ c\\\\' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: 'setTimeout\\(\\(\\)\\ =\\>\\ \\{\\ console.log\\(1,\\ \\"2\\\'3\\"\\)\\;\\ \\},\\ 100\\)', + }, + ], + [ShellQuoting.Strong]: [ + { input: 'abc', expected: '\'abc\'' }, + { input: 'ab c', expected: '\'ab c\'' }, + { input: 'ab"c', expected: '\'ab"c\'' }, + { input: 'ab\'c', expected: '\'ab\'"\'"\'c\'' }, + { input: 'ab\\ c\\', expected: '\'ab\\ c\\\'' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: '\'setTimeout(() => { console.log(1, "2\'"\'"\'3"); }, 100)\'', + }, + ], + [ShellQuoting.Weak]: [ + { input: 'abc', expected: '"abc"' }, + { input: 'ab c', expected: '"ab c"' }, + { input: 'ab"c', expected: '"ab\\"c"' }, + { input: 'ab\'c', expected: '"ab\'c"' }, + { input: 'ab\\ c\\', expected: '"ab\\ c\\\\"' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: '"setTimeout(() => { console.log(1, \\"2\'3\\"); }, 100)"', + }, + ] + }, + }, + cmd: { + // https://ss64.com/nt/syntax-esc.html + quotingFunctions: CmdQuotingFunctions, + data: { + [ShellQuoting.Escape]: [ + { input: 'abc', expected: 'abc' }, + { input: 'ab c', expected: 'ab" "c' }, + { input: 'ab"c', expected: 'ab^"c' }, + { input: 'ab\'c', expected: 'ab\'c' }, + { input: 'ab^ c^', expected: 'ab^^" "c^^' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: 'setTimeout^(^(^)" "=^>" "{" "console.log^(1," "^"2\'3^"^);" "}," "100^)', + }, + { + input: 'console.log("%PATH%")', + expected: 'console.log^(^"^%PATH^%^"^)', + }, + ], + [ShellQuoting.Strong]: [ + { input: 'abc', expected: '"abc"' }, + { input: 'ab c', expected: '"ab c"' }, + { input: 'ab"c', expected: '"ab\\"c"' }, + { input: 'ab\'c', expected: '"ab\'c"' }, + { input: 'ab^ c^', expected: '"ab^^ c^^"' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: '"setTimeout^(^(^) =^> { console.log^(1, \\"2\'3\\"^); }, 100^)"', + }, + { + input: 'console.log("%PATH%")', + expected: '"console.log^(\\""%"PATH"%"\\"^)"', + }, + ], + [ShellQuoting.Weak]: [ + { input: 'abc', expected: '"abc"' }, + { input: 'ab c', expected: '"ab c"' }, + { input: 'ab"c', expected: '"ab\\"c"' }, + { input: 'ab\'c', expected: '"ab\'c"' }, + { input: 'ab^ c^', expected: '"ab^^ c^^"' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: '"setTimeout^(^(^) =^> { console.log^(1, \\"2\'3\\"^); }, 100^)"', + }, + { + input: 'console.log("%PATH%")', + expected: '"console.log^(\\"%PATH%\\"^)"', + }, + ] + }, + }, + powershell: { + // https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules?view=powershell-6 + quotingFunctions: PowershellQuotingFunctions, + data: { + [ShellQuoting.Escape]: [ + { input: 'abc', expected: 'abc' }, + { input: 'ab c', expected: 'ab` c' }, + { input: 'ab"c', expected: 'ab`"c' }, + { input: 'ab\'c', expected: 'ab`\'c' }, + { input: 'ab` c`', expected: 'ab``` c``' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: 'setTimeout`(`(`)` =`>` `{` console.log`(1,` `"2`\'3`"`)`;` `},` 100`)', + }, + ], + [ShellQuoting.Strong]: [ + { input: 'abc', expected: '\'abc\'' }, + { input: 'ab c', expected: '\'ab c\'' }, + { input: 'ab"c', expected: '\'ab"c\'' }, + { input: 'ab\'c', expected: '\'ab\'\'c\'' }, + { input: 'ab` c`', expected: '\'ab` c`\'' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: '\'setTimeout(() => { console.log(1, "2\'\'3"); }, 100)\'', + }, + ], + [ShellQuoting.Weak]: [ + { input: 'abc', expected: '"abc"' }, + { input: 'ab c', expected: '"ab c"' }, + { input: 'ab"c', expected: '"ab`"c"' }, + { input: 'ab\'c', expected: '"ab\'c"' }, + { input: 'ab` c`', expected: '"ab` c``"' }, + { + input: 'setTimeout(() => { console.log(1, "2\'3"); }, 100)', + expected: '"setTimeout(() => { console.log(1, `"2\'3`"); }, 100)"', + }, + ] + }, + } + } as const; + + // Iter through each runtime (bash/cmd/powershell): + for (const runtime of Object.keys(testData)) { + const testInfo = testData[runtime as keyof typeof testData]; + + // Get all quoting types (escape/strong/weak): + for (const quotingType of Object.keys(testInfo.data)) { + const testInput = testInfo.data[quotingType as keyof typeof testInfo['data']]; + + // Run the test for each input: + it(`${runtime}/${quotingType}`, () => { + for (const test of testInput) { + expect(escapeForShell({ + quoting: quotingType as ShellQuoting, + value: test.input, + }, testInfo.quotingFunctions)).equal(test.expected); + } + }); + } + } + +}); diff --git a/packages/process/src/common/shell-quoting.ts b/packages/process/src/common/shell-quoting.ts new file mode 100644 index 0000000000000..b689fa25ac8a7 --- /dev/null +++ b/packages/process/src/common/shell-quoting.ts @@ -0,0 +1,232 @@ +/******************************************************************************** + * Copyright (C) 2020 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +// #region vscode + +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +// See: https://github.com/microsoft/vscode/blob/9ebb7c43bc99fd6e1a295040674d1f8e5831b9be/src/vs/vscode.d.ts#L5326-L5370 + +/** + * Defines how an argument should be quoted if it contains + * spaces or unsupported characters. + */ +export const enum ShellQuoting { + + /** + * Character escaping should be used. This for example + * uses \ on bash and ` on PowerShell. + */ + Escape = 'escape', + + /** + * Strong string quoting should be used. This for example + * uses " for Windows cmd and ' for bash and PowerShell. + * Strong quoting treats arguments as literal strings. + * Under PowerShell echo 'The value is $(2 * 3)' will + * print `The value is $(2 * 3)` + */ + Strong = 'strong', + + /** + * Weak string quoting should be used. This for example + * uses " for Windows cmd, bash and PowerShell. Weak quoting + * still performs some kind of evaluation inside the quoted + * string. Under PowerShell echo "The value is $(2 * 3)" + * will print `The value is 6` + */ + Weak = 'weak' +} + +/** + * A string that will be quoted depending on the used shell. + */ +export interface ShellQuotedString { + /** + * The actual string value. + */ + value: string; + + /** + * The quoting style to use. + */ + quoting: ShellQuoting; +} + +// #endregion vscode + +/** + * Functions that provide shell quoting capabilities. + */ +export interface ShellQuotingFunctions { + + characters: { + /** Characters that require quotes, white space is always implied. */ + needQuotes?: string + /** The character used to escape sequences. */ + escape?: string + /** The character used to quote sequences, preventing variable expansion. */ + strong?: string + /** The character used to quote sequences, allowing variable expansion. */ + weak?: string + } + + /** + * Should add escape-characters in front of forbidden characters. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + escape?(this: any, arg: string): string + + /** + * Should quote the argument in such a way that variables CANNOT be expanded. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + strong?(this: any, arg: string): string; + + /** + * Should quote the argument in such a way that variables CAN be expanded. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + weak?(this: any, arg: string): string; +} + +/** + * Converts a list of args into an escaped shell command. + * + * There are two main use cases when handling command/arguments for a shell: + * 1. User already wrote the escaped commandline, then just use that. + * 2. User wants a specific process to be invoked with some arguments. + * + * The `createShellCommandLine` function is useful for the latter. + * + * @param args Standard list of spawn/exec arguments, first item is the command. + * @param quotingFunctions Collection of functions to process arguments. + */ +export function createShellCommandLine(args: Array, quotingFunctions?: ShellQuotingFunctions): string { + return args.map(arg => escapeForShell(arg, quotingFunctions)).join(' '); +} + +/** + * Escape (or quote) a given input. + * + * @param arg Input to escape. + * @param quotingFunctions Collection of functions to process the given `arg`. + * @param quotingType Override the quoting type specified by the given `arg`. + */ +export function escapeForShell(arg: string | ShellQuotedString, quotingFunctions?: ShellQuotingFunctions, quotingType?: ShellQuoting): string { + let value: string; + let quoting: ShellQuoting | undefined = quotingType; + if (typeof arg === 'string') { + if (!quoting) { + return arg; + } + value = arg; + } else { + if (!quoting) { + quoting = arg.quoting; + } + value = arg.value; + } + if (quotingFunctions && typeof quotingFunctions[quoting] === 'function') { + return quotingFunctions[quoting]!(value); + } + return value; +} + +export const BashQuotingFunctions: Required = { + characters: { + needQuotes: '()', + escape: '\\', + strong: '\'', + weak: '"', + }, + escape(arg): string { + return arg + .replace(/[\s\\|(){}<>$&;"']/g, '\\$&'); + }, + strong(arg): string { + // ('+) becomes ('"'+"') + return `'${arg + .replace(/'+/g, '\'"$&"\'')}'`; + }, + weak(arg): string { + return `"${arg + // Escape escape-characters. + .replace(/\\"/g, '\\\\"') + // Escape user-specified double-quotes. + .replace(/"/g, '\\"') + // Escape trailing (\), we don't want the user to escape our last quote. + .replace(/\\$/g, '\\\\')}"`; + }, +}; + +export const CmdQuotingFunctions: Required = { + characters: { + weak: '"', + }, + escape(arg): string { + return arg + // Escape forbidden characters (see: cmd /?) + .replace(/["%&<>()@^|]/g, '^$&') + // Double-quote whitespaces, else we cannot escape it. + .replace(/\s+/g, '"$&"'); + }, + strong(arg): string { + return this.weak(arg) + .replace(/%/g, '"%"'); + }, + weak(arg): string { + return `"${arg + // Escape user-specified backticks. + .replace(/"/g, '\\"') + // Escape forbidden characters (see: cmd /?) + .replace(/[&<>()@^|]/g, '^$&') + // Escape trailing (`), we don't want the user to escape our last quote. + // .replace(/\\$/g, '^\\') // TODO: fix + // Escape line returns + .replace(/\r?\n/g, '^$&')}"`; + }, +}; + +export const PowershellQuotingFunctions: Required = { + characters: { + needQuotes: '()', + escape: '`', + strong: '\'', + weak: '"', + }, + escape(arg): string { + return arg.replace(/[`|{}()<>;"' ]/g, '`$&'); + }, + strong(arg): string { + // In powershell, one must write ('') for a single quote to be displayed + // within a single quoted string. + return `'${arg + .replace(/'/g, '\'\'')}'`; + }, + weak(arg): string { + return `"${arg + // Escape escape-characters. + .replace(/`"/g, '``"') + // Escape user-specified backticks. + .replace(/"/g, '`"') + // Escape trailing (`), we don't want the user to escape our last quote. + .replace(/`$/g, '``')}"`; + }, +}; diff --git a/packages/process/src/common/tests/$weird(),file=name.js b/packages/process/src/common/tests/$weird(),file=name.js new file mode 100644 index 0000000000000..7ddfb4848f8bb --- /dev/null +++ b/packages/process/src/common/tests/$weird(),file=name.js @@ -0,0 +1 @@ +console.log('FORBIDDEN_OK') diff --git a/packages/process/src/common/tests/white space.js b/packages/process/src/common/tests/white space.js new file mode 100644 index 0000000000000..45934cc30c5ab --- /dev/null +++ b/packages/process/src/common/tests/white space.js @@ -0,0 +1 @@ +console.log('WHITESPACE_OK') diff --git a/packages/process/src/node/process.ts b/packages/process/src/node/process.ts index f00a5437f893f..c5f485d3642c1 100644 --- a/packages/process/src/node/process.ts +++ b/packages/process/src/node/process.ts @@ -56,9 +56,9 @@ export enum ProcessType { * * https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options */ -export interface ProcessOptions { +export interface ProcessOptions { readonly command: string, - args?: T[], + args?: string[], options?: { // eslint-disable-next-line @typescript-eslint/no-explicit-any [key: string]: any diff --git a/packages/process/src/node/terminal-process.spec.ts b/packages/process/src/node/terminal-process.spec.ts index 93c836f295cfe..654e0604da3e7 100644 --- a/packages/process/src/node/terminal-process.spec.ts +++ b/packages/process/src/node/terminal-process.spec.ts @@ -17,7 +17,7 @@ import * as chai from 'chai'; import * as process from 'process'; import * as stream from 'stream'; import { createProcessTestContainer } from './test/process-test-container'; -import { TerminalProcessFactory, TerminalProcess } from './terminal-process'; +import { TerminalProcessFactory } from './terminal-process'; import { IProcessExitEvent, ProcessErrorEvent } from './process'; import { isWindows } from '@theia/core/lib/common/os'; @@ -103,75 +103,75 @@ describe('TerminalProcess', function (): void { }); -/** - * @FIXME - * - * For some reason, we get a lot of garbage on `stdout` when on Windows. - * Tested manually `example-browser` and `example-electron`, it seems like - * the terminals are behaving correctly, meaning that it is only a problem - * here in the tests. - */ -if (process.platform !== 'win32' || process.env.THEIA_PROCESS_TEST_OVERRIDE) { - - describe('TerminalProcess { shell: true }', function (): void { - - this.timeout(20_000); - - interface ProcessExit extends IProcessExitEvent { - output: string; - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - async function checkOutput(proc: TerminalProcess, pattern?: RegExp): Promise { - return new Promise((resolve, reject) => { - let output = ''; - proc.outputStream.on('data', chunk => output += chunk); - proc.onExit(async exit => { - if (pattern) { - expect(output).match(pattern, output); - } - resolve({ ...exit, output }); - }); - proc.onError(reject); - }); - } - - it('should execute the command as a whole if not arguments are specified', async function (): Promise { - const proc = terminalProcessFactory({ command: 'echo a b c', options: { shell: true } }); - const exit = await checkOutput(proc, /^a b c/); - expect(exit.code).eq(0); - }); - - it('should fail if user defines a full command line and arguments', async function (): Promise { - const proc = terminalProcessFactory({ command: 'echo a b c', args: [], options: { shell: true } }); - const exit = await checkOutput(proc); - expect(exit.code).not.eq(0); - }); - - it('should be able to exec using simple arguments', async function (): Promise { - const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', 'c'], options: { shell: true } }); - const exit = await checkOutput(proc, /^a b c/); - expect(exit.code).eq(0); - }); - - it('should be able to run using arguments containing whitespace', async function (): Promise { - const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', ' c'], options: { shell: true } }); - const exit = await checkOutput(proc, /^a b c/); - expect(exit.code).eq(0); - }); - - it('will fail if user specify problematic arguments', async function (): Promise { - const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', 'c"'], options: { shell: true } }); - const exit = await checkOutput(proc); - expect(exit.code).not.eq(0); - }); - - it('should be able to run using arguments specifying which quoting method to use', async function (): Promise { - const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', { value: 'c"', quoting: 'escaped' }], options: { shell: true } }); - const exit = await checkOutput(proc, /^a b c"/); - expect(exit.code).eq(0); - }); - - }); - -} +// /** +// * @FIXME +// * +// * For some reason, we get a lot of garbage on `stdout` when on Windows. +// * Tested manually `example-browser` and `example-electron`, it seems like +// * the terminals are behaving correctly, meaning that it is only a problem +// * here in the tests. +// */ +// if (process.platform !== 'win32' || process.env.THEIA_PROCESS_TEST_OVERRIDE) { + +// describe('TerminalProcess { shell: true }', function (): void { + +// this.timeout(20_000); + +// interface ProcessExit extends IProcessExitEvent { +// output: string; +// } + +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// async function checkOutput(proc: TerminalProcess, pattern?: RegExp): Promise { +// return new Promise((resolve, reject) => { +// let output = ''; +// proc.outputStream.on('data', chunk => output += chunk); +// proc.onExit(async exit => { +// if (pattern) { +// expect(output).match(pattern, output); +// } +// resolve({ ...exit, output }); +// }); +// proc.onError(reject); +// }); +// } + +// it('should execute the command as a whole if not arguments are specified', async function (): Promise { +// const proc = terminalProcessFactory({ command: 'echo a b c', options: { shell: true } }); +// const exit = await checkOutput(proc, /^a b c/); +// expect(exit.code).eq(0); +// }); + +// it('should fail if user defines a full command line and arguments', async function (): Promise { +// const proc = terminalProcessFactory({ command: 'echo a b c', args: [], options: { shell: true } }); +// const exit = await checkOutput(proc); +// expect(exit.code).not.eq(0); +// }); + +// it('should be able to exec using simple arguments', async function (): Promise { +// const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', 'c'], options: { shell: true } }); +// const exit = await checkOutput(proc, /^a b c/); +// expect(exit.code).eq(0); +// }); + +// it('should be able to run using arguments containing whitespace', async function (): Promise { +// const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', ' c'], options: { shell: true } }); +// const exit = await checkOutput(proc, /^a b c/); +// expect(exit.code).eq(0); +// }); + +// it('will fail if user specify problematic arguments', async function (): Promise { +// const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', 'c"'], options: { shell: true } }); +// const exit = await checkOutput(proc); +// expect(exit.code).not.eq(0); +// }); + +// it('should be able to run using arguments specifying which quoting method to use', async function (): Promise { +// const proc = terminalProcessFactory({ command: 'echo', args: ['a', 'b', { value: 'c"', quoting: 'escaped' }], options: { shell: true } }); +// const exit = await checkOutput(proc, /^a b c"/); +// expect(exit.code).eq(0); +// }); + +// }); + +// } diff --git a/packages/process/src/node/terminal-process.ts b/packages/process/src/node/terminal-process.ts index 58f67be76e642..00f42c4c62bf5 100644 --- a/packages/process/src/node/terminal-process.ts +++ b/packages/process/src/node/terminal-process.ts @@ -15,9 +15,9 @@ ********************************************************************************/ import { injectable, inject, named } from 'inversify'; +import { isWindows } from '@theia/core'; import { ILogger } from '@theia/core/lib/common'; import { Process, ProcessType, ProcessOptions, ProcessErrorEvent } from './process'; -import { RawProcessOptions } from './raw-process'; import { ProcessManager } from './process-manager'; import { IPty, spawn } from '@theia/node-pty'; import { MultiRingBuffer, MultiRingBufferReadableStream } from './multi-ring-buffer'; @@ -25,53 +25,12 @@ import { DevNullStream } from './dev-null-stream'; import { signame } from './utils'; import { Writable } from 'stream'; -export type QuotingType = 'escaped' | 'strong' | 'weak'; - -/** - * A `RuntimeQuotingType` represents the different ways to quote - * and escape a value in a given runtime (`sh`, `cmd`, etc...). - */ -export type RuntimeQuotingTypes = { [key in QuotingType]: string } & { shouldBeEscaped?: string[] }; -export const ShellQuoting = { - strong: "'", - weak: '"', - escaped: '\\', - shouldBeEscaped: ['$', ' ', '<', '>', '|', '{', '}', '(', ')', '\'', '"', '`'], -}; - -/** - * Map of `Runtime (string) -> ShellQuoting`, trying to cover the - * different ways in which each runtime manages quoting and escaping. - */ -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export const RuntimeQuotingMap: { [key in string]: RuntimeQuotingTypes | undefined } = { - 'bash': ShellQuoting, - 'sh': ShellQuoting, - 'cmd.exe': { - strong: '"', - weak: '"', - escaped: '^', - shouldBeEscaped: ['%', '<', '>', '{', '}', '"'], - } -}; - -/** - * Struct describing how a string should be quoted. - * To be used when sanitizing arguments for a shell task. - */ -export interface QuotedString { - value: string; - quoting: QuotingType -} - export const TerminalProcessOptions = Symbol('TerminalProcessOptions'); -export interface TerminalProcessOptions extends ProcessOptions { - options?: { - shell?: { - executable: string - args: string[] - } | boolean; - } +export interface TerminalProcessOptions extends ProcessOptions { + /** + * Windows only. Allow passing complex command lines already escaped for CommandLineToArgvW. + */ + commandLine?: string } export const TerminalProcessFactory = Symbol('TerminalProcessFactory'); @@ -79,102 +38,14 @@ export interface TerminalProcessFactory { (options: TerminalProcessOptions): TerminalProcess; } +/** + * Run arbitrary processes inside pseudo-terminals (PTY). + * + * Note: a PTY is not a shell process (bash/pwsh/cmd...) + */ @injectable() export class TerminalProcess extends Process { - /** - * Resolve the exec options based on type (shell/process). - * - * @param options - */ - protected static resolveExecOptions(options: TerminalProcessOptions): RawProcessOptions { - return options.options && options.options.shell ? - this.createShellOptions(options) : this.normalizeProcessOptions(options); - } - - /** - * Terminal options accept a special argument format when executing in a shell: - * Arguments can be of the form: { value: string, quoting: string }, specifying - * how the arg should be quoted/escaped in the shell command. - * - * @param options - */ - protected static normalizeProcessOptions(options: TerminalProcessOptions): RawProcessOptions { - return { - ...options, - args: options.args && options.args.map( - arg => typeof arg === 'string' ? arg : arg.value), - }; - } - - /** - * Build the shell execution options (`runtime ...exec-argv "command ...argv"`). - * - * @param options - */ - protected static createShellOptions(options: TerminalProcessOptions): RawProcessOptions { - const windows = process.platform === 'win32'; - let runtime: string | undefined; - let execArgs: string[] | undefined; - let command = options.command; - - // Extract user defined runtime, if any: - if (options.options && typeof options.options.shell === 'object') { - runtime = options.options.shell.executable; - execArgs = options.options.shell.args; - } - - // Apply fallback values in case no specific runtime was specified: - runtime = runtime || windows ? - process.env['COMSPEC'] || 'cmd.exe' : - process.env['SHELL'] || 'sh'; - execArgs = execArgs || windows ? - ['/c'] : ['-c']; - - // Quote function, based on the selected runtime: - const quoteCharacters = RuntimeQuotingMap[runtime] || ShellQuoting; - function quote(string: string, quoting: QuotingType): string { - - if (quoting === 'escaped') { - // Escaping most characters (https://stackoverflow.com/a/17606289/7983255) - for (const reservedSymbol of quoteCharacters.shouldBeEscaped || []) { - string = string.split(reservedSymbol).join(quoteCharacters.escaped + reservedSymbol); - } - - } else { - // Add quotes around the argument - const q = quoteCharacters[quoting]; - string = q + string + q; - } - - return string; - } - - function quoteIfWhitespace(string: string): string { - return /\s/.test(string) ? - quote(string, 'strong') : - string; - } - - // See VS Code behavior: https://code.visualstudio.com/docs/editor/tasks#_custom-tasks - // Basically, when `task.type === 'shell` && `task.args.length > 0`, `task.command` - // is only the executable to run in a shell, followed by the correctly escaped `args`. - // Else just run `task.command`. - if (options.args) { - command = quoteIfWhitespace(command); - for (const arg of options.args) { - command += ' ' + (typeof arg === 'string' ? - quoteIfWhitespace(arg) : quote(arg.value, arg.quoting)); - } - } - - return { - ...options, - command: runtime, - args: [...execArgs, command], - }; - } - protected readonly terminal: IPty | undefined; readonly outputStream = this.createOutputStream(); @@ -187,7 +58,7 @@ export class TerminalProcess extends Process { @inject(MultiRingBuffer) protected readonly ringBuffer: MultiRingBuffer, @inject(ILogger) @named('process') logger: ILogger ) { - super(processManager, logger, ProcessType.Terminal, TerminalProcess.resolveExecOptions(options)); + super(processManager, logger, ProcessType.Terminal, options); if (this.isForkOptions(this.options)) { throw new Error('terminal processes cannot be forked as of today'); @@ -196,9 +67,9 @@ export class TerminalProcess extends Process { try { this.terminal = spawn( - this.options.command, - this.options.args || [], - this.options.options || {}); + options.command, + (isWindows && options.commandLine) || options.args || [], + options.options || {}); this.terminal.on('exec', (reason: string | undefined) => { if (reason === undefined) { @@ -274,6 +145,14 @@ export class TerminalProcess extends Process { return this.terminal!.pid; } + get executable(): string { + return (this.options as ProcessOptions).command; + } + + get arguments(): string[] { + return this.options.args || []; + } + kill(signal?: string): void { if (this.terminal && this.killed === false) { this.terminal.kill(signal); diff --git a/packages/task/src/node/process/process-task-runner.ts b/packages/task/src/node/process/process-task-runner.ts index 1db29b95b6c1f..fe7707d2e2812 100644 --- a/packages/task/src/node/process/process-task-runner.ts +++ b/packages/task/src/node/process/process-task-runner.ts @@ -14,23 +14,32 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + import { injectable, inject, named } from 'inversify'; import { isWindows, isOSX, ILogger } from '@theia/core'; import { FileUri } from '@theia/core/lib/node'; import { - TerminalProcessOptions, RawProcessFactory, TerminalProcessFactory, ProcessErrorEvent, Process, - QuotedString, + TerminalProcessOptions, } from '@theia/process/lib/node'; +import { + ShellQuotedString, ShellQuotingFunctions, BashQuotingFunctions, CmdQuotingFunctions, PowershellQuotingFunctions, createShellCommandLine, ShellQuoting +} from '@theia/process/lib/common/shell-quoting'; import { TaskFactory } from './process-task'; import { TaskRunner } from '../task-runner'; import { Task } from '../task'; import { TaskConfiguration } from '../../common/task-protocol'; import { ProcessTaskError, CommandOptions } from '../../common/process/task-protocol'; import * as fs from 'fs'; +import { ShellProcess } from '@theia/terminal/lib/node/shell-process'; +import { deepClone } from '@theia/core'; /** * Task runner that runs a task as a process or a command inside a shell. @@ -58,48 +67,25 @@ export class ProcessTaskRunner implements TaskRunner { if (!taskConfig.command) { throw new Error("Process task config must have 'command' property specified"); } - try { - const { command, args, options } = this.getResolvedCommand(taskConfig); - - const processType = taskConfig.type === 'process' ? 'process' : 'shell'; - let proc: Process; - // Always spawn a task in a pty, the only difference between shell/process tasks is the // way the command is passed: // - process: directly look for an executable and pass a specific set of arguments/options. // - shell: defer the spawning to a shell that will evaluate a command line with our executable. - if (processType === 'process') { - this.logger.debug(`Task: spawning process: ${command} with ${args}`); - proc = this.terminalProcessFactory({ - command, args, options: { - ...options, - shell: false, - } - }); - } else { - // all Task types without specific TaskRunner will be run as a shell process e.g.: npm, gulp, etc. - this.logger.debug(`Task: executing command through a shell: ${command}`); - proc = this.terminalProcessFactory({ - command, args, options: { - ...options, - shell: options.shell || true, - }, - }); - } + const terminal: Process = this.terminalProcessFactory(this.getResolvedCommand(taskConfig)); // Wait for the confirmation that the process is successfully started, or has failed to start. await new Promise((resolve, reject) => { - proc.onStart(resolve); - proc.onError((error: ProcessErrorEvent) => { + terminal.onStart(resolve); + terminal.onError((error: ProcessErrorEvent) => { reject(ProcessTaskError.CouldNotRun(error.code)); }); }); return this.taskFactory({ label: taskConfig.label, - process: proc, - processType: processType, + process: terminal, + processType: taskConfig.type as 'process' | 'shell', context: ctx, config: taskConfig }); @@ -109,14 +95,10 @@ export class ProcessTaskRunner implements TaskRunner { } } - private getResolvedCommand(taskConfig: TaskConfiguration): { - command: string | undefined, - args: Array | undefined, - options: CommandOptions - } { + private getResolvedCommand(taskConfig: TaskConfiguration): TerminalProcessOptions { let systemSpecificCommand: { - command: string | undefined, - args: Array | undefined, + command: string | undefined + args: Array | undefined options: CommandOptions }; // on windows, windows-specific options, if available, take precedence @@ -146,18 +128,137 @@ export class ProcessTaskRunner implements TaskRunner { }; } - return systemSpecificCommand; + if (typeof systemSpecificCommand.command === 'undefined') { + throw new Error('The `command` field of a task cannot be undefined.'); + } + + let args: string[]; + let command = systemSpecificCommand.command; + let commandLine: string | undefined; + + if (taskConfig.type === 'shell') { + + let execArgs: string[] = []; + let quotingFunctions: ShellQuotingFunctions | undefined; + const { shell } = systemSpecificCommand.options; + + // Actual command to execute is now a shell. + // Thing to be run will be passed as an argument. + command = shell && shell.executable || ShellProcess.getShellExecutablePath(); + + if (/bash(.exe)?$/.test(command)) { + quotingFunctions = BashQuotingFunctions; + execArgs = ['-l', '-c']; + + } else if (/wsl(.exe)?$/.test(command)) { + quotingFunctions = BashQuotingFunctions; + execArgs = ['-e']; + + } else if (/cmd(.exe)?$/.test(command)) { + quotingFunctions = CmdQuotingFunctions; + execArgs = ['/S /C']; + + } else if (/(ps|pwsh|powershell)(.exe)?/.test(command)) { + quotingFunctions = PowershellQuotingFunctions; + execArgs = ['-c']; + } + + // Allow overriding shell options from task configuration. + if (shell && shell.args) { + args = [...shell.args]; + } else { + args = [...execArgs]; + } + + // Check if an argument list is defined or not. Empty is ok. + if (Array.isArray(systemSpecificCommand.args)) { + const commandLineElements = [systemSpecificCommand.command, ...systemSpecificCommand.args] + .map(arg => { + // We want to quote arguments only if needed. + if (quotingFunctions && typeof arg === 'string' && this.argumentNeedsQuotes(arg, quotingFunctions)) { + return { + quoting: ShellQuoting.Strong, + value: arg, + }; + } else { + return arg; + } + }); + const commandLineAsArgument = createShellCommandLine(commandLineElements, quotingFunctions); + args.push(commandLineAsArgument); + } else { + // No arguments are provided, so "command" is actually the full command line to execute. + args.push(systemSpecificCommand.command); + } + + if (isWindows) { + commandLine = createShellCommandLine(args, quotingFunctions); + } + + } else if (Array.isArray(systemSpecificCommand.args)) { + // Process task doesn't handle quotation: Normalize arguments from `ShellQuotedString` to raw `string`. + args = systemSpecificCommand.args.map(arg => typeof arg === 'string' ? arg : arg.value); + + } else { + args = []; + } + + return { command, args, commandLine, options }; + } + + /** + * This is task specific, to align with VS Code's behavior. + * + * When parsing arguments, VS Code will try to detect if the user already + * tried to quote things. + * + * See: https://github.com/microsoft/vscode/blob/d363b988e1e58cf49963841c498681cdc6cb55a3/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts#L1101-L1127 + * + * @param value + * @param shellQuotingOptions + */ + protected argumentNeedsQuotes(value: string, shellQuotingOptions: ShellQuotingFunctions): boolean { + const { characters } = shellQuotingOptions; + const needQuotes = new Set([' ', ...characters.needQuotes || []]); + if (!characters) { + return false; + } + if (value.length >= 2) { + const first = value[0] === characters.strong ? characters.strong : value[0] === characters.weak ? characters.weak : undefined; + if (first === value[value.length - 1]) { + return false; + } + } + let quote: string | undefined; + for (let i = 0; i < value.length; i++) { + // We found the end quote. + const ch = value[i]; + if (ch === quote) { + quote = undefined; + } else if (quote !== undefined) { + // skip the character. We are quoted. + continue; + } else if (ch === characters.escape) { + // Skip the next character + i++; + } else if (ch === characters.strong || ch === characters.weak) { + quote = ch; + } else if (needQuotes.has(ch)) { + return true; + } + } + return false; } private getSystemSpecificCommand(taskConfig: TaskConfiguration, system: 'windows' | 'linux' | 'osx' | undefined): { command: string | undefined, - args: Array | undefined, + args: Array | undefined, options: CommandOptions } { // initialize with default values from the `taskConfig` let command: string | undefined = taskConfig.command; - let args: Array | undefined = taskConfig.args; - let options: CommandOptions = taskConfig.options || {}; + let args: Array | undefined = taskConfig.args; + let options: CommandOptions = deepClone(taskConfig.options) || {}; if (system) { if (taskConfig[system].command) { @@ -171,11 +272,15 @@ export class ProcessTaskRunner implements TaskRunner { } } + if (options.cwd) { + options.cwd = this.asFsPath(options.cwd); + } + return { command, args, options }; } protected asFsPath(uriOrPath: string): string { - return (uriOrPath.startsWith('file:/')) + return (uriOrPath.startsWith('file:')) ? FileUri.fsPath(uriOrPath) : uriOrPath; } diff --git a/packages/task/src/node/task-server.slow-spec.ts b/packages/task/src/node/task-server.slow-spec.ts index 0aaf6a19c6d87..a2d120bc15558 100644 --- a/packages/task/src/node/task-server.slow-spec.ts +++ b/packages/task/src/node/task-server.slow-spec.ts @@ -14,6 +14,8 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ +// tslint:disable-next-line:no-implicit-dependencies +import 'reflect-metadata'; import { createTaskTestContainer } from './test/task-test-container'; import { BackendApplication } from '@theia/core/lib/node/backend-application'; import { TaskExitedEvent, TaskInfo, TaskServer, TaskWatcher, TaskConfiguration } from '../common'; @@ -28,10 +30,6 @@ import { TestWebSocketChannel } from '@theia/core/lib/node/messaging/test/test-w import { expect } from 'chai'; import URI from '@theia/core/lib/common/uri'; -/** - * Globals - */ - // test scripts that we bundle with tasks const commandShortRunning = './task'; const commandShortRunningOsx = './task-osx'; @@ -46,6 +44,15 @@ const bogusCommand = 'thisisnotavalidcommand'; const commandUnixNoop = 'true'; const commandWindowsNoop = 'rundll32.exe'; +/** Expects argv to be ['a', 'b', 'c'] */ +const script0 = './test-arguments-0.js'; +/** Expects argv to be ['a', 'b', ' c'] */ +const script1 = './test-arguments-1.js'; +/** Expects argv to be ['a', 'b', 'c"'] */ +const script2 = './test-arguments-2.js'; +/** Expects argv to be ['a', 'b', 'c"'] */ +const script3 = './test-arguments-3.js'; + // we use test-resources subfolder ('/packages/task/test-resources/'), // as workspace root, for these tests const wsRootUri: URI = FileUri.create(__dirname).resolve('../../test-resources'); @@ -114,7 +121,7 @@ describe('Task server / back-end', function (): void { // create task using raw process const taskInfo: TaskInfo = await taskServer.run(createProcessTaskConfig('process', executable, [someString]), wsRoot); - const p = new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { const toDispose = taskWatcher.onTaskExit((event: TaskExitedEvent) => { if (event.taskId === taskInfo.taskId && event.code === 0) { if (typeof taskInfo.terminalId === 'number') { @@ -126,37 +133,26 @@ describe('Task server / back-end', function (): void { } }); }); - - await p; }); it('task is executed successfully with cwd as a file URI', async function (): Promise { const command = isWindows ? commandShortRunningWindows : (isOSX ? commandShortRunningOsx : commandShortRunning); - const config = createProcessTaskConfig('shell', command, [], FileUri.create(wsRoot).toString()); + const config = createProcessTaskConfig('shell', command, undefined, FileUri.create(wsRoot).toString()); const taskInfo: TaskInfo = await taskServer.run(config, wsRoot); - - const p = checkSuccessfulProcessExit(taskInfo, taskWatcher); - - await p; + await checkSuccessfulProcessExit(taskInfo, taskWatcher); }); it('task is executed successfully using terminal process', async function (): Promise { const command = isWindows ? commandShortRunningWindows : (isOSX ? commandShortRunningOsx : commandShortRunning); - const taskInfo: TaskInfo = await taskServer.run(createProcessTaskConfig('shell', command, []), wsRoot); - - const p = checkSuccessfulProcessExit(taskInfo, taskWatcher); - - await p; + const taskInfo: TaskInfo = await taskServer.run(createProcessTaskConfig('shell', command, undefined), wsRoot); + await checkSuccessfulProcessExit(taskInfo, taskWatcher); }); it('task is executed successfully using raw process', async function (): Promise { const command = isWindows ? commandShortRunningWindows : (isOSX ? commandShortRunningOsx : commandShortRunning); const executable = FileUri.fsPath(wsRootUri.resolve(command)); const taskInfo: TaskInfo = await taskServer.run(createProcessTaskConfig('process', executable, [])); - - const p = checkSuccessfulProcessExit(taskInfo, taskWatcher); - - await p; + await checkSuccessfulProcessExit(taskInfo, taskWatcher); }); it('task without a specific runner is executed successfully using as a process', async function (): Promise { @@ -165,99 +161,59 @@ describe('Task server / back-end', function (): void { // there's no runner registered for the 'npm' task type const taskConfig: TaskConfiguration = createTaskConfig('npm', command, []); const taskInfo: TaskInfo = await taskServer.run(taskConfig, wsRoot); - - const p = checkSuccessfulProcessExit(taskInfo, taskWatcher); - - await p; + await checkSuccessfulProcessExit(taskInfo, taskWatcher); }); it('task can successfully execute command found in system path using a terminal process', async function (): Promise { const command = isWindows ? commandWindowsNoop : commandUnixNoop; - const opts: TaskConfiguration = createProcessTaskConfig('shell', command, []); const taskInfo: TaskInfo = await taskServer.run(opts, wsRoot); - - const p = checkSuccessfulProcessExit(taskInfo, taskWatcher); - - await p; + await checkSuccessfulProcessExit(taskInfo, taskWatcher); }); it('task can successfully execute command found in system path using a raw process', async function (): Promise { const command = isWindows ? commandWindowsNoop : commandUnixNoop; const taskInfo: TaskInfo = await taskServer.run(createProcessTaskConfig('process', command, []), wsRoot); - - const p = checkSuccessfulProcessExit(taskInfo, taskWatcher); - - await p; + await checkSuccessfulProcessExit(taskInfo, taskWatcher); }); - it('task using type "terminal" can be killed', async function (): Promise { + it('task using type "shell" can be killed', async function (): Promise { const taskInfo: TaskInfo = await taskServer.run(createTaskConfigTaskLongRunning('shell'), wsRoot); - const p = new Promise((resolve, reject) => { - taskWatcher.onTaskExit((event: TaskExitedEvent) => { - if (isWindows) { - if (event.taskId !== taskInfo.taskId || event.code === undefined) { - reject(new Error(JSON.stringify(event))); - } - resolve(event.code); - } else { - if (event.taskId !== taskInfo.taskId || event.signal === undefined) { - reject(new Error(JSON.stringify(event))); - } - resolve(event.signal); - } - }); - - taskServer.kill(taskInfo.taskId); - }); + const exitStatusPromise = getExitStatus(taskInfo, taskWatcher); + taskServer.kill(taskInfo.taskId); + const exitStatus = await exitStatusPromise; // node-pty reports different things on Linux/macOS vs Windows when // killing a process. This is not ideal, but that's how things are // currently. Ideally, its behavior should be aligned as much as // possible on what node's child_process module does. - const signalOrCode = await p; if (isWindows) { // On Windows, node-pty just reports an exit code of 0. - expect(signalOrCode).equals(0); + expect(exitStatus).equals(0); } else { // On Linux/macOS, node-pty sends SIGHUP by default, for some reason. - expect(signalOrCode).equals('SIGHUP'); + expect(exitStatus).equals('SIGHUP'); } }); it('task using type "process" can be killed', async function (): Promise { const taskInfo: TaskInfo = await taskServer.run(createTaskConfigTaskLongRunning('process'), wsRoot); - const p = new Promise((resolve, reject) => { - taskWatcher.onTaskExit((event: TaskExitedEvent) => { - if (isWindows) { - if (event.taskId !== taskInfo.taskId || event.code === undefined) { - reject(new Error(JSON.stringify(event))); - } - resolve(event.code); - } else { - if (event.taskId !== taskInfo.taskId || event.signal === undefined) { - reject(new Error(JSON.stringify(event))); - } - resolve(event.signal); - } - }); - - taskServer.kill(taskInfo.taskId); - }); + const exitStatusPromise = getExitStatus(taskInfo, taskWatcher); + taskServer.kill(taskInfo.taskId); + const exitStatus = await exitStatusPromise; // node-pty reports different things on Linux/macOS vs Windows when // killing a process. This is not ideal, but that's how things are // currently. Ideally, its behavior should be aligned as much as // possible on what node's child_process module does. - const signalOrCode = await p; if (isWindows) { // On Windows, node-pty just reports an exit code of 0. - expect(signalOrCode).equals(0); + expect(exitStatus).equals(0); } else { // On Linux/macOS, node-pty sends SIGHUP by default, for some reason. - expect(signalOrCode).equals('SIGHUP'); + expect(exitStatus).equals('SIGHUP'); } }); @@ -266,8 +222,7 @@ describe('Task server / back-end', function (): void { */ it('task using terminal process can handle command that does not exist', async function (): Promise { const taskInfo: TaskInfo = await taskServer.run(createProcessTaskConfig2('shell', bogusCommand, []), wsRoot); - - const p = new Promise((resolve, reject) => { + const code = await new Promise((resolve, reject) => { taskWatcher.onTaskExit((event: TaskExitedEvent) => { if (event.taskId !== taskInfo.taskId || event.code === undefined) { reject(new Error(JSON.stringify(event))); @@ -275,12 +230,10 @@ describe('Task server / back-end', function (): void { resolve(event.code); }); }); - // node-pty reports different things on Linux/macOS vs Windows when // killing a process. This is not ideal, but that's how things are // currently. Ideally, its behavior should be aligned as much as // possible on what node's child_process module does. - const code = await p; if (isWindows) { expect(code).equals(1); } else { @@ -355,6 +308,48 @@ describe('Task server / back-end', function (): void { }); + it('shell task should execute the command as a whole if not arguments are specified', async function (): Promise { + const taskInfo = await taskServer.run(createProcessTaskConfig2('shell', `node ${script0} debug-hint:0a a b c`)); + const exitStatus = await getExitStatus(taskInfo, taskWatcher); + expect(exitStatus).eq(0); + }); + + it('shell task should fail if user defines a full command line and arguments', async function (): Promise { + const taskInfo = await taskServer.run(createProcessTaskConfig2('shell', `node ${script0} debug-hint:0b a b c`, [])); + const exitStatus = await getExitStatus(taskInfo, taskWatcher); + expect(exitStatus).not.eq(0); + }); + + it('shell task should be able to exec using simple arguments', async function (): Promise { + const taskInfo = await taskServer.run(createProcessTaskConfig2('shell', 'node', [script0, 'debug-hint:0c', 'a', 'b', 'c'])); + const exitStatus = await getExitStatus(taskInfo, taskWatcher); + expect(exitStatus).eq(0); + }); + + it('shell task should be able to run using arguments containing whitespace', async function (): Promise { + const taskInfo = await taskServer.run(createProcessTaskConfig2('shell', 'node', [script1, 'debug-hint:1', 'a', 'b', ' c'])); + const exitStatus = await getExitStatus(taskInfo, taskWatcher); + expect(exitStatus).eq(0); + }); + + it('shell task will fail if user specify problematic arguments', async function (): Promise { + const taskInfo = await taskServer.run(createProcessTaskConfig2('shell', 'node', [script2, 'debug-hint:2', 'a', 'b', 'c"'])); + const exitStatus = await getExitStatus(taskInfo, taskWatcher); + expect(exitStatus).not.eq(0); + }); + + it('shell task should be able to run using arguments specifying which quoting method to use', async function (): Promise { + const taskInfo = await taskServer.run(createProcessTaskConfig2('shell', 'node', [script3, 'debug-hint:3', 'a', 'b', { value: 'c"', quoting: 'escape' }])); + const exitStatus = await getExitStatus(taskInfo, taskWatcher); + expect(exitStatus).eq(0); + }); + + it('shell task should be able to run using arguments with forbidden characters but no whitespace', async function (): Promise { + const taskInfo = await taskServer.run(createProcessTaskConfig2('shell', 'node', ['-e', 'setTimeout(console.log,1000,1+2)'])); + const exitStatus = await getExitStatus(taskInfo, taskWatcher); + expect(exitStatus).eq(0); + }); + }); function createTaskConfig(taskType: string, command: string, args: string[]): TaskConfiguration { @@ -378,11 +373,12 @@ function createProcessTaskConfig(processType: ProcessType, command: string, args _scope: '/source/folder', command, args, - options: { cwd: wsRoot }, + options: { cwd }, }; } -function createProcessTaskConfig2(processType: ProcessType, command: string, args?: string[]): TaskConfiguration { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function createProcessTaskConfig2(processType: ProcessType, command: string, args?: any[]): TaskConfiguration { return { label: 'test task', type: processType, @@ -400,10 +396,8 @@ function createTaskConfigTaskLongRunning(processType: ProcessType): TaskConfigur _scope: '/source/folder', options: { cwd: wsRoot }, command: commandLongRunning, - args: [], windows: { command: FileUri.fsPath(wsRootUri.resolve(commandLongRunningWindows)), - args: [], options: { cwd: wsRoot } }, osx: { @@ -413,7 +407,7 @@ function createTaskConfigTaskLongRunning(processType: ProcessType): TaskConfigur } function checkSuccessfulProcessExit(taskInfo: TaskInfo, taskWatcher: TaskWatcher): Promise { - const p = new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const toDispose = taskWatcher.onTaskExit((event: TaskExitedEvent) => { if (event.taskId === taskInfo.taskId && event.code === 0) { toDispose.dispose(); @@ -421,6 +415,20 @@ function checkSuccessfulProcessExit(taskInfo: TaskInfo, taskWatcher: TaskWatcher } }); }); +} - return p; +function getExitStatus(taskInfo: TaskInfo, taskWatcher: TaskWatcher): Promise { + return new Promise((resolve, reject) => { + taskWatcher.onTaskExit((event: TaskExitedEvent) => { + if (event.taskId === taskInfo.taskId) { + if (typeof event.signal === 'string') { + resolve(event.signal); + } else if (typeof event.code === 'number') { + resolve(event.code); + } else { + reject(new Error('no code nor signal')); + } + } + }); + }); } diff --git a/packages/task/src/node/test/task-test-container.ts b/packages/task/src/node/test/task-test-container.ts index be0900c3042f8..5386502315775 100644 --- a/packages/task/src/node/test/task-test-container.ts +++ b/packages/task/src/node/test/task-test-container.ts @@ -23,6 +23,7 @@ import filesystemBackendModule from '@theia/filesystem/lib/node/filesystem-backe import workspaceServer from '@theia/workspace/lib/node/workspace-backend-module'; import { messagingBackendModule } from '@theia/core/lib/node/messaging/messaging-backend-module'; import { ApplicationPackage } from '@theia/application-package/lib/application-package'; +import { TerminalProcess } from '@theia/process/lib/node'; export function createTaskTestContainer(): Container { const testContainer = new Container(); @@ -38,5 +39,19 @@ export function createTaskTestContainer(): Container { testContainer.load(workspaceServer); testContainer.load(terminalBackendModule); + // Make it easier to debug processes. + testContainer.rebind(TerminalProcess).to(TestTerminalProcess).inSingletonScope(); + return testContainer; } + +class TestTerminalProcess extends TerminalProcess { + + protected emitOnStarted(): void { + if (process.env['THEIA_TASK_TEST_DEBUG']) { + this.outputStream.on('data', data => console.debug(`${JSON.stringify([this.executable, ...this.arguments])} OUTPUT: ${data.toString().trim()}`)); + } + super.emitOnStarted(); + } + +} diff --git a/packages/task/test-resources/compare.js b/packages/task/test-resources/compare.js new file mode 100644 index 0000000000000..e9461b323eb03 --- /dev/null +++ b/packages/task/test-resources/compare.js @@ -0,0 +1,14 @@ +/** + * Compares if two arrays contain the same primitive values. + */ +exports.compareArrayValues = function (a, b) { + if (a.length !== b.length) { + return false + } + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) { + return false + } + } + return true +} diff --git a/packages/task/test-resources/test-arguments-0.js b/packages/task/test-resources/test-arguments-0.js new file mode 100644 index 0000000000000..c08521eee2141 --- /dev/null +++ b/packages/task/test-resources/test-arguments-0.js @@ -0,0 +1,13 @@ +const { + compareArrayValues +} = require('./compare') + +const debugHint = process.argv[2] +const args = process.argv.slice(3) + +if (compareArrayValues(args, ['a', 'b', 'c'])) { + process.exit(0) // OK +} else { + console.error(debugHint, JSON.stringify(args)) + process.exit(1) // NOT OK +} diff --git a/packages/task/test-resources/test-arguments-1.js b/packages/task/test-resources/test-arguments-1.js new file mode 100644 index 0000000000000..fcbdb48d4b556 --- /dev/null +++ b/packages/task/test-resources/test-arguments-1.js @@ -0,0 +1,13 @@ +const { + compareArrayValues +} = require('./compare') + +const debugHint = process.argv[2] +const args = process.argv.slice(3) + +if (compareArrayValues(args, ['a', 'b', ' c'])) { + process.exit(0) // OK +} else { + console.error(debugHint, JSON.stringify(args)) + process.exit(1) // NOT OK +} diff --git a/packages/task/test-resources/test-arguments-2.js b/packages/task/test-resources/test-arguments-2.js new file mode 100644 index 0000000000000..007cab96f7744 --- /dev/null +++ b/packages/task/test-resources/test-arguments-2.js @@ -0,0 +1,13 @@ +const { + compareArrayValues +} = require('./compare') + +const debugHint = process.argv[2] +const args = process.argv.slice(3) + +if (compareArrayValues(args, ['a', 'b', 'c"'])) { + process.exit(0) // OK +} else { + console.error(debugHint, JSON.stringify(args)) + process.exit(1) // NOT OK +} diff --git a/packages/task/test-resources/test-arguments-3.js b/packages/task/test-resources/test-arguments-3.js new file mode 100644 index 0000000000000..007cab96f7744 --- /dev/null +++ b/packages/task/test-resources/test-arguments-3.js @@ -0,0 +1,13 @@ +const { + compareArrayValues +} = require('./compare') + +const debugHint = process.argv[2] +const args = process.argv.slice(3) + +if (compareArrayValues(args, ['a', 'b', 'c"'])) { + process.exit(0) // OK +} else { + console.error(debugHint, JSON.stringify(args)) + process.exit(1) // NOT OK +} diff --git a/packages/terminal/src/browser/base/terminal-widget.ts b/packages/terminal/src/browser/base/terminal-widget.ts index 5297c511e3b13..958d01ebaec6e 100644 --- a/packages/terminal/src/browser/base/terminal-widget.ts +++ b/packages/terminal/src/browser/base/terminal-widget.ts @@ -16,7 +16,9 @@ import { Event } from '@theia/core'; import { BaseWidget } from '@theia/core/lib/browser'; +import { CommandLineOptions } from '@theia/process/lib/common/shell-command-builder'; import { TerminalSearchWidget } from '../search/terminal-search-widget'; +import { TerminalProcessInfo } from '../../common/base-terminal-protocol'; /** * Terminal UI widget. @@ -25,6 +27,8 @@ export abstract class TerminalWidget extends BaseWidget { abstract processId: Promise; + abstract processInfo: Promise; + /** * Start terminal and return terminal id. * @param id - terminal id. @@ -37,6 +41,17 @@ export abstract class TerminalWidget extends BaseWidget { */ abstract sendText(text: string): void; + /** + * Resolves when the command is successfully sent, this doesn't mean that it + * was evaluated. Might reject if terminal wasn't properly started yet. + * + * Note that this method will try to escape your arguments as if it was + * someone inputting everything in a shell. + * + * Supported shells: `bash`, `cmd.exe`, `wsl.exe`, `pwsh/powershell.exe` + */ + abstract executeCommand(commandOptions: CommandLineOptions): Promise; + abstract onDidOpen: Event; abstract scrollLineUp(): void; diff --git a/packages/terminal/src/browser/terminal-widget-impl.ts b/packages/terminal/src/browser/terminal-widget-impl.ts index 53e28da7744f5..7b193ea1a4e3e 100644 --- a/packages/terminal/src/browser/terminal-widget-impl.ts +++ b/packages/terminal/src/browser/terminal-widget-impl.ts @@ -23,7 +23,7 @@ import { isOSX } from '@theia/core/lib/common'; import { WorkspaceService } from '@theia/workspace/lib/browser'; import { ShellTerminalServerProxy } from '../common/shell-terminal-protocol'; import { terminalsPath } from '../common/terminal-protocol'; -import { IBaseTerminalServer } from '../common/base-terminal-protocol'; +import { IBaseTerminalServer, TerminalProcessInfo } from '../common/base-terminal-protocol'; import { TerminalWatcher } from '../common/terminal-watcher'; import { TerminalWidgetOptions, TerminalWidget } from './base/terminal-widget'; import { MessageConnection } from 'vscode-jsonrpc'; @@ -35,6 +35,7 @@ import { TerminalService } from './base/terminal-service'; import { TerminalSearchWidgetFactory, TerminalSearchWidget } from './search/terminal-search-widget'; import { TerminalCopyOnSelectionHandler } from './terminal-copy-on-selection-handler'; import { TerminalThemeService } from './terminal-theme-service'; +import { CommandLineOptions, ShellCommandBuilder } from '@theia/process/lib/common/shell-command-builder'; export const TERMINAL_WIDGET_FACTORY_ID = 'terminal'; @@ -71,6 +72,7 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget @inject(TerminalSearchWidgetFactory) protected readonly terminalSearchBoxFactory: TerminalSearchWidgetFactory; @inject(TerminalCopyOnSelectionHandler) protected readonly copyOnSelectionHandler: TerminalCopyOnSelectionHandler; @inject(TerminalThemeService) protected readonly themeService: TerminalThemeService; + @inject(ShellCommandBuilder) protected readonly shellCommandBuilder: ShellCommandBuilder; protected readonly onDidOpenEmitter = new Emitter(); readonly onDidOpen: Event = this.onDidOpenEmitter.event; @@ -273,6 +275,13 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget return this.shellTerminalServer.getProcessId(this.terminalId); } + get processInfo(): Promise { + if (!IBaseTerminalServer.validateId(this.terminalId)) { + return Promise.reject(new Error('terminal is not started')); + } + return this.shellTerminalServer.getProcessInfo(this.terminalId); + } + get lastTouchEndEvent(): TouchEvent | undefined { return this.lastTouchEnd; } @@ -478,6 +487,10 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget } } + async executeCommand(commandOptions: CommandLineOptions): Promise { + this.sendText(this.shellCommandBuilder.buildCommand(await this.processInfo, commandOptions) + '\n'); + } + scrollLineUp(): void { this.term.scrollLines(-1); } diff --git a/packages/terminal/src/common/base-terminal-protocol.ts b/packages/terminal/src/common/base-terminal-protocol.ts index 2d77900221325..b73d138d47c35 100644 --- a/packages/terminal/src/common/base-terminal-protocol.ts +++ b/packages/terminal/src/common/base-terminal-protocol.ts @@ -17,11 +17,17 @@ import { JsonRpcServer } from '@theia/core/lib/common/messaging/proxy-factory'; import { Disposable } from '@theia/core'; +export interface TerminalProcessInfo { + executable: string + arguments: string[] +} + export interface IBaseTerminalServerOptions { } export interface IBaseTerminalServer extends JsonRpcServer { create(IBaseTerminalServerOptions: object): Promise; getProcessId(id: number): Promise; + getProcessInfo(id: number): Promise; getCwdURI(id: number): Promise; resize(id: number, cols: number, rows: number): Promise; attach(id: number): Promise; diff --git a/packages/terminal/src/node/base-terminal-server.ts b/packages/terminal/src/node/base-terminal-server.ts index b832251728f1c..c59cf2fa10e9c 100644 --- a/packages/terminal/src/node/base-terminal-server.ts +++ b/packages/terminal/src/node/base-terminal-server.ts @@ -16,7 +16,7 @@ import { inject, injectable, named } from 'inversify'; import { ILogger, DisposableCollection } from '@theia/core/lib/common'; -import { IBaseTerminalServer, IBaseTerminalServerOptions, IBaseTerminalClient } from '../common/base-terminal-protocol'; +import { IBaseTerminalServer, IBaseTerminalServerOptions, IBaseTerminalClient, TerminalProcessInfo } from '../common/base-terminal-protocol'; import { TerminalProcess, ProcessManager } from '@theia/process/lib/node'; import { ShellProcess } from './shell-process'; @@ -59,6 +59,17 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer { return terminal.pid; } + async getProcessInfo(id: number): Promise { + const terminal = this.processManager.get(id); + if (!(terminal instanceof TerminalProcess)) { + throw new Error(`terminal "${id}" does not exist`); + } + return { + executable: terminal.executable, + arguments: terminal.arguments, + }; + } + async getCwdURI(id: number): Promise { const terminal = this.processManager.get(id); if (!(terminal instanceof TerminalProcess)) { diff --git a/scripts/prepare-travis.js b/scripts/prepare-travis.js index 3635d9dce59e7..61c4c76c7bcf1 100644 --- a/scripts/prepare-travis.js +++ b/scripts/prepare-travis.js @@ -1,4 +1,5 @@ #!/usr/bin/env node + /******************************************************************************** * Copyright (c) 2018-2020 TypeFox and others * diff --git a/yarn.lock b/yarn.lock index 7c5a340d17860..30c5b1edd5775 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3831,7 +3831,7 @@ colors@1.0.3: resolved "https://registry.yarnpkg.com/colors/-/colors-1.0.3.tgz#0433f44d809680fdeb60ed260f1b0c262e82a40b" integrity sha1-BDP0TYCWgP3rYO0mDxsMJi6CpAs= -colors@^1.1.2, colors@^1.2.1, colors@^1.3.3: +colors@^1.1.2, colors@^1.2.1, colors@^1.3.3, colors@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/colors/-/colors-1.4.0.tgz#c50491479d4c1bdaed2c9ced32cf7c7dc2360f78" integrity sha512-a+UqTh4kgZg/SlGvfbzDHpgRu7AAQOmmqRHJnxhRZICKFUT91brVhNNt58CMWU9PsBbv3PDCZUHbVxuDiH2mtA==