Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upArguments for external executables aren't correctly escaped #1995
Comments
|
Why do you think that The fact that Bash uses If you want to pass literally > echo `"\`"a\`"`"
"\"a\"" |
|
@andschwa
(three characters) instead of
(one character). |
|
Currently to make native.exe "\`"a\`"" |
|
Ah, I see. Re-opening. |
|
Out of a strong curiosity, what happens if you try a build using #1639? |
|
@andschwa The same. You HAVE to double-esacpe to satisify both PowerShell and native.exe "`"a`""results a StartProcess equalivent to cmd native.exe ""a"" |
|
@be5invis @douglaswth is this resolved via #2182? |
|
No, We still need to add a backslash before a backtick-escaped double quote? This does not solve the double-escaping problem. (That is, we have to escape a double quote for both PowerShell and CommandLineToArgvW.)
|
|
Since |
|
This seems like a feature request that if implemented could break a large number of already existing PowerShell scripts that use the required double escaping, so extreme care would be required with any solution. |
|
@vors Yes. |
|
@vors @douglaswth #include <stdio.h>
#include <wchar.h>
#include <Windows.h>
int main() {
LPWSTR cmdline = GetCommandLineW();
wprintf(L"Command Line : %s\n", cmdline);
int nArgs;
LPWSTR *szArglist = CommandLineToArgvW(cmdline, &nArgs);
if (NULL == szArglist) {
wprintf(L"CommandLineToArgvW failed\n");
return 0;
} else {
for (int i = 0; i < nArgs; i++) {
wprintf(L"argv[%d]: %s\n", i, szArglist[i]);
}
}
LocalFree(szArglist);
} |
|
Here is the result
|
|
@be5invis I do not disagree with you about the double escaping being annoying, but I am merely saying that a change to this would need to be backward compatible with what existing PowerShell scripts use. |
|
How many are them? I do not think there are script writers know about such double-quoting. It is a bug, not feature, and it is not documented. ???? iPhone ? 2016?9?21??01:58?Douglas Thrift <notifications@github.commailto:notifications@github.com> ??? @be5invishttps://github.com/be5invis I do not disagree with you about the double escaping being annoying, but I am merely saying that a change to this would need to be backward compatible with what existing PowerShell scripts use. You are receiving this because you were mentioned. |
|
PowerShell has been around for 9 years so there are very likely a good number of scripts out there. I found plenty of information about the need for double escaping from StackOverflow and other sources when I ran into the need for it so I don't know if I agree with your claims about nobody knowing about the need for it or that it is not documented. |
|
For the additional context, I'd like to talk a little bit about the implementation. Here, PS creates StartProcessInfo that is uses The provided API takes a single string for arguments and then it's re-parsed into an array of arguments to do the execution. Although, PowerShell may try to be smarter and provide a nicer experience, the current behavior is consistent with cmd and bash: you can copy native executable line from them and use it in powershell and it works the same. @be5invis If you know a way to enhance the expirience in non-breaking way, please line up the details. For the breaking changes, we would need to use RFC process, as described in https://github.com/PowerShell/PowerShell/blob/master/docs/dev-process/breaking-change-contract.md |
|
This applies to Windows, but when running commands on Linux or Unix, its strange that one needs to double escape quotes. On Linux processes don't have a single commandline but instead an array of arguments. Even on windows, the current behavior is inconsistent: I think arguments should either never be changed, or always be changed to meet Regarding breaking-the-contract: |
|
@vors This is extremely annoying if your argument is an variable or something else: you have to manually escape it before sending it into a native app. |
|
I think @TSlivede put it right with the inconsistency in the behavior.
I'm not sure about the bucket, but even the "clearly breaking change" bucket could potentially be changed. We want to make PowerShell better, but backward compatibility is one of our highest priorities. That's why it's not so easy. Would anybody want to start an RFC process? |
|
It would be worth investigating the use of P/Invoke instead of .Net to start a process if that avoids the need for PowerShell to add quotes to arguments. |
|
@lzybkr as far as I can tell, PInvoke would not help. https://msdn.microsoft.com/en-us/library/20y988d2.aspx (treats spaces as separators) |
|
I wasn't suggesting changing the Windows implementation. |
|
I'd try to avoid having platform-specific behavior here. It will hurt scripts portability. |
|
We're talking about calling external commands - somewhat platform dependent anyway. |
|
Well, i think it can't be really platform independent, as Windows and Linux just have different ways to call executables. In Linux a process gets an argument array while on Windows a process just gets a single commandline (one string). As Powershell adds those quotes when arguments have spaces in them, it seems to me, that powershell tries** to pass the arguments in a way, that ** (and fails, as it doesn't escape quotes) PS: What is necessary to start an RFC process? |
|
Exactly - PowerShell tries to make sure This has been a longstanding pain point on Windows, I see on reason to bring that difficulty over to *nix. To me, this feels like an implementation detail, not really needing an RFC. If we changed behavior in Windows PowerShell, it might warrant an RFC, but even then, the right change might be considered a (possibly risky) bug fix. |
|
TL;DR: The assumption that we can reliably pass any value as an argument to any program in the Microsoft Windows NT subsystem is wrong, so we should stop pretending that this is our goal. However, there is still much to be rescued if we consider the extent of the argument. When invoking native Windows executables, we should preserve original quoting. Example: CMD /CSTART="WINDOW TITLE"
{ CMD /CSTART="WINDOW TITLE" }. Ast. EndBlock. Statements. PipelineElements. CommandElements[1]
If we took the extent as the template, we would not lose anything and we could call the native executable as expected. The workaround of using a string argument works here but I do not think it is strictly technically necessary to do so, provided proper support gets implemented within PowerShell. This approach would work for all cases. Quotation marks within quotations present an insurmountable problem because there are tools that interpret backslash escape ( Note that environment variables do not represent values in |
That should be the goal though shouldn't it? It's not PowerShell's problem if a program can't interpret arguments it receives.
Are you suggesting calling a program should dynamically change the language from PowerShell to whatever the invoked program uses? You wrote that example in PowerShell, which means it should be equivalent to any of the following CMD "/CSTART=WINDOW TITLE"
CMD '/CSTART=WINDOW TITLE'
CMD /CSTART=WINDOW` TITLE |
The program
I tried to suggest that, when interfacing with external programs under Microsoft Windows NT subsystem, PowerShell has a myriad ways to encode the arguments that are all equivalent to PowerShell but not equivalent to the receiving program. Being blunt and forcing the One True Way™ of encoding arguments, without paying attention to what quoting arrangement the user actually used, is not helpful, to say it mildly. |
|
@yecril71pl I'm really confused by your comments. What exactly are you proposing here? Your use cases are all covered by
But in fact you can use environment variables with
So what am I missing? |
|
We are missing the syntax |
|
As a terrible idea, we do have the option of replacing Since equal signs are not allowed in env var names anyways, we can have As for the documentation hell this would cause... I apologize. |
Fixing these issues is what @TSlivede's PowerShell/PowerShell-RFC#90 is all about. In PowerShell/PowerShell-RFC#90 (comment) I've proposed additionally automatically compensating for the "roguishness" of batch files, given their still very widespread use as CLI entry points for high-profile software such as Azure (CLI I've just published a module, It also includes In other words: Instead of: # This command is currently broken, because the '{ "name": "foo" }' argument isn't properly passed.
curl.exe -u jdoe 'https://api.github.com/user/repos' -d '{ "name": "foo" }'you'd use the following: # OK, thanks to `ie`
ie curl.exe -u jdoe 'https://api.github.com/user/repos' -d '{ "name": "foo" }'As for the It is in essence the same problem as with There are two ways to resolve this:
|
I do not think
I am not a confident user of either, so I preferred to bring up something I am more familiar with. |
|
To be clear: the following has no impact on the points made in my previous comment.
:: OK - the "..." around & tells cmd.exe to use it verbatim
C:\>echoArgs.exe one "two & three"
Arg 0 is <one>
Arg 1 is <two & three>
Command line:
"C:\ProgramData\chocolatey\lib\echoargs\tools\EchoArgs.exe" one "two & three"Also, :: OK - the "" is recognized as an escaped "
C:\>echoArgs.exe "3"" of rain & such."
Arg 0 is <3" of rain & such.>
Command line:
"C:\ProgramData\chocolatey\lib\echoargs\tools\EchoArgs.exe" "3"" of rain & such."Unfortunately, While most CLIs on Windows support both :: !! BROKEN: cmd.exe misinterprets the & as *unquoted*, thinks it's the statement-sequencing operator,
:: !! and tries to execute `such`:
C:\>echoArgs.exe "3\" of rain & such."
Arg 0 is <3" of rain >
Command line:
"C:\ProgramData\chocolatey\lib\echoargs\tools\EchoArgs.exe" "3\" of rain
'such."' is not recognized as an internal or external command,
operable program or batch file.Therefore:
|
|
I know I'm going to catch a lot of flak here, and I really appreciate the depth of the discussion happening, but...ducks...does anyone have an example of any of this actually mattering in a real-world scenario? It's my take that we are not empowered in PowerShell to solve the "anarchy" that currently exists with Windows argument parsing. And for many of the same reasons that we can't solve the problem, there's a good reason that Windows and the VC++ compilers have chosen not to break this behavior. It's rampant, and we're only going to create a really long tail of new (and largely undecipherable) problems if we change things. For those utilities which are already cross-platform and in heavy use between Windows and Linux (e.g. Docker, k8s, Git, etc.), I don't see this problem manifesting in the real world. And for those "rogue" applications that do a poor job: they're largely legacy, Windows-only utilities. I agree that what you've described @mklement0 is largely a "correct" solution. I just don't know how to get there without really screwing things up. |
|
Pretty basic usages break:
|
|
|
|
@PowerShell/powershell-committee discussed this. We appreciate the git example which clearly shows a real world compelling example. We agreed that we should have an experimental feature early in 7.2 to validate the impact of taking such a breaking change. An additional test example shows that even PS> testexe --% -echoargs 'a b c "d e f " g h'
Arg 0 is <'a>
Arg 1 is <b>
Arg 2 is <c>
Arg 3 is <d e f >
Arg 4 is <g>
Arg 5 is <h'>This appears to be a problem in the native command parameter binder. |
|
Yeah, thank you @cspotcode. That example was definitely an aha moment for me (especially considering I've actually hit that one in the real world). I'm still concerned about the breaking change aspect, and it's my take that this is a could candidate for an experimental feature that may remain experimental over multiple versions of PowerShell, and that is absolutely not something we're sure will eventually make it. I also need to dig in more to understand the allow list / "rouge app" aspect of your RFC, @mklement0, as I'm not sure how much we want to sign up to maintain a list like that. |
|
@joeyaiello and @SteveL-MSFT, let me make a meta observation first: While it's good to see that @cspotcode's example gave you a glimpse of the problem, your responses still betray a fundamental lack of understanding and appreciation of the (magnitude of the) underlying problem (I will argue this point in a later comment). This is not a personal judgment: I fully recognize how difficult it must be to be stretched very thin and to have to make decisions on a very wide range of subjects in a short amount of time. However, this points to a structural problem: To me it seems that decisions are routinely made by the @PowerShell/powershell-committee on the basis of a superficial understanding of the problems being discussed, to the detriment of the community at large. To me, the committee's response to the issue being discussed here is the most consequential example of this structural problem to date. Therefore, I ask you to consider this: How about appointing subject-matter-specific sub-committees that the committee consults with that do have the required understanding of the issues involved? |
|
can you share the content of |
|
@TSlivede summarized the problem aptly in #13068 (comment):
As stated many times before, a core mandate of a shell is to call external executables with arguments. PowerShell currently fails to fulfill this mandate, given that arguments with embedded double quotes and empty-string arguments aren't passed correctly. As stated before, this may have been less of a problem in the Windows-only days, where the lack of capable external CLIs rarely surfaced this problem, but these days are gone, and if PowerShell wants to establish itself as a credible cross-platform shell, it must address this problem. @cspotcode's # On Unix; on Windows,
# echoArgs.exe '{ "foo": "bar" }'
# would show the same problem.
PS> /bin/echo '{ "foo": "bar" }'
{ foo: bar } # !! Argument was incorrectly passed.Leaving backward compatibility aside:
Therefore, as soon as possible, one of the following choices must be made:
Proposing an experimental feature to address a badly broken fundamental feature is wholly inadequate. Even considering use of It is a Windows-only feature that knows only On Unix, the concept of "stopping parsing" fundamentally doesn't apply: there is no command line to pass to child processes, only arrays of arguments. Thus, someone has to parse the command line into arguments before invocation, which is implicitly delegated to the
The VC++ compiler imposes a sensible, widely observed convention, to bring order to the anarchy. It is precisely adherence to this convention that is being advocated for here, which use of This alone will cover the vast majority of calls. Covering ALL calls is impossible and indeed not PowerShell's responsibility. As stated, for "rogue" CLIs that require non-conventional forms of quoting, As a courtesy, we can automatically compensate for well-known "rogue" scenarios, namely calling batch files and certain high-profile Microsoft CLIs:
|
|
@musm You can find the source code of
|
|
I think that accommodating exceptions by default is just going to lead to a similar situation as the current one, where people need to revert PowerShell's "helpfulness". If there are exceptions, it should be obvious they're being applied. Maybe something like: # Arguments passed correctly, without regard for the program's ability to handle them
& $program a "" 'c "d e" f'
# Try to pass the arguments intelligently based on the program being called
&[] $program a "" 'c "d e" f'
# Escape the arguments for a batch file, eg) " -> ""
&[bat] $program a "" 'c "d e" f'I'm really struggling to find syntax for this which isn't broken. At least this sort of makes sense if you think of it as casting the program, but casting the actual variable containing the program would require enclosing parenthesis. That, in addition to allowing people to add exceptions for whatever broken behaviour they desire should hopefully eliminate the need for |
The quotes used to construct the command line should follow the way the call is quoted in PowerShell. Therefore:
In particular:
The last row shows an example where it will not be possible to retain the original double quotes. |
I already mentioned it in this thread a year ago (it's hidden now...) that I used to get a lot of WFT moments when using ripgrep in Powershell. I couldn't understand why I couldn't search quoted strings. It ignored my quotes:
and in git bash it didn't. Now I get less this WTF moments because sadly I found this long github issue and found that passing To be honest, now I don't even dare to use Powershell to call native command when I know I might be passing Really, @mklement0 summed it up great (this should be engraved in stone somewhere)
And about breaking changes. |
|
RE: that last point of breaking changes -- I fully agree. There are many breaking changes we've tolerated for various reasons. However, more and more often it seems to be the case that some breaking changes are simply frowned upon for reasons of preference and not given proper gravity of consideration for their actual value. There are some changes like this which would massively improve the overall shell experience for anyone who needs to reach outside of PowerShell to get things done, which happens all the time. It's been agreed time and time again that the current behaviour is untenable and already largely broken for anything but the simplest usages. And yet, we're still facing this reticence to breaking changes even while there are scores of already accepted breaking changes, some of which have similarly large impact. For those asking for examples -- take a minute to visit Stack Overflow for once. I'm sure @mklement0 has a litany of examples where community help is required to help explain a breaking change in newer versions. It happens all the time. We have no excuse to not make helpful breaking changes. |
|
Whenever MSFT team repeats the same thing over and over again, we can be sure they know more than they can publicly say. We should respect their inner discipline and not pressure them. Maybe we can find a compromise. I hope I have time today to describe an alternative path with lazy migration. |
|
I do recognise that, and it's why I rarely make a point of questioning it. However, this is an open source project; if there's no possible visibility into those decisions, folks will inevitably end up frustrated. No blame to cast on either side of that coin, that's just the reality of the situation here, IMO. So yeah, having a migration path may ease that pain somewhat, but we need clear policies defined on how that has to work that will make things work for as many folks as possible. Compromise is difficult to reach when lacking information, though. I look forward to seeing what you have up your sleeve. |
|
@mklement0 you're absolutely right, and so much so that I can only respond to your meta-point right now. Unfortunately, in the cases where we aren't able to reach the level of depth required to answer a question like this, the safer approach is often to defer or reject the breaking change until we have more time to I want to make another meta-point about breaking changes, though: our telemetry implies that most PowerShell 7 users are not managing their own versions. They're running automated scripts in a managed environment that's comfortable e.g. upgrading their users from 6.2 to 7.0 (see the 2-day jump in 6.2 users becoming 7.0 users starting on 8/3; this isn't our only data point here, but it's a convenient one right now that makes the point). For these users, a breaking change that turns a perfectly working script into a non-working script is unacceptable. I also owe the community a blog on how I think about the impact of breaking changes: namely trading off the prevalence of existing usage and severity of the break against the ease of identifying and correcting the break. This one is extremely prevalent in existing scripts, confusing to identify and fix, and the breaking behavior is from total success to total failure, hence my extreme reticence to do anything here. I think it's fair to say we're not going to do anything here in 7.1, but I'm definitely open to making this an investigative priority for 7.2 (i.e. we spend more than just our Committee time discussing this.)
We're working on this. I know I've said that before, but we're extremely close (as in, I'm crafting the blog and you're probably going to see some new labels show up soon that we'll be playing with). I appreciate everyone's patience and I recognize that it's annoying to get a pithy reply out of the Committee every couple weeks when folks are pouring an immense amount of thought and consideration into the discussion. I know it looks like that means we're not thinking deeply about things, but I think we're just not expressing the depth of our discussions in as much detail as folks do here. In my own backlog, I've got a whole set of blog topics like the breaking change one around how I think about making decisions from within the Committee, but I've just never gotten the chance to sit down and pump them out. But I can see here that maybe folks would find a lot of value in that. Hope I didn't get the rails too far off in this discussion. I don't want this issue to totally become a meta-issue about the project's management, but I did want to address some of the understandable frustration I see here. I implore anyone that wants to talk about this in more detail with us to join the Community Call next week (add your questions and thoughts here and I'll be sure to address them in the call). |
|
Just a quick note on the meta-point: I appreciate the thoughtful response, @joeyaiello. As for the severity of the breaking change: The following statements seem to be at odds:
vs.
If it is already prevalent, the awkwardness and obscurity of the necessary workarounds are all the more reason to finally fix this, especially given that we should expect the number of cases to increase. I do realize that all existing workarounds will break. If avoiding that is paramount, this previously suggested approach is the way to go:
A function such as
If/when the default behavior gets fixed:
|
We can simplify adoption by mean of #13428. We can inject this with @mklement0's investigations in Engine transparently. |
Steps to reproduce
native.exewhich acquires ARGVnative.exe "`"a`""Expected behavior
ARGV[1] ==
"a"Actual behavior
ARGV[1] ==
aEnvironment data
Windows 10 x64