Skip to content

Commit 5982fb0

Browse files
authored
Ensure uses of isolate script in combination with sendCommand is correctly quoted (#11810)
* Revert "Ensure isolate script is passed as command arg when installing modules (#11447)" This reverts commit b2a0b59. * Format args in buildCommandForTerminal * Typos
1 parent 66ef1b9 commit 5982fb0

File tree

7 files changed

+24
-20
lines changed

7 files changed

+24
-20
lines changed

src/client/common/installer/moduleInstaller.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export abstract class ModuleInstaller implements IModuleInstaller {
4545
? await interpreterService.getActiveInterpreter(resource)
4646
: resource;
4747
const pythonPath = isResource(resource) ? settings.pythonPath : resource.path;
48-
const args = internalPython.execModule(executionInfo.moduleName, executionInfoArgs, true, true);
48+
const args = internalPython.execModule(executionInfo.moduleName, executionInfoArgs);
4949
if (!interpreter || interpreter.type !== InterpreterType.Unknown) {
5050
await terminalService.sendCommand(pythonPath, args, token);
5151
} else if (settings.globalModuleInstallation) {

src/client/common/process/internal/python.ts

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

4-
import '../../extensions';
54
import { _ISOLATED as ISOLATED } from './scripts';
65

76
// "python" contains functions corresponding to the various ways that
@@ -26,15 +25,10 @@ export function execCode(code: string, isolated = true): string[] {
2625
return args;
2726
}
2827

29-
export function execModule(
30-
name: string,
31-
moduleArgs: string[],
32-
isolated = true,
33-
useAsCommandArgument = false
34-
): string[] {
28+
export function execModule(name: string, moduleArgs: string[], isolated = true): string[] {
3529
const args = ['-m', name, ...moduleArgs];
3630
if (isolated) {
37-
args[0] = useAsCommandArgument ? ISOLATED.fileToCommandArgument() : ISOLATED; // replace
31+
args[0] = ISOLATED; // replace
3832
}
3933
// "code" isn't specific enough to know how to parse it,
4034
// so we only return the args.

src/client/common/terminal/helper.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ export class TerminalHelper implements ITerminalHelper {
6161
terminalShellType === TerminalShellType.powershell ||
6262
terminalShellType === TerminalShellType.powershellCore;
6363
const commandPrefix = isPowershell ? '& ' : '';
64-
return `${commandPrefix}${command.fileToCommandArgument()} ${args.join(' ')}`.trim();
64+
const formattedArgs = args.map((a) => a.toCommandArgument());
65+
66+
return `${commandPrefix}${command.fileToCommandArgument()} ${formattedArgs.join(' ')}`.trim();
6567
}
6668
public async getEnvironmentActivationCommands(
6769
terminalShellType: TerminalShellType,

src/client/interpreter/locators/services/currentPathService.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ export class CurrentPathService extends CacheableLocatorService {
9393
private async getInterpreter(options: { command: string; args?: string[] }) {
9494
try {
9595
const processService = await this.processServiceFactory.create();
96+
const pyArgs = Array.isArray(options.args) ? options.args : [];
9697
const [args, parse] = internalPython.getExecutable();
97-
const pyArgs = Array.isArray(options.args) ? options.args.concat(args) : args;
9898
return processService
99-
.exec(options.command, pyArgs, {})
99+
.exec(options.command, pyArgs.concat(args), {})
100100
.then((output) => parse(output.stdout))
101101
.then(async (value) => {
102102
if (value.length > 0 && (await this.fs.fileExists(value))) {

src/test/common/installer/moduleInstaller.unit.test.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
} from 'vscode';
2323
import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types';
2424
import { STANDARD_OUTPUT_CHANNEL } from '../../../client/common/constants';
25-
import '../../../client/common/extensions';
2625
import { CondaInstaller } from '../../../client/common/installer/condaInstaller';
2726
import { ModuleInstaller } from '../../../client/common/installer/moduleInstaller';
2827
import { PipEnvInstaller, pipenvName } from '../../../client/common/installer/pipEnvInstaller';
@@ -52,9 +51,7 @@ import {
5251
import { IServiceContainer } from '../../../client/ioc/types';
5352
import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants';
5453

55-
const isolated = path
56-
.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py')
57-
.fileToCommandArgument();
54+
const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py');
5855

5956
/* Complex test to ensure we cover all combinations:
6057
We could have written separate tests for each installer, but we'd be replicate code.

src/test/common/moduleInstaller.test.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import { ConfigurationService } from '../../client/common/configuration/service'
3737
import { CryptoUtils } from '../../client/common/crypto';
3838
import { EditorUtils } from '../../client/common/editor';
3939
import { ExperimentsManager } from '../../client/common/experiments';
40-
import '../../client/common/extensions';
4140
import { FeatureDeprecationManager } from '../../client/common/featureDeprecationManager';
4241
import {
4342
ExtensionInsidersDailyChannelRule,
@@ -144,9 +143,7 @@ import { closeActiveWindows, initializeTest } from './../initialize';
144143

145144
chai_use(chaiAsPromised);
146145

147-
const isolated = path
148-
.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py')
149-
.fileToCommandArgument();
146+
const isolated = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'pythonFiles', 'pyvsc-run-isolated.py');
150147

151148
const info: PythonInterpreter = {
152149
architecture: Architecture.Unknown,

src/test/common/terminals/helper.unit.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,20 @@ suite('Terminal Service helpers', () => {
124124
expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell ${item.name}`);
125125
});
126126
});
127+
test('Ensure spaces in args are quoted', async () => {
128+
getNamesAndValues<TerminalShellType>(TerminalShellType).forEach((item) => {
129+
const command = 'python3.7.exe';
130+
const args = ['a file.py', '1', '2'];
131+
const commandPrefix =
132+
item.value === TerminalShellType.powershell || item.value === TerminalShellType.powershellCore
133+
? '& '
134+
: '';
135+
const expectedTerminalCommand = `${commandPrefix}${command} "a file.py" 1 2`;
136+
137+
const terminalCommand = helper.buildCommandForTerminal(item.value, command, args);
138+
expect(terminalCommand).to.equal(expectedTerminalCommand, `Incorrect command for Shell ${item.name}`);
139+
});
140+
});
127141

128142
test('Ensure empty args are ignored', async () => {
129143
getNamesAndValues<TerminalShellType>(TerminalShellType).forEach((item) => {

0 commit comments

Comments
 (0)