From 1cd71a787b81cc40977c5a0dfc11d8fcfcc75a0e Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 25 Sep 2023 14:26:52 -0700 Subject: [PATCH 1/4] Fix implicit remoting proxy cmdlets to act on common parameters --- .../commands/utility/ImplicitRemotingCommands.cs | 5 ++++- .../CompatiblePSEditions.Module.Tests.ps1 | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs index 6dbbc04fea1..9379d1b23e8 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs @@ -1305,7 +1305,10 @@ private static bool IsProxyForCmdlet(Dictionary param // we are not sending CmdletBinding/DefaultParameterSet over the wire anymore // we need to infer IsProxyForCmdlet from presence of all common parameters - foreach (string commonParameterName in Cmdlet.CommonParameters) + // need to exclude `ProgressAction` which may not exist for downlevel platforms + var commonParameters = Cmdlet.CommonParameters; + commonParameters.Remove("ProgressAction"); + foreach (string commonParameterName in commonParameters) { if (!parameters.ContainsKey(commonParameterName)) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index a9c9b69b202..e6d7d488794 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -1532,4 +1532,15 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag $result[4] | Should -BeExactly 'ConvertFrom-String' $result[5] | Should -BeExactly 'CFS' } + + It 'ErrorAction should be used for cmdlet' { + try { + $out = Invoke-Expression 'get-AppLockerFileInformation NoSuch.exe -ErrorAction Stop; "after"' + } + catch { + # do nothing + } + + $out | Should -Not -Contain 'after' + } } From b0b54703eec7f165302e06353998f91cbf1feaf7 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 25 Sep 2023 16:59:29 -0700 Subject: [PATCH 2/4] address Dongbo's feedback creating copy of commonparameters and only remove progressaction for older PS --- .../commands/utility/ImplicitRemotingCommands.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs index 9379d1b23e8..bb65862d62a 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs @@ -1300,14 +1300,18 @@ private ParameterMetadata RehydrateParameterMetadata(PSObject deserializedParame parameterType); } - private static bool IsProxyForCmdlet(Dictionary parameters) + private bool IsProxyForCmdlet(Dictionary parameters) { // we are not sending CmdletBinding/DefaultParameterSet over the wire anymore // we need to infer IsProxyForCmdlet from presence of all common parameters + var commonParameters = new HashSet(Cmdlet.CommonParameters); // need to exclude `ProgressAction` which may not exist for downlevel platforms - var commonParameters = Cmdlet.CommonParameters; - commonParameters.Remove("ProgressAction"); + if ((Session.Runspace is RemoteRunspace remoteRunspace) && (remoteRunspace.ServerVersion != null) && (remoteRunspace.ServerVersion <= new Version(7, 3))) + { + commonParameters.Remove("ProgressAction"); + } + foreach (string commonParameterName in commonParameters) { if (!parameters.ContainsKey(commonParameterName)) From 88ccf445ed54d0c54891b841da64fc49bd00b78a Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 25 Sep 2023 17:22:50 -0700 Subject: [PATCH 3/4] fix codefactor --- .../commands/utility/ImplicitRemotingCommands.cs | 1 + .../CompatiblePSEditions.Module.Tests.ps1 | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs index bb65862d62a..5fcc601ca8a 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs @@ -1306,6 +1306,7 @@ private bool IsProxyForCmdlet(Dictionary parameters) // we need to infer IsProxyForCmdlet from presence of all common parameters var commonParameters = new HashSet(Cmdlet.CommonParameters); + // need to exclude `ProgressAction` which may not exist for downlevel platforms if ((Session.Runspace is RemoteRunspace remoteRunspace) && (remoteRunspace.ServerVersion != null) && (remoteRunspace.ServerVersion <= new Version(7, 3))) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index e6d7d488794..d4423b034b9 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -1538,7 +1538,7 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag $out = Invoke-Expression 'get-AppLockerFileInformation NoSuch.exe -ErrorAction Stop; "after"' } catch { - # do nothing + # do nothing as we expect an error, but execution should not continue } $out | Should -Not -Contain 'after' From 55c8f89e0bc6d1fd2a9f87f94af6aa876cd8714a Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 26 Sep 2023 10:03:13 -0700 Subject: [PATCH 4/4] address Dongbo's feedback --- .../commands/utility/ImplicitRemotingCommands.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs index 5fcc601ca8a..3b7db715953 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs @@ -1305,16 +1305,18 @@ private bool IsProxyForCmdlet(Dictionary parameters) // we are not sending CmdletBinding/DefaultParameterSet over the wire anymore // we need to infer IsProxyForCmdlet from presence of all common parameters - var commonParameters = new HashSet(Cmdlet.CommonParameters); - // need to exclude `ProgressAction` which may not exist for downlevel platforms - if ((Session.Runspace is RemoteRunspace remoteRunspace) && (remoteRunspace.ServerVersion != null) && (remoteRunspace.ServerVersion <= new Version(7, 3))) - { - commonParameters.Remove("ProgressAction"); - } + bool isDownLevelRemote = Session.Runspace is RemoteRunspace remoteRunspace + && remoteRunspace.ServerVersion is not null + && remoteRunspace.ServerVersion <= new Version(7, 3); - foreach (string commonParameterName in commonParameters) + foreach (string commonParameterName in CommonParameters) { + if (isDownLevelRemote && commonParameterName == "ProgressAction") + { + continue; + } + if (!parameters.ContainsKey(commonParameterName)) { return false;