Use module-qualified cmdlet names when invoking plugins by Tadas · Pull Request #129 · poshbotio/PoshBot

Tadas · 2018-10-24T21:58:21Z

Description

I wanted to use start cmdlet name in a plugin but discovered that command execution jobs never finish executing. Turns out that start was being resolved as Start-Process by PowerShell. PoshBot should use module-qualified cmdlet names when invoking plugin commands to ensure that the right cmdlet is called regardless of aliases.

https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_command_precedence?view=powershell-6#qualified-names

Related Issue

Motivation and Context

Enables usage of cmdlet names in plugins which might be hidden by an alias e.g. start (a default alias for Start-Process)

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

devblackops · 2018-10-25T16:26:22Z

Good idea @Tadas. Thanks!

devblackops · 2018-10-26T05:02:34Z

@Tadas I'm trying to repro your issue but can't get the problem to occur. This is what I'm trying:

function Start {
    [cmdletbinding()]
    param()

    'Start your engines'
}

or

function Start-Engine {
    [PoshBot.BotCommand(
        CommandName = 'start'
    )]
    [cmdletbinding()]
    param()

    'Start your engines'
}

or

function Start-Engine {
    [PoshBot.BotCommand(
        Aliases = 'start'
    )]
    [cmdletbinding()]
    param()

    'Start your engines'
}

All of these seem to work fine.

What does your plugin command look like that triggers this?

Tadas · 2018-10-26T23:53:59Z

It can only happen when you have an alias defined

> Get-Alias -Name start

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Alias           start -> Start-Process


> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.18262.1000
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.18262.1000
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Only your first example triggers the bug, that function is pretty much identical to what I have.

devblackops · 2018-10-27T04:26:46Z

Sorry still can't repro this, and I'm not sure why you're getting a problem either. 🤷‍♂️

PoshBot is already executing the actual command from the module, not the module-qualified name, but the actual function/cmdlet object returned from Get-Command.

When it loads the module, it gets all the module's public commands via Get-Command here.
It then stores each object in either the .FunctionInfo or .CmdletInfo property of the [Command] instance depending on if the command is of type Function or Cmdlet here. When it comes time to execute a command in a job, it adds either the .FunctionInfo or .CmdletInfo property to the hashtable that will be sent to the job here. Inside the job, it executes the actual command object here and here.

It never executes the command by name only which I can see would cause a problem with aliases of the same name.

Below is effectively what PoshBot does and even though I've added an alias for start, it still executes the module command because it's not being executed by name.

New-Alias -Name start -Value Start-Process
$c = Get-Command -Module PoshBot.PSSummit -Name start
& $c

I'd like to understand more about how you're running into this.

Tadas · 2018-10-27T22:24:03Z

Ok I think we're running into serialization nuances. Is your plugin begin executed as job?

Just before Start-Job call:

PoshBot/PoshBot/Classes/Command.ps1

Lines 133 to 138 in 5cadba8

    
           $jobParams = @{ 
        
               Name = $jobName 
        
               ScriptBlock = $outer 
        
               ArgumentList = $options 
        
           } 
        
           return (Start-Job @jobParams)

[DBG]: PS D:\Dev\_PoshBot>> $jobParams.ArgumentList.Function.GetType()

IsPublic IsSerial Name                                     BaseType                                          
-------- -------- ----                                     --------                                          
True     False    FunctionInfo                             System.Management.Automation.CommandInfo

and inside of the job:

[DBG]: [Job14]: PS D:\Documents>> l


   26:                  ParsedCommand = $options.ParsedCommand | Select-Object -ExcludeProperty $parsedCommandExcludes
   27:                  OriginalMessage = $options.OriginalMessage
   28:                  BackendType = $options.BackendType
   29:              }
   30:              Wait-Debugger
   31:*             & $cmd @namedParameters @positionalParameters
   32:          



[DBG]: [Job14]: PS D:\Documents>> $cmd.GetType()

IsPublic IsSerial Name                                     BaseType                                          
-------- -------- ----                                     --------                                          
True     True     PSObject                                 System.Object                                     



[DBG]: [Job14]: PS D:\Documents>> $cmd | gm


   TypeName: Deserialized.System.Management.Automation.FunctionInfo

Name                MemberType   Definition                                                                  
----                ----------   ----------                                                                  
GetType             Method       type GetType()                                                              
ToString            Method       string ToString(), string ToString(string format, System.IFormatProvider ...
Namespace           NoteProperty string Namespace=ReproModule                                                
CmdletBinding       Property     System.Boolean {get;set;}                                                   
CommandType         Property     System.String {get;set;}                                                    
DefaultParameterSet Property      {get;set;}                                                                 
Definition          Property     System.String {get;set;}                                                    
Description         Property      {get;set;}                                                                 
HelpFile            Property      {get;set;}                                                                 
Module              Property     System.String {get;set;}                                                    
ModuleName          Property     System.String {get;set;}                                                    
Name                Property     System.String {get;set;}                                                    
Noun                Property     System.String {get;set;}                                                    
Options             Property     System.String {get;set;}                                                    
OutputType          Property     Deserialized.System.Collections.ObjectModel.ReadOnlyCollection`1[[System....
Parameters          Property     Deserialized.System.Collections.Generic.Dictionary`2[[System.String, msco...
ParameterSets       Property     Deserialized.System.Collections.ObjectModel.ReadOnlyCollection`1[[System....
RemotingCapability  Property     System.String {get;set;}                                                    
ScriptBlock         Property     System.String {get;set;}                                                    
Source              Property     System.String {get;set;}                                                    
Verb                Property     System.String {get;set;}                                                    
Version             Property     System.Version {get;set;}                                                   
Visibility          Property     System.String {get;set;}

devblackops · 2018-10-31T04:57:08Z

Ahh. That makes sense.

I'm OK with this change to use the module qualified name.

devblackops

LGTM

Tadas added 2 commits October 24, 2018 21:47

Remove unused ModuleCommand class

d4255cf

Use module-qualified cmdlet names

eb637d7

devblackops self-requested a review October 25, 2018 16:25

devblackops self-assigned this Oct 25, 2018

devblackops added enhancement hacktoberfest labels Oct 25, 2018

devblackops approved these changes Oct 31, 2018

View reviewed changes

devblackops merged commit 03a8e9b into poshbotio:master Oct 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use module-qualified cmdlet names when invoking plugins#129

Use module-qualified cmdlet names when invoking plugins#129
devblackops merged 2 commits intoposhbotio:masterfrom
Tadas:fix/invoke-module-qualified

Tadas commented Oct 24, 2018

Uh oh!

devblackops commented Oct 25, 2018

Uh oh!

devblackops commented Oct 26, 2018

Uh oh!

Tadas commented Oct 26, 2018

Uh oh!

devblackops commented Oct 27, 2018

Uh oh!

Tadas commented Oct 27, 2018

Uh oh!

devblackops commented Oct 31, 2018

Uh oh!

devblackops left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Tadas commented Oct 24, 2018

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

devblackops commented Oct 25, 2018

Uh oh!

devblackops commented Oct 26, 2018

Uh oh!

Tadas commented Oct 26, 2018

Uh oh!

devblackops commented Oct 27, 2018

Uh oh!

Tadas commented Oct 27, 2018

Uh oh!

devblackops commented Oct 31, 2018

Uh oh!

devblackops left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants