Repository navigation
[Pre-RFC] Argument Escape Migration (Start-Process, &, and the normal cmd starting; v7.1 / v7.2) #13089
Description
Summary of the new feature/enhancement
This proposal seeks to come up with a plan for getting #1995 (argv-passing) right without having to call out to the system shell. A two-step migration plan is proposed:
- For powershell v7.1, we seek to make a proper
-ArgumentListavailable (in the sense of escaping likeSystem.Diagnostics.Processdoes) by adding a switch toStart-Processthat defaults tofalse. - For powershell v7.2, we make the correct behavior the default by:
- making the new
Start-Processswitch default to$true - making escaping the behavior for
&and normal external command calls - bumping whatever protocol version is affected
- making the new
The goal is to make sure that PowerShell pass the argv as-is to most programs on Windows and all programs on Unix. In other words, for any given string object $x, the result for the two commands are expected to be the same:
Write-Output $x
/bin/printf "%s\n" $xWin32 programs that use custom cmdline parsing will still be usable with the --% marker as well as our new switch. Unix programs always use the MSVCRT parsing rule under-the-hood in CoreFX, so they won't be negatively affected at all. For Unix, --% can be seen as a way to... poke at the internals.
After all, pwsh is supposed to be a cross-platform shell, so Windows-specific idiosyncrasies should not be part of the basic interface.
Proposed technical implementation details
As some unspecified utility class, we duplicate the PasteArguments class from CoreFX runtime. This is needed because some our logging code paths use startupInfo.Arguments already. It makes more sense to log the stuff escaped like how we invoke it than to log it wrong or using some other custom escape method.
For 7.1 and 7.2, we add a bool? EscapeArgs (name to be discussed) member to the StartProcessCommand class. This flag is to be checked in the existing if (ArgumentList != null) branch. When we decide that we shall escape the args according to the fallback rules in the first section, we do it ourselves using the new utility class.
For 7.2 we change NativeCommandParameterBinder::AppendOneNativeArgument to use the new class. (The existing code tries to do escaping, but it somehow fails. NeedQuotes is miserably wrong.)
I don't have time to check Invoke-Expression and Invoke-Command yet. I guess they are handled by the latter change?
Examples
The change to quoting behavior mainly affects strings with embedded whitespace and double quotes. Consider the snippet below:
$x = 'a" b" c d'
node -e "console.log(process.argv.splice(1).join('|'))" -- "$x"In current powershell, the string printed would be a|b c d, a result that can only be understood by one with an idea about how MSVCRT parses arguments and how we decide what to wrap quotes around for. In the proposed version, the output would be the a" b" c d, exactly as fed into the command (as Write-Output would show).
The linked PR, #13099, also changes how empty parameters are handled. They were previously omitted on the command-line, but now they are included correctly as empty native arguments. As a result, unassigned variables used will shift the parameter position by one. To avoid this, initalize it to an empty list.
# add $cpflag = @()
if ($shouldReflink) {
$cpflag = '--reflink=auto'
}
cp -a $cpflag $source $dest#13099 also enables globbing on ALL bare words. As a result, constructs like a` b* is now subject to globbing. If globbing is undesired, you should be quoting the string. It is expected that few people will rely on the old behavior, as quoted strings are easier to use anyways. A separate proposal (#13098) will address the ergonomics of needing to manually backtick-escaping everything.
A lot of old code is expected to rely on the old example 2 behavior, since that's how POSIX sh works with unquoted expansions. More complex frameworks may do its own quoting, coming into conflict with example 1. An experimental feature switch will be added that covers all three.
Activity
iSazonov commentedon Jul 3, 2020
Artoria2e5 commentedon Jul 3, 2020
Since this will be a public interface change, some protocol version bump will needed. I dont really get how the whole versioning thing works, but if we can keep the 7 while bumping the protocol version that will be great too.
Ah shit, there's a dedicated repo for RFCs and I didnt see that. Ooooops. Call this a pre-RFC then.
[-][RFC] Argument Escape Migration (Start-Process, &, and the normal cmd starting; v7.1 / 8)[/-][+][RFC] Argument Escape Migration (Start-Process, &, and the normal cmd starting; v7.1 / v7.2)[/+][-][RFC] Argument Escape Migration (Start-Process, &, and the normal cmd starting; v7.1 / v7.2)[/-][+][Pre-RFC] Argument Escape Migration (Start-Process, &, and the normal cmd starting; v7.1 / v7.2)[/+]Cmdline changes set for 7.2 (draft)
daxian-dbw commentedon Jul 5, 2020
Mingye Wang (@Artoria2e5) No you are doing the right thing. Open an issue for discussion is the first step before moving to draft an RFC.
I suggest you to include more examples to demonstrate your proposal.
TSlivede commentedon Jul 5, 2020
Just duplicating would probably not be enough. For some edgecases you could probably read my old RFC and the corresponding discussion starting at PowerShell/PowerShell-RFC#90 (comment)
TSlivede commentedon Jul 5, 2020
Specifically regarding the behavior of
Start-Processthere is also #5576 posted by Michael Klement (@mklement0)And while I do like the Idea of using
Start-Processinstead of a new operator (suggested here: #13068 (comment)) it would require to make some other features available toStart-Process: AFAIKStart-Processcan currently not redirect the out streams of the executable to the next command in the pipeline. And there are probably more differences betweenStart-Processand the call operator&...Artoria2e5 commentedon Jul 5, 2020
Wait, there's a three-year-old RFC for this?
I want to keep stuff... minimal. My plan is that rogue programs will eventually only be served by
--%, for which people will write modules around. It should get documented after all. There is the problem of being unable to pass a literal%, but people should be able to make up their own env var for it.--=is nice and shouldn't be too hard to add.I cannot think of a good interface for reusable escaping. Doing
cmdwould take either a very complicated version of the normal rules that keeps tracks of cmd's view of quoting or a non-standard one with the undocumeted"". And top it with the usual/c /k /s "verbatim cmdline here"stuff.I kind of like the idea of preserving quoting structure, but then a bareword with trailing backslashes will always throw it off. Issue 13098 can however use some inspection if I can figure out what the AST looks like with
'a 'bvsa' b'...mklement0 commentedon Jul 5, 2020
TSlivede
This is not an either-or scenario: both direct/
&-based invocation andStart-Processmust be fixed - they serve distinct purposes.If I understand Mingye Wang (@Artoria2e5)'s proposal correctly, the aim with respect to direct/
&-based invocation is to fix them in a breaking manner.However, my understanding is that there is still a fundamental unwillingness to ever take such a big breaking change.
Correct, Steve Lee (@SteveL-MSFT)?
With this in mind:
The proposal in Start-Process doesn't pass arguments with embedded whitespace and double quotes correctly #5576 is to add a new parameter to
Start-Process,-IndividualArguments, which could useSystem.Diagnostics.ProcessStartInfo.ArgumentListbehind the scenes (that on the PowerShell side it is-ArgumentListthat is the broken parameter is unfortunate, but cannot be helped).Since direct/
&-based invocation cannot be fixed, Call native operator #13068 (comment) proposes that the fix be implemented as a new operator.This would mean implementing TSlivede's RFC (Create RFC####-Improve-generation-of-argument-string-for-executables PowerShell-RFC#90 (comment)), the fundamentals of which I think are agreed upon, but a few details have yet to be worked out.
While it is, of course, unfortunate, that direct/
&-based invocation will continue to be broken, introducing something like&!is (a) concise) and (b) at least easy to communicate:&!instead of&(for PowerShell commands it will act like&already does, for external programs, it will pass arguments properly.&!, don't use direct invocation.Fixing the original features is undoubtedly preferable in an ideal world, but with the commitment to backward compatibility that isn't an option.
Artoria2e5 commentedon Jul 6, 2020
The RFC has an idea about a feature switch variable for controlling the behavior. It should be a good option for maintaining backward compatibility too if we make it scopable.
mklement0 commentedon Jul 6, 2020
Mingye Wang (@Artoria2e5)
The RFC proposes a preference variable as an opt-out mechanism - to opt out of the new, correct behavior, in order to get the old, broken behavior.
This logic contradicts the commitment to backward compatibility.
Even if we were to reverse that logic - asking for an opt-in to the new, correct behavior, PowerShell's dynamic scoping of (preference) variables makes this solution problematic - see Arguments for external executables aren't correctly escaped #1995 (comment)
A new operator, as awkward as that is, avoids these problems.
Cmdline changes set for 7.2 (draft)
jeff-hykin commentedon May 9, 2021
For anyone looking for an essentially bulletproof workaround to this issue (that adjusts itself depending on the powershell version.
Comparison
Code
This should always escape the arguments correctly, even when taking into account the planned 7.2 change here.
mklement0 commentedon May 9, 2021
Jeff Hykin (@jeff-hykin), if you're looking for something prepackaged that works robustly and also incorporates the important accommodations on Windows proposed in #15143 that the new-in-PowerShell Core 7.2.0-preview.5 experimental
PSNativeCommandArgumentPassingfeature does not and seemingly will never include (though it does work robustly on Unix), consider theiefunction from theNativemodule (authored by me).jeff-hykin commentedon May 9, 2021
Michael Klement (@mklement0) Thanks for the info, that looks like a much better long term solution.
DemiMarie commentedon May 12, 2021
Can we at least include Michael Klement (@mklement0)’s msiexec mitigation? If I understand it correctly, it is 100% compatible and
requires no special-casing whatsoeverstill needs special-casing for certain commands 😞.mklement0 commentedon May 12, 2021
Demi Marie Obenour (@DemiMarie), I suggest posting your comment in issue #15143, as that is where the official decision not to include such accommodations was announced.
microsoft-github-policy-service commentedon Nov 16, 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 16, 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 16, 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 23, 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.
Fix Start-Process argv word-splitting in Invoke-CopilotOnce
Triage pipeline step 2: per-item AI analysis on the weekly triage iss…