Powershell: The 'help' function cannot use a program specified in the '$env:PAGER' if its path contains spaces

[a-tinsmith]

Steps to reproduce

1. Set $env:PAGER to a program with spaces in the path. For example:

> $env:PAGER = "C:\Program Files\Git\usr\bin\less.exe"

1. Invoke help function:

> help gcm

Expected behavior

New pager program is used to display help contents.

Actual behavior

The help contents cannot be displayed:

& : The term 'C:\Program' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
At line:76 char:23
+             $help | & $moreCommand $moreArgs
+                       ~~~~~~~~~~~~
+ CategoryInfo          : ObjectNotFound: (C:\Program:String) [], CommandNotFoundException
+ FullyQualifiedErrorId : CommandNotFoundException

Notice how help command handles $env:PAGER:

> (gcm help).ScriptBlock
...
    else
    {
        # Respect PAGER, use more on Windows, and use less on Linux
        $moreCommand,$moreArgs = $env:PAGER -split '\s+'
        if ($moreCommand) {
            $help | & $moreCommand $moreArgs
        } elseif ($IsWindows) {
            $help | more.com
        } else {
            $help | less
        }
    }

The contents of the variable is split with \s+ pattern, which ignores any quotes or escape characters.

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      6.1.0
PSEdition                      Core
GitCommitId                    6.1.0
OS                             Microsoft Windows 10.0.17134
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[iSazonov]

It will be simple fix here https://github.com/SteveL-MSFT/PowerShell/blob/3b078bd188e59d2519464d255c140320b39e1c8e/src/System.Management.Automation/engine/InitialSessionState.cs#L4767

[mklement0]

Thanks for pinpointing the source-code location, @iSazonov

While the fix is technically not difficult, we need to get clarity on the specific formats we want to support:

• Executable-path-only: Allow $env:PAGER = "C:\Program Files\Git\usr\bin\less.exe", without requiring _embedded_ quoting, by employing heuristics?

  • If the value has embedded spaces but no embedded quoting check to see if it happens to refer to an existing executable path only (as opposed to an executable + arguments) and accept it as such?

• Executable-path-plus-arguments: What embedded quoting styles do we support? Both single- and double-quoting?

  • $env:PAGER = '"C:\Program Files\Git\usr\bin\less.exe" -w' # embedded "..." quoting
  • $env:PAGER = '''C:\Program Files\Git\usr\bin\less.exe'' -w' # embedded '...' quoting

[SteveL-MSFT]

The simplest thing here is to add a $env:PAGER_ARGS, but would be a breaking change.

[vexx32]

Hmmmm. Something simpler? A delimiter between pager and args perhaps?

$env:PAGER = 'C:\Program Files\Git\usr\bin\less.exe;-w'

Documentation necessary, of course.

[SteveL-MSFT]

@vexx32 a creative solution, but that would be setting a precedent introducing a new syntax and also less discoverable. Alternative is require quotes in the value to differentiate executable path with whitespace and parameters which is consistent with the command line. Since this is an advanced use case, perhaps documenting that is sufficient. So it would look like:

$env:PAGER = '"C:\Program Files\Git\usr\bin\less" -w'

[vexx32]

Yep, not a bad solution overall, I think.

[rkeithhill]

I suspect the set of folks using $env:PAGER is pretty small (I'm one of them) and the set of folks using $env:PAGER with arguments is very small (I'm not one of them). So $env:PAGER_ARGS wouldn't be horrible considering that I think this is an interactive use feature (and not used in scripts). That said, requiring exe paths with spaces to be quoted would be fine also.

[mklement0]

$env:PAGER is primarily used on Unix-like platforms, and at least on macOS:

• _both_ kinds of _embedded_ quoting ("..." and '...') _are_ properly recognized by man

• embedded quoting is _required_ in order for executable paths with embedded spaces to be recognized.

Therefore, I suggest:

• we also support both styles of embedded quoting.

• but, as a nod to Windows users - where paths with spaces are more likely - allow _fallback_ to interpreting a value without embedded quoting _in full_ as the binary path, so that users who don't have passing arguments on their mind (which is rare, as @rkeithhill points out) can just specify the full executable path as-is, without having to worry about _embedded_ quoting.

In short: I suggest we apply sh-style command-line parsing - to the value of $env:PAGER and fall back to interpreting a whitespace-containing value _as a whole_ as the binary path.

_Caveat_: man on macOS even recognizes _environment-variable references_ such as $HOME in $env:PAGER (unless '...' is used for embedded quoting).
I'm not sure we need to go this far in our implementation, however.

[SteveL-MSFT]

It seems we don't need to over-engineer this for a small subset of a small subset of users. Based on what @mklement0 is saying, it seems at least man has set some precedent and we should go with the embedded quoting route since some users could be using that today. I would probably just go with a simple regex matching for this.

[a-tinsmith]

I believe that following man behaviour is the most sensible thing to do. I would assume that users who set PAGER variable expect it to behave as with man, so there is no surprises for them.

[mklement0]

Here's my suggestion for a pragmatic solution (to replace the following code: https://github.com/PowerShell/PowerShell/blob/b27380dc510fe3cc2b8dbef056d882e6086dcd20/src/System.Management.Automation/engine/InitialSessionState.cs#L4238-L4245):

Important: In order to embed the code below in the verbatim string in the C# code, all " instances must be doubled.

$customPagerCommand = $null
if ($customPagerCommandLine = $env:PAGER) {

  # Sanitize the command line to prevent injection attacks.
  # Note: This sanitization is overeager in that it also escapes metacharacters
  #       inside embedded single-quoted tokens, but I doubt that that's a real-world
  #       concern.
  # Effectively, ignore anything other than simple [environment-]variable references.
  # Note:
  #       If the command needs to target environment variables, the PS-specific
  #       syntax - .e.g, $env:USER - then makes the command PS-specific on 
  #       Unix-like platforms.
  #       $HOME is the only variable that would work without $env: in PS too.
  #       Conceivably, we could interpret $var as $env:var by default, but
  #       I'm not sure that's worth the effort, and might cause confusion.
  $customPagerCommandLineSanitized = $customPagerCommandLine -replace '[(),{};@<>|]', '`$&'

  # Split the command line into tokens, respecting quoting.
  # Thanks to sanitizing, use of InvokeExpression should be safe here.
  $customPagerCommand, $customPagerArgs = Invoke-Expression "Write-Output -- $customPagerCommandLineSanitized"

  # See if the first token refers to a known command (executable), 
  # and if not, see if the command line as a whole is an executable path.
  $cmds = Get-Command $customPagerCommand, $customPagerCommandLine -ErrorAction Ignore
  if (-not $cmds) {
    # Custom command is invalid; ignore it, but issue a warning.
    Write-Warning "Ignoring invalid custom-paging utility command line specified in `$env:PAGER: $customPagerCommandLine"
    $customPagerCommand = $null # use default command
  }
  elseif ($cmds.Count -eq 1 -and $cmds[0].Source -eq $customPagerCommandLine) {
    # The full command line is an unquoted path to an existing executable
    # with embedded spaces.
    $customPagerCommand = $customPagerCommandLine
    $customPagerArgs = $null
  }
}

if ($customPagerCommand) {
  $help | & $customPagerCommand $customPagerArgs
}
elseif ($IsWindows) {
  $help | more.com
}
else {
  $help | less -Ps"Page %db?B of %D:.\. Press h for help or q to quit\.$"
}

See the code comments for tradeoffs.

Note that, as an additional courtesy, an invalid command triggers a warning and fallback to the default behavior.

The above should handle strings such as the following (showing embedded quoting only), with or without arguments:

/usr/bin/less
less -r
'less' -r
"less" -r
less -Ps"Page %db?B of %D:.\. Press h for help or q to quit\.$"
"C:\Program Files\Git\usr\bin\less.exe"
C:\Program Files\Git\usr\bin\less.exe    # as a courtesy, recognize a file path (only) with embedded spaces)

[pougetat]

Hello,
Can I work on this :) ?