-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Arguments 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. |
This appears to be fixed in 7.3: PS> testexe -echoargs "`"a`""
Arg 0 is <"a"> |
Thanks a lot @SteveL-MSFT. May I know which PR is responsible for the fix? |
I can verify double quotes are escaped correctly in v7.3.0-preview.6 when calling a
However, special characters ( echo %*
python -c "import sys; print(sys.argv)" %* For
This differs from the behavior of Command Prompt:
This causes problem for Azure CLI when there is For
This differs from the behavior of Command Prompt:
|
As noted by @jiasli, this issue is not actually fixed. |
The PowerShell behavior here is the same as
#!/bin/bash
echo $@ bburns@DESKTOP-VHJJ57D:~$ ./test.sh "a|b"
a|b
bburns@DESKTOP-VHJJ57D:~$ ./test.sh "a&b"
a&b
bburns@DESKTOP-VHJJ57D:~$ In my opinion, the bash/powershell behavior makes more sense than the |
Furthermore this seems to be a specific behavior related to import sys
print(sys.argv) C:\Users\bburns>py test.py 'a|b'
'b'' is not recognized as an internal or external command,
operable program or batch file.
C:\Users\bburns>py test.py 'a&b'
['test.py', "'a"]
'b'' is not recognized as an internal or external command,
operable program or batch file. |
This issue is about passing arguments to native executables that support passing arguments. The executable |
Steps to reproduce
native.exe
which acquires ARGVnative.exe "`"a`""
Expected behavior
ARGV[1] ==
"a"
Actual behavior
ARGV[1] ==
a
Environment data
Windows 10 x64
The text was updated successfully, but these errors were encountered: