★ wanayoo — archive 1999 https://github.com/PowerShell/PowerShell/issues/1995Nouvelle recherche | Portail wanayoo
Skip to content
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

Open
be5invis opened this issue Aug 21, 2016 · 153 comments
Open

Arguments for external executables aren't correctly escaped #1995

be5invis opened this issue Aug 21, 2016 · 153 comments

Comments

@be5invis
Copy link

@be5invis be5invis commented Aug 21, 2016

Steps to reproduce

  1. write a C program native.exe which acquires ARGV
  2. Run native.exe "`"a`""

Expected behavior

ARGV[1] == "a"

Actual behavior

ARGV[1] == a

Environment data

Windows 10 x64

Name                           Value
----                           -----
PSVersion                      5.1.14393.0
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14393.0
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
@andschwa
Copy link
Member

@andschwa andschwa commented Aug 31, 2016

Why do you think that "\"a\"" the expected behavior? My understanding of PowerShell escapes says that the actual behavior is the correct and expected behavior. ""a"" is a pair of quotes surrounding an escaped pair of quotes surrounding an a, so PowerShell interprets the outer unescaped pair as "this is a string argument" and so drops them, then interprets the escaped pair as escaped quotes and so keeps them, leaving you with "a". At no point was a \ added to the string.

The fact that Bash uses \ as an escape character is irrelevant. In PowerShell, the escape character is a backtick. See PowerShell escape characters.

If you want to pass literally "\"a\"", I believe you would use:

> echo `"\`"a\`"`"
"\"a\""
@andschwa andschwa closed this Aug 31, 2016
@be5invis
Copy link
Author

@be5invis be5invis commented Aug 31, 2016

@andschwa
Yes, escapes works fine for internal cmdlets, but things get weird when communicate with native binaries, especially on Windows.
When running native.exe ""a"", the ARGV[1] should be

"a"

(three characters)

instead of

a

(one character).

@be5invis
Copy link
Author

@be5invis be5invis commented Aug 31, 2016

Currently to make native.exe correctly receive an ARGV with two quotes and an a character, you have to use this weird call:

native.exe "\`"a\`""
@andschwa
Copy link
Member

@andschwa andschwa commented Aug 31, 2016

Ah, I see. Re-opening.

@andschwa andschwa reopened this Aug 31, 2016
@andschwa
Copy link
Member

@andschwa andschwa commented Aug 31, 2016

Out of a strong curiosity, what happens if you try a build using #1639?

@be5invis
Copy link
Author

@be5invis be5invis commented Aug 31, 2016

@andschwa The same. You HAVE to double-esacpe to satisify both PowerShell and CommandLineToArgvW. This line:

native.exe "`"a`""

results a StartProcess equalivent to cmd

native.exe ""a""
@andschwa
Copy link
Member

@andschwa andschwa commented Sep 19, 2016

@be5invis @douglaswth is this resolved via #2182?

@be5invis
Copy link
Author

@be5invis be5invis commented Sep 20, 2016

@vors
Copy link
Collaborator

@vors vors commented Sep 20, 2016

Since ""a"" is equal to '"a"', do you suggest that native.exe '"a"' should result in "\"a\""?

@douglaswth
Copy link
Contributor

@douglaswth douglaswth commented Sep 20, 2016

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.

@be5invis
Copy link
Author

@be5invis be5invis commented Sep 20, 2016

@vors Yes.
@douglaswth The double-escaping is really silly: why do we need the “inner” escapes made in the DOS era?

@be5invis
Copy link
Author

@be5invis be5invis commented Sep 20, 2016

@vors @douglaswth
This is a the C code used to show GetCommandLineW and CommandLineToArgvW results:

#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);
}
@be5invis
Copy link
Author

@be5invis be5invis commented Sep 20, 2016

Here is the result

$ ./a "a b"
Command Line : "Z:\playground\ps-cmdline\a.exe" "a b"
argv[0]: Z:\playground\ps-cmdline\a.exe
argv[1]: a b

