Autoescaping characters in launch.json args

[abhijeetviswa]

Issue Type: Bug

• VSCode Version: code-1.45.1-1
• OS Version: Manjaro Linux
• Shell: zsh

Steps to Reproduce:

1. Create a new workspace
2. Create a launch.json with type as python.
3. In the args add an element like '$test' or '\\$test'.
4. The argument passed to will be '\$test\' or '\\$test' respectively.

It seems the problem is that special characters are escaped. I've scoured the internet for a solution to prevent this from happening but I've not found one. I'm assuming this to be a bug no one has ever encountered so far since I didn't see any issues here either.

In case it is not obvious, I would like to pass the string $test directly to the application. Neither does surrounding with single quote work, nor does trying to escape $ (inside the args list i.e)

Does this issue occur when all extensions are disabled?: Yes

Thanks.

[weinand]

I cannot reproduce the problem:

In addition, please be aware that the '$hello' is substituted by the shell that is used to launch python (that's the reason why $hello doesn't show up in the output because the shell has substituted it to the empty string).

[abhijeetviswa]

Oh. I'm sorry. I probably wasn't clear enough. $hello is passed perfectly. What I want to pass is the string $hello to python itself (without the shell substituting it). I can't escape the $ using single quotes nor can I add a backslash. Here are some pics.

EDIT: In the second image, I meant to use the argument \\$hello. I missed the $. However, the parameter that's passed is still \\$hello, when it in fact should only be a single .

[weinand]

Thanks for the additional info. Now I see the problem.
The bug is that an arg $hello is passed to the shell command line without escaping the $.
VS Code should pass it as \$hello or '$hello' to avoid substitution in the shell.
So a user should never have to escape because the intermediate shell is an implementation detail (and you never know what you get). On the other hand this means that it is not possible to have the $hello be interpreted by the shell.

[abhijeetviswa]

I believe it might be best to pass it raw without escaping anything. Let the users escape it manually in the args list.

[weinand]

The user doesn't know whether a shell is used at all. So how should she know what escaping is needed.

[ZigZagT]

Hey @weinand I'm facing the same issue. In my case it's the [ and ] characters. This is quite common for people using next.js. Can I expect VSCode to automatically escape these characters as well as the '"s anytime soon?

[weinand]

@BananaWanted please provide an example for how you use next.js from the command line.

[ZigZagT]

@weinand
Here's the launch.json config I'm using:

    {
      "type": "node",
      "name": "jest (current file)",
      "request": "launch",
      "args": ["--runInBand", "--runTestsByPath", "--no-coverage", "'${relativeFile}'"],
      "cwd": "${workspaceFolder}",
      "console": "integratedTerminal",
      "internalConsoleOptions": "neverOpen",
      "timeout": 100000,
      "program": "${workspaceFolder}/node_modules/.bin/jest"
}

command ran by vscode:

cd my_workspace; /usr/local/bin/node --inspect-brk=20152 node_modules/.bin/jest --runInBand --runTestsByPath --no-coverage \'path_to_next_app/pages_tests/[dynamic_route_param]/test.js\' 

# output: zsh: no matches found: 'path_to_next_app/pages_tests/[dynamic_route_param]/test.js'

Expected command to run:

# I manually wrapped the path with `''`. This worked since a recent update. Sorry I dont' know which update changed the behavior
cd my_workspace; /usr/local/bin/node --inspect-brk=20152 node_modules/.bin/jest --runInBand --runTestsByPath --no-coverage 'path_to_next_app/pages_tests/[dynamic_route_param]/test.js' 

[weinand]

@BananaWanted how do you make ${relativeFile} evaluate to some path with square brackets? Do you really have a folder named [dynamic_route_param]?

Why do you need to quote ${relativeFile} with single quotes at all?

For me the square brackets are left untouched without quoting:

[ZigZagT]

Hi @weinand thanks for looking into this issue.

The square brackets are literally part of the file name. In this case, the folder is exactly named [dynamic_route_param]. According to the next.js team they chose the [] character base on cross-filesystem compatibility considerations.

[] is used for shell's pattern matching
. AFAIK both bash and zsh use [] for pattern matching. So in order to pass the correct path to the launched command, they need to be quoted. Before VSCode doing auto escaping I added the '' marks around ${relativeFile} to workaround this issue.

Leaving [] arguments untouched while escaping quotation marks is unwanted behavior. This makes there's no way to ever escape those characters in the shell.

To fix this, VSCode should also escape them whenever it escapes quotation marks (' and "), or VSCode can also choose not to do any escaping at all and just leave it to the user to decide.

[weinand]

@BananaWanted thanks for the explanation.
My bash does not seem to interpret square brackets:

... but we will consider to add square brackets to the list of escaped characters (but not for this milestone).

[int19h]

My apologies for a belated follow-up!

This issue has an interesting history. Originally, our implementation of the debug adapter propagated debuggee arguments via command line, and thus VSCode's lack of escaping resulted in shell expansion for them. It was not really an intended decision on our part, but our users have discovered it, and started using it as such, as we later found out.

When we rewrote the debug adapter, one of the changes was argument handling - now they were propagated as raw JSON from the "launch" request via a socket. This disabled shell escaping, and we got user complaints about this, e.g. microsoft/debugpy#86 (also an example of a scenario that this feature inadvertently enabled).

Since the change was technically breaking, and we knew that we had users relying on past behavior, we re-implemented argument passing again. But because we already ran into a scenario where shell quoting was not desirable, this time it was made configurable - by default, we reverted back to old behavior of using the shell to pass them, but users could say "argsExpansion": "none" in launch.json to circumvent the shell.

Unfortunately, this change on VSCode side effectively broke our setting, since now it doesn't matter anymore whether the arguments are passed via the shell or not. Which also means that the bug linked above has to be re-opened, since it has been regressed - but we no longer have the means to fix it.

I think it's reasonable to not have shell expansion by default - it can be confusing to users who are unfamiliar with it, and are just trying to pass a string to their app, not to mention being shell-specific; and, on the other hand, the scenarios where it's useful are advanced ones, and so the user can be reasonably expected to know (or read the docs to find out) that they need to enable it. But as it is, we're basically breaking a bunch of launch.json setups that worked fine before, and for which there's no different way to do what they were doing - there are no workarounds here, so far as I can see.

So, my suggestion is to provide some way to reflect this configurability in the "runInTerminal" request, so that debug adapters can make use of it to request escaping if needed, and surface it in their "launch" request schema. It would be even better if it can be done on an argument-by-argument basis, instead of the setup that we had, where the setting applied to all arguments at once. If it all goes into "args", it would be easier for debug adapters to use, as well, since they could just marshal the config setting as is (i.e. wouldn't have to figure out the syntax and semantics for their own setting), with only such additions as the adapter needs to launch the debuggee.

[int19h]

With respect to "runInTerminal" shell being an implementation detail - I don't think users see it that way. They see the terminal and the shell during launch, and they know that it uses the default shell they configured in the settings, so there's no obvious reason as to why access to features of that shell is so restricted.

If the launch command were passed to the shell as an argument (bash -c ... etc), instead of "typed" as input in the terminal, such that the command is not visible at all, I think that would truly make the shell an implementation detail, and lower those expectations. That would also mean that the terminal automatically exits once debug session is over, however. From our perspective, this is actually preferable, because those leftover debug terminals end up confusing the users (they often don't even notice that it's a new terminal, different from the one they were using before for manual CLI work, and just continue to use it as such).