Skip to content

Commit

Permalink
process/task/terminal: refactor escaping/quoting
Browse files Browse the repository at this point in the history
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.

Signed-off-by: Paul Maréchal <[email protected]>
  • Loading branch information
paul-marechal committed Jan 7, 2020
1 parent 8ddf4b6 commit 6ce6613
Show file tree
Hide file tree
Showing 27 changed files with 1,279 additions and 352 deletions.
3 changes: 2 additions & 1 deletion packages/debug/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@
"test": "theiaext test"
},
"devDependencies": {
"@theia/ext-scripts": "^0.14.0"
"@theia/ext-scripts": "^0.14.0",
"colors": "^1.4.0"
},
"nyc": {
"extends": "../../configs/nyc.json"
Expand Down
16 changes: 12 additions & 4 deletions packages/debug/src/browser/debug-session-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { IWebSocket } from 'vscode-ws-jsonrpc/lib/socket/socket';
import { DebugAdapterPath } from '../common/debug-service';
import { ContributionProvider } from '@theia/core/lib/common/contribution-provider';
import { FileSystem } from '@theia/filesystem/lib/common';
import { RunInTerminalUtility } from '@theia/process/lib/common/run-in-terminal';

/**
* DebugSessionContribution symbol for DI.
Expand Down Expand Up @@ -114,6 +115,8 @@ export class DefaultDebugSessionFactory implements DebugSessionFactory {
protected readonly debugPreferences: DebugPreferences;
@inject(FileSystem)
protected readonly fileSystem: FileSystem;
@inject(RunInTerminalUtility)
protected readonly runInTerminalUtility: RunInTerminalUtility;

get(sessionId: string, options: DebugSessionOptions): DebugSession {
const connection = new DebugSessionConnection(
Expand All @@ -123,9 +126,10 @@ export class DefaultDebugSessionFactory implements DebugSessionFactory {
resolve(channel);
}, { reconnecting: false })
),
this.getTraceOutputChannel());

return new DebugSession(
this.getTraceOutputChannel()
);
// Constructor injections
const debugSession = new DebugSession(
sessionId,
options,
connection,
Expand All @@ -134,7 +138,11 @@ export class DefaultDebugSessionFactory implements DebugSessionFactory {
this.breakpoints,
this.labelProvider,
this.messages,
this.fileSystem);
this.fileSystem,
);
// Property injections
debugSession['runInTerminalUtility'] = this.runInTerminalUtility;
return debugSession;
}

protected getTraceOutputChannel(): OutputChannel | undefined {
Expand Down
13 changes: 11 additions & 2 deletions packages/debug/src/browser/debug-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { SourceBreakpoint, ExceptionBreakpoint } from './breakpoint/breakpoint-m
import { FileSystem } from '@theia/filesystem/lib/common';
import { TerminalWidgetOptions, TerminalWidget } from '@theia/terminal/lib/browser/base/terminal-widget';
import { DebugFunctionBreakpoint } from './model/debug-function-breakpoint';
import { RunInTerminalUtility } from '@theia/process/lib/common/run-in-terminal';

export enum DebugState {
Inactive,
Expand All @@ -64,6 +65,9 @@ export class DebugSession implements CompositeTreeElement {

protected readonly toDispose = new DisposableCollection();

// @inject(RunInTerminalUtility)
protected runInTerminalUtility: RunInTerminalUtility;

constructor(
readonly id: string,
readonly options: DebugSessionOptions,
Expand Down Expand Up @@ -379,8 +383,13 @@ export class DebugSession implements CompositeTreeElement {

protected async runInTerminal({ arguments: { title, cwd, args, env } }: DebugProtocol.RunInTerminalRequest): Promise<DebugProtocol.RunInTerminalResponse['body']> {
const terminal = await this.doCreateTerminal({ title, cwd, env });
terminal.sendText(args.join(' ') + '\n');
return { processId: await terminal.processId };
// TODO: Getting a property shouldn't map to a promise for an RPC call.
// Either change the way the API works or transform the attribute
// into a function.
// Parallelize promise resolution by requesting both now:
const { processId, processInfo } = terminal;
terminal.sendText(this.runInTerminalUtility.prepareCommandLine(await processInfo, { cwd, args, env }) + '\n');
return { processId: await processId };
}

protected async doCreateTerminal(options: TerminalWidgetOptions): Promise<TerminalWidget> {
Expand Down
4 changes: 4 additions & 0 deletions packages/process/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
22 changes: 22 additions & 0 deletions packages/process/src/common/process-common-module.ts
Original file line number Diff line number Diff line change
@@ -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 { RunInTerminalUtility } from './run-in-terminal';

export default new ContainerModule((bind, unbind, isBound, rebind) => {
bind(RunInTerminalUtility).toSelf().inSingletonScope();
});
Loading

0 comments on commit 6ce6613

Please sign in to comment.