$ ./a 'a b'
Command Line : "Z:\playground\ps-cmdline\a.exe" "a b"
argv[0]: Z:\playground\ps-cmdline\a.exe
argv[1]: a b

$ ./a 'a"b'
Command Line : "Z:\playground\ps-cmdline\a.exe" a"b
argv[0]: Z:\playground\ps-cmdline\a.exe
argv[1]: ab

$ ./a 'a"b"c'
Command Line : "Z:\playground\ps-cmdline\a.exe" a"b"c
argv[0]: Z:\playground\ps-cmdline\a.exe
argv[1]: abc

$ ./a 'a\"b\"c'
Command Line : "Z:\playground\ps-cmdline\a.exe" a\"b\"c
argv[0]: Z:\playground\ps-cmdline\a.exe
argv[1]: a"b"c
@douglaswth
Copy link
Contributor

@douglaswth douglaswth commented Sep 20, 2016

@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.

@be5invis
Copy link
Author

@be5invis be5invis commented Sep 20, 2016

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.
Reply to this email directly, view it on GitHubhttps://github.com//issues/1995#issuecomment-248381045, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAOp20f_W0mTl2YiJKi_flQBJUKaeAnLks5qsB7ZgaJpZM4JpVin.

@douglaswth
Copy link
Contributor

@douglaswth douglaswth commented Sep 20, 2016

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.

@vors
Copy link
Collaborator

@vors vors commented Sep 20, 2016

For the additional context, I'd like to talk a little bit about the implementation.
PowerShell calls .NET API to spawn a new process, which calls a Win32 API (on windows).

Here, PS creates StartProcessInfo that is uses
https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/NativeCommandProcessor.cs#L1063

The provided API takes a single string for arguments and then it's re-parsed into an array of arguments to do the execution.
The rules of this re-parsing are not controlled by PowerShell. It's a Win32 API (and fortunately, it consistent in dotnet core and unix rules).
Particularly, this contract describes the \ and " behavior.

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

@TSlivede
Copy link

@TSlivede TSlivede commented Oct 3, 2016

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.
Therefore arguments in powershell should be the same as those, that are passed to the executable, instead of merging all arguments and then resplitting.

Even on windows, the current behavior is inconsistent:
If an argument contains no spaces, it is passed unchanged.
If an argument contains spaces, if it will be surrounded by quotes, to keep it together through CommandLineToArgvW call. => Argument is changed to meet CommandLineToArgvW requirement.
But if argument contains quotes, those are not escaped. => Argument is not changed, although CommandLineToArgvW requires this.

I think arguments should either never be changed, or always be changed to meet CommandLineToArgvW requirements, but not in half of the cases.

Regarding breaking-the-contract:
As I couldn't find any official documentation about double escaping, I'd consider this as category "Bucket 2: Reasonable Grey Area", so there are chances to change this, or am I wrong?

@be5invis
Copy link
Author

@be5invis be5invis commented Oct 3, 2016

