process/task/terminal: refactor escaping/quoting

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>
eclipse-theia · Jan 21, 2020 · 8d55945 · 8d55945
1 parent e9449a0
commit 8d55945
Show file tree

Hide file tree

Showing 30 changed files with 1,510 additions and 462 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,80 +1,80 @@
 sudo: required
 language: node_js
-node_js: '10'
+node_js: "10"
 git:
   depth: 1
 cache:
   yarn: true
   directories:
-# All directories need to be listed here, because Travis does not support globs.
-# Auto generated by scripts/prepare-travis
-# start_cache_directories
-  - /tmp/vscode-ripgrep-cache-1.5.7
-  - dev-packages/application-manager/node_modules
-  - dev-packages/application-package/node_modules
-  - dev-packages/cli/node_modules
-  - dev-packages/electron/node_modules
-  - examples/api-samples/node_modules
-  - examples/browser/node_modules
-  - examples/electron/node_modules
-  - node_modules
-  - packages/callhierarchy/node_modules
-  - packages/console/node_modules
-  - packages/core/node_modules
-  - packages/debug-nodejs/node_modules
-  - packages/debug/node_modules
-  - packages/editor-preview/node_modules
-  - packages/editor/node_modules
-  - packages/editorconfig/node_modules
-  - packages/file-search/node_modules
-  - packages/filesystem/node_modules
-  - packages/getting-started/node_modules
-  - packages/git/node_modules
-  - packages/java-debug/node_modules
-  - packages/java/node_modules
-  - packages/json/node_modules
-  - packages/keymaps/node_modules
-  - packages/languages/node_modules
-  - packages/markers/node_modules
-  - packages/merge-conflicts/node_modules
-  - packages/messages/node_modules
-  - packages/metrics/node_modules
-  - packages/mini-browser/node_modules
-  - packages/monaco/node_modules
-  - packages/navigator/node_modules
-  - packages/outline-view/node_modules
-  - packages/output/node_modules
-  - packages/plugin-dev/node_modules
-  - packages/plugin-ext-vscode/node_modules
-  - packages/plugin-ext/node_modules
-  - packages/plugin-metrics/node_modules
-  - packages/plugin/node_modules
-  - packages/preferences/node_modules
-  - packages/preview/node_modules
-  - packages/process/node_modules
-  - packages/python/node_modules
-  - packages/scm/node_modules
-  - packages/search-in-workspace/node_modules
-  - packages/task/node_modules
-  - packages/terminal/node_modules
-  - packages/textmate-grammars/node_modules
-  - packages/tslint/node_modules
-  - packages/typehierarchy/node_modules
-  - packages/typescript/node_modules
-  - packages/userstorage/node_modules
-  - packages/variable-resolver/node_modules
-  - packages/workspace/node_modules
-# end_cache_directories
-  - packages/java-debug/download
-  - packages/debug-nodejs/download
+    # All directories need to be listed here, because Travis does not support globs.
+    # Auto generated by scripts/prepare-travis
+    # start_cache_directories
+    - /tmp/vscode-ripgrep-cache-1.5.7
+    - dev-packages/application-manager/node_modules
+    - dev-packages/application-package/node_modules
+    - dev-packages/cli/node_modules
+    - dev-packages/electron/node_modules
+    - examples/api-samples/node_modules
+    - examples/browser/node_modules
+    - examples/electron/node_modules
+    - node_modules
+    - packages/callhierarchy/node_modules
+    - packages/console/node_modules
+    - packages/core/node_modules
+    - packages/debug-nodejs/node_modules
+    - packages/debug/node_modules
+    - packages/editor-preview/node_modules
+    - packages/editor/node_modules
+    - packages/editorconfig/node_modules
+    - packages/file-search/node_modules
+    - packages/filesystem/node_modules
+    - packages/getting-started/node_modules
+    - packages/git/node_modules
+    - packages/java-debug/node_modules
+    - packages/java/node_modules
+    - packages/json/node_modules
+    - packages/keymaps/node_modules
+    - packages/languages/node_modules
+    - packages/markers/node_modules
+    - packages/merge-conflicts/node_modules
+    - packages/messages/node_modules
+    - packages/metrics/node_modules
+    - packages/mini-browser/node_modules
+    - packages/monaco/node_modules
+    - packages/navigator/node_modules
+    - packages/outline-view/node_modules
+    - packages/output/node_modules
+    - packages/plugin-dev/node_modules
+    - packages/plugin-ext-vscode/node_modules
+    - packages/plugin-ext/node_modules
+    - packages/plugin-metrics/node_modules
+    - packages/plugin/node_modules
+    - packages/preferences/node_modules
+    - packages/preview/node_modules
+    - packages/process/node_modules
+    - packages/python/node_modules
+    - packages/scm/node_modules
+    - packages/search-in-workspace/node_modules
+    - packages/task/node_modules
+    - packages/terminal/node_modules
+    - packages/textmate-grammars/node_modules
+    - packages/tslint/node_modules
+    - packages/typehierarchy/node_modules
+    - packages/typescript/node_modules
+    - packages/userstorage/node_modules
+    - packages/variable-resolver/node_modules
+    - packages/workspace/node_modules
+    # end_cache_directories
+    - packages/java-debug/download
+    - packages/debug-nodejs/download
 before_cache:
   # Runs before the cache is updated, after successful CI
   - rm -f node_modules/@theia/electron/post-install.log
   - rm -rf node_modules/@theia/electron/download
   - rm -rf node_modules/electron
 branches:
   only:
