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

[ProxyCommand]::Create doesn't generate dynamic param block for function that has dynamic parameters #4792

Open
daxian-dbw opened this issue Sep 9, 2017 · 14 comments · May be fixed by #20392
Labels
In-PR Indicates that a PR is out for the issue Issue-Enhancement the issue is more of a feature request than a bug

Comments

@daxian-dbw
Copy link
Member

Steps to reproduce

function ProxyTest {
    [CmdletBinding(DefaultParameterSetName='Name')]
    param (
        [Parameter(ParameterSetName="Name", Mandatory=$true)]
        [ValidateSet("Orange", "Apple")]
        [string] $Name,

        [Parameter(ParameterSetName="Id", Mandatory=$true)]
        [ValidateRange(1,5)]
        [int] $Id,

        [Parameter(ValueFromPipeline)]
        [string] $Message
    )

    DynamicParam {
        $ParamAttrib  = [parameter]::new()
        $ParamAttrib.Mandatory  = $true

        $AttribColl = [System.Collections.ObjectModel.Collection[System.Attribute]]::new()
        $AttribColl.Add($ParamAttrib)

        $RuntimeParam  = [System.Management.Automation.RuntimeDefinedParameter]::new('LastName', [string], $AttribColl)
        $RuntimeParamDic  = [System.Management.Automation.RuntimeDefinedParameterDictionary]::new()
        $RuntimeParamDic.Add('LastName',  $RuntimeParam)
        return  $RuntimeParamDic
    }

    Begin {
        $AllMessages = @()
        if ($PSCmdlet.ParameterSetName -eq "Name") {
            $MyString = $Name, $PSBoundParameters['LastName'] -join ","
        } else {
            $MyString = $Id, $PSBoundParameters['LastName'] -join ","
        }
    }

    Process {
        if ($Message) {
            $AllMessages += $Message
        }
    }

    End {
        $MyString + " - " + ($AllMessages -join ";")
    }
}

$cmdinfo = Get-Command ProxyTest
$cmdMetadata = [System.Management.Automation.CommandMetadata]::new($cmdinfo)
[System.Management.Automation.ProxyCommand]::Create($cmdMetadata, "HelpContent", $true)

Expected behavior

There is dynamic parameters defined in the function ProxyTest:

PS> $cmdinfo.Parameters.GetEnumerator() | ? { $_.Value.IsDynamic }

Key      Value
---      -----
LastName System.Management.Automation.ParameterMetadata

And when calling [ProxyCommand]::Create, the $true is the value for parameter bool generateDynamicParameters, so it should generate the dynamic param block in the proxy script.
However, the returned script has DynamicParam block in it.

Actual behavior

No DynamicParam block generated.
But if it's a binary cmdlet instead of a function, then the dynamicparam block is correctly generated.

Environment data

This happens in powershell core and windows powershell.
The cause might be that the constructor public CommandMetadata(CommandInfo commandInfo) never check if dynamic parameters are defined for CommandInfo arguments that are other than CmdletInfo. This code in internal CommandMetadata(ScriptBlock scriptblock .. probably should be moved to Init(ScriptBlock scriptBlock, string name, bool shouldGenerateCommonParameters).

@TravisEz13 TravisEz13 added the Issue-Enhancement the issue is more of a feature request than a bug label Apr 16, 2018
@thomasmktong
Copy link

Any suggested way to workaround this issue? Thanks.

@lzybkr
Copy link
Member

lzybkr commented Apr 30, 2018

Many years ago I used the following code in a proxy for Get-ChildItem:

    dynamicparam
    {
        $argList = @($psboundparameters.getenumerator() | % { "-$($_.Key)"; $_.Value })
        
        $wrappedCmd = Get-Command Get-ChildItem -Type Cmdlet -ArgumentList $argList
        $providerParams = @($wrappedCmd.Parameters.GetEnumerator() | Where-Object { $_.Value.IsDynamic })
        if ($providerParams.Length -gt 0)
        {
            $paramDictionary = new-object System.Management.Automation.RuntimeDefinedParameterDictionary
            foreach ($param in $providerParams)
            {
                $param = $param.Value
                $dynParam1 = new-object System.Management.Automation.RuntimeDefinedParameter $param.Name, $param.ParameterType, $param.Attributes
                $paramDictionary.Add($param.Name, $dynParam1)            
            }
            
            return $paramDictionary
        }
    }

Worth noting - this takes the current arguments and passes them to Get-Command which would otherwise return possibly incorrect dynamic parameters, e.g. if the current directory was in the file system, but the command was passed a certificate path.

There are problems with how Get-Command does parameter binding, so this approach isn't quite perfect, but it's the most general approach I'm aware of.

@mklement0
Copy link
Contributor

Worth adding that including dynamic parameters is the default behavior for compiled cmdlets, and therefore also should be for (advanced) functions and scripts (which are equally affected).

@IISResetMe has found a workaround via System.Management.Automation.ProxyCommand.GetDynamicParam, demonstrated in this Stack Overflow answer.

This method automatically generates code similar to @lzybkr's above; running
[System.Management.Automation.ProxyCommand]::GetDynamicParam((Get-Command ProxyTest)) yields:

    try {
        $targetCmd = $ExecutionContext.InvokeCommand.GetCommand('ProxyTest', [System.Management.Automation.CommandTypes]::Function, $PSBoundParameters)
        $dynamicParams = @($targetCmd.Parameters.GetEnumerator() | Microsoft.PowerShell.Core\Where-Object { $_.Value.IsDynamic })
        if ($dynamicParams.Length -gt 0)
        {
            $paramDictionary = [Management.Automation.RuntimeDefinedParameterDictionary]::new()
            foreach ($param in $dynamicParams)
            {
                $param = $param.Value

                if(-not $MyInvocation.MyCommand.Parameters.ContainsKey($param.Name))
                {
                    $dynParam = [Management.Automation.RuntimeDefinedParameter]::new($param.Name, $param.ParameterType, $param.Attributes)
                    $paramDictionary.Add($param.Name, $dynParam)
                }
            }

            return $paramDictionary
        }
    } catch {
        throw
    }

@StevenBucher98
Copy link
Collaborator

I am getting the expected output on PS 7.2+, marking as resolved.

@StevenBucher98 StevenBucher98 added the Resolution-Fixed The issue is fixed. label Jun 6, 2022
@ghost
Copy link

ghost commented Jun 8, 2022

This issue has been marked as fixed and has not had any activity for 1 day. It has been closed for housekeeping purposes.

@ghost ghost closed this as completed Jun 8, 2022
@mklement0
Copy link
Contributor

@StevenBucher98, unfortunately, this is not fixed, which you can verify as follows (after running the code in the initial post):

[System.Management.Automation.ProxyCommand]::Create($cmdMetadata, "HelpContent", $true) -match 'dynamicparam'

It should return $true, but still returns $false as of PowerShell 7.3.1 / PowerShell 7.4.0-preview.1

@StevenBucher98
Copy link
Collaborator

Yep you are right @mklement0, not sure what I did back in June but I will reopen 😄

@ghost
Copy link

ghost commented Dec 29, 2022

This issue has been marked as fixed and has not had any activity for 1 day. It has been closed for housekeeping purposes.

@ghost ghost closed this as completed Dec 29, 2022
@mklement0
Copy link
Contributor

Glad to hear it, @StevenBucher98, but it looks like you'll have to re-open again and remove the Resolution-Fixed label.

@StevenBucher98 StevenBucher98 removed the Resolution-Fixed The issue is fixed. label Dec 29, 2022
@ghost ghost added the In-PR Indicates that a PR is out for the issue label Jan 24, 2023
@ghost ghost added In-PR Indicates that a PR is out for the issue and removed In-PR Indicates that a PR is out for the issue labels Mar 14, 2023
@ghost ghost added In-PR Indicates that a PR is out for the issue and removed In-PR Indicates that a PR is out for the issue labels Apr 28, 2023
@ghost ghost removed the In-PR Indicates that a PR is out for the issue label Jun 10, 2023
@fsackur
Copy link

fsackur commented Sep 28, 2023

Hi, I just hit this issue myself.

It looks like the overload for CommandMetadata.Init for cmdlets sets _implementsDynamicParameters by calling ConstructCmdletMetadataUsingReflection, but the Init overload for scriptblocks does not:

private void Init(ScriptBlock scriptBlock, string name, bool shouldGenerateCommonParameters)

My ignorance may be showing, but I think I've fixed it in https://github.com/fsackur/PowerShell/tree/Fix_CommandMetadata_ImplementsDynamicParameters

Output from the repro snippet
[CmdletBinding(DefaultParameterSetName='Name')]
param(
    [Parameter(ParameterSetName='Name', Mandatory=$true)]
    [ValidateSet('Orange','Apple')]
    [string]
    ${Name},

    [Parameter(ParameterSetName='Id', Mandatory=$true)]
    [ValidateRange(1, 5)]
    [int]
    ${Id},

    [Parameter(ValueFromPipeline=$true)]
    [string]
    ${Message})


dynamicparam
{
    try {
        $targetCmd = $ExecutionContext.InvokeCommand.GetCommand('ProxyTest', [System.Management.Automation.CommandTypes]::Function, $PSBoundParameters)
        $dynamicParams = @($targetCmd.Parameters.GetEnumerator() | Microsoft.PowerShell.Core\Where-Object { $_.Value.IsDynamic })
        if ($dynamicParams.Length -gt 0)
        {
            $paramDictionary = [Management.Automation.RuntimeDefinedParameterDictionary]::new()
            foreach ($param in $dynamicParams)
            {
                $param = $param.Value

                if(-not $MyInvocation.MyCommand.Parameters.ContainsKey($param.Name))
                {
                    $dynParam = [Management.Automation.RuntimeDefinedParameter]::new($param.Name, $param.ParameterType, $param.Attributes)
                    $paramDictionary.Add($param.Name, $dynParam)
                }
            }

            return $paramDictionary
        }
    } catch {
        throw
    }
}

begin
{
    try {
        $outBuffer = $null
        if ($PSBoundParameters.TryGetValue('OutBuffer', [ref]$outBuffer))
        {
            $PSBoundParameters['OutBuffer'] = 1
        }

        $wrappedCmd = $ExecutionContext.InvokeCommand.GetCommand('ProxyTest', [System.Management.Automation.CommandTypes]::Function)
        $scriptCmd = {& $wrappedCmd @PSBoundParameters }

        $steppablePipeline = $scriptCmd.GetSteppablePipeline()
        $steppablePipeline.Begin($PSCmdlet)
    } catch {
        throw
    }
}

process
{
    try {
        $steppablePipeline.Process($_)
    } catch {
        throw
    }
}

end
{
    try {
        $steppablePipeline.End()
    } catch {
        throw
    }
}

clean
{
    if ($null -ne $steppablePipeline) {
        $steppablePipeline.Clean()
    }
}
<#
HelpContent
#>

Would a maintainer be willing to look at a PR?

@fsackur fsackur linked a pull request Sep 28, 2023 that will close this issue
22 tasks
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR Indicates that a PR is out for the issue label Sep 28, 2023
@fsackur
Copy link

fsackur commented Dec 7, 2023

@mklement0 Whatever broke the build on mac is fixed, and my PR is green. It's only small - any chance of a review?

@mklement0
Copy link
Contributor

@fsackur, others are better qualified to conduct reviews, perhaps @daxian-dbw or @IISResetMe.

@fsackur
Copy link

fsackur commented Jan 11, 2024

Hi @daxian-dbw or @IISResetMe - this is a small change, but the bug bothered me and cost me a day; would either of you be willing to review it?

@phatmandrake
Copy link

I also would find this incredibly useful for a module I'm working on that I have to work around with a ton more effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In-PR Indicates that a PR is out for the issue Issue-Enhancement the issue is more of a feature request than a bug
Projects
None yet
8 participants