Skip to content

Ensure string prototypes extension extends are unique enough #18870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/client/common/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ declare interface String {
* Appropriately formats a string so it can be used as an argument for a command in a shell.
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
*/
toCommandArgument(): string;
toCommandArgumentForPythonExt(): string;
/**
* Appropriately formats a a file path so it can be used as an argument for a command in a shell.
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
*/
fileToCommandArgument(): string;
fileToCommandArgumentForPythonExt(): string;
/**
* String.format() implementation.
* Tokens such as {0}, {1} will be replaced with corresponding positional arguments.
Expand Down Expand Up @@ -69,7 +69,7 @@ String.prototype.splitLines = function (
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
* @param {String} value.
*/
String.prototype.toCommandArgument = function (this: string): string {
String.prototype.toCommandArgumentForPythonExt = function (this: string): string {
if (!this) {
return this;
}
Expand All @@ -82,11 +82,11 @@ String.prototype.toCommandArgument = function (this: string): string {
* Appropriately formats a a file path so it can be used as an argument for a command in a shell.
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
*/
String.prototype.fileToCommandArgument = function (this: string): string {
String.prototype.fileToCommandArgumentForPythonExt = function (this: string): string {
if (!this) {
return this;
}
return this.toCommandArgument().replace(/\\/g, '/');
return this.toCommandArgumentForPythonExt().replace(/\\/g, '/');
};

/**
Expand Down
4 changes: 2 additions & 2 deletions src/client/common/installer/condaInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ export class CondaInstaller extends ModuleInstaller {
if (info && info.name) {
// If we have the name of the conda environment, then use that.
args.push('--name');
args.push(info.name.toCommandArgument());
args.push(info.name.toCommandArgumentForPythonExt());
} else if (info && info.path) {
// Else provide the full path to the environment path.
args.push('--prefix');
args.push(info.path.fileToCommandArgument());
args.push(info.path.fileToCommandArgumentForPythonExt());
}
if (flags & ModuleInstallFlags.updateDependencies) {
args.push('--update-deps');
Expand Down
3 changes: 2 additions & 1 deletion src/client/common/installer/moduleInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ export abstract class ModuleInstaller implements IModuleInstaller {
const argv = [command, ...args];
// Concat these together to make a set of quoted strings
const quoted = argv.reduce(
(p, c) => (p ? `${p} ${c.toCommandArgument()}` : `${c.toCommandArgument()}`),
(p, c) =>
p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`,
'',
);
await processService.shellExec(quoted);
Expand Down
6 changes: 3 additions & 3 deletions src/client/common/process/internal/scripts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function normalizeSelection(): [string[], (out: string) => string] {
// printEnvVariables.py

export function printEnvVariables(): [string[], (out: string) => NodeJS.ProcessEnv] {
const script = path.join(SCRIPTS_DIR, 'printEnvVariables.py').fileToCommandArgument();
const script = path.join(SCRIPTS_DIR, 'printEnvVariables.py').fileToCommandArgumentForPythonExt();
const args = [script];

function parse(out: string): NodeJS.ProcessEnv {
Expand All @@ -113,11 +113,11 @@ export function shell_exec(command: string, lockfile: string, shellArgs: string[
// could be anything.
return [
script,
command.fileToCommandArgument(),
command.fileToCommandArgumentForPythonExt(),
// The shell args must come after the command
// but before the lockfile.
...shellArgs,
lockfile.fileToCommandArgument(),
lockfile.fileToCommandArgumentForPythonExt(),
];
}

Expand Down
2 changes: 1 addition & 1 deletion src/client/common/process/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class ProcessLogger implements IProcessLogger {
return;
}
let command = args
? [fileOrCommand, ...args].map((e) => e.trimQuotes().toCommandArgument()).join(' ')
? [fileOrCommand, ...args].map((e) => e.trimQuotes().toCommandArgumentForPythonExt()).join(' ')
: fileOrCommand;
const info = [`> ${this.getDisplayCommands(command)}`];
if (options && options.cwd) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ export class Bash extends VenvBaseActivationCommandProvider {
if (!scriptFile) {
return;
}
return [`source ${scriptFile.fileToCommandArgument()}`];
return [`source ${scriptFile.fileToCommandArgumentForPythonExt()}`];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ export class CommandPromptAndPowerShell extends VenvBaseActivationCommandProvide
}

if (targetShell === TerminalShellType.commandPrompt && scriptFile.endsWith('activate.bat')) {
return [scriptFile.fileToCommandArgument()];
return [scriptFile.fileToCommandArgumentForPythonExt()];
} else if (
(targetShell === TerminalShellType.powershell || targetShell === TerminalShellType.powershellCore) &&
scriptFile.endsWith('Activate.ps1')
) {
return [`& ${scriptFile.fileToCommandArgument()}`];
return [`& ${scriptFile.fileToCommandArgumentForPythonExt()}`];
} else if (targetShell === TerminalShellType.commandPrompt && scriptFile.endsWith('Activate.ps1')) {
// lets not try to run the powershell file from command prompt (user may not have powershell)
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,11 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
const interpreterPath = await this.condaService.getInterpreterPathForEnvironment(envInfo);
const condaPath = await this.condaService.getCondaFileFromInterpreter(interpreterPath, envInfo.name);
if (condaPath) {
const activatePath = path.join(path.dirname(condaPath), 'activate').fileToCommandArgument();
const activatePath = path
.join(path.dirname(condaPath), 'activate')
.fileToCommandArgumentForPythonExt();
const firstActivate = this.platform.isWindows ? activatePath : `source ${activatePath}`;
return [firstActivate, `conda activate ${condaEnv.toCommandArgument()}`];
return [firstActivate, `conda activate ${condaEnv.toCommandArgumentForPythonExt()}`];
}
}
}
Expand Down Expand Up @@ -116,15 +118,15 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
const condaScriptsPath: string = path.dirname(condaExePath);
// prefix the cmd with the found path, and ensure it's quoted properly
activateCmd = path.join(condaScriptsPath, activateCmd);
activateCmd = activateCmd.toCommandArgument();
activateCmd = activateCmd.toCommandArgumentForPythonExt();
}

return activateCmd;
}

public async getWindowsCommands(condaEnv: string): Promise<string[] | undefined> {
const activate = await this.getWindowsActivateCommand();
return [`${activate} ${condaEnv.toCommandArgument()}`];
return [`${activate} ${condaEnv.toCommandArgumentForPythonExt()}`];
}
}

Expand All @@ -135,16 +137,16 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
* Extension will not attempt to work around issues by trying to setup shell for user.
*/
export async function _getPowershellCommands(condaEnv: string): Promise<string[] | undefined> {
return [`conda activate ${condaEnv.toCommandArgument()}`];
return [`conda activate ${condaEnv.toCommandArgumentForPythonExt()}`];
}

async function getFishCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
// https://github.com/conda/conda/blob/be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe/shell/etc/fish/conf.d/conda.fish#L18-L28
return [`${condaFile.fileToCommandArgument()} activate ${condaEnv.toCommandArgument()}`];
return [`${condaFile.fileToCommandArgumentForPythonExt()} activate ${condaEnv.toCommandArgumentForPythonExt()}`];
}

async function getUnixCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
const condaDir = path.dirname(condaFile);
const activateFile = path.join(condaDir, 'activate');
return [`source ${activateFile.fileToCommandArgument()} ${condaEnv.toCommandArgument()}`];
return [`source ${activateFile.fileToCommandArgumentForPythonExt()} ${condaEnv.toCommandArgumentForPythonExt()}`];
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma
}
}
const execName = this.pipEnvExecution.executable;
return [`${execName.fileToCommandArgument()} shell`];
return [`${execName.fileToCommandArgumentForPythonExt()} shell`];
}

