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

Argument with colon in it is incorrectly handled if not quoted #23819

Open
5 tasks done
CrendKing opened this issue May 19, 2024 · 12 comments
Open
5 tasks done

Argument with colon in it is incorrectly handled if not quoted #23819

CrendKing opened this issue May 19, 2024 · 12 comments
Labels
Needs-Triage The issue is new and needs to be triaged by a work group. WG-Language parser, language semantics

Comments

@CrendKing
Copy link

Prerequisites

Steps to reproduce

I understand the -<arg_name>:<arg_value> syntax in PowerShell. But it seems there lacks a way to tell PowerShell to simply passing the argument as-is without parsing, without requiring caller to explicitly quoting the whole thing. I found the issue #6292 (comment) discussed this 6 years ago, but I can't find any fix for that issue. If I'm missing a solution to this, please let me know.

Why does this matter? Because some applications like ffmpeg extensively use the -<arg_name>:<arg_value> syntax. There is no way to write a function to wrap such application cleanly.

function test {
    Write-Host $args
}

function Call-ffmpeg {
    ffmpeg $args
}

test -a:b 1

Call-ffmpeg -i test.mp4 -filter:v null -f null -

Expected behavior

-a:b 1

<ffmpeg successfully processing the file>

Actual behavior

-a: b 1

<ffmpeg reports "Unable to choose an output format for 'null'; use a standard extension for the filename or specify the format manually.">

Error details

No response

Environment data

PSVersion                      7.4.2
PSEdition                      Core
GitCommitId                    7.4.2
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

No response

@CrendKing CrendKing added the Needs-Triage The issue is new and needs to be triaged by a work group. label May 19, 2024
@mklement0
Copy link
Contributor

mklement0 commented May 19, 2024

Unfortunately, there are a number of long-standing bugs relating to argument-passing to external (native) programs.

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

@jborean93
Copy link
Collaborator

In this case the problem isn't with the native parser but just how parameter binding in PowerShell works in general. Calling a native executable command directly will go through one binder that passes through -foo:bar as is but when you call a function it'll go through the normal binder where -foo:bar is treated as -foo bar.

So by the time Call-ffmpeg is calling the exe, the $args would have already converted -filter:v into distinct arguments -filter, v as that's how the parser and normal function binder works. You can see the difference between calling it in the function vs calling it normally.

function Call-ffmpeg {
    C:\temp\print_argv.exe $args
}

Call-ffmpeg -i test.mp4 -filter:v null -f null -

"C:\temp\print_argv.exe" -i test.mp4 -filter: v null -f null -
[0] -i
[1] test.mp4
[2] -filter:
[3] v
[4] null
[5] -f
[6] null
[7] -

C:\temp\print_argv.exe -i test.mp4 -filter:v null -f null -

"C:\temp\print_argv.exe" -i test.mp4 -filter:v null -f null -
[0] -i
[1] test.mp4
[2] -filter:v
[3] null
[4] -f
[5] null
[6] -

While I can see an argument about this being a bug I personally think it's just how things work and changing the behaviour today would probably result in more problems.

The simplest solution here is to just quote your argument or provide it as an array to your wrapper with the value already provided as a string:

function Call-ffmpeg {
    ffmpeg @args
}

Call-ffmpeg -i test.mp4 '-foo:bar'
Call-ffmpeg @('-i', 'test.mp4', '-foo:bar')

@CrendKing
Copy link
Author

Ideally there could be something like a

function Call-ffmpeg {
    [CmdletBinding(PassArgumentsAsIs = $true)]  # <--
    param(
        [Parameter(Mandatory, ValueFromRemainingArguments)]
        $args
    )
    ...
}

that allows user to change how the "binder" works.

@mklement0
Copy link
Contributor

mklement0 commented May 20, 2024

@jborean93, you're right, the problem is the parameter binder for PowerShell commands in this case.

That it is a bug, even by the rules of PowerShell commands, is exemplified by the following:

  • -- is not honored:
# !! -> '-foo: bar' - space inserted (due to mistaken interpretation as a parameter name-value pair)
Write-Host -- -foo:bar
  • -switch:$false isn't passed through with @args splatting:
# -> !! turns into the equivalent of -foo:$TRUE
& { & { param([switch] $foo) $PSBoundParameters } @args } -foo:$false

As for fixing the bug in the context of relaying arguments to native programs - probably the most likely scenario - @BrucePay suggested the following in the aforementioned #6360 (see there for more context):

One way to fix this is to propagate the "NoSpace" token property into metadata on the corresponding string value. The NativeCommandParameterBinder could then check for this metadata when converting its arg array into a string. It's probably worth thinking about this a bit more to see if there are other cases (especially *nix specific cases) that should be addressed.

@StevenBucher98 StevenBucher98 added the WG-Language parser, language semantics label May 20, 2024
@jhoneill
Copy link

Why does this matter? Because some applications like ffmpeg extensively use the -<arg_name>:<arg_value> syntax. There is no way to write a function to wrap such application cleanly.

It is perfectly possible to call command like the ffmpeg one you gave from PowerShell .
The problem is that you seem to want to run a PowerShell command as if it were ffmpeg with normal PowerShell-isms about commands suspended.

Really the job of an external-command wrapper is to take parameters in PowerShell style (naming each one, allowing them to be in any order if named, suggesting names but removing names when they are not compatible with parameters already provided,
providing tab completion for values where possible yada yada yada) and transforming those into whatever weird and/or wonderful way of doing things seemed a good idea to the external command's authors when they first though about it.

