From 1c27e21dcf41e6435a025d94ada04cf40c9cecc8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Jun 2019 13:28:59 -0700 Subject: [PATCH 1/4] Disable quoting paths sent to the debugger as args --- news/2 Fixes/5861.md | 1 + src/client/api.ts | 4 +-- .../DebugClients/launcherProvider.ts | 19 +++++++++-- .../launcherProvider.unit.test.ts | 32 +++++++++++++++++-- 4 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 news/2 Fixes/5861.md diff --git a/news/2 Fixes/5861.md b/news/2 Fixes/5861.md new file mode 100644 index 000000000000..5bff0305615a --- /dev/null +++ b/news/2 Fixes/5861.md @@ -0,0 +1 @@ +Disable quoting of paths sent to the debugger as arguments. \ No newline at end of file diff --git a/src/client/api.ts b/src/client/api.ts index 71c857e6e903..177dc997397d 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -4,7 +4,7 @@ 'use strict'; import { traceError } from './common/logger'; -import { RemoteDebuggerLauncherScriptProvider } from './debugger/debugAdapter/DebugClients/launcherProvider'; +import { RemoteDebuggerExternalLauncherScriptProvider } from './debugger/debugAdapter/DebugClients/launcherProvider'; /* * Do not introduce any breaking changes to this API. @@ -42,7 +42,7 @@ export function buildApi(ready: Promise) { }), debug: { async getRemoteLauncherCommand(host: string, port: number, waitUntilDebuggerAttaches: boolean = true): Promise { - return new RemoteDebuggerLauncherScriptProvider().getLauncherArgs({ host, port, waitUntilDebuggerAttaches }); + return new RemoteDebuggerExternalLauncherScriptProvider().getLauncherArgs({ host, port, waitUntilDebuggerAttaches }); } } }; diff --git a/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts b/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts index 455db85cde63..32d8f794f372 100644 --- a/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts +++ b/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts @@ -15,7 +15,7 @@ export class NoDebugLauncherScriptProvider implements IDebugLauncherScriptProvid constructor(@optional() private script: string = pathToScript) { } public getLauncherArgs(options: LocalDebugOptions): string[] { const customDebugger = options.customDebugger ? '--custom' : '--default'; - return [this.script.fileToCommandArgument(), customDebugger, '--nodebug', '--client', '--host', options.host, '--port', options.port.toString()]; + return [this.script, customDebugger, '--nodebug', '--client', '--host', options.host, '--port', options.port.toString()]; } } @@ -23,11 +23,26 @@ export class DebuggerLauncherScriptProvider implements IDebugLauncherScriptProvi constructor(@optional() private script: string = pathToScript) { } public getLauncherArgs(options: LocalDebugOptions): string[] { const customDebugger = options.customDebugger ? '--custom' : '--default'; - return [this.script.fileToCommandArgument(), customDebugger, '--client', '--host', options.host, '--port', options.port.toString()]; + return [this.script, customDebugger, '--client', '--host', options.host, '--port', options.port.toString()]; } } export class RemoteDebuggerLauncherScriptProvider implements IRemoteDebugLauncherScriptProvider { + constructor(@optional() private script: string = pathToScript) { } + public getLauncherArgs(options: RemoteDebugOptions): string[] { + const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait'] : []; + return [this.script, '--default', '--host', options.host, '--port', options.port.toString()].concat(waitArgs); + } +} + +/** + * This class is used to provide the launch scripts so external code can launch the debugger. + * As we're passing command arguments, we need to ensure the file paths are quoted. + * @export + * @class RemoteDebuggerExternalLauncherScriptProvider + * @implements {IRemoteDebugLauncherScriptProvider} + */ +export class RemoteDebuggerExternalLauncherScriptProvider implements IRemoteDebugLauncherScriptProvider { constructor(@optional() private script: string = pathToScript) { } public getLauncherArgs(options: RemoteDebugOptions): string[] { const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait'] : []; diff --git a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts index a3f67e3c22dd..e60401e33712 100644 --- a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts +++ b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts @@ -7,7 +7,7 @@ import { expect } from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; import { EXTENSION_ROOT_DIR } from '../../../../client/common/constants'; -import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDebuggerLauncherScriptProvider } from '../../../../client/debugger/debugAdapter/DebugClients/launcherProvider'; +import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDebuggerLauncherScriptProvider, RemoteDebuggerExternalLauncherScriptProvider } from '../../../../client/debugger/debugAdapter/DebugClients/launcherProvider'; const expectedPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py'); @@ -26,7 +26,7 @@ suite('Debugger - Launcher Script Provider', () => { { testName: 'When path to ptvsd launcher contains spaces', path: path.join('path', 'to', 'ptvsd_launcher', 'with spaces'), - expectedPath: '"path/to/ptvsd_launcher/with spaces"' + expectedPath: 'path/to/ptvsd_launcher/with spaces' } ]; @@ -64,4 +64,32 @@ suite('Debugger - Launcher Script Provider', () => { }); }); }); + + suite('External Debug Launcher', () => + [ + { + testName: 'When path to ptvsd launcher does not contains spaces', + path: path.join('path', 'to', 'ptvsd_launcher'), + expectedPath: 'path/to/ptvsd_launcher' + }, + { + testName: 'When path to ptvsd launcher contains spaces', + path: path.join('path', 'to', 'ptvsd_launcher', 'with spaces'), + expectedPath: '"path/to/ptvsd_launcher/with spaces"' + } + ].forEach(testParams => { + suite(testParams.testName, async () => { + test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => { + const args = new RemoteDebuggerExternalLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false }); + const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234']; + expect(args).to.be.deep.equal(expectedArgs); + }); + test('Test remote debug launcher args (and wait for debugger to attach)', async () => { + const args = new RemoteDebuggerExternalLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true }); + const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait']; + expect(args).to.be.deep.equal(expectedArgs); + }); + }); + }); +}); }); From e4c177be15ae3c058ec485f5431e31b847d17db6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Jun 2019 13:35:50 -0700 Subject: [PATCH 2/4] Remove unnecessary class --- .../debugAdapter/DebugClients/launcherProvider.ts | 8 -------- .../debugClients/launcherProvider.unit.test.ts | 12 +----------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts b/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts index 32d8f794f372..466c1f1707fd 100644 --- a/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts +++ b/src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts @@ -27,14 +27,6 @@ export class DebuggerLauncherScriptProvider implements IDebugLauncherScriptProvi } } -export class RemoteDebuggerLauncherScriptProvider implements IRemoteDebugLauncherScriptProvider { - constructor(@optional() private script: string = pathToScript) { } - public getLauncherArgs(options: RemoteDebugOptions): string[] { - const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait'] : []; - return [this.script, '--default', '--host', options.host, '--port', options.port.toString()].concat(waitArgs); - } -} - /** * This class is used to provide the launch scripts so external code can launch the debugger. * As we're passing command arguments, we need to ensure the file paths are quoted. diff --git a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts index e60401e33712..1c3bb5b9233e 100644 --- a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts +++ b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts @@ -7,7 +7,7 @@ import { expect } from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; import { EXTENSION_ROOT_DIR } from '../../../../client/common/constants'; -import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDebuggerLauncherScriptProvider, RemoteDebuggerExternalLauncherScriptProvider } from '../../../../client/debugger/debugAdapter/DebugClients/launcherProvider'; +import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDebuggerExternalLauncherScriptProvider } from '../../../../client/debugger/debugAdapter/DebugClients/launcherProvider'; const expectedPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py'); @@ -52,16 +52,6 @@ suite('Debugger - Launcher Script Provider', () => { const expectedArgs = [testParams.expectedPath, '--custom', '--nodebug', '--client', '--host', 'something', '--port', '1234']; expect(args).to.be.deep.equal(expectedArgs); }); - test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => { - const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false }); - const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234']; - expect(args).to.be.deep.equal(expectedArgs); - }); - test('Test remote debug launcher args (and wait for debugger to attach)', async () => { - const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true }); - const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait']; - expect(args).to.be.deep.equal(expectedArgs); - }); }); }); From 4e2b8785be67048c56e5a13c4e19f7990ca9bb04 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Jun 2019 13:36:42 -0700 Subject: [PATCH 3/4] Fix typo --- .../debugAdapter/debugClients/launcherProvider.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts index 1c3bb5b9233e..e8bbac63606b 100644 --- a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts +++ b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts @@ -55,7 +55,7 @@ suite('Debugger - Launcher Script Provider', () => { }); }); - suite('External Debug Launcher', () => + suite('External Debug Launcher', () => { [ { testName: 'When path to ptvsd launcher does not contains spaces', @@ -81,5 +81,5 @@ suite('Debugger - Launcher Script Provider', () => { }); }); }); -}); + }); }); From 2766704241d11e171a3690a5621eccdcc185e2bd Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Jun 2019 13:55:10 -0700 Subject: [PATCH 4/4] Fixes to tests --- .../debugAdapter/debugClients/launcherProvider.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts index e8bbac63606b..8aeeed217b0a 100644 --- a/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts +++ b/src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts @@ -21,12 +21,12 @@ suite('Debugger - Launcher Script Provider', () => { { testName: 'When path to ptvsd launcher does not contains spaces', path: path.join('path', 'to', 'ptvsd_launcher'), - expectedPath: 'path/to/ptvsd_launcher' + expectedPath: path.join('path', 'to', 'ptvsd_launcher') }, { testName: 'When path to ptvsd launcher contains spaces', path: path.join('path', 'to', 'ptvsd_launcher', 'with spaces'), - expectedPath: 'path/to/ptvsd_launcher/with spaces' + expectedPath: path.join('path', 'to', 'ptvsd_launcher', 'with spaces') } ];