Use replaceAll when replacing separators

[karthiknadig]

See here:
https://github.com/microsoft/vscode-python/security/code-scanning/10?query=ref%3Arefs%2Fheads%2Fmain

Issue Location:
https://github.com/microsoft/vscode-python/blob/2021.1.502429796/src/client/pythonEnvironments/base/info/interpreter.ts
https://github.com/microsoft/vscode-python/blob/2021.1.502429796/src/client/pythonEnvironments/info/interpreter.ts

[KamalSinghKhanna]

Hi, I would like to work on this issue,can you suggest me ways to get started?

[karthiknadig]

@KamalSinghKhanna go ahead

[vijaya-lakshmi-venkatraman]

Hello,
I would like to pick this issue if still available.

So the required changes are :

1. In /pythonEnvironments/base/info/interpreter.ts (Line 73)

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

2. In /pythonEnvironments/info/interpreter.ts (Line 60)

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

Is that right?

[karthiknadig]

@vijaya-lakshmi-venkatraman That is correct.

Edit: You may have to add the replaceAll API to the string prototype. src\client\common\extensions.ts

[vijaya-lakshmi-venkatraman]

Thanks @karthiknadig.
I would like to take this up if @KamalSinghKhanna is not working on this anymore.

[karthiknadig]

@vijaya-lakshmi-venkatraman go ahead.

[KamalSinghKhanna]

Is this issue still open or @vijaya-lakshmi-venkatraman is still working on this issue

[karthiknadig]

One thing to note. there is no replaceAll API. There are two ways one could do this. Either extend string prototype to add a replaceAll. Or use pattern matching to replace all.

if you decide to extend string, you can do that here src\client\common\extensions.ts . Then all you have to do is include it in that file where you consume replaceAll

[IceJinx33]

Hi, is this issue still open or is @KamalSinghKhanna or @vijaya-lakshmi-venkatraman working on this? If not could I try working on this?

[KamalSinghKhanna]

Hi @IceJinx33 , feel free to work on this.

[IceJinx33]

@karthiknadig, The original Code QL issue link link doesn't seem to be working. Where can I verify the correct requirements of the code?

[karthiknadig]

@IceJinx33 This is what the CodeQL link says:

[IceJinx33]

@karthiknadig, I extended the string in src\client\common\extensions.ts to have a replaceAll function, which simply uses a RegExp global search 'g' to replace an old substring with a new one. However, the old substring needs to be escaped properly to get a correct RegExp, and the escaping function from MDN Web Docs escapes a limited set of special characters.

replaceAll is used in the following files for replacing '\\' with '\\\\'
https://github.com/microsoft/vscode-python/blob/2021.1.502429796/src/client/pythonEnvironments/base/info/interpreter.ts
https://github.com/microsoft/vscode-python/blob/2021.1.502429796/src/client/pythonEnvironments/info/interpreter.ts

In the extensions.unit.tests.ts, I added the following tests for the above use case and they pass when tested using npm run test:unittests -- --grep='String Extensions' after compilation.

Are there any test cases I could be missing? I'm also not really sure if the escaping function covers all cases, so is the way I've documented it okay? Or should I think of a different approach?

[karthiknadig]

@IceJinx33 This is fantastic. Can you open a PR.