Repository navigation
Changed behaviour for leading double quotes during command calls #18614
Description
Prerequisites
- Write a descriptive title.
- Make sure you are able to repro it on the latest released version
- Search the existing issues.
- Refer to the FAQ.
- Refer to Differences between Windows PowerShell 5.1 and PowerShell.
Steps to reproduce
The behaviour of string inputs during command calls seems to have changed from PowerShell Core 7.2.6 (and also Powershell 5.1!) to version 7.3.0.
While this might be partially as designed, it should at least be noted as a breaking change (with an explanation what exactly changed and in which scenarious this will take effect ), which it is currently not.
One example would be to use double quotes for a parameter, which could be argued is wrong anyways. But we had a script that used a parameter that way and it broke after the update.
Example:
ping "www.google.com" works in both versions
ping ""www.google.com"" works only up to PowerShell Core 7.2.6 (and Powershell 5.1) but no longer in 7.3.0 (this is also true for even more quotes in the command)
The error message generated is:
Bad parameter www.google.com.
Another would be if you use git command rev-list with an empty string in a variable used for the "from" branch in the "from"..."to" parameters. In these examples (note: I'm using git version 2.33.1.windows.1) you first have to change into a git repository, change to the master branch and define variable $TargetBranch as being an empty string.
So...
- Change into git repository
git checkout master$TargetBranch = ""
then the following command works up to PowerShell Core 7.2.6 (and again Powershell 5.1) but no longer in 7.3.0:
git rev-list $TargetBranch..origin/master
Instead of no output, as it should be, the error message generated is:
fatal: ambiguous argument '': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
interestingly if you set $TargetBranch to $null ( $TargetBranch = $null ) it will work again. But again, it's a breaking change, as we used an empty string up to now for the $TargetBranch variable.
It seems to me that the handling of input parameters for commands has changed from maybe an actual object directly forwarded to the command to producing an actual string with quotes out of the parameter and passing it as a string to the command, but maybe I'm mistaken.
Expected behavior
Either same behaviour as up to PowerShell Core 7.2.6, or if these changes are by design than at least listening it as a breaking change here: https://learn.microsoft.com/en-us/powershell/scripting/whats-new/what-s-new-in-powershell-73?view=powershell-7.3#breaking-changes-and-improvements
Also a better explanation (than mine) what exactly changed and in which scenarious this will take effect would be nice (as I can only guess that something in the interpretation of string inputs with quotes changed during call of commands).Actual behavior
Changed behaviour of string input during command calls (for example double quotes will break a command parameter now, some parameters where no quotes are expected may also break a command if the parameter is an empty string like in the git example)Error details
For example...
`Bad parameter www.google.com.`
or
fatal: ambiguous argument '': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'according to the used example (see "Steps to reproduce")
### Environment data
```powershell
Name Value
---- -----
PSVersion 7.3.0
PSEdition Core
GitCommitId 7.3.0
OS Microsoft Windows 10.0.19044
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0
Visuals
No response
Activity
jhoneill commentedon Nov 18, 2022
This is a duplicate - see my comment here
#18568 (comment)
addendum Steve Lee (@SteveL-MSFT) while this is causing a problem and we don't have at least very clear information for people that this breaking change exists we probably should NOT close the issues that are being opened for it.
I think this one of those "between a rock and a hard place" things. The change fixes what is generally accepted as a bug. But anyone who was aware of the bug had worked around it and has code in place for it. Now that code breaks, and a script which needs to run on current pwsh (>= 7.3.0) and out-of-the-box PowerShell on Windows (5.1) needs to set the option for legacy. The fix (as it stands) may be more of a problem than the bug was
echalone commentedon Nov 18, 2022
Oh thank you! It's really not well documented, I wasn't able to find anything on it, now I got a better understanding :)
SteveL-MSFT commentedon Nov 21, 2022
I'll bring it up at next committee meeting what should be done here.
SteveL-MSFT commentedon Dec 14, 2022
Markus Szumovski (@echalone) the breaking change was intentional to resolve other issues like the ability to pass an empty parameter value to a native command and using
$nullis the correct way to indicate nothing should be passed. What is the real world use case forping ""google.com""?echalone commentedon Dec 14, 2022
The ping example was just an easy example for you to understand and recreate one aspect of the problem. However we had a case with double quotes in a script and the update broke the script.
The git example is a more real life example that broke yet another skript all our build processes depended on.
Yes it's clear by now that this breaking change was intentional, but you did in no way communicate this breaking change to us end users or what was going on. You guys have to imagine millions of skripts in the wild are depending on your PowerShell releases. Such things could bring a whole company to a standstill, so at least better communication and documentation of breaking changes, as well as putting more thought into potential risks, would be appreciated.
Users could have also changed that behavior before if they wished so, if I understood correctly, so maybe just better communicating that this possibility exists would have been a better idea than just changing its default behavior.
By the way you still have not released the new PowerShellGet version 3 or fixed the bug in version 2 either that would reinstall every updated script from the global scope to the user scope. You are not accepting pull requests any more for version 2 so I can't fix that bug for you but you're still shipping out the buggy version 2 with Powershell Core for years now. Maybe you can bring that up on a meeting too?
Here is the 3 year old pull request by me that would change one line and fix the problem once and for all:
PowerShell/PowerShellGetv2#592
SteveL-MSFT commentedon Jan 18, 2023
@PowerShell/powershell-committee discussed this. We agree that these use cases should all behave the same:
Where the double quote gets resolved and concatenated with any remaining characters and treated as a single token. In all these examples, a single argument
abcis passed to foo. The issue is in the parser such that double quotes within the string or at the end of the string do get resolved correctly as being part of the string. It's just that double quotes at the start finds the ending double quote and treats that token as finished and remaining characters becomes a new token. This last behavior we agreed was incorrect and should be fixed. We do have concerns of impact although it seems like bucket 3 so we request this to be an Experimental Feature to change that behavior.Note that contents within the double string will also get resolved and concatenated such that:
would resolve to
ab cas a single argument. Only whitespace outside of enclosed double quotes would indicate a new token/argument.[-]Changed behaviour for string inputs during command calls[/-][+]Changed behaviour for leading double quotes during command calls[/+]SteveL-MSFT commentedon Jan 20, 2023
As I was trying to figure out where in the tokenizer this is happening, it turns out to be a bit more complicated than expected. For example:
These are all valid as 3 tokens although it might not do what the user expected. However, a more real-world scenario might be:
In this case, this is 3 tokens redirecting an empty string to a file. The current tokenizer logic enables this and changing it would introduce risk. Special casing to alphanumeric and underscore would solve the original problem, but seems may be incomplete.
Jason Shirk (@lzybkr), Patrick Meinecke (@SeeminglyScience) , Dongbo Wang (@daxian-dbw) any suggestions here?
SteveL-MSFT commentedon Feb 2, 2023
Maybe a way to fix this would be in the parameter binder instead of the tokenizer
JamesWTruher commentedon Feb 3, 2023
it should be possible to inspect this token, and if it ends with a quote and there are no spaces between that token and the subsequent token they can be combined into a single string.
That would catch things like:
'a b'c,''c,"a${null}b"cmicrosoft-github-policy-service commentedon Nov 15, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 15, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 15, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 22, 2023
This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.