$args is a throwback to V1 of PowerShell and does not work as soon as you add [cmdletBinding()] or declare parameter attributes```
function foo { param($bar) $bar ; $args}

foo 1 2 3
1
2
3
function foo { param([Parameter(ValueFromPipeline)]$bar) $bar ; $args}
foo 1 2 3
foo: A positional parameter cannot be found that accepts argument '2'.

 if not officially deprecated, its use is widely considered to be ill advised.   Using `externalCommand $args` is a pretty bad idea all round, but doing it right does mean more code in the wrapper than you might like.      


 



@CrendKing
Copy link
Author

@jhoneill I'm so sorry I don't understand what you are talking about. What I asked for is a way to remove this inconsistency as follows:

  1. Run ffmpeg -i test.mp4 -filter:v null -f null - in PowerShell. Every works fine.
  2. User wants to always hide the ffmpeg banner. Since ffmpeg doesn't allow any environment variable or config file to turn it off, user has to always pass the -hide_banner argument. Since the only way in PowerShell to make a command alias with arguments included is function, naturally user creates
function ffmpeg {
    D:\ffmpeg\bin\ffmpeg.exe -hide_banner $args
}
  1. Suddenly the same ffmpeg -i test.mp4 -filter:v null -f null - no longer works. User has to do ffmpeg -i test.mp4 '-filter:v' null -f null - from now on.

In Bash, one can just alias ffmpeg="D:\ffmpeg\bin\ffmpeg.exe -hide_banner" and it works. It is a problem that was solved 35 years ago since 1989, yet PowerShell still struggles today. Are we expecting too much?

@MartinGC94
Copy link
Contributor

You have a valid point but bringing up the age of the tool as if that's the reason why it's not working like Bash is silly. PowerShell aliases are simply different from Bash. I don't like the way Bash handles whitespace but I don't expect them to change it just because other shells do it differently.

You can do whatever you want inside a function so just add some additional processing inside your function to join the colon parameters with your value, here's an example to get you started:

    function ffmpeg
    {
        $Param = ""
        $NewArgs = foreach ($Item in $args)
        {
            if ($Item.EndsWith(':'))
            {
                $Param = $Item
                continue
            }

            if ($Param -ne "")
            {
                $Param + $Item
                $Param = ""
            }
            else
            {
                $Item
            }
        }

        D:\ffmpeg\bin\ffmpeg.exe -hide_banner $NewArgs
    }

@CrendKing
Copy link
Author

But why do we have to do this if without the function the "binder" can already correctly process the colons? Why can't there be an option to allow user to use that "binder" for function? I brought up Bash because I think Bash got their priority right. The consistency of syntax with or without function should be the most important to be preserved. PowerShell already has advanced parameter attributes. It should have the colon splatting optional unless a special attribute presents, not the other way:

param(
    [switch]$a,
    [Parameter(ColonSplat)]
    $b,
    $c
)

test -a:$false -b:value_of_b -c:this_is_part_of_the_whole_argument_not_just_value_of_c

@mklement0
Copy link
Contributor

mklement0 commented May 22, 2024

@CrendKing

@jhoneill
Copy link

@jhoneill I'm so sorry I don't understand what you are talking about.

OK. Basic premise is that PowerShell commands should all work in the same way. That means they don't have unnamed parameters, and the use of $Args has been discouraged for as long as I can remember.

If you write a function to do a bunch of things which use ffmpeg that function should work like everything else in PowerShell , and should take parameters in PowerShell style and translate them when calling ffmpeg.

What I asked for is a way to remove this inconsistency as follows:

  1. Run ffmpeg -i test.mp4 -filter:v null -f null - in PowerShell. Every works fine.

So far so good, and you can build a function to call that...

  1. User wants to always hide the ffmpeg banner. Since ffmpeg doesn't allow any environment variable or config file to turn it off, user has to always pass the -hide_banner argument. Since the only way in PowerShell to make a command alias with arguments included is function,

Ah, what I hadn't picked up from the first post is that your not doing a bunch of stuff which involves ffmpeg, you just want an alias-with-parameters which PowerShell doesn't naturally do.
One way to do what bash does and replace ffmpeg with ffmpeg.exe -no_banner is
function ffmpeg {Invoke-Expression ($MyInvocation.statement -replace "^.*ffmpeg", "ffmpeg.exe -hide_banner") }

invoke-expression has its detractors but it's the lesser of multiple evils here

This

function ffmpeg {
    D:\ffmpeg\bin\ffmpeg.exe -hide_banner $args
}

Is the way many new to PowerShell users who expect their scripts to refer to arguments 1 , 2 and 3 etc would do it writing a PowerShell wrapper for all the parameters to do a simple fix like this is horribly laborious - but when it it is fixing a particularly awkward utility PowerShell's assumptions about prepping stuff for a PowerShell command mean the roof falls in.

In Bash, one can just alias ffmpeg="D:\ffmpeg\bin\ffmpeg.exe -hide_banner" and it works.

Yup. PowerShell aliases were to replace names. Re-writing the command and all its parameters, use $MyInvocation.statement not args

It is a problem that was solved 35 years ago since 1989, yet PowerShell still struggles today. Are we expecting too much?

Like so many other things if you know where to look PowerShell had a solution back at the start.

@CrendKing
Copy link
Author

Thank you! Invoke-Expression + $MyInvocation.Statement is indeed a trick to solve the "dumb" aliasing use case.

@mklement0
Copy link
Contributor

mklement0 commented May 23, 2024

@CrendKing:

While it's always good to know a workaround, we shouldn't lose sight of the bigger picture:

  • There is no justification for PowerShell's current behavior: something that doesn't bind to a formally declared parameter must be retained in its original form via $args / @args, especially when it is explicitly designated as not being a named argument, following --.

The - obscure - Invoke-Expression workaround may work in simple cases, but it isn't robust, because blindly re-invoking a statement's source code, as reflected in $MyInvocation.Statement, can repeat operations that have side effects.

Here's a simple example (run on Windows):

function foo { iex ($MyInvocation.Statement -replace '^foo\b', 'cmd /c echo') }
$i = 0; foo (++$i)

This prints 2 rather than 1, because the ++ operation was performed twice.

(It also doesn't address automatic support for pipeline input, though you can't fault it for that, as even fixing the bug at hand wouldn't address that; only #12962 (comment) would.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Triage The issue is new and needs to be triaged by a work group. WG-Language parser, language semantics
Projects
None yet
Development

No branches or pull requests

6 participants