public async getActivationCommandsForInterpreter(pythonPath: string): Promise<string[] | undefined> {
Expand All @@ -51,6 +51,6 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma
}

const execName = this.pipEnvExecution.executable;
return [`${execName.fileToCommandArgument()} shell`];
return [`${execName.fileToCommandArgumentForPythonExt()} shell`];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class PyEnvActivationCommandProvider implements ITerminalActivationComman
return;
}

return [`pyenv shell ${interpreter.envName.toCommandArgument()}`];
return [`pyenv shell ${interpreter.envName.toCommandArgumentForPythonExt()}`];
}

public async getActivationCommandsForInterpreter(
Expand All @@ -40,6 +40,6 @@ export class PyEnvActivationCommandProvider implements ITerminalActivationComman
return;
}

return [`pyenv shell ${interpreter.envName.toCommandArgument()}`];
return [`pyenv shell ${interpreter.envName.toCommandArgumentForPythonExt()}`];
}
}
4 changes: 2 additions & 2 deletions src/client/common/terminal/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ export class TerminalHelper implements ITerminalHelper {
terminalShellType === TerminalShellType.powershell ||
terminalShellType === TerminalShellType.powershellCore;
const commandPrefix = isPowershell ? '& ' : '';
const formattedArgs = args.map((a) => a.toCommandArgument());
const formattedArgs = args.map((a) => a.toCommandArgumentForPythonExt());

return `${commandPrefix}${command.fileToCommandArgument()} ${formattedArgs.join(' ')}`.trim();
return `${commandPrefix}${command.fileToCommandArgumentForPythonExt()} ${formattedArgs.join(' ')}`.trim();
}
public async getEnvironmentActivationCommands(
terminalShellType: TerminalShellType,
Expand Down
7 changes: 6 additions & 1 deletion src/client/debugger/extension/adapter/remoteLaunchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ type RemoteDebugOptions = {

export function getDebugpyLauncherArgs(options: RemoteDebugOptions, debuggerPath: string = pathToDebugger) {
const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait-for-client'] : [];
return [debuggerPath.fileToCommandArgument(), '--listen', `${options.host}:${options.port}`, ...waitArgs];
return [
debuggerPath.fileToCommandArgumentForPythonExt(),
'--listen',
`${options.host}:${options.port}`,
...waitArgs,
];
}

export function getDebugpyPackagePath(): string {
Expand Down
4 changes: 2 additions & 2 deletions src/client/interpreter/activation/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
let command: string | undefined;
let [args, parse] = internalScripts.printEnvVariables();
args.forEach((arg, i) => {
args[i] = arg.toCommandArgument();
args[i] = arg.toCommandArgumentForPythonExt();
});
interpreter = interpreter ?? (await this.interpreterService.getActiveInterpreter(resource));
if (interpreter?.envType === EnvironmentType.Conda) {
Expand All @@ -185,7 +185,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
});
if (pythonArgv) {
// Using environment prefix isn't needed as the marker script already takes care of it.
command = [...pythonArgv, ...args].map((arg) => arg.toCommandArgument()).join(' ');
command = [...pythonArgv, ...args].map((arg) => arg.toCommandArgumentForPythonExt()).join(' ');
}
}
if (!command) {
Expand Down
5 changes: 4 additions & 1 deletion src/client/pythonEnvironments/base/info/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ export async function getInterpreterInfo(
const argv = [info.command, ...info.args];

// Concat these together to make a set of quoted strings
const quoted = argv.reduce((p, c) => (p ? `${p} ${c.toCommandArgument()}` : `${c.toCommandArgument()}`), '');
const quoted = argv.reduce(
(p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
'',
);

// Try shell execing the command, followed by the arguments. This will make node kill the process if it
// takes too long.
Expand Down
5 changes: 4 additions & 1 deletion src/client/pythonEnvironments/info/executable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ export async function getExecutablePath(
const info = copyPythonExecInfo(python, args);
const argv = [info.command, ...info.args];
// Concat these together to make a set of quoted strings
const quoted = argv.reduce((p, c) => (p ? `${p} ${c.toCommandArgument()}` : `${c.toCommandArgument()}`), '');
const quoted = argv.reduce(
(p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
'',
);
const result = await shellExec(quoted, { timeout: timeout ?? 15000 });
const executable = parse(result.stdout.trim());
if (executable === '') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
const workspaceRoot = workspaceUri ? workspaceUri.uri.fsPath : defaultWorkspace;
const managePyPath = workspaceRoot.length === 0 ? 'manage.py' : path.join(workspaceRoot, 'manage.py');

return copyPythonExecInfo(info, [managePyPath.fileToCommandArgument(), 'shell']);
return copyPythonExecInfo(info, [managePyPath.fileToCommandArgumentForPythonExt(), 'shell']);
}

public async getExecuteFileArgs(resource?: Uri, executeArgs: string[] = []): Promise<PythonExecInfo> {
Expand Down
6 changes: 4 additions & 2 deletions src/client/terminals/codeExecution/terminalCodeExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {

public async executeFile(file: Uri) {
await this.setCwdForFileExecution(file);
const { command, args } = await this.getExecuteFileArgs(file, [file.fsPath.fileToCommandArgument()]);
const x = file.fsPath;
const hello = x.fileToCommandArgumentForPythonExt();
const { command, args } = await this.getExecuteFileArgs(file, [hello]);

await this.getTerminalService(file).sendCommand(command, args);
}
Expand Down Expand Up @@ -106,7 +108,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
await this.getTerminalService(file).sendText(`${fileDrive}:`);
}
}
await this.getTerminalService(file).sendText(`cd ${fileDirPath.fileToCommandArgument()}`);
await this.getTerminalService(file).sendText(`cd ${fileDirPath.fileToCommandArgumentForPythonExt()}`);
}
}
}
8 changes: 6 additions & 2 deletions src/test/api.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ suite('Extension API', () => {
instance(serviceManager),
instance(serviceContainer),
).debug.getRemoteLauncherCommand(debuggerHost, debuggerPort, waitForAttach);
const expectedArgs = [debuggerPath.fileToCommandArgument(), '--listen', `${debuggerHost}:${debuggerPort}`];
const expectedArgs = [
debuggerPath.fileToCommandArgumentForPythonExt(),
'--listen',
`${debuggerHost}:${debuggerPort}`,
];

expect(args).to.be.deep.equal(expectedArgs);
});
Expand All @@ -98,7 +102,7 @@ suite('Extension API', () => {
instance(serviceContainer),
).debug.getRemoteLauncherCommand(debuggerHost, debuggerPort, waitForAttach);
const expectedArgs = [
debuggerPath.fileToCommandArgument(),
debuggerPath.fileToCommandArgumentForPythonExt(),
'--listen',
`${debuggerHost}:${debuggerPort}`,
'--wait-for-client',
Expand Down
22 changes: 11 additions & 11 deletions src/test/common/extensions.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,47 @@ import { asyncFilter } from '../../client/common/utils/arrayUtils';
suite('String Extensions', () => {
test('Should return empty string for empty arg', () => {
const argTotest = '';
expect(argTotest.toCommandArgument()).to.be.equal('');
expect(argTotest.toCommandArgumentForPythonExt()).to.be.equal('');
});
test('Should quote an empty space', () => {
const argTotest = ' ';
expect(argTotest.toCommandArgument()).to.be.equal('" "');
expect(argTotest.toCommandArgumentForPythonExt()).to.be.equal('" "');
});
test('Should not quote command arguments without spaces', () => {
const argTotest = 'one.two.three';
expect(argTotest.toCommandArgument()).to.be.equal(argTotest);
expect(argTotest.toCommandArgumentForPythonExt()).to.be.equal(argTotest);
});
test('Should quote command arguments with spaces', () => {
const argTotest = 'one two three';
expect(argTotest.toCommandArgument()).to.be.equal(`"${argTotest}"`);
expect(argTotest.toCommandArgumentForPythonExt()).to.be.equal(`"${argTotest}"`);
});
test('Should quote command arguments containing ampersand', () => {
const argTotest = 'one&twothree';
expect(argTotest.toCommandArgument()).to.be.equal(`"${argTotest}"`);
expect(argTotest.toCommandArgumentForPythonExt()).to.be.equal(`"${argTotest}"`);
});
test('Should return empty string for empty path', () => {
const fileToTest = '';
expect(fileToTest.fileToCommandArgument()).to.be.equal('');
expect(fileToTest.fileToCommandArgumentForPythonExt()).to.be.equal('');
});
test('Should not quote file argument without spaces', () => {
const fileToTest = 'users/test/one';
expect(fileToTest.fileToCommandArgument()).to.be.equal(fileToTest);
expect(fileToTest.fileToCommandArgumentForPythonExt()).to.be.equal(fileToTest);
});
test('Should quote file argument with spaces', () => {
const fileToTest = 'one two three';
expect(fileToTest.fileToCommandArgument()).to.be.equal(`"${fileToTest}"`);
expect(fileToTest.fileToCommandArgumentForPythonExt()).to.be.equal(`"${fileToTest}"`);
});
test('Should replace all back slashes with forward slashes (irrespective of OS)', () => {
const fileToTest = 'c:\\users\\user\\conda\\scripts\\python.exe';
expect(fileToTest.fileToCommandArgument()).to.be.equal(fileToTest.replace(/\\/g, '/'));
expect(fileToTest.fileToCommandArgumentForPythonExt()).to.be.equal(fileToTest.replace(/\\/g, '/'));
});
test('Should replace all back slashes with forward slashes (irrespective of OS) and quoted when file has spaces', () => {
const fileToTest = 'c:\\users\\user namne\\conda path\\scripts\\python.exe';
expect(fileToTest.fileToCommandArgument()).to.be.equal(`"${fileToTest.replace(/\\/g, '/')}"`);
expect(fileToTest.fileToCommandArgumentForPythonExt()).to.be.equal(`"${fileToTest.replace(/\\/g, '/')}"`);
});
test('Should replace all back slashes with forward slashes (irrespective of OS) and quoted when file has spaces', () => {
const fileToTest = 'c:\\users\\user namne\\conda path\\scripts\\python.exe';
expect(fileToTest.fileToCommandArgument()).to.be.equal(`"${fileToTest.replace(/\\/g, '/')}"`);
expect(fileToTest.fileToCommandArgumentForPythonExt()).to.be.equal(`"${fileToTest.replace(/\\/g, '/')}"`);
});
test('Should leave string unchanged', () => {
expect('something {0}'.format()).to.be.equal('something {0}');
Expand Down
Loading