From f8785f80214be4fc18876476ee3863ea406e7ffb Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 6 Sep 2024 16:04:32 -0700 Subject: [PATCH] debug: fix some unsoundness in type convertsions in the ext host (#227846) Refs #211878 --- .../api/common/extHostDebugService.ts | 90 +++++++++++-------- .../workbench/api/node/extHostDebugService.ts | 37 ++++---- .../workbench/contrib/debug/common/debug.ts | 1 - 3 files changed, 69 insertions(+), 59 deletions(-) diff --git a/src/vs/workbench/api/common/extHostDebugService.ts b/src/vs/workbench/api/common/extHostDebugService.ts index d072857559cb4..fe33d966e66ea 100644 --- a/src/vs/workbench/api/common/extHostDebugService.ts +++ b/src/vs/workbench/api/common/extHostDebugService.ts @@ -3,34 +3,34 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import type * as vscode from 'vscode'; +import { coalesce } from '../../../base/common/arrays.js'; import { asPromise } from '../../../base/common/async.js'; import { CancellationToken } from '../../../base/common/cancellation.js'; import { Emitter, Event } from '../../../base/common/event.js'; -import { URI, UriComponents } from '../../../base/common/uri.js'; import { Disposable as DisposableCls, toDisposable } from '../../../base/common/lifecycle.js'; +import { ThemeIcon as ThemeIconUtils } from '../../../base/common/themables.js'; +import { URI, UriComponents } from '../../../base/common/uri.js'; import { ExtensionIdentifier, IExtensionDescription } from '../../../platform/extensions/common/extensions.js'; import { createDecorator } from '../../../platform/instantiation/common/instantiation.js'; import { ISignService } from '../../../platform/sign/common/sign.js'; import { IWorkspaceFolder } from '../../../platform/workspace/common/workspace.js'; -import { DebugSessionUUID, ExtHostDebugServiceShape, IBreakpointsDeltaDto, IThreadFocusDto, IStackFrameFocusDto, IDebugSessionDto, IFunctionBreakpointDto, ISourceMultiBreakpointDto, MainContext, MainThreadDebugServiceShape } from './extHost.protocol.js'; -import { IExtHostEditorTabs } from './extHostEditorTabs.js'; -import { IExtHostExtensionService } from './extHostExtensionService.js'; -import { IExtHostRpcService } from './extHostRpcService.js'; -import { Breakpoint, DataBreakpoint, DebugAdapterExecutable, DebugAdapterInlineImplementation, DebugAdapterNamedPipeServer, DebugAdapterServer, DebugConsoleMode, Disposable, FunctionBreakpoint, Location, Position, setBreakpointId, SourceBreakpoint, DebugThread, DebugStackFrame, ThemeIcon } from './extHostTypes.js'; -import { IExtHostWorkspace } from './extHostWorkspace.js'; import { AbstractDebugAdapter } from '../../contrib/debug/common/abstractDebugAdapter.js'; -import { MainThreadDebugVisualization, IAdapterDescriptor, IConfig, IDebugAdapter, IDebugAdapterExecutable, IDebugAdapterNamedPipeServer, IDebugAdapterServer, IDebugVisualization, IDebugVisualizationContext, IDebuggerContribution, DebugVisualizationType, IDebugVisualizationTreeItem } from '../../contrib/debug/common/debug.js'; +import { DebugVisualizationType, IAdapterDescriptor, IConfig, IDebugAdapter, IDebugAdapterExecutable, IDebugAdapterImpl, IDebugAdapterNamedPipeServer, IDebugAdapterServer, IDebuggerContribution, IDebugVisualization, IDebugVisualizationContext, IDebugVisualizationTreeItem, MainThreadDebugVisualization } from '../../contrib/debug/common/debug.js'; import { convertToDAPaths, convertToVSCPaths, isDebuggerMainContribution } from '../../contrib/debug/common/debugUtils.js'; import { ExtensionDescriptionRegistry } from '../../services/extensions/common/extensionDescriptionRegistry.js'; import { Dto } from '../../services/extensions/common/proxyIdentifier.js'; -import type * as vscode from 'vscode'; -import { IExtHostConfiguration } from './extHostConfiguration.js'; -import { IExtHostVariableResolverProvider } from './extHostVariableResolverService.js'; -import { ThemeIcon as ThemeIconUtils } from '../../../base/common/themables.js'; +import { DebugSessionUUID, ExtHostDebugServiceShape, IBreakpointsDeltaDto, IDebugSessionDto, IFunctionBreakpointDto, ISourceMultiBreakpointDto, IStackFrameFocusDto, IThreadFocusDto, MainContext, MainThreadDebugServiceShape } from './extHost.protocol.js'; import { IExtHostCommands } from './extHostCommands.js'; -import * as Convert from './extHostTypeConverters.js'; -import { coalesce } from '../../../base/common/arrays.js'; +import { IExtHostConfiguration } from './extHostConfiguration.js'; +import { IExtHostEditorTabs } from './extHostEditorTabs.js'; +import { IExtHostExtensionService } from './extHostExtensionService.js'; +import { IExtHostRpcService } from './extHostRpcService.js'; import { IExtHostTesting } from './extHostTesting.js'; +import * as Convert from './extHostTypeConverters.js'; +import { Breakpoint, DataBreakpoint, DebugAdapterExecutable, DebugAdapterInlineImplementation, DebugAdapterNamedPipeServer, DebugAdapterServer, DebugConsoleMode, DebugStackFrame, DebugThread, Disposable, FunctionBreakpoint, Location, Position, setBreakpointId, SourceBreakpoint, ThemeIcon } from './extHostTypes.js'; +import { IExtHostVariableResolverProvider } from './extHostVariableResolverService.js'; +import { IExtHostWorkspace } from './extHostWorkspace.js'; export const IExtHostDebugService = createDecorator('IExtHostDebugService'); @@ -578,8 +578,8 @@ export abstract class ExtHostDebugServiceBase extends DisposableCls implements I return variableResolver.resolveAnyAsync(ws, config); } - protected createDebugAdapter(adapter: IAdapterDescriptor, session: ExtHostDebugSession): AbstractDebugAdapter | undefined { - if (adapter.type === 'implementation') { + protected createDebugAdapter(adapter: vscode.DebugAdapterDescriptor, session: ExtHostDebugSession): AbstractDebugAdapter | undefined { + if (adapter instanceof DebugAdapterInlineImplementation) { return new DirectDebugAdapter(adapter.implementation); } return undefined; @@ -600,9 +600,7 @@ export abstract class ExtHostDebugServiceBase extends DisposableCls implements I throw new Error(`Couldn't find a debug adapter descriptor for debug type '${session.type}' (extension might have failed to activate)`); } - const adapterDescriptor = this.convertToDto(daDescriptor); - - const da = this.createDebugAdapter(adapterDescriptor, session); + const da = this.createDebugAdapter(daDescriptor, session); if (!da) { throw new Error(`Couldn't create a debug adapter for type '${session.type}'.`); } @@ -891,35 +889,49 @@ export abstract class ExtHostDebugServiceBase extends DisposableCls implements I // private & dto helpers private convertToDto(x: vscode.DebugAdapterDescriptor): Dto { - if (x instanceof DebugAdapterExecutable) { - return { - type: 'executable', - command: x.command, - args: x.args, - options: x.options - } satisfies IDebugAdapterExecutable; + return this.convertExecutableToDto(x); } else if (x instanceof DebugAdapterServer) { - return { - type: 'server', - port: x.port, - host: x.host - } satisfies IDebugAdapterServer; + return this.convertServerToDto(x); } else if (x instanceof DebugAdapterNamedPipeServer) { - return { - type: 'pipeServer', - path: x.path - } satisfies IDebugAdapterNamedPipeServer; + return this.convertPipeServerToDto(x); } else if (x instanceof DebugAdapterInlineImplementation) { - return { - type: 'implementation', - implementation: x.implementation - } as Dto; + return this.convertImplementationToDto(x); } else { throw new Error('convertToDto unexpected type'); } } + protected convertExecutableToDto(x: DebugAdapterExecutable): IDebugAdapterExecutable { + return { + type: 'executable', + command: x.command, + args: x.args, + options: x.options + }; + } + + protected convertServerToDto(x: DebugAdapterServer): IDebugAdapterServer { + return { + type: 'server', + port: x.port, + host: x.host + }; + } + + protected convertPipeServerToDto(x: DebugAdapterNamedPipeServer): IDebugAdapterNamedPipeServer { + return { + type: 'pipeServer', + path: x.path + }; + } + + protected convertImplementationToDto(x: DebugAdapterInlineImplementation): IDebugAdapterImpl { + return { + type: 'implementation', + }; + } + private getAdapterDescriptorFactoryByType(type: string): vscode.DebugAdapterDescriptorFactory | undefined { const results = this._adapterFactories.filter(p => p.type === type); if (results.length > 0) { diff --git a/src/vs/workbench/api/node/extHostDebugService.ts b/src/vs/workbench/api/node/extHostDebugService.ts index ed9f4f688260a..33e1b4d34ac94 100644 --- a/src/vs/workbench/api/node/extHostDebugService.ts +++ b/src/vs/workbench/api/node/extHostDebugService.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import type * as vscode from 'vscode'; import { createCancelablePromise, firstParallel, timeout } from '../../../base/common/async.js'; import { IDisposable } from '../../../base/common/lifecycle.js'; import * as platform from '../../../base/common/platform.js'; @@ -11,23 +12,21 @@ import { IExternalTerminalService } from '../../../platform/externalTerminal/com import { LinuxExternalTerminalService, MacExternalTerminalService, WindowsExternalTerminalService } from '../../../platform/externalTerminal/node/externalTerminalService.js'; import { ISignService } from '../../../platform/sign/common/sign.js'; import { SignService } from '../../../platform/sign/node/signService.js'; +import { AbstractDebugAdapter } from '../../contrib/debug/common/abstractDebugAdapter.js'; +import { ExecutableDebugAdapter, NamedPipeDebugAdapter, SocketDebugAdapter } from '../../contrib/debug/node/debugAdapter.js'; +import { hasChildProcesses, prepareCommand } from '../../contrib/debug/node/terminals.js'; +import { ExtensionDescriptionRegistry } from '../../services/extensions/common/extensionDescriptionRegistry.js'; +import { IExtHostCommands } from '../common/extHostCommands.js'; +import { ExtHostConfigProvider, IExtHostConfiguration } from '../common/extHostConfiguration.js'; import { ExtHostDebugServiceBase, ExtHostDebugSession } from '../common/extHostDebugService.js'; import { IExtHostEditorTabs } from '../common/extHostEditorTabs.js'; import { IExtHostExtensionService } from '../common/extHostExtensionService.js'; import { IExtHostRpcService } from '../common/extHostRpcService.js'; import { IExtHostTerminalService } from '../common/extHostTerminalService.js'; -import { DebugAdapterExecutable, ThemeIcon } from '../common/extHostTypes.js'; +import { IExtHostTesting } from '../common/extHostTesting.js'; +import { DebugAdapterExecutable, DebugAdapterNamedPipeServer, DebugAdapterServer, ThemeIcon } from '../common/extHostTypes.js'; import { IExtHostVariableResolverProvider } from '../common/extHostVariableResolverService.js'; import { IExtHostWorkspace } from '../common/extHostWorkspace.js'; -import { AbstractDebugAdapter } from '../../contrib/debug/common/abstractDebugAdapter.js'; -import { IAdapterDescriptor } from '../../contrib/debug/common/debug.js'; -import { ExecutableDebugAdapter, NamedPipeDebugAdapter, SocketDebugAdapter } from '../../contrib/debug/node/debugAdapter.js'; -import { hasChildProcesses, prepareCommand } from '../../contrib/debug/node/terminals.js'; -import { ExtensionDescriptionRegistry } from '../../services/extensions/common/extensionDescriptionRegistry.js'; -import type * as vscode from 'vscode'; -import { ExtHostConfigProvider, IExtHostConfiguration } from '../common/extHostConfiguration.js'; -import { IExtHostCommands } from '../common/extHostCommands.js'; -import { IExtHostTesting } from '../common/extHostTesting.js'; export class ExtHostDebugService extends ExtHostDebugServiceBase { @@ -50,16 +49,16 @@ export class ExtHostDebugService extends ExtHostDebugServiceBase { super(extHostRpcService, workspaceService, extensionService, configurationService, editorTabs, variableResolver, commands, testing); } - protected override createDebugAdapter(adapter: IAdapterDescriptor, session: ExtHostDebugSession): AbstractDebugAdapter | undefined { - switch (adapter.type) { - case 'server': - return new SocketDebugAdapter(adapter); - case 'pipeServer': - return new NamedPipeDebugAdapter(adapter); - case 'executable': - return new ExecutableDebugAdapter(adapter, session.type); + protected override createDebugAdapter(adapter: vscode.DebugAdapterDescriptor, session: ExtHostDebugSession): AbstractDebugAdapter | undefined { + if (adapter instanceof DebugAdapterExecutable) { + return new ExecutableDebugAdapter(this.convertExecutableToDto(adapter), session.type); + } else if (adapter instanceof DebugAdapterServer) { + return new SocketDebugAdapter(this.convertServerToDto(adapter)); + } else if (adapter instanceof DebugAdapterNamedPipeServer) { + return new NamedPipeDebugAdapter(this.convertPipeServerToDto(adapter)); + } else { + return super.createDebugAdapter(adapter, session); } - return super.createDebugAdapter(adapter, session); } protected override daExecutableFromPackage(session: ExtHostDebugSession, extensionRegistry: ExtensionDescriptionRegistry): DebugAdapterExecutable | undefined { diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index 798dcc4e79784..cc6138857e7c7 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -911,7 +911,6 @@ export interface IDebugAdapterInlineImpl extends IDisposable { export interface IDebugAdapterImpl { readonly type: 'implementation'; - readonly implementation: IDebugAdapterInlineImpl; } export type IAdapterDescriptor = IDebugAdapterExecutable | IDebugAdapterServer | IDebugAdapterNamedPipeServer | IDebugAdapterImpl;