Skip to content

Commit da33bdb

Browse files
jborean93jborean93
committed
Harden PromptForChoiceMultipleSelection host call
Fixes `IHostUISupportsMultipleChoiceSelection.PromptForChoice` over PowerShell remoting when `defaultChoices` is anything other than a `Collection<int>` or `$null`. There are two fixes in this commit. The first is to make sure that `PromptForChoice` serializes the `defaultChoices` value as `Collection<int>`. This ensures that unpatched clients connecting to a patched server can deserialize the expected value for this argument. The second is to support decoding a host method array value when the expected type is `IEnumerable<int>`. This ensures that patches clients connecting to an unpatched server can deserialize the array serialized value.
1 parent 0b909d0 commit da33bdb

4 files changed

Lines changed: 238 additions & 5 deletions

File tree

‎src/System.Management.Automation/engine/remoting/common/WireDataFormat/RemoteHostEncoder.cs‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -487,10 +487,22 @@ internal static object DecodeObject(object obj, Type type)
487487
}
488488
else if (obj is PSObject && IsGenericIEnumerableOfInt(type))
489489
{
490-
// we cannot create an instance of interface type like IEnumerable
491-
// Since a Collection implements IEnumerable, falling back to use
492-
// that.
493-
return DecodeCollection((PSObject)obj, typeof(Collection<int>));
490+
// We cannot create an instance of an interface type like IEnumerable<int>,
491+
// so decode into a concrete type that implements it. EncodeObject picks
492+
// the wire format from the runtime type so older servers that send an int[]
493+
// using the array format while a Collection<int> uses the ArrayList format.
494+
PSObject psObject = (PSObject)obj;
495+
if (psObject.BaseObject is ArrayList)
496+
{
497+
return DecodeCollection(psObject, typeof(Collection<int>));
498+
}
499+
else if (psObject.Properties[RemoteDataNameStrings.MethodArrayElements] is not null &&
500+
psObject.Properties[RemoteDataNameStrings.MethodArrayLengths] is not null)
501+
{
502+
return DecodeArray(psObject, typeof(int[]));
503+
}
504+
505+
throw RemoteHostExceptions.NewRemoteHostDataDecodingNotSupportedException(type);
494506
}
495507
else if (obj is PSObject && type == typeof(RemoteHostCall))
496508
{

‎src/System.Management.Automation/engine/remoting/server/ServerRemoteHostUserInterface.cs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,22 @@ public Collection<int> PromptForChoice(string caption,
7676
Collection<ChoiceDescription> choices,
7777
IEnumerable<int> defaultChoices)
7878
{
79+
// The remote host encoder picks the wire format from the runtime
80+
// type while the client decodes IEnumerable<int> as a
81+
// Collection<int>. Normalize to Collection<int> so that arrays,
82+
// lists, or any other IEnumerable<int> can round trip.
83+
Collection<int> defaultChoicesCollection = null;
84+
if (defaultChoices is not null)
85+
{
86+
defaultChoicesCollection = new Collection<int>();
87+
foreach (int choice in defaultChoices)
88+
{
89+
defaultChoicesCollection.Add(choice);
90+
}
91+
}
92+
7993
return _serverMethodExecutor.ExecuteMethod<Collection<int>>(RemoteHostMethodId.PromptForChoiceMultipleSelection,
80-
new object[] { caption, message, choices, defaultChoices });
94+
new object[] { caption, message, choices, defaultChoicesCollection });
8195
}
8296

8397
/// <summary>

