From b1947313c1e768b26911f28eb23bb7aa091461eb Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 9 Nov 2017 09:13:34 -0800 Subject: [PATCH] Revert refactoring changes that broke remoting to Windows PowerShell 5.1 (#5321) - Revert refactoring changes so that it doesn't break remoting to Windows PowerShell 5.1 - Fix the rest of the supported types for remoting - Put remoting sensitive private members in a descriptive region --- .../engine/ProgressRecord.cs | 90 +++++------ .../engine/hostifaces/ChoiceDescription.cs | 27 ++-- .../engine/hostifaces/FieldDescription.cs | 113 ++++++++++---- .../hostifaces/MshHostRawUserInterface.cs | 142 +++++++++++++----- .../WireDataFormat/RemoteSessionCapability.cs | 71 ++++++--- 5 files changed, 306 insertions(+), 137 deletions(-) diff --git a/src/System.Management.Automation/engine/ProgressRecord.cs b/src/System.Management.Automation/engine/ProgressRecord.cs index f63ba0392a..630407434b 100644 --- a/src/System.Management.Automation/engine/ProgressRecord.cs +++ b/src/System.Management.Automation/engine/ProgressRecord.cs @@ -66,9 +66,9 @@ namespace System.Management.Automation throw PSTraceSource.NewArgumentException("activity", ProgressRecordStrings.ArgMayNotBeNullOrEmpty, "statusDescription"); } - _id = activityId; - _activity = activity; - _status = statusDescription; + this.id = activityId; + this.activity = activity; + this.status = statusDescription; } /// @@ -77,14 +77,14 @@ namespace System.Management.Automation /// internal ProgressRecord(ProgressRecord other) { - _activity = other._activity; - _currentOperation = other._currentOperation; - _id = other._id; - _parentId = other._parentId; - _percent = other._percent; - _secondsRemaining = other._secondsRemaining; - _status = other._status; - _type = other._type; + this.activity = other.activity; + this.currentOperation = other.currentOperation; + this.id = other.id; + this.parentId = other.parentId; + this.percent = other.percent; + this.secondsRemaining = other.secondsRemaining; + this.status = other.status; + this.type = other.type; } /// @@ -100,7 +100,7 @@ namespace System.Management.Automation { get { - return _id; + return id; } } @@ -133,7 +133,7 @@ namespace System.Management.Automation { get { - return _parentId; + return parentId; } set { @@ -141,7 +141,7 @@ namespace System.Management.Automation { throw PSTraceSource.NewArgumentException("value", ProgressRecordStrings.ParentActivityIdCantBeActivityId); } - _parentId = value; + parentId = value; } } @@ -165,7 +165,7 @@ namespace System.Management.Automation { get { - return _activity; + return activity; } set { @@ -173,7 +173,7 @@ namespace System.Management.Automation { throw PSTraceSource.NewArgumentException("value", ProgressRecordStrings.ArgMayNotBeNullOrEmpty, "value"); } - _activity = value; + activity = value; } } @@ -191,7 +191,7 @@ namespace System.Management.Automation { get { - return _status; + return status; } set { @@ -199,7 +199,7 @@ namespace System.Management.Automation { throw PSTraceSource.NewArgumentException("value", ProgressRecordStrings.ArgMayNotBeNullOrEmpty, "value"); } - _status = value; + status = value; } } @@ -219,13 +219,13 @@ namespace System.Management.Automation { get { - return _currentOperation; + return currentOperation; } set { // null or empty string is allowed - _currentOperation = value; + currentOperation = value; } } @@ -244,7 +244,7 @@ namespace System.Management.Automation { get { - return _percent; + return percent; } set { @@ -257,7 +257,7 @@ namespace System.Management.Automation "value", value, ProgressRecordStrings.PercentMayNotBeMoreThan100, "PercentComplete"); } - _percent = value; + percent = value; } } @@ -283,13 +283,13 @@ namespace System.Management.Automation { get { - return _secondsRemaining; + return secondsRemaining; } set { // negative values are allowed - _secondsRemaining = value; + secondsRemaining = value; } } @@ -307,7 +307,7 @@ namespace System.Management.Automation { get { - return _type; + return type; } set { @@ -316,7 +316,7 @@ namespace System.Management.Automation throw PSTraceSource.NewArgumentException("value"); } - _type = value; + type = value; } } @@ -343,14 +343,14 @@ namespace System.Management.Automation String.Format( System.Globalization.CultureInfo.CurrentCulture, "parent = {0} id = {1} act = {2} stat = {3} cur = {4} pct = {5} sec = {6} type = {7}", - _parentId, - _id, - _activity, - _status, - _currentOperation, - _percent, - _secondsRemaining, - _type); + parentId, + id, + activity, + status, + currentOperation, + percent, + secondsRemaining, + type); } #endregion @@ -467,29 +467,33 @@ namespace System.Management.Automation #endregion - [DataMemberAttribute()] - private int _id; + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell [DataMemberAttribute()] - private int _parentId = -1; + private int id; [DataMemberAttribute()] - private string _activity; + private int parentId = -1; [DataMemberAttribute()] - private string _status; + private string activity; [DataMemberAttribute()] - private string _currentOperation; + private string status; [DataMemberAttribute()] - private int _percent = -1; + private string currentOperation; [DataMemberAttribute()] - private int _secondsRemaining = -1; + private int percent = -1; [DataMemberAttribute()] - private ProgressRecordType _type = ProgressRecordType.Processing; + private int secondsRemaining = -1; + + [DataMemberAttribute()] + private ProgressRecordType type = ProgressRecordType.Processing; + + #endregion #region Serialization / deserialization for remoting diff --git a/src/System.Management.Automation/engine/hostifaces/ChoiceDescription.cs b/src/System.Management.Automation/engine/hostifaces/ChoiceDescription.cs index 48a7af3407..72b9e86086 100644 --- a/src/System.Management.Automation/engine/hostifaces/ChoiceDescription.cs +++ b/src/System.Management.Automation/engine/hostifaces/ChoiceDescription.cs @@ -19,6 +19,14 @@ namespace System.Management.Automation.Host public sealed class ChoiceDescription { + + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell compatibility with Windows PowerShell + + private readonly string label = null; + private string helpMessage = ""; + + #endregion + /// /// /// Initializes an new instance of ChoiceDescription and defines the Label value. @@ -46,7 +54,7 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentException("label", DescriptionsStrings.NullOrEmptyErrorTemplate, "label"); } - _label = label; + this.label = label; } /// @@ -92,8 +100,8 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentNullException("helpMessage"); } - _label = label; - _helpMessage = helpMessage; + this.label = label; + this.helpMessage = helpMessage; } /// @@ -118,9 +126,9 @@ namespace System.Management.Automation.Host { get { - Dbg.Assert(_label != null, "label should not be null"); + Dbg.Assert(this.label != null, "label should not be null"); - return _label; + return this.label; } } @@ -149,9 +157,9 @@ namespace System.Management.Automation.Host { get { - Dbg.Assert(_helpMessage != null, "helpMessage should not be null"); + Dbg.Assert(this.helpMessage != null, "helpMessage should not be null"); - return _helpMessage; + return this.helpMessage; } set { @@ -160,12 +168,9 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentNullException("value"); } - _helpMessage = value; + this.helpMessage = value; } } - - private readonly string _label = null; - private string _helpMessage = ""; } } diff --git a/src/System.Management.Automation/engine/hostifaces/FieldDescription.cs b/src/System.Management.Automation/engine/hostifaces/FieldDescription.cs index 7825ab2891..8d02deab24 100644 --- a/src/System.Management.Automation/engine/hostifaces/FieldDescription.cs +++ b/src/System.Management.Automation/engine/hostifaces/FieldDescription.cs @@ -57,13 +57,19 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentException("name", DescriptionsStrings.NullOrEmptyErrorTemplate, "name"); } - Name = name; + this.name = name; } /// /// Gets the name of the field. /// - public string Name { get; } = null; + public string Name + { + get + { + return name; + } + } /// @@ -123,14 +129,14 @@ namespace System.Management.Automation.Host { get { - if (String.IsNullOrEmpty(_parameterTypeName)) + if (String.IsNullOrEmpty(parameterTypeName)) { // the default if the type name is not specified is 'string' SetParameterType(typeof(string)); } - return _parameterTypeName; + return parameterTypeName; } } @@ -156,14 +162,14 @@ namespace System.Management.Automation.Host { get { - if (String.IsNullOrEmpty(_parameterTypeFullName)) + if (String.IsNullOrEmpty(parameterTypeFullName)) { // the default if the type name is not specified is 'string' SetParameterType(typeof(string)); } - return _parameterTypeFullName; + return parameterTypeFullName; } } @@ -189,14 +195,14 @@ namespace System.Management.Automation.Host { get { - if (String.IsNullOrEmpty(_parameterAssemblyFullName)) + if (String.IsNullOrEmpty(parameterAssemblyFullName)) { // the default if the type name is not specified is 'string' SetParameterType(typeof(string)); } - return _parameterAssemblyFullName; + return parameterAssemblyFullName; } } @@ -235,9 +241,9 @@ namespace System.Management.Automation.Host { get { - Dbg.Assert(_label != null, "label should not be null"); + Dbg.Assert(label != null, "label should not be null"); - return _label; + return label; } set { @@ -246,7 +252,7 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentNullException("value"); } - _label = value; + label = value; } } @@ -275,9 +281,9 @@ namespace System.Management.Automation.Host { get { - Dbg.Assert(_helpMessage != null, "helpMessage should not be null"); + Dbg.Assert(helpMessage != null, "helpMessage should not be null"); - return _helpMessage; + return helpMessage; } set { @@ -286,7 +292,7 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentNullException("value"); } - _helpMessage = value; + helpMessage = value; } } @@ -299,7 +305,17 @@ namespace System.Management.Automation.Host public bool - IsMandatory { get; set; } = true; + IsMandatory + { + get + { + return isMandatory; + } + set + { + isMandatory = value; + } + } /// /// @@ -317,8 +333,20 @@ namespace System.Management.Automation.Host public PSObject - DefaultValue { get; set; } = null; + DefaultValue + { + get + { + return defaultValue; + } + set + { + // null is allowed. + + defaultValue = value; + } + } /// /// @@ -332,7 +360,7 @@ namespace System.Management.Automation.Host Collection Attributes { - get { return _metadata ?? (_metadata = new Collection()); } + get { return metadata ?? (metadata = new Collection()); } } /// @@ -356,7 +384,7 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentException("nameOfType", DescriptionsStrings.NullOrEmptyErrorTemplate, "nameOfType"); } - _parameterTypeName = nameOfType; + parameterTypeName = nameOfType; } @@ -382,7 +410,7 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentException("fullNameOfType", DescriptionsStrings.NullOrEmptyErrorTemplate, "fullNameOfType"); } - _parameterTypeFullName = fullNameOfType; + parameterTypeFullName = fullNameOfType; } @@ -408,7 +436,7 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentException("fullNameOfAssembly", DescriptionsStrings.NullOrEmptyErrorTemplate, "fullNameOfAssembly"); } - _parameterAssemblyFullName = fullNameOfAssembly; + parameterAssemblyFullName = fullNameOfAssembly; } /// @@ -419,7 +447,17 @@ namespace System.Management.Automation.Host /// determine if this field description was /// modified by the remoting protocol layer /// and take appropriate actions - internal bool ModifiedByRemotingProtocol { get; set; } = false; + internal bool ModifiedByRemotingProtocol + { + get + { + return modifiedByRemotingProtocol; + } + set + { + modifiedByRemotingProtocol = value; + } + } /// /// Indicates if this field description @@ -429,18 +467,37 @@ namespace System.Management.Automation.Host /// not cast strings to an arbitrary type, /// but let the server-side do the type conversion /// - internal bool IsFromRemoteHost { get; set; } = false; + internal bool IsFromRemoteHost + { + get + { + return isFromRemoteHost; + } + set + { + isFromRemoteHost = value; + } + } #region Helper #endregion Helper - private string _label = ""; - private string _parameterTypeName = null; - private string _parameterTypeFullName = null; - private string _parameterAssemblyFullName = null; - private string _helpMessage = ""; + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell - private Collection _metadata = new Collection(); + private readonly string name = null; + private string label = ""; + private string parameterTypeName = null; + private string parameterTypeFullName = null; + private string parameterAssemblyFullName = null; + private string helpMessage = ""; + private bool isMandatory = true; + + private PSObject defaultValue = null; + private Collection metadata = new Collection(); + private bool modifiedByRemotingProtocol = false; + private bool isFromRemoteHost = false; + + #endregion } } diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostRawUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostRawUserInterface.cs index a57f190797..843cfb7ae5 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostRawUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostRawUserInterface.cs @@ -25,10 +25,13 @@ namespace System.Management.Automation.Host public struct Coordinates { - // DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell + private int x; private int y; + #endregion + /// /// /// Gets and sets the X coordinate @@ -267,10 +270,13 @@ namespace System.Management.Automation.Host public struct Size { - // DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell + private int width; private int height; + #endregion + /// /// /// Gets and sets the Width @@ -618,34 +624,56 @@ namespace System.Management.Automation.Host public struct KeyInfo { + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell + + private int virtualKeyCode; + private char character; + private ControlKeyStates controlKeyState; + private bool keyDown; + + #endregion + /// /// /// Gets and set device-independent key /// /// - public int VirtualKeyCode { get; set; } - + public int VirtualKeyCode + { + get { return virtualKeyCode; } + set { virtualKeyCode = value; } + } /// /// Gets and set unicode Character of the key /// - public char Character { get; set; } - + public char Character + { + get { return character; } + set { character = value; } + } /// /// State of the control keys. /// - public ControlKeyStates ControlKeyState { get; set; } - + public ControlKeyStates ControlKeyState + { + get { return controlKeyState; } + set { controlKeyState = value; } + } /// /// Gets and set the status of whether this instance is generated by a key pressed or released /// - public bool KeyDown { get; set; } + public bool KeyDown + { + get { return keyDown; } + set { keyDown = value; } + } /// /// @@ -683,10 +711,10 @@ namespace System.Management.Automation.Host bool keyDown ) { - VirtualKeyCode = virtualKeyCode; - Character = ch; - ControlKeyState = controlKeyState; - KeyDown = keyDown; + this.virtualKeyCode = virtualKeyCode; + this.character = ch; + this.controlKeyState = controlKeyState; + this.keyDown = keyDown; } /// @@ -851,14 +879,26 @@ namespace System.Management.Automation.Host public struct Rectangle { + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell + + private int left; + private int top; + private int right; + private int bottom; + + #endregion + /// /// /// Gets and sets the left side of the rectangle /// /// - public int Left { get; set; } - + public int Left + { + get { return left; } + set { left = value; } + } /// /// @@ -866,8 +906,11 @@ namespace System.Management.Automation.Host /// /// - public int Top { get; set; } - + public int Top + { + get { return top; } + set { top = value; } + } /// /// @@ -875,8 +918,11 @@ namespace System.Management.Automation.Host /// /// - public int Right { get; set; } - + public int Right + { + get { return right; } + set { right = value; } + } /// /// @@ -884,8 +930,11 @@ namespace System.Management.Automation.Host /// /// - public int Bottom { get; set; } - + public int Bottom + { + get { return bottom; } + set { bottom = value; } + } /// /// @@ -933,10 +982,10 @@ namespace System.Management.Automation.Host throw PSTraceSource.NewArgumentException("bottom", MshHostRawUserInterfaceStrings.LessThanErrorTemplate, "bottom", "top"); } - Left = left; - Top = top; - Right = right; - Bottom = bottom; + this.left = left; + this.top = top; + this.right = right; + this.bottom = bottom; } @@ -1161,14 +1210,26 @@ namespace System.Management.Automation.Host public struct BufferCell { + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell + + private char character; + private ConsoleColor foregroundColor; + private ConsoleColor backgroundColor; + private BufferCellType bufferCellType; + + #endregion + /// /// /// Gets and sets the character value /// /// - public char Character { get; set; } - + public char Character + { + get { return character; } + set { character = value; } + } // we reuse System.ConsoleColor - it's in the core assembly, and I think it would be confusing to create another // essentially identical enum @@ -1179,7 +1240,11 @@ namespace System.Management.Automation.Host /// /// - public ConsoleColor ForegroundColor { get; set; } + public ConsoleColor ForegroundColor + { + get { return foregroundColor; } + set { foregroundColor = value; } + } /// /// @@ -1187,7 +1252,11 @@ namespace System.Management.Automation.Host /// /// - public ConsoleColor BackgroundColor { get; set; } + public ConsoleColor BackgroundColor + { + get { return backgroundColor; } + set { backgroundColor = value; } + } /// /// @@ -1195,8 +1264,11 @@ namespace System.Management.Automation.Host /// /// - public BufferCellType BufferCellType { get; set; } - + public BufferCellType BufferCellType + { + get { return bufferCellType; } + set { bufferCellType = value; } + } /// /// @@ -1228,10 +1300,10 @@ namespace System.Management.Automation.Host public BufferCell(char character, ConsoleColor foreground, ConsoleColor background, BufferCellType bufferCellType) { - Character = character; - ForegroundColor = foreground; - BackgroundColor = background; - BufferCellType = bufferCellType; + this.character = character; + this.foregroundColor = foreground; + this.backgroundColor = background; + this.bufferCellType = bufferCellType; } diff --git a/src/System.Management.Automation/engine/remoting/common/WireDataFormat/RemoteSessionCapability.cs b/src/System.Management.Automation/engine/remoting/common/WireDataFormat/RemoteSessionCapability.cs index cec2926cfe..69863ade1b 100644 --- a/src/System.Management.Automation/engine/remoting/common/WireDataFormat/RemoteSessionCapability.cs +++ b/src/System.Management.Automation/engine/remoting/common/WireDataFormat/RemoteSessionCapability.cs @@ -19,12 +19,32 @@ namespace System.Management.Automation.Remoting /// internal class RemoteSessionCapability { - internal Version ProtocolVersion { get; set; } + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell - internal Version PSVersion { get; } - internal Version SerializationVersion { get; } - internal RemotingDestination RemotingDestination { get; } - private static byte[] s_timeZoneInByteFormat; + private Version _psversion; + private Version _serversion; + private Version _protocolVersion; + private RemotingDestination _remotingDestination; + private static byte[] _timeZoneInByteFormat; + private TimeZoneInfo _timeZone; + + #endregion + + internal Version ProtocolVersion + { + get + { + return _protocolVersion; + } + set + { + _protocolVersion = value; + } + } + + internal Version PSVersion { get { return _psversion; } } + internal Version SerializationVersion { get { return _serversion; } } + internal RemotingDestination RemotingDestination { get { return _remotingDestination; } } /// /// Constructor for RemoteSessionCapability. @@ -33,13 +53,13 @@ namespace System.Management.Automation.Remoting /// internal RemoteSessionCapability(RemotingDestination remotingDestination) { - ProtocolVersion = RemotingConstants.ProtocolVersion; + _protocolVersion = RemotingConstants.ProtocolVersion; // PS Version 3 is fully backward compatible with Version 2 // In the remoting protocol sense, nothing is changing between PS3 and PS2 // For negotiation to succeed with old client/servers we have to use 2. - PSVersion = new Version(2, 0); //PSVersionInfo.PSVersion; - SerializationVersion = PSVersionInfo.SerializationVersion; - RemotingDestination = remotingDestination; + _psversion = new Version(2,0); //PSVersionInfo.PSVersion; + _serversion = PSVersionInfo.SerializationVersion; + _remotingDestination = remotingDestination; } internal RemoteSessionCapability(RemotingDestination remotingDestination, @@ -47,10 +67,10 @@ namespace System.Management.Automation.Remoting Version psVersion, Version serVersion) { - ProtocolVersion = protocolVersion; - PSVersion = psVersion; - SerializationVersion = serVersion; - RemotingDestination = remotingDestination; + _protocolVersion = protocolVersion; + _psversion = psVersion; + _serversion = serVersion; + _remotingDestination = remotingDestination; } /// @@ -76,7 +96,7 @@ namespace System.Management.Automation.Remoting /// internal static byte[] GetCurrentTimeZoneInByteFormat() { - if (null == s_timeZoneInByteFormat) + if (null == _timeZoneInByteFormat) { Exception e = null; try @@ -88,7 +108,7 @@ namespace System.Management.Automation.Remoting stream.Seek(0, SeekOrigin.Begin); byte[] result = new byte[stream.Length]; stream.Read(result, 0, (int)stream.Length); - s_timeZoneInByteFormat = result; + _timeZoneInByteFormat = result; } } catch (ArgumentNullException ane) @@ -108,17 +128,21 @@ namespace System.Management.Automation.Remoting // ignore it and dont try to serialize again. if (null != e) { - s_timeZoneInByteFormat = Utils.EmptyArray(); + _timeZoneInByteFormat = Utils.EmptyArray(); } } - return s_timeZoneInByteFormat; + return _timeZoneInByteFormat; } /// /// Gets the TimeZone of the destination machine. This may be null /// - internal TimeZoneInfo TimeZone { get; set; } + internal TimeZoneInfo TimeZone + { + get { return _timeZone; } + set { _timeZone = value; } + } } /// @@ -146,9 +170,13 @@ namespace System.Management.Automation.Remoting /// /// Data. /// - // DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting + + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell + private Dictionary data; + #endregion + /// /// Private constructor to force use of Create. /// @@ -346,10 +374,13 @@ namespace System.Management.Automation.Remoting private readonly bool _isHostNull; - // DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting + #region DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell + private readonly HostDefaultData _hostDefaultData; private bool _useRunspaceHost; + #endregion + /// /// Is host raw ui null. ///