Skip to content

Commit 33697c0

Browse files
authored
Always quote Python executable path (#964)
* Always quote Python executable path On Windows, this would cause pahts like "C:\Program Files\" to fail launching the Python command as the blank space was not escaped properly. Add also a unit test to validate this change. Signed-off-by: Francesco Giancane <me@fgiancane8.dev> * stringUtils.ts: remove call to .replace() method for paths There is no need anymore to replace backslashes with forward slashes for paths on Windows, as the Python extension's `toCommandArgumentForPythonExt` function already handles this correctly. Signed-off-by: Francesco Giancane <me@fgiancane8.dev> * stringUtils.ts: restore replace call when normalizing string * remoteLaunchers: ensure test is normalizing paths Explicitly call `fileToCommandArgumentForPythonExt` when computing a path with spaces and check results are as expected. * stringUtils.ts: replace backslashes with forward-slashes only on non-Windows platforms. * Revert "remoteLaunchers: ensure test is normalizing paths" This reverts commit 32822e0. * remoteLaunchers.ts: feed platform-specific data to tests Use backward slashes on Windows and forward slashes on Unix-like OSes when testing correct behavior of path strings quotation. * Copy-paste error in comment. * remoteLaunchers.unit.test.ts: fix formatting issues --------- Signed-off-by: Francesco Giancane <me@fgiancane8.dev>
1 parent e0ec60c commit 33697c0

File tree

4 files changed

+41
-5
lines changed

4 files changed

+41
-5
lines changed

src/extension/common/stringUtils.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
import { getOSType, OSType } from './platform';
5+
46
/**
57
* Replaces all instances of a substring with a new substring.
68
*/
@@ -58,5 +60,12 @@ export function fileToCommandArgumentForPythonExt(source: string): string {
5860
if (!source) {
5961
return source;
6062
}
61-
return toCommandArgumentForPythonExt(source).replace(/\\/g, '/');
63+
64+
let result = toCommandArgumentForPythonExt(source);
65+
66+
if (getOSType() !== OSType.Windows) {
67+
result = result.replace(/\\/g, '/');
68+
}
69+
70+
return result;
6271
}

src/extension/debugger/adapter/factory.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,20 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
8080

8181
let executable = command.shift() ?? 'python';
8282

83+
// Always ensure interpreter/command is quoted if necessary. Previously this was
84+
// only done in the debugAdapterPath branch which meant that in the common case
85+
// (using the built‑in adapter path) an interpreter path containing spaces would
86+
// be passed unquoted, resulting in a fork/spawn failure on Windows. See bug
87+
// report for details.
88+
executable = fileToCommandArgumentForPythonExt(executable);
89+
8390
// "logToFile" is not handled directly by the adapter - instead, we need to pass
8491
// the corresponding CLI switch when spawning it.
8592
const logArgs = configuration.logToFile ? ['--log-dir', EXTENSION_ROOT_DIR] : [];
8693

8794
if (configuration.debugAdapterPath !== undefined) {
8895
const args = command.concat([configuration.debugAdapterPath, ...logArgs]);
8996
traceLog(`DAP Server launched with command: ${executable} ${args.join(' ')}`);
90-
executable = fileToCommandArgumentForPythonExt(executable);
9197
return new DebugAdapterExecutable(executable, args);
9298
}
9399

src/test/unittest/adapter/factory.unit.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,23 @@ suite('Debugging - Adapter Factory', () => {
304304

305305
assert.deepStrictEqual(descriptor, debugExecutable);
306306
});
307-
test('Add quotes to interpreter path with spaces', async () => {
307+
test('Add quotes to interpreter path with spaces (default adapter path)', async () => {
308+
const session = createSession({});
309+
const interpreterPathSpaces = 'path/to/python interpreter with spaces';
310+
const interpreterPathSpacesQuoted = `"${interpreterPathSpaces}"`;
311+
const debugExecutable = new DebugAdapterExecutable(interpreterPathSpacesQuoted, [debugAdapterPath]);
312+
313+
getInterpreterDetailsStub.resolves({ path: [interpreterPathSpaces] });
314+
const interpreterSpacePath: PythonEnvironment = createInterpreter(interpreterPathSpaces, '3.7.4-test');
315+
// Add architecture for completeness.
316+
(interpreterSpacePath as any).architecture = Architecture.Unknown;
317+
resolveEnvironmentStub.withArgs(interpreterPathSpaces).resolves(interpreterSpacePath);
318+
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
319+
320+
assert.deepStrictEqual(descriptor, debugExecutable);
321+
});
322+
323+
test('Add quotes to interpreter path with spaces when debugAdapterPath is specified', async () => {
308324
const customAdapterPath = 'custom/debug/adapter/customAdapterPath';
309325
const session = createSession({ debugAdapterPath: customAdapterPath });
310326
const interpreterPathSpaces = 'path/to/python interpreter with spaces';

src/test/unittest/adapter/remoteLaunchers.unit.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,23 @@ import * as path from 'path';
88
import { EXTENSION_ROOT_DIR } from '../../../extension/common/constants';
99
import '../../../extension/common/promiseUtils';
1010
import * as launchers from '../../../extension/debugger/adapter/remoteLaunchers';
11+
import { getOSType, OSType } from '../../../extension/common/platform';
12+
13+
function osExpectedPath(windowsPath: string, unixPath: string): string {
14+
return getOSType() === OSType.Windows ? windowsPath : unixPath;
15+
}
1116

1217
suite('External debugpy Debugger Launcher', () => {
1318
[
1419
{
1520
testName: 'When path to debugpy does not contains spaces',
1621
path: path.join('path', 'to', 'debugpy'),
17-
expectedPath: 'path/to/debugpy',
22+
expectedPath: osExpectedPath('path\\to\\debugpy', 'path/to/debugpy'),
1823
},
1924
{
2025
testName: 'When path to debugpy contains spaces',
2126
path: path.join('path', 'to', 'debugpy', 'with spaces'),
22-
expectedPath: '"path/to/debugpy/with spaces"',
27+
expectedPath: osExpectedPath('"path\\to\\debugpy\\with spaces"', '"path/to/debugpy/with spaces"'),
2328
},
2429
].forEach((testParams) => {
2530
suite(testParams.testName, async () => {

0 commit comments

Comments
 (0)