‎test/powershell/engine/Remoting/RemoteHostCalls.Tests.ps1‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
Describe "Remote host method call tests" -Tag Feature {
5+
BeforeAll {
6+
if (-not ('RemoteHostCallsTest.TestHost' -as [type])) {
7+
Add-Type -TypeDefinition @'
8+
using System;
9+
using System.Collections.Generic;
10+
using System.Collections.ObjectModel;
11+
using System.Globalization;
12+
using System.Management.Automation;
13+
using System.Management.Automation.Host;
14+
using System.Security;
15+
16+
namespace RemoteHostCallsTest
17+
{
18+
public class TestHostUserInterface : PSHostUserInterface, IHostUISupportsMultipleChoiceSelection
19+
{
20+
public int[] ReceivedDefaultChoices;
21+
public Collection<int> ChoicesToReturn = new Collection<int>();
22+
23+
public override PSHostRawUserInterface RawUI => null;
24+
public override string ReadLine() => throw new NotImplementedException();
25+
public override SecureString ReadLineAsSecureString() => throw new NotImplementedException();
26+
public override void Write(string value) { }
27+
public override void Write(ConsoleColor foregroundColor, ConsoleColor backgroundColor, string value) { }
28+
public override void WriteLine(string value) { }
29+
public override void WriteErrorLine(string value) { }
30+
public override void WriteDebugLine(string message) { }
31+
public override void WriteProgress(long sourceId, ProgressRecord record) { }
32+
public override void WriteVerboseLine(string message) { }
33+
public override void WriteWarningLine(string message) { }
34+
35+
public override Dictionary<string, PSObject> Prompt(string caption, string message, Collection<FieldDescription> descriptions)
36+
=> throw new NotImplementedException();
37+
38+
public override PSCredential PromptForCredential(string caption, string message, string userName, string targetName)
39+
=> throw new NotImplementedException();
40+
41+
public override PSCredential PromptForCredential(string caption, string message, string userName, string targetName, PSCredentialTypes allowedCredentialTypes, PSCredentialUIOptions options)
42+
=> throw new NotImplementedException();
43+
44+
public override int PromptForChoice(string caption, string message, Collection<ChoiceDescription> choices, int defaultChoice)
45+
=> throw new NotImplementedException();
46+
47+
public Collection<int> PromptForChoice(string caption, string message, Collection<ChoiceDescription> choices, IEnumerable<int> defaultChoices)
48+
{
49+
ReceivedDefaultChoices = defaultChoices == null ? null : new List<int>(defaultChoices).ToArray();
50+
return ChoicesToReturn;
51+
}
52+
}
53+
54+
public class TestHost : PSHost
55+
{
56+
private readonly TestHostUserInterface _ui = new TestHostUserInterface();
57+
private readonly Guid _instanceId = Guid.NewGuid();
58+
59+
public override CultureInfo CurrentCulture => CultureInfo.CurrentCulture;
60+
public override CultureInfo CurrentUICulture => CultureInfo.CurrentUICulture;
61+
public override Guid InstanceId => _instanceId;
62+
public override string Name => "RemoteHostCallsTest";
63+
public override PSHostUserInterface UI => _ui;
64+
public override Version Version => new Version(1, 0);
65+
public override void EnterNestedPrompt() => throw new NotImplementedException();
66+
public override void ExitNestedPrompt() => throw new NotImplementedException();
67+
public override void NotifyBeginApplication() { }
68+
public override void NotifyEndApplication() { }
69+
public override void SetShouldExit(int exitCode) { }
70+
}
71+
}
72+
'@
73+
}
74+
75+
$testHost = [RemoteHostCallsTest.TestHost]::new()
76+
77+
# Connecting to the current process over the named pipe gives a remote
78+
# runspace whose host calls are marshalled back to our custom host.
79+
$connInfo = [System.Management.Automation.Runspaces.NamedPipeConnectionInfo]::new($PID)
80+
$runspace = [runspacefactory]::CreateRunspace($testHost, $connInfo)
81+
$runspace.Open()
82+
}
83+
84+
AfterAll {
85+
if ($runspace) {
86+
$runspace.Dispose()
87+
}
88+
}
89+
90+
Context "IHostUISupportsMultipleChoiceSelection.PromptForChoice" {
91+
BeforeEach {
92+
$testHost.UI.ReceivedDefaultChoices = $null
93+
$testHost.UI.ChoicesToReturn.Clear()
94+
}
95+
96+
It "Passes default choices as <Name>" -TestCases @(
97+
@{ Name = 'int[]'; DefaultChoices = '[int[]]@(0, 2)'; Expected = @(0, 2) }
98+
@{ Name = 'List[int]'; DefaultChoices = '[System.Collections.Generic.List[int]]@(0, 2)'; Expected = @(0, 2) }
99+
@{ Name = 'Collection[int]'; DefaultChoices = '[System.Collections.ObjectModel.Collection[int]]@(0, 2)'; Expected = @(0, 2) }
100+
@{ Name = 'empty int[]'; DefaultChoices = '[int[]]@()'; Expected = @() }
101+
@{ Name = 'null'; DefaultChoices = '$null'; Expected = $null }
102+
) {
103+
param ($DefaultChoices, $Expected)
104+
105+
$testHost.UI.ChoicesToReturn.Add(1)
106+
$testHost.UI.ChoicesToReturn.Add(2)
107+
108+
$ps = [PowerShell]::Create()
109+
try {
110+
$ps.Runspace = $runspace
111+
$null = $ps.AddScript(@"
112+
`$choices = [System.Collections.ObjectModel.Collection[System.Management.Automation.Host.ChoiceDescription]]@('&a', '&b', '&c')
113+
`$Host.UI.PromptForChoice('caption', 'message', `$choices, $DefaultChoices)
114+
"@)
115+
$actual = $ps.Invoke()
116+
117+
$ps.Streams.Error | Should -BeNullOrEmpty
118+
$actual | Should -Be @(1, 2)
119+
120+
if ($null -eq $Expected) {
121+
$null -eq $testHost.UI.ReceivedDefaultChoices | Should -BeTrue
122+
}
123+
else {
124+
$null -eq $testHost.UI.ReceivedDefaultChoices | Should -BeFalse
125+
$testHost.UI.ReceivedDefaultChoices -join ',' | Should -BeExactly ($Expected -join ',')
126+
}
127+
}
128+
finally {
129+
$ps.Dispose()
130+
}
131+
}
132+
}
133+
}

‎test/xUnit/csharp/test_RemoteHostEncoder.cs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Collections.ObjectModel;
7+
using System.IO;
8+
using System.Linq;
9+
using System.Management.Automation;
10+
using System.Management.Automation.Host;
11+
using System.Management.Automation.Remoting;
12+
using Xunit;
13+
14+
namespace PSTests.Parallel
15+
{
16+
public static class RemoteHostEncoderTests
17+
{
18+
public static TheoryData<IEnumerable<int>> MultipleChoiceDefaultChoices => new()
19+
{
20+
new int[] { 0, 2 },
21+
Array.Empty<int>(),
22+
new Collection<int> { 0, 2 },
23+
new Collection<int>(),
24+
};
25+
26+
[Theory]
27+
[MemberData(nameof(MultipleChoiceDefaultChoices))]
28+
public static void PromptForChoiceMultipleSelectionDefaultChoicesRoundTrip(IEnumerable<int> defaultChoices)
29+
{
30+
// Older servers pass the defaultChoices value through as is so the
31+
// client needs to be able to decode both the array and collection
32+
// wire formats.
33+
Collection<ChoiceDescription> choices = new()
34+
{
35+
new ChoiceDescription("&a"),
36+
new ChoiceDescription("&b"),
37+
new ChoiceDescription("&c"),
38+
};
39+
RemoteHostCall call = new(
40+
1,
41+
RemoteHostMethodId.PromptForChoiceMultipleSelection,
42+
new object[] { "caption", "message", choices, defaultChoices });
43+
44+
RemoteHostCall decoded = RemoteHostCall.Decode(SerializeForRemoting(call.Encode()));
45+
46+
Assert.Equal(RemoteHostMethodId.PromptForChoiceMultipleSelection, decoded.MethodId);
47+
IEnumerable<int> actual = Assert.IsAssignableFrom<IEnumerable<int>>(decoded.Parameters[3]);
48+
Assert.Equal(defaultChoices.ToArray(), actual.ToArray());
49+
}
50+
51+
[Fact]
52+
public static void DecodeEnumerableOfIntWithUnknownFormatFails()
53+
{
54+
PSObject data = new();
55+
data.Properties.Add(new PSNoteProperty("Foo", "Bar"));
56+
57+
PSRemotingDataStructureException exc = Assert.Throws<PSRemotingDataStructureException>(
58+
() => RemoteHostEncoder.DecodeObject(data, typeof(IEnumerable<int>)));
59+
Assert.Equal(
60+
string.Format(RemotingErrorIdStrings.RemoteHostDataDecodingNotSupported, typeof(IEnumerable<int>)),
61+
exc.Message);
62+
}
63+
64+
private static PSObject SerializeForRemoting(PSObject data)
65+
{
66+
Fragmentor fragmentor = new(32 * 1024, null);
67+
using MemoryStream stream = new();
68+
fragmentor.SerializeToBytes(data, stream);
69+
stream.Position = 0;
70+
71+
return fragmentor.DeserializeToPSObject(stream);
72+
}
73+
}
74+
}

0 commit comments

Comments
 (0)