-  - master
+    - master
 env:
   global:
     - CXX=g++-4.8
@@ -83,13 +83,13 @@ addons:
   apt:
     update: true
     sources:
-    - ubuntu-toolchain-r-test
+      - ubuntu-toolchain-r-test
     packages:
-    - g++-4.8
-    - libsecret-1-dev
-    - xvfb
-    - libx11-dev
-    - libxkbfile-dev
+      - g++-4.8
+      - libsecret-1-dev
+      - xvfb
+      - libx11-dev
+      - libxkbfile-dev
     chrome: stable
 jdk:
   - oraclejdk9
@@ -115,50 +115,51 @@ script:
 notifications:
   webhooks:
     urls:
-    - https://webhooks.gitter.im/e/c42ddc125fe6bbfccb48
+      - https://webhooks.gitter.im/e/c42ddc125fe6bbfccb48
     on_success: change
     on_failure: always
     on_start: never
 jobs:
   fast_finish: true
   include:
-  - stage: test
-    os: linux
-  - os: osx
-    env: CXX=c++
-    before_script: skip
-    script:
-      - travis_retry yarn test:theia
-  - os: windows
-    env:
-    - CXX=c++
-    - YARN_GPG=no
-    before_script: skip
-    script:
-      - travis_retry yarn test:theia
-  - stage: deploy
-    if: NOT type IN (cron, pull_request)
-    os: linux
-    before_script: skip
-    script: skip
-    install: skip
-    before_deploy:
-      - |
-        if ! [ "$BEFORE_DEPLOY_RUN" ]; then
-          export BEFORE_DEPLOY_RUN=1
-          printf "//registry.npmjs.org/:_authToken=${NPM_AUTH_TOKEN}\n" >> ~/.npmrc
-          THEIA_SKIP_NPM_PREPARE=1 yarn install --skip-integrity-check # fix cache we meddled-with
-          yarn run docs
-        fi
-    deploy:
-      - provider: script
-        script: yarn run publish:next
-        on:
-          branch: master
-        skip_cleanup: true
-      - provider: pages
-        skip_cleanup: true
-        github-token: $GITHUB_TOKEN
-        local-dir: gh-pages
-        on:
-          branch: master
+    - stage: test
+      os: linux
+    - os: osx
+      env: CXX=c++
+      before_script: skip
+      script:
+        - travis_retry yarn test:theia
+    - os: windows
+      env:
+        - CXX=c++
+        - YARN_GPG=no
+      before_script: skip
+      script:
+        # - travis_retry yarn test:theia
+        - npx run test @theia/process
+    - stage: deploy
+      if: NOT type IN (cron, pull_request)
+      os: linux
+      before_script: skip
+      script: skip
+      install: skip
+      before_deploy:
+        - |
+          if ! [ "$BEFORE_DEPLOY_RUN" ]; then
+            export BEFORE_DEPLOY_RUN=1
+            printf "//registry.npmjs.org/:_authToken=${NPM_AUTH_TOKEN}\n" >> ~/.npmrc
+            THEIA_SKIP_NPM_PREPARE=1 yarn install --skip-integrity-check # fix cache we meddled-with
+            yarn run docs
+          fi
+      deploy:
+        - provider: script
+          script: yarn run publish:next
+          on:
+            branch: master
+          skip_cleanup: true
+        - provider: pages
+          skip_cleanup: true
+          github-token: $GITHUB_TOKEN
+          local-dir: gh-pages
+          on:
+            branch: master
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@
 
 Breaking changes:
 
+- `TerminalProcess` instances will not handle shell escaping anymore. It will be up to callers to provide all the arguments required for shells to process commands.
+- Moved shell escaping utilities into `@theia/process/lib/common/shell-quoting` and `@theia/process/lib/common/shell-command-builder` for creating shell inputs.
 - Updated `example-browser` and `example-electron` applications to remove extensions which are instead contributed by VS Code builtin extensions [#6883](https://github.com/eclipse-theia/theia/pull/6883)
   - Extensions removed from the example applications are deprecated and will be removed in the future. If adopters/extenders would like to continue
   using the deprecated extensions, they must be self-maintained and can be accessed through the repository's Git history.
@@ -29,6 +31,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',
@@ -40,7 +43,9 @@ Breaking changes:
                 }
             });
         ```
+
       - later from a file:
+
         ```ts
             @inject(MonacoThemingService)
             protected readonly monacoThemeService: MonacoThemingService;
@@ -52,6 +57,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`.
@@ -71,6 +77,11 @@ Breaking changes:
   Before these attributes have to be computed for all nodes and stored as a part of the layout.
   From now on they will be computed only on demand for visible nodes.
   It decreases requirements to the local storage and allows to invalidate node appearance by simply rerendering a tree.
+  - [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.
 
 ## v0.14.0
 

diff --git a/package.json b/package.json
@@ -22,6 +22,7 @@
     "@types/webdriverio": "^4.7.0",
     "chai": "^4.1.0",
     "chai-string": "^1.4.0",
+    "colors": "^1.4.0",
     "concurrently": "^3.5.0",
     "electron-mocha": "~3.5.0",
     "ignore-styles": "^5.0.1",

diff --git 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
@@ -379,8 +379,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> {

diff --git 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
@@ -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();
+});