Skip to content

[Pre-RFC] Argument Escape Migration (Start-Process, &, and the normal cmd starting; v7.1 / v7.2) #13089

@Artoria2e5

Description

@Artoria2e5

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 -ArgumentList available (in the sense of escaping like System.Diagnostics.Process does) by adding a switch to Start-Process that defaults to false.
  • For powershell v7.2, we make the correct behavior the default by:
    • making the new Start-Process switch default to $true
    • making escaping the behavior for & and normal external command calls
    • bumping whatever protocol version is affected

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" $x

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Issue-Enhancementthe issue is more of a feature request than a bugResolution-No ActivityIssue has had no activity for 6 months or moreWG-Enginecore PowerShell engine, interpreter, and runtime

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions