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.

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 <paul.marechal@ericsson.com>
  • Loading branch information
paul-marechal committed Apr 2, 2020
1 parent ce39872 commit 5e45d2b
Show file tree
Hide file tree
Showing 30 changed files with 1,557 additions and 376 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Breaking changes:

- [plugin] removed `configStorage` argument from `PluginManager.registerPlugin`.
Use `PluginManager.configStorage` property instead. [#7265](https://github.com/eclipse-theia/theia/pull/7265#discussion_r399956070)
- [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.

## v1.0.0

Expand Down Expand Up @@ -288,6 +290,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',
Expand All @@ -299,7 +302,9 @@ Breaking changes:
}
});
```

- later from a file:

```ts
@inject(MonacoThemingService)
protected readonly monacoThemeService: MonacoThemingService;
Expand All @@ -311,6 +316,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`.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/node/backend-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ export class BackendApplication {
@inject(ApplicationPackage)
protected readonly applicationPackage: ApplicationPackage;

private readonly _performanceObserver: PerformanceObserver;

constructor(
@inject(ContributionProvider) @named(BackendApplicationContribution)
protected readonly contributionsProvider: ContributionProvider<BackendApplicationContribution>,
Expand All @@ -123,7 +125,7 @@ export class BackendApplication {
process.on('SIGTERM', () => process.exit(0));

// Create performance observer
new PerformanceObserver(list => {
this._performanceObserver = new PerformanceObserver(list => {
for (const item of list.getEntries()) {
const contribution = `Backend ${item.name}`;
if (item.duration > TIMER_WARNING_THRESHOLD) {
Expand All @@ -132,7 +134,8 @@ export class BackendApplication {
console.debug(`${contribution} took: ${item.duration.toFixed(1)} ms`);
}
}
}).observe({
});
this._performanceObserver.observe({
entryTypes: ['measure']
});

Expand Down Expand Up @@ -259,6 +262,7 @@ export class BackendApplication {
}
}
console.info('<<< All backend contributions have been stopped.');
this._performanceObserver.disconnect();
}

protected async serveGzipped(contentType: string, req: express.Request, res: express.Response, next: express.NextFunction): Promise<void> {
Expand Down
1 change: 0 additions & 1 deletion packages/debug/src/browser/debug-session-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ export class DefaultDebugSessionFactory implements DebugSessionFactory {
}, { reconnecting: false })
),
this.getTraceOutputChannel());

return new DebugSession(
sessionId,
options,
Expand Down
5 changes: 3 additions & 2 deletions packages/debug/src/browser/debug-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,9 @@ 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, 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<TerminalWidget> {
Expand Down
6 changes: 5 additions & 1 deletion packages/process/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@
"description": "Theia process support.",
"dependencies": {
"@theia/core": "^1.0.0",
"@theia/node-pty": "0.7.8-theia004",
"@theia/node-pty": "0.9.0-theia.6",
"string-argv": "^0.1.1"
},
"publishConfig": {
"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 { ShellCommandBuilder } from './shell-command-builder';

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

0 comments on commit 5e45d2b

Please sign in to comment.