[plugin] fix #6092: make sure command arguments are passed safely via…

… jsonrpc

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>

packages/plugin-ext/src/common/plugin-api-rpc-model.ts

@@ -322,7 +322,6 @@ export interface DocumentLinkProvider {
 
 export interface CodeLensSymbol {
     range: Range;
-    id?: string;
     command?: Command;
 }
 

packages/plugin-ext/src/plugin/command-registry.ts

@@ -19,6 +19,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as theia from '@theia/plugin';
+import * as model from '../common/plugin-api-rpc-model';
 import { CommandRegistryExt, PLUGIN_RPC_CONTEXT as Ext, CommandRegistryMain } from '../common/plugin-api-rpc';
 import { RPCProtocol } from '../common/rpc-protocol';
 import { Disposable } from './types-impl';
@@ -153,26 +154,47 @@ export class CommandsConverter {
     /**
      * Convert to a command that can be safely passed over JSON-RPC.
      */
-    toSafeCommand(command: theia.Command, disposables: DisposableCollection): theia.Command {
+    toSafeCommand(command: undefined, disposables: DisposableCollection): undefined;
+    toSafeCommand(command: theia.Command, disposables: DisposableCollection): model.Command;
+    toSafeCommand(command: theia.Command | undefined, disposables: DisposableCollection): model.Command | undefined;
+    toSafeCommand(command: theia.Command | undefined, disposables: DisposableCollection): model.Command | undefined {
+        if (!command) {
+            return undefined;
+        }
+        const result = this.toInternalCommand(command);
+        if (KnownCommands.mapped(result.id)) {
+            return result;
+        }
+
         if (!this.isSafeCommandRegistered) {
             this.commands.registerCommand({ id: this.safeCommandId }, this.executeSafeCommand, this);
             this.isSafeCommandRegistered = true;
         }
 
-        const result: theia.Command = {};
-        Object.assign(result, command);
-
         if (command.command && command.arguments && command.arguments.length > 0) {
             const id = this.handle++;
             this.commandsMap.set(id, command);
             disposables.push(new Disposable(() => this.commandsMap.delete(id)));
-            result.command = this.safeCommandId;
+            result.id = this.safeCommandId;
             result.arguments = [id];
         }
 
         return result;
     }
 
+    protected toInternalCommand(external: theia.Command): model.Command {
+        // we're deprecating Command.id, so it has to be optional.
+        // Existing code will have compiled against a non - optional version of the field, so asserting it to exist is ok
+        // tslint:disable-next-line: no-any
+        return KnownCommands.map((external.command || external.id)!, external.arguments, (mappedId: string, mappedArgs: any[]) =>
+            ({
+                id: mappedId,
+                title: external.title || external.label || ' ',
+                tooltip: external.tooltip,
+                arguments: mappedArgs
+            }));
+    }
+
     // tslint:disable-next-line:no-any
     private executeSafeCommand<R>(...args: any[]): PromiseLike<R | undefined> {
         const command = this.commandsMap.get(args[0]);

packages/plugin-ext/src/plugin/languages.ts

@@ -78,6 +78,7 @@ import { FoldingProviderAdapter } from './languages/folding';
 import { ColorProviderAdapter } from './languages/color';
 import { RenameAdapter } from './languages/rename';
 import { Event } from '@theia/core/lib/common/event';
+import { CommandRegistryImpl } from './command-registry';
 
 type Adapter = CompletionAdapter |
     SignatureHelpAdapter |
@@ -109,7 +110,10 @@ export class LanguagesExtImpl implements LanguagesExt {
     private callId = 0;
     private adaptersMap = new Map<number, Adapter>();
 
-    constructor(rpc: RPCProtocol, private readonly documents: DocumentsExtImpl) {
+    constructor(
+        rpc: RPCProtocol,
+        private readonly documents: DocumentsExtImpl,
+        private readonly commands: CommandRegistryImpl) {
         this.proxy = rpc.getProxy(PLUGIN_RPC_CONTEXT.LANGUAGES_MAIN);
         this.diagnostics = new Diagnostics(rpc);
     }
@@ -225,7 +229,7 @@ export class LanguagesExtImpl implements LanguagesExt {
     }
 
     registerCompletionItemProvider(selector: theia.DocumentSelector, provider: theia.CompletionItemProvider, triggerCharacters: string[]): theia.Disposable {
-        const callId = this.addNewAdapter(new CompletionAdapter(provider, this.documents));
+        const callId = this.addNewAdapter(new CompletionAdapter(provider, this.documents, this.commands));
         this.proxy.$registerCompletionSupport(callId, this.transformDocumentSelector(selector), triggerCharacters, CompletionAdapter.hasResolveSupport(provider));
         return this.createDisposable(callId);
     }
@@ -395,7 +399,7 @@ export class LanguagesExtImpl implements LanguagesExt {
         pluginModel: PluginModel,
         metadata?: theia.CodeActionProviderMetadata
     ): theia.Disposable {
-        const callId = this.addNewAdapter(new CodeActionAdapter(provider, this.documents, this.diagnostics, pluginModel ? pluginModel.id : ''));
+        const callId = this.addNewAdapter(new CodeActionAdapter(provider, this.documents, this.diagnostics, pluginModel ? pluginModel.id : '', this.commands));
         this.proxy.$registerQuickFixProvider(
             callId,
             this.transformDocumentSelector(selector),
@@ -416,7 +420,7 @@ export class LanguagesExtImpl implements LanguagesExt {
 
     // ### Code Lens Provider begin
     registerCodeLensProvider(selector: theia.DocumentSelector, provider: theia.CodeLensProvider): theia.Disposable {
-        const callId = this.addNewAdapter(new CodeLensAdapter(provider, this.documents));
+        const callId = this.addNewAdapter(new CodeLensAdapter(provider, this.documents, this.commands));
         const eventHandle = typeof provider.onDidChangeCodeLenses === 'function' ? this.nextCallId() : undefined;
         this.proxy.$registerCodeLensSupport(callId, this.transformDocumentSelector(selector), eventHandle);
         let result = this.createDisposable(callId);

packages/plugin-ext/src/plugin/languages/code-action.ts

@@ -22,14 +22,17 @@ import * as Converter from '../type-converters';
 import { DocumentsExtImpl } from '../documents';
 import { Diagnostics } from './diagnostics';
 import { CodeActionKind } from '../types-impl';
+import { CommandRegistryImpl } from '../command-registry';
+import { DisposableCollection } from '@theia/core/lib/common/disposable';
 
 export class CodeActionAdapter {
 
     constructor(
         private readonly provider: theia.CodeActionProvider,
         private readonly document: DocumentsExtImpl,
         private readonly diagnostics: Diagnostics,
-        private readonly pluginId: string
+        private readonly pluginId: string,
+        private readonly commands: CommandRegistryImpl
     ) { }
 
     provideCodeAction(resource: URI, rangeOrSelection: Range | Selection,
@@ -60,6 +63,8 @@ export class CodeActionAdapter {
             if (!Array.isArray(commandsOrActions) || commandsOrActions.length === 0) {
                 return undefined!;
             }
+            // TODO cache toDispose and dispose it
+            const toDispose = new DisposableCollection();
             const result: monaco.languages.CodeAction[] = [];
             for (const candidate of commandsOrActions) {
                 if (!candidate) {
@@ -68,7 +73,7 @@ export class CodeActionAdapter {
                 if (CodeActionAdapter._isCommand(candidate)) {
                     result.push({
                         title: candidate.title || '',
-                        command: Converter.toInternalCommand(candidate)
+                        command: this.commands.converter.toSafeCommand(candidate, toDispose)
                     });
                 } else {
                     if (codeActionContext.only) {
@@ -83,7 +88,7 @@ export class CodeActionAdapter {
 
                     result.push({
                         title: candidate.title,
-                        command: candidate.command && Converter.toInternalCommand(candidate.command),
+                        command: this.commands.converter.toSafeCommand(candidate.command, toDispose),
                         diagnostics: candidate.diagnostics && candidate.diagnostics.map(Converter.convertDiagnosticToMarkerData) as monaco.editor.IMarker[],
                         edit: candidate.edit && Converter.fromWorkspaceEdit(candidate.edit) as monaco.languages.WorkspaceEdit,
                         kind: candidate.kind && candidate.kind.value

packages/plugin-ext/src/plugin/languages/completion.ts

@@ -22,15 +22,19 @@ import * as Converter from '../type-converters';
 import { mixin } from '../../common/types';
 import { Position } from '../../common/plugin-api-rpc';
 import { CompletionContext, CompletionResultDto, Completion, CompletionDto } from '../../common/plugin-api-rpc-model';
+import { CommandRegistryImpl } from '../command-registry';
+import { DisposableCollection } from '@theia/core/lib/common/disposable';
 
 export class CompletionAdapter {
     private cacheId = 0;
-    private cache = new Map<number, theia.CompletionItem[]>();
+    private readonly cache = new Map<number, theia.CompletionItem[]>();
+    private readonly disposables = new Map<number, DisposableCollection>();
 
-    constructor(private readonly delegate: theia.CompletionItemProvider,
-        private readonly documents: DocumentsExtImpl) {
-
-    }
+    constructor(
+        private readonly delegate: theia.CompletionItemProvider,
+        private readonly documents: DocumentsExtImpl,
+        private readonly commands: CommandRegistryImpl
+    ) { }
 
     provideCompletionItems(resource: URI, position: Position, context: CompletionContext, token: theia.CancellationToken): Promise<CompletionResultDto | undefined> {
         const document = this.documents.getDocumentData(resource);
@@ -43,6 +47,10 @@ export class CompletionAdapter {
         const pos = Converter.toPosition(position);
         return Promise.resolve(this.delegate.provideCompletionItems(doc, pos, token, context)).then(value => {
             const id = this.cacheId++;
+
+            const toDispose = new DisposableCollection();
+            this.disposables.set(id, toDispose);
+
             const result: CompletionResultDto = {
                 id,
                 completions: [],
@@ -102,9 +110,13 @@ export class CompletionAdapter {
         });
     }
 
-    releaseCompletionItems(id: number): Promise<void> {
+    async releaseCompletionItems(id: number): Promise<void> {
         this.cache.delete(id);
-        return Promise.resolve();
+        const toDispose = this.disposables.get(id);
+        if (toDispose) {
+            toDispose.dispose();
+            this.disposables.delete(id);
+        }
     }
 
     private convertCompletionItem(item: theia.CompletionItem, position: theia.Position, defaultRange: theia.Range, id: number, parentId: number): CompletionDto | undefined {
@@ -113,6 +125,11 @@ export class CompletionAdapter {
             return undefined;
         }
 
+        const toDispose = this.disposables.get(id);
+        if (!toDispose) {
+            throw Error('DisposableCollection is missing...');
+        }
+
         const result: CompletionDto = {
             id,
             parentId,
@@ -125,7 +142,7 @@ export class CompletionAdapter {
             preselect: item.preselect,
             insertText: '',
             additionalTextEdits: item.additionalTextEdits && item.additionalTextEdits.map(Converter.fromTextEdit),
-            command: undefined,   // TODO: implement this: this.commands.toInternal(item.command),
+            command: this.commands.converter.toSafeCommand(item.command, toDispose),
             commitCharacters: item.commitCharacters
         };
 

packages/plugin-ext/src/plugin/languages/lens.ts

@@ -20,18 +20,22 @@ import { DocumentsExtImpl } from '../documents';
 import { CodeLensSymbol } from '../../common/plugin-api-rpc-model';
 import * as Converter from '../type-converters';
 import { ObjectIdentifier } from '../../common/object-identifier';
+import { CommandRegistryImpl } from '../command-registry';
+import { DisposableCollection } from '@theia/core/lib/common/disposable';
 
 /** Adapts the calls from main to extension thread for providing/resolving the code lenses. */
 export class CodeLensAdapter {
 
     private static readonly BAD_CMD: theia.Command = { command: 'missing', title: '<<MISSING COMMAND>>' };
 
     private cacheId = 0;
-    private cache = new Map<number, theia.CodeLens>();
+    private readonly cache = new Map<number, theia.CodeLens>();
+    private readonly disposables = new Map<number, DisposableCollection>();
 
     constructor(
         private readonly provider: theia.CodeLensProvider,
         private readonly documents: DocumentsExtImpl,
+        private readonly commands: CommandRegistryImpl
     ) { }
 
     provideCodeLenses(resource: URI, token: theia.CancellationToken): Promise<CodeLensSymbol[] | undefined> {
@@ -45,36 +49,44 @@ export class CodeLensAdapter {
         return Promise.resolve(this.provider.provideCodeLenses(doc, token)).then(lenses => {
             if (Array.isArray(lenses)) {
                 return lenses.map(lens => {
-                    const id = this.cacheId++;
+                    const cacheId = this.cacheId++;
+                    const toDispose = new DisposableCollection();
                     const lensSymbol = ObjectIdentifier.mixin({
                         range: Converter.fromRange(lens.range)!,
-                        command: lens.command ? Converter.toInternalCommand(lens.command) : undefined
-                    }, id);
-                    this.cache.set(id, lens);
+                        command: this.commands.converter.toSafeCommand(lens.command, toDispose)
+                    }, cacheId);
+                    // TODO: invalidate caches and dispose command handlers
+                    this.cache.set(cacheId, lens);
+                    this.disposables.set(cacheId, toDispose);
                     return lensSymbol;
                 });
             }
             return undefined;
         });
     }
 
-    resolveCodeLens(resource: URI, symbol: CodeLensSymbol, token: theia.CancellationToken): Promise<CodeLensSymbol | undefined> {
-        const lens = this.cache.get(ObjectIdentifier.of(symbol));
+    async resolveCodeLens(resource: URI, symbol: CodeLensSymbol, token: theia.CancellationToken): Promise<CodeLensSymbol | undefined> {
+        const cacheId = ObjectIdentifier.of(symbol);
+        const lens = this.cache.get(cacheId);
         if (!lens) {
-            return Promise.resolve(undefined);
+            return undefined;
         }
 
-        let resolve: Promise<theia.CodeLens | undefined>;
-        if (typeof this.provider.resolveCodeLens !== 'function' || lens.isResolved) {
-            resolve = Promise.resolve(lens);
-        } else {
-            resolve = Promise.resolve(this.provider.resolveCodeLens(lens, token));
+        let newLens: theia.CodeLens | undefined;
+        if (typeof this.provider.resolveCodeLens === 'function' && !lens.isResolved) {
+            newLens = await this.provider.resolveCodeLens(lens, token);
+            if (token.isCancellationRequested) {
+                return undefined;
+            }
         }
+        newLens = newLens || lens;
 
-        return resolve.then(newLens => {
-            newLens = newLens || lens;
-            symbol.command = Converter.toInternalCommand(newLens.command ? newLens.command : CodeLensAdapter.BAD_CMD);
-            return symbol;
-        });
+        const disposables = this.disposables.get(cacheId);
+        if (!disposables) {
+            // already been disposed of
+            return undefined;
+        }
+        symbol.command = this.commands.converter.toSafeCommand(newLens.command ? newLens.command : CodeLensAdapter.BAD_CMD, disposables);
+        return symbol;
     }
 }

packages/plugin-ext/src/plugin/plugin-context.ts

@@ -160,7 +160,7 @@ export function createAPIFactory(
     const statusBarMessageRegistryExt = new StatusBarMessageRegistryExt(rpc);
     const terminalExt = rpc.set(MAIN_RPC_CONTEXT.TERMINAL_EXT, new TerminalServiceExtImpl(rpc));
     const outputChannelRegistryExt = new OutputChannelRegistryExt(rpc);
-    const languagesExt = rpc.set(MAIN_RPC_CONTEXT.LANGUAGES_EXT, new LanguagesExtImpl(rpc, documents));
+    const languagesExt = rpc.set(MAIN_RPC_CONTEXT.LANGUAGES_EXT, new LanguagesExtImpl(rpc, documents, commandRegistry));
     const treeViewsExt = rpc.set(MAIN_RPC_CONTEXT.TREE_VIEWS_EXT, new TreeViewsExtImpl(rpc, commandRegistry));
     const webviewExt = rpc.set(MAIN_RPC_CONTEXT.WEBVIEWS_EXT, new WebviewsExtImpl(rpc));
     const tasksExt = rpc.set(MAIN_RPC_CONTEXT.TASKS_EXT, new TasksExtImpl(rpc));