@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.
An "auto-escaping" operator may help. like ^"a"" -> "a\""`

@vors
Copy link
Collaborator

@vors vors commented Oct 3, 2016

I think @TSlivede put it right with the inconsistency in the behavior.

I think arguments should either never be changed, or always be changed to meet CommandLineToArgvW requirements, but not in half of the cases.

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.
We have a great community and I'm confident that we can find consensus.

Would anybody want to start an RFC process?

@lzybkr
Copy link
Member

@lzybkr lzybkr commented Oct 3, 2016

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.

@vors
Copy link
Collaborator

@vors vors commented Oct 3, 2016

@lzybkr as far as I can tell, PInvoke would not help.
And this is where unix and windows APIs are different:

https://msdn.microsoft.com/en-us/library/20y988d2.aspx (treats spaces as separators)
https://linux.die.net/man/3/execvp (doesn't treat spaces as separators)

@lzybkr
Copy link
Member

@lzybkr lzybkr commented Oct 3, 2016

I wasn't suggesting changing the Windows implementation.

@vors
Copy link
Collaborator

@vors vors commented Oct 3, 2016

I'd try to avoid having platform-specific behavior here. It will hurt scripts portability.
I think we can consider changing windows behavior in a non-breaking way. I.e. with preference variable. And then we can have different defaults or something like that.

@lzybkr
Copy link
Member

@lzybkr lzybkr commented Oct 3, 2016

We're talking about calling external commands - somewhat platform dependent anyway.

@TSlivede
Copy link

@TSlivede TSlivede commented Oct 3, 2016

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).
(compare the more basic
CreateProcess -> commandline (https://msdn.microsoft.com/library/windows/desktop/ms682425)
and
execve -> command array (https://linux.die.net/man/2/execve)
)

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 CommandLineToArgvW splits the commandline to the arguments that were originally given in powershell. (This way a typical c-program gets the same arguments in its argv array as a powershell function gets as $args.)
This would perfectly match to just passing the arguments to the linux systemcall (as suggested via p/invoke).

** (and fails, as it doesn't escape quotes)

PS: What is necessary to start an RFC process?

@lzybkr
Copy link
Member

@lzybkr lzybkr commented Oct 3, 2016

Exactly - PowerShell tries to make sure CommandLineToArgvW produces the correct command and after reparsing what PowerShell has already parsed.

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.

@yecril71pl
Copy link
Contributor

@yecril71pl yecril71pl commented Aug 8, 2020

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"

The system cannot find the file WINDOW.

 { CMD /CSTART="WINDOW TITLE" }. Ast. EndBlock. Statements. PipelineElements. CommandElements[1]
StringConstantType
BareWord
Value
/CSTART=WINDOW TITLE
StaticType
System.String
Extent
/CSTART="WINDOW TITLE"
Parent
CMD /CSTART="WINDOW TITLE"

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 (TASKLIST "\\\"PROGRAM FILES") and tools that do not (DIR "\""PROGRAM FILES" /B) and tools that do not bother (TITLE A " B). However, if we were to escape, the standard escape with backslashes poisons all common file managing tools because they simply do not support quotation marks at all and double backslashes \\ mean something entirely different to them (try DIR "\\\"PROGRAM FILES" /B), so sending an argument with a quotation mark inside should be a run-time error. But we cannot throw an error because we do not know which one is which. While using the normal escaping mechanism should not cause any harm to arguments that do not contain quotation marks, we cannot be sure that, when applied to arguments that do contain them and fed to a tool that does not support quotation marks as values, it would necessarily cause an aborting error rather than unexpected behaviour, and unexpected behaviour would be very bad indeed. This is a serious burden we place upon the user. In addition, we shall never be able to provide for ‘don’t care’ tools (CMD /CECHO='A " B').

Note that environment variables do not represent values in CMD, they represent code fragments that are reparsed as environment variables are expanded, and there is no provision for reliably treating them as arguments to other commands. CMD just does not operate on objects on any kind, not even strings, which seems to be the root cause of the present conundrum.

@Dabombber
Copy link

@Dabombber Dabombber commented Aug 9, 2020

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.

That should be the goal though shouldn't it? It's not PowerShell's problem if a program can't interpret arguments it receives.

When invoking native Windows executables, we should preserve original quoting. Example:

CMD /CSTART="WINDOW TITLE"

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
@yecril71pl
Copy link
Contributor

@yecril71pl yecril71pl commented Aug 9, 2020

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.

That should be the goal though shouldn't it? It's not PowerShell's problem if a program can't interpret arguments it receives.

The program CMD can interpret the argument /CECHO=A " B but PowerShell cannot pass it without distorting it.

When invoking native Windows executables, we should preserve original quoting. Example:

CMD /CSTART="WINDOW TITLE"

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

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.

@imgx64
Copy link

@imgx64 imgx64 commented Aug 9, 2020

@yecril71pl I'm really confused by your comments. What exactly are you proposing here? Your use cases are all covered by --%. You dismissed it earlier by saying

--% requires a predefined command line, so its utility is limited.

But in fact you can use environment variables with --%. Try This:

PS > $env:mytitle='WINDOW TITLE'
PS > cmd --% /CSTART="%mytitle%"

So what am I missing?

@yecril71pl
Copy link
Contributor

@yecril71pl yecril71pl commented Aug 9, 2020

We are missing the syntax CMD /CSTART="$mytitle", without leaking things to ENV:.

@Artoria2e5
Copy link

@Artoria2e5 Artoria2e5 commented Aug 9, 2020

As a terrible idea, we do have the option of replacing Environment.ExpandEnvironmentVariables with something else. There's no native implementation on Unix anyways, and I don't believe the stuff it processes would become performance-critical when rewritten in C#.

Since equal signs are not allowed in env var names anyways, we can have %=$a% mean $a. This wouldn't break anything existing while allowing for some very flexible (and possibly bad) extensions like making it work like JS's template strings. Hell, we can define %VARNAME=$var% as some sort of fallback syntax too.

As for the documentation hell this would cause... I apologize.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Aug 11, 2020

  • We do not have a parsing problem.

  • What we do have is a problem with how PowerShell passes the verbatim, stringified arguments that have resulted from its parsing to external (native) executables:

    • On Windows, the problem is that the command line to invoke the external executable with that is constructed behind the scenes does not adhere to the most widely used convention for quoting arguments, as detailed in the the Microsoft C/C++ compiler documentation's Parsing C++ command-Line arguments section.

      • What happens currently isn't even that a different convention is used: presumably due to an oversight, the command lines that are constructed are situationally syntactically fundamentally broken, depending on the specifics of the arguments, relating to a combination of embedded double quotes and spaces as well as empty-string arguments.

      • Ultimately, the problem is the fundamental architecture of process creation on Windows: You're forced to encode the arguments to pass to a process as a command line - a single string representing all arguments - rather than passing them as an array of arguments (which is how Unix-like platforms do it). The need to pass a command line requires quoting and escaping rules to be implemented, and it is ultimately up to each program how to interpret the command line it is given. In effect, this amounts to needlessly forcing programs to be a mini-shell of sorts: they're forced to re-perform the task that the shell has already performed, which is task that should be the purview of a shell only (as is the case on Unix), namely parsing a command line into individual arguments. In a nutshell, this is the anarchy that is argument passing on Windows.

      • In practice, the anarchy is mitigated by most programs adhering to the aforementioned convention, and new programs being developed are highly likely to adhere to that convention, primarily because widely used runtimes underpinning console applications implement these conventions (such as the Microsoft C/C++ / .NET runtimes). The sensible solution is therefore:

        • Make PowerShell adhere to this convention when building the command line behind the scenes.
        • For "rogue" programs that do not adhere to this convention - which notably includes cmd.exe, batch files, and Microsoft utilities such as msiexec.exe and msdeploy.exe - provide a mechanism to explicitly control the command line passed to the target executable; this is what --%, the stop-parsing symbol provides - albeit quite awkwardly.
    • On Unix, the problem is that a command line is being constructed at all - instead, the array of verbatim arguments should be passed as-is, which .NET Core now supports (since v2.1, via ProcessStartInfo.ArgumentList; it should always have supported this, given that - sensibly - there are no command lines, only argument arrays, when a process is created on Unix-like platforms).

      • Once we use ProcessStartInfo.ArgumentList, all problems on Unix go away.

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 az is implemented as a batch file, az.cmd).
Similarly, we should consider doing the same for msiexec.exe and msdeploy.exe and perhaps other high-profile "rogue" Microsoft CLIs.


I've just published a module, Native, (Install-Module Native -Scope CurrentUser) that addresses all of the above via its ie function (short for invoke (external) executable; it is a more complete implementation of the iep function introduced above).

It also includes ins (Invoke-NativeShell), which addresses #13068, and dbea (Debug-ExecutableArguments) for diagnosing argument passing - see #13068 (comment) for details.

In other words: ie can serve as an unobtrusive stopgap while we wait for this issue to be fixed, simply by prefixing invocations with ie as the command:

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 CMD /CSTART="WINDOW TITLE" example (whose more idiomatic form is cmd /c start "WINDOW TITLE", which does already work):

It is in essence the same problem as with prop="<value with spaces>" arguments for msiexec / msdeploy: PowerShell - justifiably - transforms /CSTART="WINDOW TITLE" into "/CSTART=WINDOW TITLE", which, however breaks the cmd.exe invocation.

There are two ways to resolve this:

  • Delegate to ins / Invoke-NativeShell (note that the use of cmd.exe /c is effectively implied):

    • ins 'START="WINDOW TITLE"'
    • If you use an expandable string, you can embed PowerShell values in the command string.
      • $title = 'window title'; ins "START=`"$title`""
  • Alternatively, use the current --% implementation, but beware its limitations:

    • cmd --% /CSTART="WINDOW TITLE"
    • As discussed, a problematic limitation of --% is that the only way to embed PowerShell values is to use an aux. environment variable and reference it with %...% syntax:
      • $env:_title = 'window title'; cmd --% /CSTART="%_title%"
      • To avoid this limitation, --% should always have been implemented with a single string argument - e.g.,
        cmd --% '/CSTART="WINDOW TITLE"' or cmd --% "/CSTART=`"$title`"" - but this can't be changed without breaking backward compatibility, so a new symbol would have to be introduced - personally, I don't see the need for one.
@yecril71pl
Copy link
Contributor

@yecril71pl yecril71pl commented Aug 11, 2020

  • they're forced to re-perform the task that the shell has already performed

I do not think CMD.EXE splits command lines into arguments, the only thing that is needed is to find out which executable to call and the rest is just the command line as written by the user (after environment variable substitutions, which are done without any regard for argument boundaries). Of course, internal shell commands are an exception here.

It is in essence the same problem as with prop="<value with spaces>" arguments for msiexec / msdeploy

I am not a confident user of either, so I preferred to bring up something I am more familiar with.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Aug 11, 2020

To be clear: the following has no impact on the points made in my previous comment.

I do not think CMD.EXE splits command lines into arguments

  • It can get away without explicit splitting when calling external executables (commands run by another executable in a child process), but it does have to do it for batch files.

  • Even when calling external executables it needs to be aware of argument boundaries, so as to determine whether a given metacharacter (e.g. &) has syntactic function or whether it is part of a double-quoted argument and therefore to be treated as a literal:

:: 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, cmd.exe recognizes embedded " chars. in "..." strings are recognized if escaped as "":

:: 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, cmd.exe only supports (the Windows-only) "" and not also the more widely used \" (which is what POSIX-like shells on Unix exclusively use - note: shells, not programs, because programs just see the array of verbatim arguments that result from the shell's parsing).

While most CLIs on Windows support both "" and \", some only understand \" (notably Perl and Ruby), and then you're in trouble:

:: !! 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:

  • Avoid calling cmd.exe directly, if possible.

    • Call external executables directly (once this issue is fixed) or via ie (for now), using PowerShell's syntax.
  • If you do have to call cmd.exe, use ins / Invoke-NativeShell for general simplicity and, specifically, for how easy it is to embed PowerShell variable and expression values into the command line.

    • A legitimate reason to still call cmd.exe directly is to compensate for PowerShell's lack of support for raw byte data in the pipeline - see this SO answer for an example.
@joeyaiello
Copy link
Member

@joeyaiello joeyaiello commented Aug 12, 2020

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.

@cspotcode
Copy link

@cspotcode cspotcode commented Aug 12, 2020

Pretty basic usages break:

❯ git commit --allow-empty -m 'this is what we call a "commit message" which contains arbitrary text, often with punctuation'
error: pathspec 'message which contains arbitrary text, often with punctuation' did not match any file(s) known to git
❯ $a = 'this is what we call a "commit message" which contains arbitrary text, often with punctuation'
❯ git commit --allow-empty -m "$a"
error: pathspec 'message which contains arbitrary text, often with punctuation' did not match any file(s) known to git
❯ $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.0.3
PSEdition                      Core
GitCommitId                    7.0.3
OS                             Microsoft Windows 10.0.19042
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0
@yecril71pl
Copy link
Contributor

@yecril71pl yecril71pl commented Aug 12, 2020

git.exe is a well-behaved application, so the fix in PowerShell will be straightforward, albeit requiring all scripters out there to revert their smart work-arounds. cmd.exe is harder to adapt to and requires a much more considerate approach that may solve a few problems but probably not all of them. Which is actually appalling, considering that PowerShell started as a Windows NT tool. I understand this question as whether there is a real-life scenario when an ill-behaved legacy utility like cmd.exe will be called from PowerShell in a way that causes problems in the interface. PowerShell tried to approach this problem by duplicating most of the functionality in cmd.exe, so as to make cmd.exe redundant. This is also possible for other tools, for example MSI can be operated via ActiveX, although doing so requires considerable knowledge. So is there anything essential that is not covered?

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Aug 12, 2020

@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 --% has a problem even though it should have been unparsed:

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.

@joeyaiello
Copy link
Member

@joeyaiello joeyaiello commented Aug 12, 2020

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.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Aug 13, 2020

@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?

@musm
Copy link

@musm musm commented Aug 13, 2020

can you share the content of testexe SteveL-MSFT, just want to make sure !

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Aug 13, 2020

@TSlivede summarized the problem aptly in #13068 (comment):

PowerShell on the other hand claims to be a shell (until #1995 is solved, I won't say that it is a shell)

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 git example is a good one; any executable to which you want to pass a JSON string - e.g., curl - is another:

# 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:

  • On Unix, the problem is trivially and completely solved by using ProcessStartInfo.ArgumentList behind the scenes.

  • On Windows, the problem is trivially and mostly solved by using ProcessStartInfo.ArgumentList behind the scenes.

    • For edge cases ("rogue" CLIs), there's the (poorly implemented) --%
    • As a courtesy, we can compensate for certain well-known edge cases to lessen the need for --% - see below.

Therefore, as soon as possible, one of the following choices must be made:

  • Realize the importance of making argument-passing work properly and fix it at the expense of backward compatibility.

  • If backward compatibility is really paramount, provide a new operator or a function such as the ie function from the Native module that fixes the problem, and widely publicize it as the only reliable way to invoke external executables.

Proposing an experimental feature to address a badly broken fundamental feature is wholly inadequate.


@SteveL-MSFT

Even considering use of --% as the solution to this problem is fundamentally misguided:

It is a Windows-only feature that knows only "..." quoting and %...%-style environment-variable references.

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 ProcessStartInfo class, via its .Arguments property, which on Unix uses the Windows conventions for parsing a command line - and therefore recognizes "..." quoting (with escaping of embedded " as "" or \") only.

--% is a Windows-only feature whose only legitimate purpose is to call "rogue" CLIs.


@joeyaiello

that Windows and the VC++ compilers have chosen not to break this behavior.

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 ProcessStartInfo.ArgumentList would automatically give us.

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, --% must be used (or ins / Invoke-NativeShell from the Native module).

As a courtesy, we can automatically compensate for well-known "rogue" scenarios, namely calling batch files and certain high-profile Microsoft CLIs:

  • The batch-file case is a generic one, and easily explained and conceptualized (e.g, pass a&b as "a&b", even though it shouldn't require quoting) - it will avoid the need for use of --% with all CLIs that use batch files as their entry point (which is quite common), such as Azure's az.cmd

  • The alternative to hard-coding exceptions for specific CLIs - which admittedly can get confusing - is to detect the following pattern in the arguments that result from PowerShell's parsing - <word>=<value with spaces> - and, instead of passing "<word>=<value with spaces>", as currently happens, to pass <word>="<value with spaces>"; the latter satisfies the "rogue" CLIs, while also being accepted by convention-adhering CLIs; e.g., echoArgs "foo=bar baz" ultimately sees the same first argument as echoArgs --% foo="bar baz"

@daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Aug 13, 2020

@musm You can find the source code of TestExe at https://github.com/PowerShell/PowerShell/blob/master/test/tools/TestExe/TestExe.cs.

GitHub
PowerShell for every system! Contribute to PowerShell/PowerShell development by creating an account on GitHub.
@Dabombber
Copy link

@Dabombber Dabombber commented Aug 13, 2020

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 exception class they register would then have a method for determining if it's applicable to the program (for intelligent invocation), and an escaping method where you just throw the command abstract syntax tree at it and it returns the argument array/string.

@yecril71pl
Copy link
Contributor

@yecril71pl yecril71pl commented Aug 13, 2020

  • instead of passing "<word>=<value with spaces>", as currently happens, to pass <word>="<value with spaces>"

The quotes used to construct the command line should follow the way the call is quoted in PowerShell. Therefore:

  1. A call that does not contain double quotes in its text should put double quotes around the whole value.
  2. A call that contains double quotes should retain them as written in the script if possible.

In particular:

script argument command line
p=l of v "p=l of v"
p=l` of` v "p=l of v"
p="l of v" p="l of v"
p="l of v"'a s m' p="l of v"a" s m"
p="l of v"' s m' p="l of v s m"

The last row shows an example where it will not be possible to retain the original double quotes.

@mpawelski
Copy link

@mpawelski mpawelski commented Aug 13, 2020

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?

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:

rg '"quoted"'

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 Powershel is totally broken. Recent "git.exe" example is also great.

To be honest, now I don't even dare to use Powershell to call native command when I know I might be passing " in string as parameter. I know I might get wrong result or error.

Really, @mklement0 summed it up great (this should be engraved in stone somewhere)

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.

And about breaking changes.
Recently coworker wrote to me that my script didn't worked on his machine. I was running it only on Powershel Core and he was running it on Windows Powershell. Turns out Out-File -Encoding utf8 encoded file with "BOM" on Windows Powershell and without BOM on Powershel Core. This is somehow unreleted example but show that there are already subtle breaking changes in Powershel and this is good because we are eliminating quirks and intuitive behavior from a language that is famous for it. It would be great if Powershel team was a bit more lenient when it comes to breaking changes now that we have cross platform Powershell that is shipping outside of Windows and that we know that Windows Powershell is in "maintenence" mode and will be usable forever if you really want it to run some old script that broke in newer version of Powershell.

@vexx32
Copy link
Collaborator

@vexx32 vexx32 commented Aug 13, 2020

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.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Aug 13, 2020

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.

@vexx32
Copy link
Collaborator

@vexx32 vexx32 commented Aug 13, 2020

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. 😉

@joeyaiello
Copy link
Member

@joeyaiello joeyaiello commented Aug 13, 2020

@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.)

How about appointing subject-matter-specific sub-committees that the committee consults with that do have the required understanding of the issues involved?

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).

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Aug 13, 2020

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:

does anyone have an example of any of this actually mattering in a real-world scenario

vs.

This one is extremely prevalent in existing scripts, confusing to identify and fix

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:

provide a new operator or a function such as the ie function from the Native module that fixes the problem, and widely publicize it as the only reliable way to invoke external executables.

A function such as ie would allow people to opt-into the correct behavior with minimal fuss, as a stopgap, without burdening the language with a new syntactical element (an operator), whose sole raison d'être would be to work around a legacy bug deemed too breaking to change:

  • The stopgap would provide officially sanctioned access to the correct behavior (no reliance on experimental features).
  • For as long as the stopgap is necessary, it would need to be widely publicized and properly documented.

If/when the default behavior gets fixed:

  • the function can be modified to just defer to it, so as not to break code that uses it.
  • new code can be written without needing the function anymore.
@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Aug 14, 2020

A function such as ie would allow people to opt-into the correct behavior with minimal fuss, as a stopgap

We can simplify adoption by mean of #13428. We can inject this with @mklement0's investigations in Engine transparently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Shell
  
In progress
Linked pull requests

Successfully merging a pull request may close this issue.

You can’t perform that action at this time.