From 9b87b4ec1c24f658151db4fbd2d63c514a3cd4c6 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sun, 21 Apr 2024 21:07:06 +0100 Subject: [PATCH 01/13] Removed BerConverter.Decode usage Replaced this with the managed AsnDecoder, removing PInvoke from a potential hot path. --- .../Protocols/common/DirectoryControl.cs | 141 +++++++++++------- 1 file changed, 83 insertions(+), 58 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index f47bc50318b94..59b77523b85f6 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -4,6 +4,7 @@ using System.Collections; using System.ComponentModel; using System.Diagnostics; +using System.Formats.Asn1; using System.Runtime.InteropServices; using System.Runtime.Versioning; using System.Security.Principal; @@ -136,95 +137,119 @@ internal static void TransformControls(DirectoryControl[] controls) { Debug.Assert(controls[i] != null); byte[] value = controls[i].GetValue(); + Span asnSpan = value; + bool asnReadSuccessful; + if (controls[i].Type == "1.2.840.113556.1.4.319") { - // The control is a PageControl. - object[] result = BerConverter.Decode("{iO}", value); - Debug.Assert((result != null) && (result.Length == 2)); + // The control is a PageControl, as described in RFC 2696. + byte[] cookie = Array.Empty(); + + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + Debug.Assert(sequenceContentLength > 0); + + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan.Slice(sequenceContentOffset), AsnEncodingRules.BER, out int size, out int bytesConsumed); + Debug.Assert(asnReadSuccessful); + asnSpan = asnSpan.Slice(sequenceContentOffset + bytesConsumed); - int size = (int)result[0]; - // user expects cookie with length 0 as paged search is done. - byte[] cookie = (byte[])result[1] ?? Array.Empty(); + asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out int _, out int cookieLength, out _); + if (asnReadSuccessful) + { + cookie = new byte[cookieLength]; + + // user expects cookie with length 0 as paged search is done. + asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, cookie, AsnEncodingRules.BER, out _, out _); + Debug.Assert(asnReadSuccessful); + } - PageResultResponseControl pageControl = new PageResultResponseControl(size, cookie, controls[i].IsCritical, controls[i].GetValue()); + PageResultResponseControl pageControl = new PageResultResponseControl(size, cookie, controls[i].IsCritical, value); controls[i] = pageControl; } else if (controls[i].Type == "1.2.840.113556.1.4.1504") { - // The control is an AsqControl. - object[] o = BerConverter.Decode("{e}", value); - Debug.Assert((o != null) && (o.Length == 1)); + // The control is an AsqControl, as described in MS-ADTS section 3.1.1.3.4.1.18 + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + Debug.Assert(sequenceContentLength > 0); - int result = (int)o[0]; - AsqResponseControl asq = new AsqResponseControl(result, controls[i].IsCritical, controls[i].GetValue()); + ResultCode result = AsnDecoder.ReadEnumeratedValue(asnSpan.Slice(sequenceContentOffset), AsnEncodingRules.BER, out _); + + AsqResponseControl asq = new AsqResponseControl(result, controls[i].IsCritical, value); controls[i] = asq; } else if (controls[i].Type == "1.2.840.113556.1.4.841") { - // The control is a DirSyncControl. - object[] o = BerConverter.Decode("{iiO}", value); - Debug.Assert(o != null && o.Length == 3); + // The control is a DirSyncControl, as described in MS-ADTS section 3.1.1.3.4.1.3 + byte[] dirsyncCookie; + + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + Debug.Assert(sequenceContentLength > 0); + asnSpan = asnSpan.Slice(sequenceContentOffset); + + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int moreData, out int bytesConsumed); + Debug.Assert(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); - int moreData = (int)o[0]; - int count = (int)o[1]; - byte[] dirsyncCookie = (byte[])o[2]; + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int count, out bytesConsumed); + Debug.Assert(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); - DirSyncResponseControl dirsync = new DirSyncResponseControl(dirsyncCookie, (moreData == 0 ? false : true), count, controls[i].IsCritical, controls[i].GetValue()); + dirsyncCookie = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out _); + + DirSyncResponseControl dirsync = new DirSyncResponseControl(dirsyncCookie, moreData != 0, count, controls[i].IsCritical, value); controls[i] = dirsync; } else if (controls[i].Type == "1.2.840.113556.1.4.474") { - // The control is a SortControl. - int result; + // The control is a SortControl, as described in RFC 2891. + ResultCode result; string attribute = null; - object[] o = BerConverter.TryDecode("{ea}", value, out bool decodeSucceeded); - // decode might fail as AD for example never returns attribute name, we don't want to unnecessarily throw and catch exception - if (decodeSucceeded) - { - Debug.Assert(o != null && o.Length == 2); - result = (int)o[0]; - attribute = (string)o[1]; - } - else - { - // decoding might fail as attribute is optional - o = BerConverter.Decode("{e}", value); - Debug.Assert(o != null && o.Length == 1); + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + Debug.Assert(sequenceContentLength > 0); + asnSpan = asnSpan.Slice(sequenceContentOffset); + + result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out int bytesConsumed); + asnSpan = asnSpan.Slice(bytesConsumed); - result = (int)o[0]; + // Attribute name is optional: AD for example never returns attribute name + asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out Asn1Tag octetStringTag, out _, out _, out _); + if (asnReadSuccessful) + { + attribute = AsnDecoder.ReadCharacterString(asnSpan, AsnEncodingRules.BER, UniversalTagNumber.UTF8String, out _, octetStringTag); } - SortResponseControl sort = new SortResponseControl((ResultCode)result, attribute, controls[i].IsCritical, controls[i].GetValue()); + SortResponseControl sort = new SortResponseControl(result, attribute, controls[i].IsCritical, value); controls[i] = sort; } else if (controls[i].Type == "2.16.840.1.113730.3.4.10") { - // The control is a VlvResponseControl. - int position; - int count; - int result; + // The control is a VlvResponseControl, as described in MS-ADTS 3.1.1.3.4.1.17 + ResultCode result; byte[] context = null; - object[] o = BerConverter.TryDecode("{iieO}", value, out bool decodeSucceeded); - if (decodeSucceeded) - { - Debug.Assert(o != null && o.Length == 4); - position = (int)o[0]; - count = (int)o[1]; - result = (int)o[2]; - context = (byte[])o[3]; - } - else + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + Debug.Assert(sequenceContentLength > 0); + asnSpan = asnSpan.Slice(sequenceContentOffset); + + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int position, out int bytesConsumed); + Debug.Assert(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); + + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int count, out bytesConsumed); + Debug.Assert(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); + + result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out bytesConsumed); + asnSpan = asnSpan.Slice(bytesConsumed); + + asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out _, out int octetStringLength, out _); + if (asnReadSuccessful) { - o = BerConverter.Decode("{iie}", value); - Debug.Assert(o != null && o.Length == 3); - position = (int)o[0]; - count = (int)o[1]; - result = (int)o[2]; + context = new byte[octetStringLength]; + _ = AsnDecoder.TryReadOctetString(asnSpan, context, AsnEncodingRules.BER, out _, out _); } - VlvResponseControl vlv = new VlvResponseControl(position, count, context, (ResultCode)result, controls[i].IsCritical, controls[i].GetValue()); + VlvResponseControl vlv = new VlvResponseControl(position, count, context, result, controls[i].IsCritical, value); controls[i] = vlv; } } @@ -253,9 +278,9 @@ public override byte[] GetValue() public class AsqResponseControl : DirectoryControl { - internal AsqResponseControl(int result, bool criticality, byte[] controlValue) : base("1.2.840.113556.1.4.1504", controlValue, criticality, true) + internal AsqResponseControl(ResultCode result, bool criticality, byte[] controlValue) : base("1.2.840.113556.1.4.1504", controlValue, criticality, true) { - Result = (ResultCode)result; + Result = result; } public ResultCode Result { get; } From 31600a4d501d95c33f24eef268b294c446e1102f Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 23 Apr 2024 20:06:03 +0100 Subject: [PATCH 02/13] Removed BerConverter.Encode usage Also removed the manual API calls to ldap_create_sort_control - this is now built in managed code. This then has knock-on effects to eliminate the SortKeyInterop classes. --- .../Interop/Linux/OpenLdap/Interop.Ldap.cs | 3 - .../Interop/Windows/Wldap32/Interop.Ldap.cs | 4 - .../Protocols/Interop/LdapPal.Linux.cs | 2 - .../Protocols/Interop/LdapPal.Windows.cs | 2 - .../Protocols/Interop/SortKeyInterop.Linux.cs | 18 - .../Interop/SortKeyInterop.Windows.cs | 18 - .../Protocols/Interop/SortKeyInterop.cs | 35 -- .../Protocols/common/DirectoryControl.cs | 350 +++++++++++------- 8 files changed, 218 insertions(+), 214 deletions(-) delete mode 100644 src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Linux.cs delete mode 100644 src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Windows.cs delete mode 100644 src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.cs diff --git a/src/libraries/Common/src/Interop/Linux/OpenLdap/Interop.Ldap.cs b/src/libraries/Common/src/Interop/Linux/OpenLdap/Interop.Ldap.cs index cdb419fa001f1..a910d35edad5c 100644 --- a/src/libraries/Common/src/Interop/Linux/OpenLdap/Interop.Ldap.cs +++ b/src/libraries/Common/src/Interop/Linux/OpenLdap/Interop.Ldap.cs @@ -221,9 +221,6 @@ internal static partial int ldap_sasl_interactive_bind( [LibraryImport(Libraries.OpenLdap, EntryPoint = "ldap_first_reference")] public static partial IntPtr ldap_first_reference(ConnectionHandle ldapHandle, IntPtr result); - [LibraryImport(Libraries.OpenLdap, EntryPoint = "ldap_create_sort_control")] - public static partial int ldap_create_sort_control(ConnectionHandle handle, IntPtr keys, byte critical, ref IntPtr control); - [LibraryImport(Libraries.OpenLdap, EntryPoint = "ldap_control_free")] public static partial int ldap_control_free(IntPtr control); diff --git a/src/libraries/Common/src/Interop/Windows/Wldap32/Interop.Ldap.cs b/src/libraries/Common/src/Interop/Windows/Wldap32/Interop.Ldap.cs index 8af474fe948ba..299149a309f3d 100644 --- a/src/libraries/Common/src/Interop/Windows/Wldap32/Interop.Ldap.cs +++ b/src/libraries/Common/src/Interop/Windows/Wldap32/Interop.Ldap.cs @@ -186,10 +186,6 @@ internal static partial class Ldap [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] public static partial int ldap_parse_reference(ConnectionHandle ldapHandle, IntPtr result, ref IntPtr referrals); - [LibraryImport(Libraries.Wldap32, EntryPoint = "ldap_create_sort_controlW")] - [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] - public static partial int ldap_create_sort_control(ConnectionHandle handle, IntPtr keys, byte critical, ref IntPtr control); - [LibraryImport(Libraries.Wldap32, EntryPoint = "ldap_control_freeW")] [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] public static partial int ldap_control_free(IntPtr control); diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Linux.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Linux.cs index 15b3cd86effaf..7ea8214638a97 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Linux.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Linux.cs @@ -19,8 +19,6 @@ internal static int CompareDirectoryEntries(ConnectionHandle ldapHandle, string internal static void FreeDirectoryControls(IntPtr value) => Interop.Ldap.ldap_controls_free(value); - internal static int CreateDirectorySortControl(ConnectionHandle handle, IntPtr keys, byte critical, ref IntPtr control) => Interop.Ldap.ldap_create_sort_control(handle, keys, critical, ref control); - internal static int DeleteDirectoryEntry(ConnectionHandle ldapHandle, string dn, IntPtr servercontrol, IntPtr clientcontrol, ref int messageNumber) => Interop.Ldap.ldap_delete_ext(ldapHandle, dn, servercontrol, clientcontrol, ref messageNumber); internal static int ExtendedDirectoryOperation(ConnectionHandle ldapHandle, string oid, BerVal data, IntPtr servercontrol, IntPtr clientcontrol, ref int messageNumber) => diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Windows.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Windows.cs index a0f6133f66bbb..d7d74d5c735ba 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Windows.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Windows.cs @@ -19,8 +19,6 @@ internal static int CompareDirectoryEntries(ConnectionHandle ldapHandle, string internal static void FreeDirectoryControls(IntPtr value) => Interop.Ldap.ldap_controls_free(value); - internal static int CreateDirectorySortControl(ConnectionHandle handle, IntPtr keys, byte critical, ref IntPtr control) => Interop.Ldap.ldap_create_sort_control(handle, keys, critical, ref control); - internal static int DeleteDirectoryEntry(ConnectionHandle ldapHandle, string dn, IntPtr servercontrol, IntPtr clientcontrol, ref int messageNumber) => Interop.Ldap.ldap_delete_ext(ldapHandle, dn, servercontrol, clientcontrol, ref messageNumber); internal static int ExtendedDirectoryOperation(ConnectionHandle ldapHandle, string oid, BerVal data, IntPtr servercontrol, IntPtr clientcontrol, ref int messageNumber) => diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Linux.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Linux.cs deleted file mode 100644 index 5fdffa0ff82ee..0000000000000 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Linux.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections; -using System.ComponentModel; -using System.Diagnostics; -using System.Runtime.InteropServices; -using System.Runtime.Versioning; -using System.Security.Principal; -using System.Text; - -namespace System.DirectoryServices.Protocols -{ - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] - internal partial struct SortKeyInterop - { - } -} diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Windows.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Windows.cs deleted file mode 100644 index 8d5ae5420309e..0000000000000 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.Windows.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections; -using System.ComponentModel; -using System.Diagnostics; -using System.Runtime.InteropServices; -using System.Runtime.Versioning; -using System.Security.Principal; -using System.Text; - -namespace System.DirectoryServices.Protocols -{ - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] - internal partial struct SortKeyInterop - { - } -} diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.cs deleted file mode 100644 index 61852e8a7c6d6..0000000000000 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/SortKeyInterop.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections; -using System.ComponentModel; -using System.Diagnostics; -using System.Runtime.InteropServices; -using System.Runtime.Versioning; -using System.Security.Principal; -using System.Text; - -namespace System.DirectoryServices.Protocols -{ - // Declared as partial in order to be able to set the different StructLayout - // attributes in the Windows and Linux specific files. - // This is a layout-controlled struct, do not alter property ordering. - internal partial struct SortKeyInterop - { - public SortKeyInterop(SortKey sortKey) - { - if (sortKey == null) - throw new ArgumentNullException(nameof(sortKey)); - - AttributeName = sortKey.AttributeName; - MatchingRule = sortKey.MatchingRule; - ReverseOrder = sortKey.ReverseOrder; - } - - internal string AttributeName { get; set; } - - internal string MatchingRule { get; set; } - - internal bool ReverseOrder { get; set; } - } -} diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index 59b77523b85f6..c1ab4f38fa182 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -258,6 +258,8 @@ internal static void TransformControls(DirectoryControl[] controls) public class AsqRequestControl : DirectoryControl { + private static UTF8Encoding s_utf8Encoding = new(false, true); + public AsqRequestControl() : base("1.2.840.113556.1.4.1504", null, true, true) { } @@ -271,7 +273,28 @@ public AsqRequestControl(string attributeName) : this() public override byte[] GetValue() { - _directoryControlValue = BerConverter.Encode("{s}", new object[] { AttributeName }); + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.18. + * ASQRequestValue ::= SEQUENCE { + * sourceAttribute OCTET STRING } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) + { + if (!string.IsNullOrEmpty(AttributeName)) + { + int octetStringLength = s_utf8Encoding.GetByteCount(AttributeName); + Span tmpValue = octetStringLength < 256 ? stackalloc byte[octetStringLength] : new byte[octetStringLength]; + + s_utf8Encoding.GetBytes(AttributeName, tmpValue); + writer.WriteOctetString(tmpValue); + } + else + { + writer.WriteOctetString([]); + } + } + _directoryControlValue = writer.Encode(); return base.GetValue(); } } @@ -288,6 +311,8 @@ internal AsqResponseControl(ResultCode result, bool criticality, byte[] controlV public class CrossDomainMoveControl : DirectoryControl { + private static readonly UTF8Encoding s_utf8Encoding = new UTF8Encoding(false); + public CrossDomainMoveControl() : base("1.2.840.113556.1.4.521", null, true, true) { } @@ -301,18 +326,20 @@ public CrossDomainMoveControl(string targetDomainController) : this() public override byte[] GetValue() { - if (TargetDomainController != null) + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.2. + * "When sending this control to the DC, the controlValue field is set to a UTF-8 string + * containing the fully qualified domain name of a DC in the domain to which the object + * is to be moved. The string is not BER-encoded." + */ + if (!string.IsNullOrEmpty(TargetDomainController)) { - UTF8Encoding encoder = new UTF8Encoding(); - byte[] bytes = encoder.GetBytes(TargetDomainController); + int byteCount = s_utf8Encoding.GetByteCount(TargetDomainController); // Allocate large enough space for the '\0' character. - _directoryControlValue = new byte[bytes.Length + 2]; - for (int i = 0; i < bytes.Length; i++) - { - _directoryControlValue[i] = bytes[i]; - } + _directoryControlValue = new byte[byteCount + 2]; + s_utf8Encoding.GetBytes(TargetDomainController, _directoryControlValue); } + return base.GetValue(); } } @@ -350,7 +377,18 @@ public ExtendedDNFlag Flag } public override byte[] GetValue() { - _directoryControlValue = BerConverter.Encode("{i}", new object[] { (int)Flag }); + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.5. + * ExtendedDNRequestValue ::= SEQUENCE { + * Flag INTEGER } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) + { + writer.WriteInteger((int)Flag); + } + _directoryControlValue = writer.Encode(); + return base.GetValue(); } } @@ -385,7 +423,18 @@ public SecurityDescriptorFlagControl(SecurityMasks masks) : this() public override byte[] GetValue() { - _directoryControlValue = BerConverter.Encode("{i}", new object[] { (int)SecurityMasks }); + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.11. + * SDFlagsRequestValue ::= SEQUENCE { + * Flags INTEGER } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) + { + writer.WriteInteger((int)SecurityMasks); + } + _directoryControlValue = writer.Encode(); + return base.GetValue(); } } @@ -414,7 +463,18 @@ public SearchOption SearchOption public override byte[] GetValue() { - _directoryControlValue = BerConverter.Encode("{i}", new object[] { (int)SearchOption }); + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.12. + * SearchOptionsRequestValue ::= SEQUENCE { + * Flags INTEGER } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) + { + writer.WriteInteger((int)SearchOption); + } + _directoryControlValue = writer.Encode(); + return base.GetValue(); } } @@ -457,14 +517,32 @@ public string ServerName public override byte[] GetValue() { - byte[] tmpValue = null; - if (ServerName != null) + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.16. + * VerifyNameRequestValue ::= SEQUENCE { + * Flags INTEGER, + * ServerName OCTET STRING } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) { - UnicodeEncoding unicode = new UnicodeEncoding(); - tmpValue = unicode.GetBytes(ServerName); + writer.WriteInteger(Flag); + + if (!string.IsNullOrEmpty(ServerName)) + { + int serverNameLength = Encoding.Unicode.GetByteCount(ServerName); + Span tmpValue = serverNameLength < 256 ? stackalloc byte[serverNameLength] : new byte[serverNameLength]; + + Encoding.Unicode.GetBytes(ServerName, tmpValue); + writer.WriteOctetString(tmpValue); + } + else + { + writer.WriteOctetString([]); + } } + _directoryControlValue = writer.Encode(); - _directoryControlValue = BerConverter.Encode("{io}", new object[] { Flag, tmpValue }); return base.GetValue(); } } @@ -530,8 +608,22 @@ public int AttributeCount public override byte[] GetValue() { - object[] o = new object[] { (int)Option, AttributeCount, _dirsyncCookie }; - _directoryControlValue = BerConverter.Encode("{iio}", o); + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.3. + * DirSyncRequestValue ::= SEQUENCE { + * Flags INTEGER, + * MaxBytes INTEGER, + * Cookie OCTET STRING } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) + { + writer.WriteInteger((int)Option); + writer.WriteInteger(AttributeCount); + writer.WriteOctetString(_dirsyncCookie); + } + _directoryControlValue = writer.Encode(); + return base.GetValue(); } } @@ -624,8 +716,20 @@ public byte[] Cookie public override byte[] GetValue() { - object[] o = new object[] { PageSize, _pageCookie }; - _directoryControlValue = BerConverter.Encode("{io}", o); + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in RFC2696. + * realSearchControlValue ::= SEQUENCE { + * size INTEGER, + * cookie OCTET STRING } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) + { + writer.WriteInteger(PageSize); + writer.WriteOctetString(_pageCookie); + } + _directoryControlValue = writer.Encode(); + return base.GetValue(); } } @@ -663,6 +767,10 @@ public byte[] Cookie public class SortRequestControl : DirectoryControl { + private static UTF8Encoding s_utf8Encoding = new(false, true); + private static Asn1Tag s_orderingRuleTag = new(TagClass.ContextSpecific, 0, false); + private static Asn1Tag s_reverseOrderTag = new(TagClass.ContextSpecific, 1, false); + private SortKey[] _keys = Array.Empty(); public SortRequestControl(params SortKey[] sortKeys) : base("1.2.840.113556.1.4.473", null, true, true) { @@ -732,93 +840,56 @@ public SortKey[] SortKeys } } - public override unsafe byte[] GetValue() + public override byte[] GetValue() { - SortKeyInterop[] nativeSortKeys = new SortKeyInterop[_keys.Length]; - for (int i = 0; i < _keys.Length; ++i) - { - nativeSortKeys[i] = new SortKeyInterop(_keys[i]); - } + AsnWriter writer = new(AsnEncodingRules.BER); - IntPtr control = IntPtr.Zero; - int structSize = Marshal.SizeOf(); - int keyCount = nativeSortKeys.Length; - IntPtr memHandle = Utility.AllocHGlobalIntPtrArray(keyCount + 1); - - try + /* This is as laid out in RFC2891. + * SortKeyList ::= SEQUENCE OF SEQUENCE { + * attributeType AttributeDescription, + * orderingRule [0] MatchingRuleId OPTIONAL, + * reverseOrder [1] BOOLEAN DEFAULT FALSE } + * */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) { - void** pMemHandle = (void**)memHandle; - IntPtr sortPtr = IntPtr.Zero; - int i = 0; - for (i = 0; i < keyCount; i++) - { - sortPtr = Marshal.AllocHGlobal(structSize); - Marshal.StructureToPtr(nativeSortKeys[i], sortPtr, false); - pMemHandle[i] = (void*)sortPtr; - } - pMemHandle[i] = null; - - bool critical = IsCritical; - int error = LdapPal.CreateDirectorySortControl(UtilityHandle.GetHandle(), memHandle, critical ? (byte)1 : (byte)0, ref control); + Span scratchSpace = stackalloc byte[256]; - if (error != 0) + for (int i = 0; i < _keys.Length; i++) { - if (LdapErrorMappings.IsLdapError(error)) - { - string errorMessage = LdapErrorMappings.MapResultCode(error); - throw new LdapException(error, errorMessage); - } - else + SortKey key = _keys[i]; + + using (AsnWriter.Scope keySequence = writer.PushSequence()) { - throw new LdapException(error); - } - } + if (!string.IsNullOrEmpty(key.AttributeName)) + { + int octetStringLength = s_utf8Encoding.GetByteCount(key.AttributeName); + Span tmpValue = octetStringLength < scratchSpace.Length ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; - LdapControl managedControl = new LdapControl(); - Marshal.PtrToStructure(control, managedControl); - BerVal value = managedControl.ldctl_value; - // reinitialize the value - _directoryControlValue = null; - if (value != null) - { - _directoryControlValue = new byte[value.bv_len]; - Marshal.Copy(value.bv_val, _directoryControlValue, 0, value.bv_len); - } - } - finally - { - if (control != IntPtr.Zero) - { - LdapPal.FreeDirectoryControl(control); - } + s_utf8Encoding.GetBytes(key.AttributeName, tmpValue); + writer.WriteOctetString(tmpValue); + } + else + { + writer.WriteOctetString([]); + } - if (memHandle != IntPtr.Zero) - { - //release the memory from the heap - for (int i = 0; i < keyCount; i++) - { - IntPtr tempPtr = Marshal.ReadIntPtr(memHandle, IntPtr.Size * i); - if (tempPtr != IntPtr.Zero) + if (!string.IsNullOrEmpty(key.MatchingRule)) + { + int octetStringLength = s_utf8Encoding.GetByteCount(key.MatchingRule); + Span tmpValue = octetStringLength < scratchSpace.Length ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; + + s_utf8Encoding.GetBytes(key.MatchingRule, tmpValue); + writer.WriteOctetString(tmpValue, s_orderingRuleTag); + } + + if (key.ReverseOrder) { - // free the marshalled name - IntPtr ptr = Marshal.ReadIntPtr(tempPtr); - if (ptr != IntPtr.Zero) - { - Marshal.FreeHGlobal(ptr); - } - // free the marshalled rule - ptr = Marshal.ReadIntPtr(tempPtr, IntPtr.Size); - if (ptr != IntPtr.Zero) - { - Marshal.FreeHGlobal(ptr); - } - - Marshal.FreeHGlobal(tempPtr); + writer.WriteBoolean(key.ReverseOrder, s_reverseOrderTag); } } - Marshal.FreeHGlobal(memHandle); } } + _directoryControlValue = writer.Encode(); return base.GetValue(); } @@ -839,6 +910,9 @@ internal SortResponseControl(ResultCode result, string attributeName, bool criti public class VlvRequestControl : DirectoryControl { + private static Asn1Tag s_byOffsetChoiceTag = new(TagClass.ContextSpecific, 0, true); + private static Asn1Tag s_greaterThanOrEqualChoiceTag = new(TagClass.ContextSpecific, 1, false); + private int _before; private int _after; private int _offset; @@ -969,47 +1043,48 @@ public byte[] ContextId public override byte[] GetValue() { - var seq = new StringBuilder(10); - var objList = new ArrayList(); - - // first encode the before and the after count. - seq.Append("{ii"); - objList.Add(BeforeCount); - objList.Add(AfterCount); - - // encode Target if it is not null - if (Target.Length != 0) - { - seq.Append('t'); - objList.Add(0x80 | 0x1); - seq.Append('o'); - objList.Add(Target); - } - else + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.17. + * VLVRequestValue ::= SEQUENCE { + * beforeCount INTEGER, + * afterCount INTEGER, + * CHOICE { + * byoffset [0] SEQUENCE { + * offset INTEGER, + * contentCount INTEGER }, + * greaterThanOrEqual [1] AssertionValue }, + * contextID OCTET STRING OPTIONAL } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) { - seq.Append("t{"); - objList.Add(0xa0); - seq.Append("ii"); - objList.Add(Offset); - objList.Add(EstimateCount); - seq.Append('}'); - } + // first encode the before and the after count. + writer.WriteInteger(BeforeCount); + writer.WriteInteger(AfterCount); - // encode the contextID if present - if (ContextId.Length != 0) - { - seq.Append('o'); - objList.Add(ContextId); - } + // encode Target if it is not null + if (_target != null && _target.Length > 0) + { + writer.WriteOctetString(_target, s_greaterThanOrEqualChoiceTag); + } + else + { + using (AsnWriter.Scope innerSequence = writer.PushSequence(s_byOffsetChoiceTag)) + { + writer.WriteInteger(Offset); + writer.WriteInteger(EstimateCount); + } + } + + // encode the contextID if present + if (_context != null && _context.Length > 0) + { + writer.WriteOctetString(_context); + } - seq.Append('}'); - object[] values = new object[objList.Count]; - for (int i = 0; i < objList.Count; i++) - { - values[i] = objList[i]; } + _directoryControlValue = writer.Encode(); - _directoryControlValue = BerConverter.Encode(seq.ToString(), values); return base.GetValue(); } } @@ -1065,7 +1140,18 @@ public QuotaControl(SecurityIdentifier querySid) : this() public override byte[] GetValue() { - _directoryControlValue = BerConverter.Encode("{o}", new object[] { _sid }); + AsnWriter writer = new(AsnEncodingRules.BER); + + /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.19. + * QuotaRequestValue ::= SEQUENCE { + * querySID OCTET STRING } + */ + using (AsnWriter.Scope outerSequence = writer.PushSequence()) + { + writer.WriteOctetString(_sid); + } + _directoryControlValue = writer.Encode(); + return base.GetValue(); } } From a152c84be4d937a0d92e5639d0d5a92581af5bbd Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 23 Apr 2024 20:10:40 +0100 Subject: [PATCH 03/13] Regression in CrossDomainMoveControl --- .../DirectoryServices/Protocols/common/DirectoryControl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index c1ab4f38fa182..d94475dd53b9b 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -331,7 +331,7 @@ public override byte[] GetValue() * containing the fully qualified domain name of a DC in the domain to which the object * is to be moved. The string is not BER-encoded." */ - if (!string.IsNullOrEmpty(TargetDomainController)) + if (TargetDomainController != null) { int byteCount = s_utf8Encoding.GetByteCount(TargetDomainController); From 9461057df05b2eba8bc64de1a2cc0fe95acccf0d Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 23 Apr 2024 21:08:42 +0100 Subject: [PATCH 04/13] Correcting test values Most of the Control tests were hardcoded to the output of BerConverter, which uses four-byte lengths in all cases. This behaviour is now different: the same output is returned across all platforms for .NET, and remains unchanged for .NET Framework. This should also close issue 34679. --- .../tests/AsqRequestControlTests.cs | 27 ++++++------ .../tests/DirSyncRequestControlTests.cs | 42 ++++++++++++++----- .../tests/ExtendedDNControlTests.cs | 12 +++++- .../tests/PageResultRequestControlTests.cs | 27 +++++++++--- .../tests/QuotaControlTests.cs | 11 ++++- .../tests/SearchOptionsControlTests.cs | 12 +++++- .../SecurityDescriptorFlagControlTests.cs | 15 +++++-- .../tests/SortRequestControlTests.cs | 13 +++--- .../tests/VerifyNameControlTests.cs | 24 ++++++++--- .../tests/VlvRequestControlTests.cs | 39 +++++++++++++---- 10 files changed, 165 insertions(+), 57 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/AsqRequestControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/AsqRequestControlTests.cs index d2704307152d5..3aa7f6097e810 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/AsqRequestControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/AsqRequestControlTests.cs @@ -19,25 +19,26 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.1504", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 } : new byte[] { 48, 2, 4, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 }; +#else + var expected = new byte[] { 48, 2, 4, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } public static IEnumerable Ctor_String_Test_data() { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - yield return new object[] { null, new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 } }; - yield return new object[] { "", new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 } }; - yield return new object[] { "A", new byte[] { 48, 132, 0, 0, 0, 3, 4, 1, 65 } }; - } - else - { - yield return new object[] { null, new byte[] { 48, 2, 4, 0 } }; - yield return new object[] { "", new byte[] { 48, 2, 4, 0 } }; - yield return new object[] { "A", new byte[] { 48, 3, 4, 1, 65 } }; - } +#if NETFRAMEWORK + yield return new object[] { null, new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 } }; + yield return new object[] { "", new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 } }; + yield return new object[] { "A", new byte[] { 48, 132, 0, 0, 0, 3, 4, 1, 65 } }; +#else + yield return new object[] { null, new byte[] { 48, 2, 4, 0 } }; + yield return new object[] { "", new byte[] { 48, 2, 4, 0 } }; + yield return new object[] { "A", new byte[] { 48, 3, 4, 1, 65 } }; +#endif } [Theory] diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/DirSyncRequestControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/DirSyncRequestControlTests.cs index da6bcfbdf5b69..4699d1e008fed 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/DirSyncRequestControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/DirSyncRequestControlTests.cs @@ -22,15 +22,25 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.841", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } : new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 }; +#else + var expected = new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } public static IEnumerable Ctor_Cookie_Data() { - yield return new object[] { null, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } : new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; - yield return new object[] { new byte[0], (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } : new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; - yield return new object[] { new byte[] { 97, 98, 99 }, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 13, 2, 1, 0, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } : new byte[] { 48, 13, 2, 1, 0, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } }; +#if NETFRAMEWORK + yield return new object[] { null, new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[0], new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[] { 97, 98, 99 }, new byte[] { 48, 132, 0, 0, 0, 13, 2, 1, 0, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } }; +#else + yield return new object[] { null, new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[0], new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[] { 97, 98, 99 }, new byte[] { 48, 13, 2, 1, 0, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } }; +#endif } [Theory] @@ -51,9 +61,15 @@ public void Ctor_Cookie(byte[] cookie, byte[] expectedValue) public static IEnumerable Ctor_Cookie_Options_Data() { - yield return new object[] { null, DirectorySynchronizationOptions.None, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } : new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; - yield return new object[] { new byte[0], DirectorySynchronizationOptions.None - 1, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 13, 2, 4, 255, 255, 255, 255, 2, 3, 16, 0, 0, 4, 0 } : new byte[] { 48, 10, 2, 1, 255, 2, 3, 16, 0, 0, 4, 0 } }; - yield return new object[] { new byte[] { 97, 98, 99 }, DirectorySynchronizationOptions.ObjectSecurity, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 13, 2, 1, 1, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } : new byte[] { 48, 13, 2, 1, 1, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } }; +#if NETFRAMEWORK + yield return new object[] { null, DirectorySynchronizationOptions.None, new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[0], DirectorySynchronizationOptions.None - 1, new byte[] { 48, 132, 0, 0, 0, 13, 2, 4, 255, 255, 255, 255, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[] { 97, 98, 99 }, DirectorySynchronizationOptions.ObjectSecurity, new byte[] { 48, 132, 0, 0, 0, 13, 2, 1, 1, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } }; +#else + yield return new object[] { null, DirectorySynchronizationOptions.None, new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[0], DirectorySynchronizationOptions.None - 1, new byte[] { 48, 10, 2, 1, 255, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[] { 97, 98, 99 }, DirectorySynchronizationOptions.ObjectSecurity, new byte[] { 48, 13, 2, 1, 1, 2, 3, 16, 0, 0, 4, 3, 97, 98, 99 } }; +#endif } [Theory] @@ -74,9 +90,15 @@ public void Ctor_Cookie_Options(byte[] cookie, DirectorySynchronizationOptions o public static IEnumerable Ctor_Cookie_Options_AttributeCount_Data() { - yield return new object[] { null, DirectorySynchronizationOptions.None, 1048576, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } : new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; - yield return new object[] { new byte[0], DirectorySynchronizationOptions.None - 1, 0, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 11, 2, 4, 255, 255, 255, 255, 2, 1, 0, 4, 0 } : new byte[] { 48, 8, 2, 1, 255, 2, 1, 0, 4, 0 } }; - yield return new object[] { new byte[] { 97, 98, 99 }, DirectorySynchronizationOptions.ObjectSecurity, 10, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 11, 2, 1, 1, 2, 1, 10, 4, 3, 97, 98, 99 } : new byte[] { 48, 11, 2, 1, 1, 2, 1, 10, 4, 3, 97, 98, 99 } }; +#if NETFRAMEWORK + yield return new object[] { null, DirectorySynchronizationOptions.None, 1048576, new byte[] { 48, 132, 0, 0, 0, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[0], DirectorySynchronizationOptions.None - 1, 0, new byte[] { 48, 132, 0, 0, 0, 11, 2, 4, 255, 255, 255, 255, 2, 1, 0, 4, 0 } }; + yield return new object[] { new byte[] { 97, 98, 99 }, DirectorySynchronizationOptions.ObjectSecurity, 10, new byte[] { 48, 132, 0, 0, 0, 11, 2, 1, 1, 2, 1, 10, 4, 3, 97, 98, 99 } }; +#else + yield return new object[] { null, DirectorySynchronizationOptions.None, 1048576, new byte[] { 48, 10, 2, 1, 0, 2, 3, 16, 0, 0, 4, 0 } }; + yield return new object[] { new byte[0], DirectorySynchronizationOptions.None - 1, 0, new byte[] { 48, 8, 2, 1, 255, 2, 1, 0, 4, 0 } }; + yield return new object[] { new byte[] { 97, 98, 99 }, DirectorySynchronizationOptions.ObjectSecurity, 10, new byte[] { 48, 11, 2, 1, 1, 2, 1, 10, 4, 3, 97, 98, 99 } }; +#endif } [Theory] diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/ExtendedDNControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/ExtendedDNControlTests.cs index 284d916613b23..d7fe7a9aca68c 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/ExtendedDNControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/ExtendedDNControlTests.cs @@ -19,7 +19,11 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.529", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 0 } : new byte[] { 48, 3, 2, 1, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 0 }; +#else + var expected = new byte[] { 48, 3, 2, 1, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } @@ -32,7 +36,11 @@ public void Ctor_Flag() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.529", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 1 } : new byte[] { 48, 3, 2, 1, 1 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 1 }; +#else + var expected = new byte[] { 48, 3, 2, 1, 1 }; +#endif Assert.Equal(expected, control.GetValue()); } diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/PageResultRequestControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/PageResultRequestControlTests.cs index 537874c7f4fbd..5fab45ff1cb0e 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/PageResultRequestControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/PageResultRequestControlTests.cs @@ -20,14 +20,23 @@ public void Ctor_Default() Assert.Equal(512, control.PageSize); Assert.Equal("1.2.840.113556.1.4.319", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 6, 2, 2, 2, 0, 4, 0 } : new byte[] { 48, 6, 2, 2, 2, 0, 4, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 6, 2, 2, 2, 0, 4, 0 }; +#else + var expected = new byte[] { 48, 6, 2, 2, 2, 0, 4, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } public static IEnumerable Ctor_PageSize_Data() { - yield return new object[] { 0, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 0, 4, 0 } : new byte[] { 48, 5, 2, 1, 0, 4, 0 } }; - yield return new object[] { 10, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 10, 4, 0 } : new byte[] { 48, 5, 2, 1, 10, 4, 0 } }; +#if NETFRAMEWORK + yield return new object[] { 0, new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 0, 4, 0 } }; + yield return new object[] { 10, new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 10, 4, 0 } }; +#else + yield return new object[] { 0, new byte[] { 48, 5, 2, 1, 0, 4, 0 } }; + yield return new object[] { 10, new byte[] { 48, 5, 2, 1, 10, 4, 0 } }; +#endif } [Theory] @@ -52,9 +61,15 @@ public void Ctor_NegativePageSize_ThrowsArgumentException() public static IEnumerable Ctor_Cookie_Data() { - yield return new object[] { null, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 6, 2, 2, 2, 0, 4, 0 } : new byte[] { 48, 6, 2, 2, 2, 0, 4, 0 } }; - yield return new object[] { new byte[0], (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 6, 2, 2, 2, 0, 4, 0 } : new byte[] { 48, 6, 2, 2, 2, 0, 4, 0 } }; - yield return new object[] { new byte[] { 1, 2, 3, }, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 9, 2, 2, 2, 0, 4, 3, 1, 2, 3 } : new byte[] { 48, 9, 2, 2, 2, 0, 4, 3, 1, 2, 3 } }; +#if NETFRAMEWORK + yield return new object[] { null, new byte[] { 48, 132, 0, 0, 0, 6, 2, 2, 2, 0, 4, 0 } }; + yield return new object[] { new byte[0], new byte[] { 48, 132, 0, 0, 0, 6, 2, 2, 2, 0, 4, 0 } }; + yield return new object[] { new byte[] { 1, 2, 3, }, new byte[] { 48, 132, 0, 0, 0, 9, 2, 2, 2, 0, 4, 3, 1, 2, 3 } }; +#else + yield return new object[] { null, new byte[] { 48, 6, 2, 2, 2, 0, 4, 0 } }; + yield return new object[] { new byte[0], new byte[] { 48, 6, 2, 2, 2, 0, 4, 0 } }; + yield return new object[] { new byte[] { 1, 2, 3, }, new byte[] { 48, 9, 2, 2, 2, 0, 4, 3, 1, 2, 3 } }; +#endif } [Theory] diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/QuotaControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/QuotaControlTests.cs index b1ddaa9efa8e8..c89086f4066f5 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/QuotaControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/QuotaControlTests.cs @@ -20,14 +20,23 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.1852", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 } : new byte[] { 48, 2, 4, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 }; +#else + var expected = new byte[] { 48, 2, 4, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } public static IEnumerable Ctor_QuerySid_TestData() { +#if NETFRAMEWORK yield return new object[] { new SecurityIdentifier("S-1-5-32-544"), new byte[] { 48, 132, 0, 0, 0, 18, 4, 16, 1, 2, 0, 0, 0, 0, 0, 5, 32, 0, 0, 0, 32, 2, 0, 0 } }; yield return new object[] { null, new byte[] { 48, 132, 0, 0, 0, 2, 4, 0 } }; +#else + yield return new object[] { new SecurityIdentifier("S-1-5-32-544"), new byte[] { 48, 18, 4, 16, 1, 2, 0, 0, 0, 0, 0, 5, 32, 0, 0, 0, 32, 2, 0, 0 } }; + yield return new object[] { null, new byte[] { 48, 2, 4, 0 } }; +#endif } [Theory] diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/SearchOptionsControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/SearchOptionsControlTests.cs index 2a30d7dc3a522..f2e2b1b6c46d9 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/SearchOptionsControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/SearchOptionsControlTests.cs @@ -19,7 +19,11 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.1340", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 1 } : new byte[] { 48, 3, 2, 1, 1 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 1 }; +#else + var expected = new byte[] { 48, 3, 2, 1, 1 }; +#endif Assert.Equal(expected, control.GetValue()); } @@ -32,7 +36,11 @@ public void Ctor_Flags() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.1340", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 2 } : new byte[] { 48, 3, 2, 1, 2 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 2 }; +#else + var expected = new byte[] { 48, 3, 2, 1, 2 }; +#endif Assert.Equal(expected, control.GetValue()); } diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/SecurityDescriptorFlagControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/SecurityDescriptorFlagControlTests.cs index 6ec7955faa782..a973cfd141af9 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/SecurityDescriptorFlagControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/SecurityDescriptorFlagControlTests.cs @@ -19,14 +19,23 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.801", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 0 } : new byte[] { 48, 3, 2, 1, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 0 }; +#else + var expected = new byte[] { 48, 3, 2, 1, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } public static IEnumerable Ctor_Flags_Data() { - yield return new object[] { SecurityMasks.Group, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 2 } : new byte[] { 48, 3, 2, 1, 2 } }; - yield return new object[] { SecurityMasks.None - 1, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 6, 2, 4, 255, 255, 255, 255 } : new byte[] { 48, 3, 2, 1, 255 } }; +#if NETFRAMEWORK + yield return new object[] { SecurityMasks.Group, new byte[] { 48, 132, 0, 0, 0, 3, 2, 1, 2 } }; + yield return new object[] { SecurityMasks.None - 1, new byte[] { 48, 132, 0, 0, 0, 6, 2, 4, 255, 255, 255, 255 } }; +#else + yield return new object[] { SecurityMasks.Group, new byte[] { 48, 3, 2, 1, 2 } }; + yield return new object[] { SecurityMasks.None - 1, new byte[] { 48, 3, 2, 1, 255 } }; +#endif } [Theory] diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/SortRequestControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/SortRequestControlTests.cs index 50185cf8d52ef..aeed0e772e198 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/SortRequestControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/SortRequestControlTests.cs @@ -11,7 +11,6 @@ namespace System.DirectoryServices.Protocols.Tests public class SortRequestControlTests { [Theory] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34679", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] [InlineData(true)] [InlineData(false)] public void Ctor_SortKeys(bool critical) @@ -31,16 +30,16 @@ public void Ctor_SortKeys(bool critical) } control.IsCritical = critical; - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? - // WLDAP formatted ASN.1 - new byte[] { 48, 132, 0, 0, 0, 43, 48, 132, 0, 0, 0, 17, 4, 5,110, +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 43, 48, 132, 0, 0, 0, 17, 4, 5,110, 97, 109, 101, 49, 128, 5, 114, 117, 108, 101, 49, 129, 1, 255, 48, 132, 0, 0, 0, 14, 4, 5, 110, 97, 109, 101, - 50, 128, 5, 114, 117, 108, 101, 50 } : - // OpenLdap formatted ASN.1 - new byte[] { 48, 35, 48, 17, 4, 5, 110, 97, 109, 101, 49, 128, 5, + 50, 128, 5, 114, 117, 108, 101, 50 }; +#else + var expected = new byte[] { 48, 35, 48, 17, 4, 5, 110, 97, 109, 101, 49, 128, 5, 114, 117, 108, 101, 49, 129, 1, 255, 48, 14, 4, 5, 110, 97, 109, 101, 50, 128, 5, 114, 117, 108, 101, 50 }; +#endif Assert.Equal(expected, control.GetValue()); } diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/VerifyNameControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/VerifyNameControlTests.cs index 5e7469d27ed01..c967963483387 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/VerifyNameControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/VerifyNameControlTests.cs @@ -20,14 +20,23 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("1.2.840.113556.1.4.1338", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 0, 4, 0 } : new byte[] { 48, 5, 2, 1, 0, 4, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 0, 4, 0 }; +#else + var expected = new byte[] { 48, 5, 2, 1, 0, 4, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } public static IEnumerable Ctor_ServerName_Data() { - yield return new object[] { "", (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 0, 4, 0 } : new byte[] { 48, 5, 2, 1, 0, 4, 0 } }; - yield return new object[] { "S", (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 7, 2, 1, 0, 4, 2, 83, 0 } : new byte[] { 48, 7, 2, 1, 0, 4, 2, 83, 0 } }; +#if NETFRAMEWORK + yield return new object[] { "", new byte[] { 48, 132, 0, 0, 0, 5, 2, 1, 0, 4, 0 } }; + yield return new object[] { "S", new byte[] { 48, 132, 0, 0, 0, 7, 2, 1, 0, 4, 2, 83, 0 } }; +#else + yield return new object[] { "", new byte[] { 48, 5, 2, 1, 0, 4, 0 } }; + yield return new object[] { "S", new byte[] { 48, 7, 2, 1, 0, 4, 2, 83, 0 } }; +#endif } [Theory] @@ -46,8 +55,13 @@ public void Ctor_ServerName(string serverName, byte[] expectedValue) public static IEnumerable Ctor_ServerName_Flag_Data() { - yield return new object[] { "", -1, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 8, 2, 4, 255, 255, 255, 255, 4, 0 } : new byte[] { 48, 5, 2, 1, 255, 4, 0 } }; - yield return new object[] { "S", 10, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 7, 2, 1, 10, 4, 2, 83, 0 } : new byte[] { 48, 7, 2, 1, 10, 4, 2, 83, 0 } }; +#if NETFRAMEWORK + yield return new object[] { "", -1, new byte[] { 48, 132, 0, 0, 0, 8, 2, 4, 255, 255, 255, 255, 4, 0 } }; + yield return new object[] { "S", 10, new byte[] { 48, 132, 0, 0, 0, 7, 2, 1, 10, 4, 2, 83, 0 } }; +#else + yield return new object[] { "", -1, new byte[] { 48, 5, 2, 1, 255, 4, 0 } }; + yield return new object[] { "S", 10, new byte[] { 48, 7, 2, 1, 10, 4, 2, 83, 0 } }; +#endif } [Theory] diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/VlvRequestControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/VlvRequestControlTests.cs index a259bd11c7e8a..00f964200a9d9 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/VlvRequestControlTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/VlvRequestControlTests.cs @@ -24,14 +24,23 @@ public void Ctor_Default() Assert.True(control.ServerSide); Assert.Equal("2.16.840.1.113730.3.4.9", control.Type); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 } : new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 }; +#else + var expected = new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 }; +#endif Assert.Equal(expected, control.GetValue()); } public static IEnumerable Ctor_BeforeCount_AfterCount_Offset_Data() { - yield return new object[] { 0, 0, 0, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 } : new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 } }; - yield return new object[] { 10, 10, 10, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 10, 2, 1, 10, 160, 132, 0, 0, 0, 6, 2, 1, 10, 2, 1, 0 } : new byte[] { 48, 14, 2, 1, 10, 2, 1, 10, 160, 6, 2, 1, 10, 2, 1, 0 } }; +#if NETFRAMEWORK + yield return new object[] { 0, 0, 0, new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 } }; + yield return new object[] { 10, 10, 10, new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 10, 2, 1, 10, 160, 132, 0, 0, 0, 6, 2, 1, 10, 2, 1, 0 } }; +#else + yield return new object[] { 0, 0, 0, new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 } }; + yield return new object[] { 10, 10, 10, new byte[] { 48, 14, 2, 1, 10, 2, 1, 10, 160, 6, 2, 1, 10, 2, 1, 0 } }; +#endif } [Theory] @@ -54,8 +63,13 @@ public void Ctor_BeforeCount_AfterCount_Offset(int beforeCount, int afterCount, public static IEnumerable Ctor_BeforeCount_AfterCount_StringTarget_Data() { - yield return new object[] { 0, 0, null, new byte[0], (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 } : new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 } }; - yield return new object[] { 10, 10, "abc", new byte[] { 97, 98, 99 }, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 11, 2, 1, 10, 2, 1, 10, 129, 3, 97, 98, 99 } : new byte[] { 48, 11, 2, 1, 10, 2, 1, 10, 129, 3, 97, 98, 99 } }; +#if NETFRAMEWORK + yield return new object[] { 0, 0, null, new byte[0], new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 } }; + yield return new object[] { 10, 10, "abc", new byte[] { 97, 98, 99 }, new byte[] { 48, 132, 0, 0, 0, 11, 2, 1, 10, 2, 1, 10, 129, 3, 97, 98, 99 } }; +#else + yield return new object[] { 0, 0, null, new byte[0], new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 } }; + yield return new object[] { 10, 10, "abc", new byte[] { 97, 98, 99 }, new byte[] { 48, 11, 2, 1, 10, 2, 1, 10, 129, 3, 97, 98, 99 } }; +#endif } [Theory] @@ -79,8 +93,13 @@ public void Ctor_BeforeCount_AfterCount_StringTarget(int beforeCount, int afterC public static IEnumerable Ctor_BeforeCount_AfterCount_ByteArrayTarget_Data() { - yield return new object[] { 0, 0, null, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 } : new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 } }; - yield return new object[] { 10, 10, new byte[] { 1, 2, 3 }, (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 11, 2, 1, 10, 2, 1, 10, 129, 3, 1, 2, 3 } : new byte[] { 48, 11, 2, 1, 10, 2, 1, 10, 129, 3, 1, 2, 3 } }; +#if NETFRAMEWORK + yield return new object[] { 0, 0, null, new byte[] { 48, 132, 0, 0, 0, 18, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0 } }; + yield return new object[] { 10, 10, new byte[] { 1, 2, 3 }, new byte[] { 48, 132, 0, 0, 0, 11, 2, 1, 10, 2, 1, 10, 129, 3, 1, 2, 3 } }; +#else + yield return new object[] { 0, 0, null, new byte[] { 48, 14, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0 } }; + yield return new object[] { 10, 10, new byte[] { 1, 2, 3 }, new byte[] { 48, 11, 2, 1, 10, 2, 1, 10, 129, 3, 1, 2, 3 } }; +#endif } [Theory] @@ -146,7 +165,11 @@ public void ContextId_Set_GetReturnsExpected() Assert.NotSame(contextId, control.ContextId); Assert.Equal(contextId, control.ContextId); - var expected = (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) ? new byte[] { 48, 132, 0, 0, 0, 23, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0, 4, 3, 1, 2, 3 } : new byte[] { 48, 19, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0, 4, 3, 1, 2, 3 }; +#if NETFRAMEWORK + var expected = new byte[] { 48, 132, 0, 0, 0, 23, 2, 1, 0, 2, 1, 0, 160, 132, 0, 0, 0, 6, 2, 1, 0, 2, 1, 0, 4, 3, 1, 2, 3 }; +#else + var expected = new byte[] { 48, 19, 2, 1, 0, 2, 1, 0, 160, 6, 2, 1, 0, 2, 1, 0, 4, 3, 1, 2, 3 }; +#endif Assert.Equal(expected, control.GetValue()); } } From cb07eb46c1ef6a3e33d88e7647673c58f54f6a3e Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 23 Apr 2024 21:09:16 +0100 Subject: [PATCH 05/13] Removed SortKeyInterop from .csproj --- .../src/System.DirectoryServices.Protocols.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj b/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj index 60b7742c23b61..ff1aba7d99d73 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj +++ b/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj @@ -51,7 +51,6 @@ - @@ -64,7 +63,6 @@ - @@ -79,7 +77,6 @@ - From 73dd1bc80e21f14eb5b7d4e76dcac201bb0e5661 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 23 Apr 2024 21:20:27 +0100 Subject: [PATCH 06/13] Small optimisations Reduce number of copies required in TransformControls, and enable these copies to take advantage of newer intrinsics where available. --- .../Protocols/common/DirectoryControl.cs | 79 ++++--------------- 1 file changed, 16 insertions(+), 63 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index d94475dd53b9b..31ae3ce18a1cc 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -100,11 +100,7 @@ public DirectoryControl(string type, byte[] value, bool isCritical, bool serverS if (value != null) { - _directoryControlValue = new byte[value.Length]; - for (int i = 0; i < value.Length; i++) - { - _directoryControlValue[i] = value[i]; - } + _directoryControlValue = value.AsSpan().ToArray(); } IsCritical = isCritical; ServerSide = serverSide; @@ -117,12 +113,7 @@ public virtual byte[] GetValue() return Array.Empty(); } - byte[] tempValue = new byte[_directoryControlValue.Length]; - for (int i = 0; i < _directoryControlValue.Length; i++) - { - tempValue[i] = _directoryControlValue[i]; - } - return tempValue; + return _directoryControlValue.AsSpan().ToArray(); } public string Type { get; } @@ -136,7 +127,7 @@ internal static void TransformControls(DirectoryControl[] controls) for (int i = 0; i < controls.Length; i++) { Debug.Assert(controls[i] != null); - byte[] value = controls[i].GetValue(); + byte[] value = controls[i]._directoryControlValue ?? Array.Empty(); Span asnSpan = value; bool asnReadSuccessful; @@ -258,7 +249,7 @@ internal static void TransformControls(DirectoryControl[] controls) public class AsqRequestControl : DirectoryControl { - private static UTF8Encoding s_utf8Encoding = new(false, true); + private static readonly UTF8Encoding s_utf8Encoding = new(false, true); public AsqRequestControl() : base("1.2.840.113556.1.4.1504", null, true, true) { @@ -577,13 +568,7 @@ public byte[] Cookie return Array.Empty(); } - byte[] tempCookie = new byte[_dirsyncCookie.Length]; - for (int i = 0; i < tempCookie.Length; i++) - { - tempCookie[i] = _dirsyncCookie[i]; - } - - return tempCookie; + return _dirsyncCookie.AsSpan().ToArray(); } set => _dirsyncCookie = value; } @@ -648,13 +633,7 @@ public byte[] Cookie return Array.Empty(); } - byte[] tempCookie = new byte[_dirsyncCookie.Length]; - for (int i = 0; i < tempCookie.Length; i++) - { - tempCookie[i] = _dirsyncCookie[i]; - } - - return tempCookie; + return _dirsyncCookie.AsSpan().ToArray(); } } @@ -703,13 +682,7 @@ public byte[] Cookie return Array.Empty(); } - byte[] tempCookie = new byte[_pageCookie.Length]; - for (int i = 0; i < _pageCookie.Length; i++) - { - tempCookie[i] = _pageCookie[i]; - } - - return tempCookie; + return _pageCookie.AsSpan().ToArray(); } set => _pageCookie = value; } @@ -753,12 +726,7 @@ public byte[] Cookie return Array.Empty(); } - byte[] tempCookie = new byte[_pageCookie.Length]; - for (int i = 0; i < _pageCookie.Length; i++) - { - tempCookie[i] = _pageCookie[i]; - } - return tempCookie; + return _pageCookie.AsSpan().ToArray(); } } @@ -767,9 +735,9 @@ public byte[] Cookie public class SortRequestControl : DirectoryControl { - private static UTF8Encoding s_utf8Encoding = new(false, true); - private static Asn1Tag s_orderingRuleTag = new(TagClass.ContextSpecific, 0, false); - private static Asn1Tag s_reverseOrderTag = new(TagClass.ContextSpecific, 1, false); + private static readonly UTF8Encoding s_utf8Encoding = new(false, true); + private static readonly Asn1Tag s_orderingRuleTag = new(TagClass.ContextSpecific, 0, false); + private static readonly Asn1Tag s_reverseOrderTag = new(TagClass.ContextSpecific, 1, false); private SortKey[] _keys = Array.Empty(); public SortRequestControl(params SortKey[] sortKeys) : base("1.2.840.113556.1.4.473", null, true, true) @@ -910,8 +878,8 @@ internal SortResponseControl(ResultCode result, string attributeName, bool criti public class VlvRequestControl : DirectoryControl { - private static Asn1Tag s_byOffsetChoiceTag = new(TagClass.ContextSpecific, 0, true); - private static Asn1Tag s_greaterThanOrEqualChoiceTag = new(TagClass.ContextSpecific, 1, false); + private static readonly Asn1Tag s_byOffsetChoiceTag = new(TagClass.ContextSpecific, 0, true); + private static readonly Asn1Tag s_greaterThanOrEqualChoiceTag = new(TagClass.ContextSpecific, 1, false); private int _before; private int _after; @@ -1012,12 +980,7 @@ public byte[] Target return Array.Empty(); } - byte[] tempContext = new byte[_target.Length]; - for (int i = 0; i < tempContext.Length; i++) - { - tempContext[i] = _target[i]; - } - return tempContext; + return _target.AsSpan().ToArray(); } set => _target = value; } @@ -1031,12 +994,7 @@ public byte[] ContextId return Array.Empty(); } - byte[] tempContext = new byte[_context.Length]; - for (int i = 0; i < tempContext.Length; i++) - { - tempContext[i] = _context[i]; - } - return tempContext; + return _context.AsSpan().ToArray(); } set => _context = value; } @@ -1114,12 +1072,7 @@ public byte[] ContextId return Array.Empty(); } - byte[] tempContext = new byte[_context.Length]; - for (int i = 0; i < tempContext.Length; i++) - { - tempContext[i] = _context[i]; - } - return tempContext; + return _context.AsSpan().ToArray(); } } From c5c95983bf19c17806e93027a5095afbb5c67595 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 24 Apr 2024 19:07:27 +0100 Subject: [PATCH 07/13] AD compatibility test corrections Windows domain controllers may return a distinguished name starting with OU=, rather than ou=. --- .../tests/DirectoryServicesProtocolsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs index 00433bae9875c..64c8487c7b481 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs @@ -669,7 +669,7 @@ public void TestSortedSearch() Assert.Equal(1, searchResponse.Controls.Length); Assert.True(searchResponse.Controls[0] is SortResponseControl); Assert.True(searchResponse.Entries.Count > 0); - Assert.Equal("ou=ProtocolsSubGroup10.9," + dn + "," + LdapConfiguration.Configuration.SearchDn, searchResponse.Entries[0].DistinguishedName); + Assert.Equal("ou=ProtocolsSubGroup10.9," + dn + "," + LdapConfiguration.Configuration.SearchDn, searchResponse.Entries[0].DistinguishedName, true); } finally { From 33e4f25e49456439fe64d39295811a2bd27d2970 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 27 Apr 2024 14:22:21 +0100 Subject: [PATCH 08/13] Performance improvement, bugfix Preallocating space for AsnWriter buffers to reduce memory usage. Correctly handling attribute names in SortControls. --- .../Protocols/common/DirectoryControl.cs | 57 ++++++++++++------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index 31ae3ce18a1cc..758a9b2b7da1a 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -90,6 +90,8 @@ public bool ReverseOrder public class DirectoryControl { + internal static readonly UTF8Encoding s_utf8Encoding = new(false, true); + internal byte[] _directoryControlValue; public DirectoryControl(string type, byte[] value, bool isCritical, bool serverSide) @@ -124,6 +126,8 @@ public virtual byte[] GetValue() internal static void TransformControls(DirectoryControl[] controls) { + Span scratchSpace = stackalloc byte[256]; + for (int i = 0; i < controls.Length; i++) { Debug.Assert(controls[i] != null); @@ -203,10 +207,13 @@ internal static void TransformControls(DirectoryControl[] controls) asnSpan = asnSpan.Slice(bytesConsumed); // Attribute name is optional: AD for example never returns attribute name - asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out Asn1Tag octetStringTag, out _, out _, out _); + asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out Asn1Tag octetStringTag, out _, out int octetStringLength, out _); if (asnReadSuccessful) { - attribute = AsnDecoder.ReadCharacterString(asnSpan, AsnEncodingRules.BER, UniversalTagNumber.UTF8String, out _, octetStringTag); + Span attributeNameBuffer = octetStringLength < scratchSpace.Length ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; + + _ = AsnDecoder.TryReadOctetString(asnSpan, attributeNameBuffer, AsnEncodingRules.BER, out _, out _); + attribute = s_utf8Encoding.GetString(attributeNameBuffer); } SortResponseControl sort = new SortResponseControl(result, attribute, controls[i].IsCritical, value); @@ -249,8 +256,6 @@ internal static void TransformControls(DirectoryControl[] controls) public class AsqRequestControl : DirectoryControl { - private static readonly UTF8Encoding s_utf8Encoding = new(false, true); - public AsqRequestControl() : base("1.2.840.113556.1.4.1504", null, true, true) { } @@ -264,7 +269,8 @@ public AsqRequestControl(string attributeName) : this() public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 4 + (AttributeName?.Length ?? 0); + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.18. * ASQRequestValue ::= SEQUENCE { @@ -302,8 +308,6 @@ internal AsqResponseControl(ResultCode result, bool criticality, byte[] controlV public class CrossDomainMoveControl : DirectoryControl { - private static readonly UTF8Encoding s_utf8Encoding = new UTF8Encoding(false); - public CrossDomainMoveControl() : base("1.2.840.113556.1.4.521", null, true, true) { } @@ -368,7 +372,8 @@ public ExtendedDNFlag Flag } public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 8; + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.5. * ExtendedDNRequestValue ::= SEQUENCE { @@ -414,7 +419,8 @@ public SecurityDescriptorFlagControl(SecurityMasks masks) : this() public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 8; + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.11. * SDFlagsRequestValue ::= SEQUENCE { @@ -454,7 +460,8 @@ public SearchOption SearchOption public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 8; + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.12. * SearchOptionsRequestValue ::= SEQUENCE { @@ -508,7 +515,8 @@ public string ServerName public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 10 + 2 * (ServerName?.Length ?? 0); + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.16. * VerifyNameRequestValue ::= SEQUENCE { @@ -593,7 +601,8 @@ public int AttributeCount public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 16 + (_dirsyncCookie?.Length ?? 0); + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.3. * DirSyncRequestValue ::= SEQUENCE { @@ -605,7 +614,7 @@ public override byte[] GetValue() { writer.WriteInteger((int)Option); writer.WriteInteger(AttributeCount); - writer.WriteOctetString(_dirsyncCookie); + writer.WriteOctetString(_dirsyncCookie ?? []); } _directoryControlValue = writer.Encode(); @@ -689,7 +698,8 @@ public byte[] Cookie public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 6 + (_pageCookie?.Length ?? 1); + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in RFC2696. * realSearchControlValue ::= SEQUENCE { @@ -735,10 +745,10 @@ public byte[] Cookie public class SortRequestControl : DirectoryControl { - private static readonly UTF8Encoding s_utf8Encoding = new(false, true); private static readonly Asn1Tag s_orderingRuleTag = new(TagClass.ContextSpecific, 0, false); private static readonly Asn1Tag s_reverseOrderTag = new(TagClass.ContextSpecific, 1, false); + private int _keysAsnLength; private SortKey[] _keys = Array.Empty(); public SortRequestControl(params SortKey[] sortKeys) : base("1.2.840.113556.1.4.473", null, true, true) { @@ -752,10 +762,12 @@ public SortRequestControl(params SortKey[] sortKeys) : base("1.2.840.113556.1.4. } } + _keysAsnLength = 0; _keys = new SortKey[sortKeys.Length]; for (int i = 0; i < sortKeys.Length; i++) { _keys[i] = new SortKey(sortKeys[i].AttributeName, sortKeys[i].MatchingRule, sortKeys[i].ReverseOrder); + _keysAsnLength += 13 + (sortKeys[i].AttributeName?.Length ?? 0) + (sortKeys[i].MatchingRule?.Length ?? 0); } } @@ -767,6 +779,7 @@ public SortRequestControl(string attributeName, string matchingRule, bool revers { SortKey key = new SortKey(attributeName, matchingRule, reverseOrder); _keys = new SortKey[] { key }; + _keysAsnLength = 13 + (attributeName?.Length ?? 0) + (matchingRule?.Length ?? 0); } public SortKey[] SortKeys @@ -800,17 +813,20 @@ public SortKey[] SortKeys } } + _keysAsnLength = 0; _keys = new SortKey[value.Length]; for (int i = 0; i < value.Length; i++) { _keys[i] = new SortKey(value[i].AttributeName, value[i].MatchingRule, value[i].ReverseOrder); + _keysAsnLength += 13 + (value[i].AttributeName?.Length ?? 0) + (value[i].MatchingRule?.Length ?? 0); } } } public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 12 + _keysAsnLength; + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in RFC2891. * SortKeyList ::= SEQUENCE OF SEQUENCE { @@ -903,8 +919,7 @@ public VlvRequestControl(int beforeCount, int afterCount, string target) : this( AfterCount = afterCount; if (target != null) { - UTF8Encoding encoder = new UTF8Encoding(); - _target = encoder.GetBytes(target); + _target = s_utf8Encoding.GetBytes(target); } } @@ -1001,7 +1016,8 @@ public byte[] ContextId public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 16 + (_target?.Length ?? 12) + (_context?.Length ?? 1); + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.17. * VLVRequestValue ::= SEQUENCE { @@ -1093,7 +1109,8 @@ public QuotaControl(SecurityIdentifier querySid) : this() public override byte[] GetValue() { - AsnWriter writer = new(AsnEncodingRules.BER); + int sizeEstimate = 4 + (_sid?.Length ?? 0); + AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.19. * QuotaRequestValue ::= SEQUENCE { From 6b487c221325738c244f1a5073a366c7572db38f Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 11 May 2024 13:28:57 +0100 Subject: [PATCH 09/13] Following code review Corrected off-by-one error in stack allocation comparison. Made some stackallocs constant (unless a meaningful constant size would be >256 bytes) and reviewed these constant values based on real-world attribute lengths/server names. --- .../Protocols/common/DirectoryControl.cs | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index 758a9b2b7da1a..d434e7dcd0f7c 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -90,6 +90,17 @@ public bool ReverseOrder public class DirectoryControl { + // Scratch buffer allocations with sizes which are below this threshold should be made on the stack. + // This is partially based on RFC1035, which specifies that a label in a domain name should be < 64 characters. + // If a server name is specified as an FQDN, this will be at least three labels in an AD environment - up to + // 192 characters. Doubling this to allow for Unicode encoding, then rounding to the nearest power of two + // yields 512. + internal const int ServerNameStackAllocationThreshold = 512; + // Scratch buffer allocations with sizes which are below this threshold should be made on the stack. + // This is based on the Active Directory schema. The largest attribute name here is msDS-FailedInteractiveLogonCountAtLastSuccessfulLogon, + // which is 53 characters long. This is rounded up to the nearest power of two. + internal const int AttributeNameStackAllocationThreshold = 64; + internal static readonly UTF8Encoding s_utf8Encoding = new(false, true); internal byte[] _directoryControlValue; @@ -126,7 +137,7 @@ public virtual byte[] GetValue() internal static void TransformControls(DirectoryControl[] controls) { - Span scratchSpace = stackalloc byte[256]; + Span attributeNameScratchSpace = stackalloc byte[AttributeNameStackAllocationThreshold]; for (int i = 0; i < controls.Length; i++) { @@ -210,7 +221,7 @@ internal static void TransformControls(DirectoryControl[] controls) asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out Asn1Tag octetStringTag, out _, out int octetStringLength, out _); if (asnReadSuccessful) { - Span attributeNameBuffer = octetStringLength < scratchSpace.Length ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; + Span attributeNameBuffer = octetStringLength <= AttributeNameStackAllocationThreshold ? attributeNameScratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; _ = AsnDecoder.TryReadOctetString(asnSpan, attributeNameBuffer, AsnEncodingRules.BER, out _, out _); attribute = s_utf8Encoding.GetString(attributeNameBuffer); @@ -281,7 +292,8 @@ public override byte[] GetValue() if (!string.IsNullOrEmpty(AttributeName)) { int octetStringLength = s_utf8Encoding.GetByteCount(AttributeName); - Span tmpValue = octetStringLength < 256 ? stackalloc byte[octetStringLength] : new byte[octetStringLength]; + // This trades slightly increased stack usage for the improved codegen which comes from a constant value. + Span tmpValue = octetStringLength <= AttributeNameStackAllocationThreshold ? stackalloc byte[AttributeNameStackAllocationThreshold].Slice(0, octetStringLength) : new byte[octetStringLength]; s_utf8Encoding.GetBytes(AttributeName, tmpValue); writer.WriteOctetString(tmpValue); @@ -490,7 +502,6 @@ public TreeDeleteControl() : base("1.2.840.113556.1.4.805", null, true, true) { public class VerifyNameControl : DirectoryControl { private string _serverName; - public VerifyNameControl() : base("1.2.840.113556.1.4.1338", null, true, true) { } public VerifyNameControl(string serverName) : this() @@ -530,7 +541,9 @@ public override byte[] GetValue() if (!string.IsNullOrEmpty(ServerName)) { int serverNameLength = Encoding.Unicode.GetByteCount(ServerName); - Span tmpValue = serverNameLength < 256 ? stackalloc byte[serverNameLength] : new byte[serverNameLength]; + // This differs from AsqRequest - it doesn't allocate ServerNameStackAllocationThreshold and provide a slice into it, because the size of this + // constant is such that the larger stack allocation would outweigh the benefit of a constant-value stackalloc. + Span tmpValue = serverNameLength <= ServerNameStackAllocationThreshold ? stackalloc byte[serverNameLength] : new byte[serverNameLength]; Encoding.Unicode.GetBytes(ServerName, tmpValue); writer.WriteOctetString(tmpValue); @@ -836,7 +849,11 @@ public override byte[] GetValue() * */ using (AsnWriter.Scope outerSequence = writer.PushSequence()) { - Span scratchSpace = stackalloc byte[256]; + // This scratch space is used for storing attribute names and matching rule OIDs. + // Active Directory's valid matching rule OIDs are listed in MS-ADTS 3.1.1.3.4.1.13, + // with a maximum length of 23 characters - within the attribute name stack allocation + // threshold. + Span scratchSpace = stackalloc byte[AttributeNameStackAllocationThreshold]; for (int i = 0; i < _keys.Length; i++) { @@ -847,7 +864,7 @@ public override byte[] GetValue() if (!string.IsNullOrEmpty(key.AttributeName)) { int octetStringLength = s_utf8Encoding.GetByteCount(key.AttributeName); - Span tmpValue = octetStringLength < scratchSpace.Length ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; + Span tmpValue = octetStringLength <= AttributeNameStackAllocationThreshold ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; s_utf8Encoding.GetBytes(key.AttributeName, tmpValue); writer.WriteOctetString(tmpValue); @@ -860,7 +877,7 @@ public override byte[] GetValue() if (!string.IsNullOrEmpty(key.MatchingRule)) { int octetStringLength = s_utf8Encoding.GetByteCount(key.MatchingRule); - Span tmpValue = octetStringLength < scratchSpace.Length ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; + Span tmpValue = octetStringLength <= AttributeNameStackAllocationThreshold ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; s_utf8Encoding.GetBytes(key.MatchingRule, tmpValue); writer.WriteOctetString(tmpValue, s_orderingRuleTag); From b4bbdc740f3dad7eb75d625600241da5bf6dcd4b Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 24 May 2024 21:55:22 +0100 Subject: [PATCH 10/13] Implemented further code review comments * Added limited caching of AsnWriters, as small/medium/large buckets. * Replaced several Debug.Asserts with BerExceptions. * Changed the type of raised exception to retain backwards-compatibility. * Added extra checks on BER content lengths to match BerConverter. --- .../Protocols/common/DirectoryControl.cs | 291 +++++++++++------- 1 file changed, 173 insertions(+), 118 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index d434e7dcd0f7c..5749f12c16975 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -5,6 +5,7 @@ using System.ComponentModel; using System.Diagnostics; using System.Formats.Asn1; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Versioning; using System.Security.Principal; @@ -105,6 +106,22 @@ public class DirectoryControl internal byte[] _directoryControlValue; + [ThreadStatic] + private static AsnWriter t_smallWriter; + [ThreadStatic] + private static AsnWriter t_mediumWriter; + [ThreadStatic] + private static AsnWriter t_largeWriter; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static AsnWriter GetWriter(int expectedSize) + => expectedSize switch + { + > 0 and <= 32 => t_smallWriter ??= new AsnWriter(AsnEncodingRules.BER, 32), + > 32 and <= 128 => t_mediumWriter ??= new AsnWriter(AsnEncodingRules.BER, 128), + _ => t_largeWriter ??= new AsnWriter(AsnEncodingRules.BER, 256) + }; + public DirectoryControl(string type, byte[] value, bool isCritical, bool serverSide) { ArgumentNullException.ThrowIfNull(type); @@ -139,129 +156,149 @@ internal static void TransformControls(DirectoryControl[] controls) { Span attributeNameScratchSpace = stackalloc byte[AttributeNameStackAllocationThreshold]; - for (int i = 0; i < controls.Length; i++) + try { - Debug.Assert(controls[i] != null); - byte[] value = controls[i]._directoryControlValue ?? Array.Empty(); - Span asnSpan = value; - bool asnReadSuccessful; - - if (controls[i].Type == "1.2.840.113556.1.4.319") + for (int i = 0; i < controls.Length; i++) { - // The control is a PageControl, as described in RFC 2696. - byte[] cookie = Array.Empty(); + Debug.Assert(controls[i] != null); + byte[] value = controls[i]._directoryControlValue ?? Array.Empty(); + Span asnSpan = value; + bool asnReadSuccessful; + + if (controls[i].Type == "1.2.840.113556.1.4.319") + { + // The control is a PageControl, as described in RFC 2696. + byte[] cookie = Array.Empty(); - AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); - Debug.Assert(sequenceContentLength > 0); + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + ThrowUnless(sequenceContentLength > 0); - asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan.Slice(sequenceContentOffset), AsnEncodingRules.BER, out int size, out int bytesConsumed); - Debug.Assert(asnReadSuccessful); - asnSpan = asnSpan.Slice(sequenceContentOffset + bytesConsumed); + asnSpan = asnSpan.Slice(sequenceContentOffset, sequenceContentLength); + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int size, out int bytesConsumed); + ThrowUnless(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out int _, out int cookieLength, out _); - if (asnReadSuccessful) - { - cookie = new byte[cookieLength]; + asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out int _, out int cookieLength, out _); + if (asnReadSuccessful) + { + cookie = new byte[cookieLength]; + + // user expects cookie with length 0 as paged search is done. + asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, cookie, AsnEncodingRules.BER, out bytesConsumed, out _); + ThrowUnless(asnReadSuccessful && asnSpan.Length == bytesConsumed); + } - // user expects cookie with length 0 as paged search is done. - asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, cookie, AsnEncodingRules.BER, out _, out _); - Debug.Assert(asnReadSuccessful); + PageResultResponseControl pageControl = new PageResultResponseControl(size, cookie, controls[i].IsCritical, value); + controls[i] = pageControl; } + else if (controls[i].Type == "1.2.840.113556.1.4.1504") + { + // The control is an AsqControl, as described in MS-ADTS section 3.1.1.3.4.1.18 + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + ThrowUnless(sequenceContentLength > 0); - PageResultResponseControl pageControl = new PageResultResponseControl(size, cookie, controls[i].IsCritical, value); - controls[i] = pageControl; - } - else if (controls[i].Type == "1.2.840.113556.1.4.1504") - { - // The control is an AsqControl, as described in MS-ADTS section 3.1.1.3.4.1.18 - AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); - Debug.Assert(sequenceContentLength > 0); + ResultCode result = AsnDecoder.ReadEnumeratedValue(asnSpan.Slice(sequenceContentOffset, sequenceContentLength), AsnEncodingRules.BER, out _); - ResultCode result = AsnDecoder.ReadEnumeratedValue(asnSpan.Slice(sequenceContentOffset), AsnEncodingRules.BER, out _); + AsqResponseControl asq = new AsqResponseControl(result, controls[i].IsCritical, value); + controls[i] = asq; + } + else if (controls[i].Type == "1.2.840.113556.1.4.841") + { + // The control is a DirSyncControl, as described in MS-ADTS section 3.1.1.3.4.1.3 + byte[] dirsyncCookie; - AsqResponseControl asq = new AsqResponseControl(result, controls[i].IsCritical, value); - controls[i] = asq; - } - else if (controls[i].Type == "1.2.840.113556.1.4.841") - { - // The control is a DirSyncControl, as described in MS-ADTS section 3.1.1.3.4.1.3 - byte[] dirsyncCookie; + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + ThrowUnless(sequenceContentLength > 0); + asnSpan = asnSpan.Slice(sequenceContentOffset, sequenceContentLength); - AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); - Debug.Assert(sequenceContentLength > 0); - asnSpan = asnSpan.Slice(sequenceContentOffset); + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int moreData, out int bytesConsumed); + ThrowUnless(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int moreData, out int bytesConsumed); - Debug.Assert(asnReadSuccessful); - asnSpan = asnSpan.Slice(bytesConsumed); + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int count, out bytesConsumed); + ThrowUnless(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int count, out bytesConsumed); - Debug.Assert(asnReadSuccessful); - asnSpan = asnSpan.Slice(bytesConsumed); + dirsyncCookie = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); + ThrowUnless(asnSpan.Length == bytesConsumed); - dirsyncCookie = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out _); + DirSyncResponseControl dirsync = new DirSyncResponseControl(dirsyncCookie, moreData != 0, count, controls[i].IsCritical, value); + controls[i] = dirsync; + } + else if (controls[i].Type == "1.2.840.113556.1.4.474") + { + // The control is a SortControl, as described in RFC 2891. + ResultCode result; + string attribute = null; - DirSyncResponseControl dirsync = new DirSyncResponseControl(dirsyncCookie, moreData != 0, count, controls[i].IsCritical, value); - controls[i] = dirsync; - } - else if (controls[i].Type == "1.2.840.113556.1.4.474") - { - // The control is a SortControl, as described in RFC 2891. - ResultCode result; - string attribute = null; + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + ThrowUnless(sequenceContentLength > 0); + asnSpan = asnSpan.Slice(sequenceContentOffset, sequenceContentLength); - AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); - Debug.Assert(sequenceContentLength > 0); - asnSpan = asnSpan.Slice(sequenceContentOffset); + result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out int bytesConsumed); + asnSpan = asnSpan.Slice(bytesConsumed); - result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out int bytesConsumed); - asnSpan = asnSpan.Slice(bytesConsumed); + // Attribute name is optional: AD for example never returns attribute name + asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out _, out int octetStringLength, out _); + if (asnReadSuccessful) + { + Span attributeNameBuffer = octetStringLength <= AttributeNameStackAllocationThreshold ? attributeNameScratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; - // Attribute name is optional: AD for example never returns attribute name - asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out Asn1Tag octetStringTag, out _, out int octetStringLength, out _); - if (asnReadSuccessful) - { - Span attributeNameBuffer = octetStringLength <= AttributeNameStackAllocationThreshold ? attributeNameScratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; + asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, attributeNameBuffer, AsnEncodingRules.BER, out bytesConsumed, out _); + ThrowUnless(asnReadSuccessful && asnSpan.Length == bytesConsumed); + attribute = s_utf8Encoding.GetString(attributeNameBuffer); + } - _ = AsnDecoder.TryReadOctetString(asnSpan, attributeNameBuffer, AsnEncodingRules.BER, out _, out _); - attribute = s_utf8Encoding.GetString(attributeNameBuffer); + SortResponseControl sort = new SortResponseControl(result, attribute, controls[i].IsCritical, value); + controls[i] = sort; } + else if (controls[i].Type == "2.16.840.1.113730.3.4.10") + { + // The control is a VlvResponseControl, as described in MS-ADTS 3.1.1.3.4.1.17 + ResultCode result; + byte[] context = null; - SortResponseControl sort = new SortResponseControl(result, attribute, controls[i].IsCritical, value); - controls[i] = sort; - } - else if (controls[i].Type == "2.16.840.1.113730.3.4.10") - { - // The control is a VlvResponseControl, as described in MS-ADTS 3.1.1.3.4.1.17 - ResultCode result; - byte[] context = null; + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); + ThrowUnless(sequenceContentLength > 0); + asnSpan = asnSpan.Slice(sequenceContentOffset, sequenceContentLength); - AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); - Debug.Assert(sequenceContentLength > 0); - asnSpan = asnSpan.Slice(sequenceContentOffset); + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int position, out int bytesConsumed); + ThrowUnless(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int position, out int bytesConsumed); - Debug.Assert(asnReadSuccessful); - asnSpan = asnSpan.Slice(bytesConsumed); + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int count, out bytesConsumed); + ThrowUnless(asnReadSuccessful); + asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int count, out bytesConsumed); - Debug.Assert(asnReadSuccessful); - asnSpan = asnSpan.Slice(bytesConsumed); + result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out bytesConsumed); + asnSpan = asnSpan.Slice(bytesConsumed); - result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out bytesConsumed); - asnSpan = asnSpan.Slice(bytesConsumed); + asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out _, out int octetStringLength, out _); + if (asnReadSuccessful) + { + context = new byte[octetStringLength]; + asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, context, AsnEncodingRules.BER, out bytesConsumed, out _); + ThrowUnless(asnReadSuccessful && asnSpan.Length == bytesConsumed); + } - asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out _, out int octetStringLength, out _); - if (asnReadSuccessful) - { - context = new byte[octetStringLength]; - _ = AsnDecoder.TryReadOctetString(asnSpan, context, AsnEncodingRules.BER, out _, out _); + VlvResponseControl vlv = new VlvResponseControl(position, count, context, result, controls[i].IsCritical, value); + controls[i] = vlv; } - - VlvResponseControl vlv = new VlvResponseControl(position, count, context, result, controls[i].IsCritical, value); - controls[i] = vlv; } } + catch (AsnContentException asnEx) + { + throw new BerConversionException(SR.BerConversionError, asnEx); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void ThrowUnless(bool condition) + { + if (!condition) + { + throw new BerConversionException(); + } } } @@ -281,13 +318,14 @@ public AsqRequestControl(string attributeName) : this() public override byte[] GetValue() { int sizeEstimate = 4 + (AttributeName?.Length ?? 0); - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.18. * ASQRequestValue ::= SEQUENCE { * sourceAttribute OCTET STRING } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { if (!string.IsNullOrEmpty(AttributeName)) { @@ -304,6 +342,8 @@ public override byte[] GetValue() } } _directoryControlValue = writer.Encode(); + writer.Reset(); + return base.GetValue(); } } @@ -384,18 +424,19 @@ public ExtendedDNFlag Flag } public override byte[] GetValue() { - int sizeEstimate = 8; - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: 8); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.5. * ExtendedDNRequestValue ::= SEQUENCE { * Flag INTEGER } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { writer.WriteInteger((int)Flag); } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -431,18 +472,19 @@ public SecurityDescriptorFlagControl(SecurityMasks masks) : this() public override byte[] GetValue() { - int sizeEstimate = 8; - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: 8); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.11. * SDFlagsRequestValue ::= SEQUENCE { * Flags INTEGER } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { writer.WriteInteger((int)SecurityMasks); } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -472,18 +514,19 @@ public SearchOption SearchOption public override byte[] GetValue() { - int sizeEstimate = 8; - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: 8); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.12. * SearchOptionsRequestValue ::= SEQUENCE { * Flags INTEGER } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { writer.WriteInteger((int)SearchOption); } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -527,14 +570,15 @@ public string ServerName public override byte[] GetValue() { int sizeEstimate = 10 + 2 * (ServerName?.Length ?? 0); - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.16. * VerifyNameRequestValue ::= SEQUENCE { * Flags INTEGER, * ServerName OCTET STRING } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { writer.WriteInteger(Flag); @@ -554,6 +598,7 @@ public override byte[] GetValue() } } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -615,21 +660,23 @@ public int AttributeCount public override byte[] GetValue() { int sizeEstimate = 16 + (_dirsyncCookie?.Length ?? 0); - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.3. * DirSyncRequestValue ::= SEQUENCE { * Flags INTEGER, * MaxBytes INTEGER, * Cookie OCTET STRING } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { writer.WriteInteger((int)Option); writer.WriteInteger(AttributeCount); writer.WriteOctetString(_dirsyncCookie ?? []); } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -712,19 +759,21 @@ public byte[] Cookie public override byte[] GetValue() { int sizeEstimate = 6 + (_pageCookie?.Length ?? 1); - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + writer.Reset(); /* This is as laid out in RFC2696. * realSearchControlValue ::= SEQUENCE { * size INTEGER, * cookie OCTET STRING } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { writer.WriteInteger(PageSize); writer.WriteOctetString(_pageCookie); } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -839,15 +888,16 @@ public SortKey[] SortKeys public override byte[] GetValue() { int sizeEstimate = 12 + _keysAsnLength; - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + writer.Reset(); /* This is as laid out in RFC2891. * SortKeyList ::= SEQUENCE OF SEQUENCE { * attributeType AttributeDescription, * orderingRule [0] MatchingRuleId OPTIONAL, * reverseOrder [1] BOOLEAN DEFAULT FALSE } * */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { // This scratch space is used for storing attribute names and matching rule OIDs. // Active Directory's valid matching rule OIDs are listed in MS-ADTS 3.1.1.3.4.1.13, @@ -859,7 +909,7 @@ public override byte[] GetValue() { SortKey key = _keys[i]; - using (AsnWriter.Scope keySequence = writer.PushSequence()) + using (writer.PushSequence()) { if (!string.IsNullOrEmpty(key.AttributeName)) { @@ -891,6 +941,7 @@ public override byte[] GetValue() } } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -1034,8 +1085,9 @@ public byte[] ContextId public override byte[] GetValue() { int sizeEstimate = 16 + (_target?.Length ?? 12) + (_context?.Length ?? 1); - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.17. * VLVRequestValue ::= SEQUENCE { * beforeCount INTEGER, @@ -1047,7 +1099,7 @@ public override byte[] GetValue() * greaterThanOrEqual [1] AssertionValue }, * contextID OCTET STRING OPTIONAL } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { // first encode the before and the after count. writer.WriteInteger(BeforeCount); @@ -1075,6 +1127,7 @@ public override byte[] GetValue() } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } @@ -1127,17 +1180,19 @@ public QuotaControl(SecurityIdentifier querySid) : this() public override byte[] GetValue() { int sizeEstimate = 4 + (_sid?.Length ?? 0); - AsnWriter writer = new(AsnEncodingRules.BER, sizeEstimate); + AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.19. * QuotaRequestValue ::= SEQUENCE { * querySID OCTET STRING } */ - using (AsnWriter.Scope outerSequence = writer.PushSequence()) + using (writer.PushSequence()) { writer.WriteOctetString(_sid); } _directoryControlValue = writer.Encode(); + writer.Reset(); return base.GetValue(); } From e12d44712112f24d795c0e8913c45b1b0895204b Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 25 May 2024 09:39:30 +0100 Subject: [PATCH 11/13] Remainder of code review feedback DirectoryControl payload parsing is stricter, throwing exception when invalid sequences are discovered instead of optional OCTET STRINGs. --- .../Protocols/common/DirectoryControl.cs | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index 5749f12c16975..2140f0d10c302 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -178,15 +178,15 @@ internal static void TransformControls(DirectoryControl[] controls) ThrowUnless(asnReadSuccessful); asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out int _, out int cookieLength, out _); - if (asnReadSuccessful) + // If present, the remaining bytes in the control are expected to be an octet string. + if (!asnSpan.IsEmpty) { - cookie = new byte[cookieLength]; - - // user expects cookie with length 0 as paged search is done. - asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, cookie, AsnEncodingRules.BER, out bytesConsumed, out _); - ThrowUnless(asnReadSuccessful && asnSpan.Length == bytesConsumed); + // user expects cookie with length 0 as paged search is done. In this situation, there'll still be two bytes + // for the ASN.1 tag. + cookie = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); + asnSpan = asnSpan.Slice(bytesConsumed); } + ThrowUnless(asnSpan.IsEmpty); PageResultResponseControl pageControl = new PageResultResponseControl(size, cookie, controls[i].IsCritical, value); controls[i] = pageControl; @@ -238,17 +238,29 @@ internal static void TransformControls(DirectoryControl[] controls) result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out int bytesConsumed); asnSpan = asnSpan.Slice(bytesConsumed); - // Attribute name is optional: AD for example never returns attribute name - asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out _, out int octetStringLength, out _); - if (asnReadSuccessful) + // If present, the remaining bytes in the control are expected to be an octet string. + if (!asnSpan.IsEmpty) { - Span attributeNameBuffer = octetStringLength <= AttributeNameStackAllocationThreshold ? attributeNameScratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; + // Attribute name is optional: AD for example never returns attribute name + scoped Span attributeNameBuffer; + + if (asnSpan.Length <= AttributeNameStackAllocationThreshold) + { + asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, attributeNameScratchSpace, AsnEncodingRules.BER, out bytesConsumed, out int octetStringLength); + Debug.Assert(asnReadSuccessful); + attributeNameBuffer = attributeNameScratchSpace.Slice(0, octetStringLength); + } + else + { + attributeNameBuffer = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); + } + asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, attributeNameBuffer, AsnEncodingRules.BER, out bytesConsumed, out _); - ThrowUnless(asnReadSuccessful && asnSpan.Length == bytesConsumed); attribute = s_utf8Encoding.GetString(attributeNameBuffer); } + ThrowUnless(asnSpan.IsEmpty); + SortResponseControl sort = new SortResponseControl(result, attribute, controls[i].IsCritical, value); controls[i] = sort; } @@ -273,13 +285,15 @@ internal static void TransformControls(DirectoryControl[] controls) result = AsnDecoder.ReadEnumeratedValue(asnSpan, AsnEncodingRules.BER, out bytesConsumed); asnSpan = asnSpan.Slice(bytesConsumed); - asnReadSuccessful = AsnDecoder.TryReadEncodedValue(asnSpan, AsnEncodingRules.BER, out _, out _, out int octetStringLength, out _); - if (asnReadSuccessful) + // If present, the remaining bytes in the control are expected to be an octet string. + if (!asnSpan.IsEmpty) { - context = new byte[octetStringLength]; - asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, context, AsnEncodingRules.BER, out bytesConsumed, out _); - ThrowUnless(asnReadSuccessful && asnSpan.Length == bytesConsumed); + // user expects cookie with length 0 as paged search is done. In this situation, there'll still be two bytes + // for the ASN.1 tag. + context = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); + asnSpan = asnSpan.Slice(bytesConsumed); } + ThrowUnless(asnSpan.IsEmpty); VlvResponseControl vlv = new VlvResponseControl(position, count, context, result, controls[i].IsCritical, value); controls[i] = vlv; From 57fc5f628a087582cae573577b69beb7b0606f46 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 24 Jul 2024 22:23:15 +0100 Subject: [PATCH 12/13] First response to code review feedback * Ensured that all references to DirectoryControl names in comments are accurate. * Specified the name of the ASN.1 structure which is being parsed in the comments. * Added explanatory comment stating that BER INTEGER values are being interpreted as 32-bit signed integers. --- .../Protocols/common/DirectoryControl.cs | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index 2140f0d10c302..9c18bda76dfbd 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -89,6 +89,11 @@ public bool ReverseOrder } } + // The encoding and decoding of LDAP directory controls interprets BER INTEGER values as 32-bit signed integers. + // Although BER INTEGERS can exceed this data type's maximum value, previous versions of DirectoryControl (and + // its derived classes) encoded and decoded these types by passing the "i" format specifier to BerConverter. The + // .NET Framework continues to do so. There is therefore historical precedent to justify limiting BER INTEGER + // values to this data type when using AsnDecoder and AsnWriter. public class DirectoryControl { // Scratch buffer allocations with sizes which are below this threshold should be made on the stack. @@ -167,8 +172,8 @@ internal static void TransformControls(DirectoryControl[] controls) if (controls[i].Type == "1.2.840.113556.1.4.319") { - // The control is a PageControl, as described in RFC 2696. - byte[] cookie = Array.Empty(); + // The control is a PageResultResponseControl. The structure of its value is described as a realSearchControlValue structure in RFC 2696. + byte[] cookie; AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); ThrowUnless(sequenceContentLength > 0); @@ -178,14 +183,10 @@ internal static void TransformControls(DirectoryControl[] controls) ThrowUnless(asnReadSuccessful); asnSpan = asnSpan.Slice(bytesConsumed); - // If present, the remaining bytes in the control are expected to be an octet string. - if (!asnSpan.IsEmpty) - { - // user expects cookie with length 0 as paged search is done. In this situation, there'll still be two bytes - // for the ASN.1 tag. - cookie = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); - asnSpan = asnSpan.Slice(bytesConsumed); - } + // The remaining bytes in the control are expected to be the cookie (an octet string.) + // A cookie with length 0 will be sent when paged search is done. In this situation, the ASN.1 tag will still consume two bytes. + cookie = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); + asnSpan = asnSpan.Slice(bytesConsumed); ThrowUnless(asnSpan.IsEmpty); PageResultResponseControl pageControl = new PageResultResponseControl(size, cookie, controls[i].IsCritical, value); @@ -193,25 +194,27 @@ internal static void TransformControls(DirectoryControl[] controls) } else if (controls[i].Type == "1.2.840.113556.1.4.1504") { - // The control is an AsqControl, as described in MS-ADTS section 3.1.1.3.4.1.18 + // The control is an AsqResponseControl. The structure of its value is described as an ASQResponseValue in MS-ADTS section 3.1.1.3.4.1.18. + ResultCode result; + AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); ThrowUnless(sequenceContentLength > 0); - ResultCode result = AsnDecoder.ReadEnumeratedValue(asnSpan.Slice(sequenceContentOffset, sequenceContentLength), AsnEncodingRules.BER, out _); + result = AsnDecoder.ReadEnumeratedValue(asnSpan.Slice(sequenceContentOffset, sequenceContentLength), AsnEncodingRules.BER, out _); AsqResponseControl asq = new AsqResponseControl(result, controls[i].IsCritical, value); controls[i] = asq; } else if (controls[i].Type == "1.2.840.113556.1.4.841") { - // The control is a DirSyncControl, as described in MS-ADTS section 3.1.1.3.4.1.3 + // The control is a DirSyncResponseControl. The structure of its value is described as a DirSyncResponseValue in MS-ADTS section 3.1.1.3.4.1.3. byte[] dirsyncCookie; AsnDecoder.ReadSequence(asnSpan, AsnEncodingRules.BER, out int sequenceContentOffset, out int sequenceContentLength, out _); ThrowUnless(sequenceContentLength > 0); asnSpan = asnSpan.Slice(sequenceContentOffset, sequenceContentLength); - asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int moreData, out int bytesConsumed); + asnReadSuccessful = AsnDecoder.TryReadInt32(asnSpan, AsnEncodingRules.BER, out int moreResults, out int bytesConsumed); ThrowUnless(asnReadSuccessful); asnSpan = asnSpan.Slice(bytesConsumed); @@ -222,12 +225,12 @@ internal static void TransformControls(DirectoryControl[] controls) dirsyncCookie = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); ThrowUnless(asnSpan.Length == bytesConsumed); - DirSyncResponseControl dirsync = new DirSyncResponseControl(dirsyncCookie, moreData != 0, count, controls[i].IsCritical, value); + DirSyncResponseControl dirsync = new DirSyncResponseControl(dirsyncCookie, moreResults != 0, count, controls[i].IsCritical, value); controls[i] = dirsync; } else if (controls[i].Type == "1.2.840.113556.1.4.474") { - // The control is a SortControl, as described in RFC 2891. + // The control is a SortResponseControl. The structure of its value is described as a SortResult in RFC 2891. ResultCode result; string attribute = null; @@ -266,7 +269,7 @@ internal static void TransformControls(DirectoryControl[] controls) } else if (controls[i].Type == "2.16.840.1.113730.3.4.10") { - // The control is a VlvResponseControl, as described in MS-ADTS 3.1.1.3.4.1.17 + // The control is a VlvResponseControl. The structure of its value is described as a VLVResponseValue in MS-ADTS 3.1.1.3.4.1.17. ResultCode result; byte[] context = null; @@ -288,7 +291,7 @@ internal static void TransformControls(DirectoryControl[] controls) // If present, the remaining bytes in the control are expected to be an octet string. if (!asnSpan.IsEmpty) { - // user expects cookie with length 0 as paged search is done. In this situation, there'll still be two bytes + // The user expects cookie with length 0 as paged search is done. In this situation, there'll still be two bytes // for the ASN.1 tag. context = AsnDecoder.ReadOctetString(asnSpan, AsnEncodingRules.BER, out bytesConsumed); asnSpan = asnSpan.Slice(bytesConsumed); @@ -1126,7 +1129,7 @@ public override byte[] GetValue() } else { - using (AsnWriter.Scope innerSequence = writer.PushSequence(s_byOffsetChoiceTag)) + using (writer.PushSequence(s_byOffsetChoiceTag)) { writer.WriteInteger(Offset); writer.WriteInteger(EstimateCount); From f1aca424bdb1316de5ddb1059c7aa03e9fd75349 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 26 Jul 2024 20:27:49 +0100 Subject: [PATCH 13/13] Further code review changes * Refactored LDAP string writing logic into a separate WriteLdapString method in AsnWriterExtensions. * Replaced separate AsnWriter instances with a single one (reduced memory usage when compared to multiple instances being active, even with varying buffer sizes.) * Resolved backwards-compatibility issue when attempting to process an attribute name which contains invalid UTF-8 characters. * Correcting expected ASN tag when parsing an attribute name in a sort result. --- .../System.DirectoryServices.Protocols.csproj | 1 + .../Protocols/common/AsnWriterExtensions.cs | 36 +++++ .../Protocols/common/DirectoryControl.cs | 132 +++++------------- 3 files changed, 70 insertions(+), 99 deletions(-) create mode 100644 src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/AsnWriterExtensions.cs diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj b/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj index ff1aba7d99d73..a14b7fa4d14b8 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj +++ b/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj @@ -26,6 +26,7 @@ + diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/AsnWriterExtensions.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/AsnWriterExtensions.cs new file mode 100644 index 0000000000000..4891d9e587513 --- /dev/null +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/AsnWriterExtensions.cs @@ -0,0 +1,36 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Formats.Asn1; +using System.Text; + +namespace System.DirectoryServices.Protocols +{ + internal static class AsnWriterExtensions + { + public static void WriteLdapString(this AsnWriter writer, string value, Encoding stringEncoding, bool mandatory = true, Asn1Tag? tag = null) + { + // A typical stack allocation threshold would be 256 bytes. A higher threshold has been chosen because an LdapString can be + // used to serialize server names. A server name is defined by RF1035, which specifies that a label in a domain name should + // be < 64 characters. If a server name is specified as an FQDN, this will be at least three labels in an AD environment - + // up to 192 characters. Doubling this to allow for Unicode encoding, then rounding to the nearest power of two yields 512. + const int StackAllocationThreshold = 512; + + if (!string.IsNullOrEmpty(value)) + { + int octetStringLength = stringEncoding.GetByteCount(value); + // Allocate space on the stack. There's a modest codegen advantage to a constant-value stackalloc. + Span tmpValue = octetStringLength <= StackAllocationThreshold + ? stackalloc byte[StackAllocationThreshold].Slice(0, octetStringLength) + : new byte[octetStringLength]; + + stringEncoding.GetBytes(value, tmpValue); + writer.WriteOctetString(tmpValue, tag); + } + else if (mandatory) + { + writer.WriteOctetString([], tag); + } + } + } +} diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs index 9c18bda76dfbd..b44a7e73a670e 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/DirectoryControl.cs @@ -4,6 +4,7 @@ using System.Collections; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Formats.Asn1; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -96,12 +97,6 @@ public bool ReverseOrder // values to this data type when using AsnDecoder and AsnWriter. public class DirectoryControl { - // Scratch buffer allocations with sizes which are below this threshold should be made on the stack. - // This is partially based on RFC1035, which specifies that a label in a domain name should be < 64 characters. - // If a server name is specified as an FQDN, this will be at least three labels in an AD environment - up to - // 192 characters. Doubling this to allow for Unicode encoding, then rounding to the nearest power of two - // yields 512. - internal const int ServerNameStackAllocationThreshold = 512; // Scratch buffer allocations with sizes which are below this threshold should be made on the stack. // This is based on the Active Directory schema. The largest attribute name here is msDS-FailedInteractiveLogonCountAtLastSuccessfulLogon, // which is 53 characters long. This is rounded up to the nearest power of two. @@ -112,20 +107,11 @@ public class DirectoryControl internal byte[] _directoryControlValue; [ThreadStatic] - private static AsnWriter t_smallWriter; - [ThreadStatic] - private static AsnWriter t_mediumWriter; - [ThreadStatic] - private static AsnWriter t_largeWriter; + private static AsnWriter? t_writer; - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static AsnWriter GetWriter(int expectedSize) - => expectedSize switch - { - > 0 and <= 32 => t_smallWriter ??= new AsnWriter(AsnEncodingRules.BER, 32), - > 32 and <= 128 => t_mediumWriter ??= new AsnWriter(AsnEncodingRules.BER, 128), - _ => t_largeWriter ??= new AsnWriter(AsnEncodingRules.BER, 256) - }; + [MemberNotNull(nameof(t_writer))] + internal static AsnWriter GetWriter() + => t_writer ??= new AsnWriter(AsnEncodingRules.BER); public DirectoryControl(string type, byte[] value, bool isCritical, bool serverSide) { @@ -249,7 +235,8 @@ internal static void TransformControls(DirectoryControl[] controls) if (asnSpan.Length <= AttributeNameStackAllocationThreshold) { - asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, attributeNameScratchSpace, AsnEncodingRules.BER, out bytesConsumed, out int octetStringLength); + asnReadSuccessful = AsnDecoder.TryReadOctetString(asnSpan, attributeNameScratchSpace, + AsnEncodingRules.BER, out bytesConsumed, out int octetStringLength, SortResponseControl.AttributeNameTag); Debug.Assert(asnReadSuccessful); attributeNameBuffer = attributeNameScratchSpace.Slice(0, octetStringLength); } @@ -307,6 +294,10 @@ internal static void TransformControls(DirectoryControl[] controls) { throw new BerConversionException(SR.BerConversionError, asnEx); } + catch (DecoderFallbackException decEx) + { + throw new BerConversionException(SR.BerConversionError, decEx); + } } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -334,8 +325,7 @@ public AsqRequestControl(string attributeName) : this() public override byte[] GetValue() { - int sizeEstimate = 4 + (AttributeName?.Length ?? 0); - AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.18. @@ -344,19 +334,7 @@ public override byte[] GetValue() */ using (writer.PushSequence()) { - if (!string.IsNullOrEmpty(AttributeName)) - { - int octetStringLength = s_utf8Encoding.GetByteCount(AttributeName); - // This trades slightly increased stack usage for the improved codegen which comes from a constant value. - Span tmpValue = octetStringLength <= AttributeNameStackAllocationThreshold ? stackalloc byte[AttributeNameStackAllocationThreshold].Slice(0, octetStringLength) : new byte[octetStringLength]; - - s_utf8Encoding.GetBytes(AttributeName, tmpValue); - writer.WriteOctetString(tmpValue); - } - else - { - writer.WriteOctetString([]); - } + writer.WriteLdapString(AttributeName, s_utf8Encoding); } _directoryControlValue = writer.Encode(); writer.Reset(); @@ -434,14 +412,16 @@ public ExtendedDNFlag Flag set { if (value < ExtendedDNFlag.HexString || value > ExtendedDNFlag.StandardString) + { throw new InvalidEnumArgumentException(nameof(value), (int)value, typeof(ExtendedDNFlag)); + } _flag = value; } } public override byte[] GetValue() { - AsnWriter writer = GetWriter(expectedSize: 8); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.5. @@ -489,7 +469,7 @@ public SecurityDescriptorFlagControl(SecurityMasks masks) : this() public override byte[] GetValue() { - AsnWriter writer = GetWriter(expectedSize: 8); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.11. @@ -523,7 +503,9 @@ public SearchOption SearchOption set { if (value < SearchOption.DomainScope || value > SearchOption.PhantomRoot) + { throw new InvalidEnumArgumentException(nameof(value), (int)value, typeof(SearchOption)); + } _searchOption = value; } @@ -531,7 +513,7 @@ public SearchOption SearchOption public override byte[] GetValue() { - AsnWriter writer = GetWriter(expectedSize: 8); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.12. @@ -586,8 +568,7 @@ public string ServerName public override byte[] GetValue() { - int sizeEstimate = 10 + 2 * (ServerName?.Length ?? 0); - AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.16. @@ -599,20 +580,7 @@ public override byte[] GetValue() { writer.WriteInteger(Flag); - if (!string.IsNullOrEmpty(ServerName)) - { - int serverNameLength = Encoding.Unicode.GetByteCount(ServerName); - // This differs from AsqRequest - it doesn't allocate ServerNameStackAllocationThreshold and provide a slice into it, because the size of this - // constant is such that the larger stack allocation would outweigh the benefit of a constant-value stackalloc. - Span tmpValue = serverNameLength <= ServerNameStackAllocationThreshold ? stackalloc byte[serverNameLength] : new byte[serverNameLength]; - - Encoding.Unicode.GetBytes(ServerName, tmpValue); - writer.WriteOctetString(tmpValue); - } - else - { - writer.WriteOctetString([]); - } + writer.WriteLdapString(ServerName, Encoding.Unicode); } _directoryControlValue = writer.Encode(); writer.Reset(); @@ -676,8 +644,7 @@ public int AttributeCount public override byte[] GetValue() { - int sizeEstimate = 16 + (_dirsyncCookie?.Length ?? 0); - AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.3. @@ -775,8 +742,7 @@ public byte[] Cookie public override byte[] GetValue() { - int sizeEstimate = 6 + (_pageCookie?.Length ?? 1); - AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in RFC2696. @@ -824,10 +790,9 @@ public byte[] Cookie public class SortRequestControl : DirectoryControl { - private static readonly Asn1Tag s_orderingRuleTag = new(TagClass.ContextSpecific, 0, false); - private static readonly Asn1Tag s_reverseOrderTag = new(TagClass.ContextSpecific, 1, false); + private static readonly Asn1Tag s_orderingRuleTag = new(TagClass.ContextSpecific, 0); + private static readonly Asn1Tag s_reverseOrderTag = new(TagClass.ContextSpecific, 1); - private int _keysAsnLength; private SortKey[] _keys = Array.Empty(); public SortRequestControl(params SortKey[] sortKeys) : base("1.2.840.113556.1.4.473", null, true, true) { @@ -841,12 +806,10 @@ public SortRequestControl(params SortKey[] sortKeys) : base("1.2.840.113556.1.4. } } - _keysAsnLength = 0; _keys = new SortKey[sortKeys.Length]; for (int i = 0; i < sortKeys.Length; i++) { _keys[i] = new SortKey(sortKeys[i].AttributeName, sortKeys[i].MatchingRule, sortKeys[i].ReverseOrder); - _keysAsnLength += 13 + (sortKeys[i].AttributeName?.Length ?? 0) + (sortKeys[i].MatchingRule?.Length ?? 0); } } @@ -858,7 +821,6 @@ public SortRequestControl(string attributeName, string matchingRule, bool revers { SortKey key = new SortKey(attributeName, matchingRule, reverseOrder); _keys = new SortKey[] { key }; - _keysAsnLength = 13 + (attributeName?.Length ?? 0) + (matchingRule?.Length ?? 0); } public SortKey[] SortKeys @@ -892,20 +854,17 @@ public SortKey[] SortKeys } } - _keysAsnLength = 0; _keys = new SortKey[value.Length]; for (int i = 0; i < value.Length; i++) { _keys[i] = new SortKey(value[i].AttributeName, value[i].MatchingRule, value[i].ReverseOrder); - _keysAsnLength += 13 + (value[i].AttributeName?.Length ?? 0) + (value[i].MatchingRule?.Length ?? 0); } } } public override byte[] GetValue() { - int sizeEstimate = 12 + _keysAsnLength; - AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in RFC2891. @@ -916,39 +875,14 @@ public override byte[] GetValue() * */ using (writer.PushSequence()) { - // This scratch space is used for storing attribute names and matching rule OIDs. - // Active Directory's valid matching rule OIDs are listed in MS-ADTS 3.1.1.3.4.1.13, - // with a maximum length of 23 characters - within the attribute name stack allocation - // threshold. - Span scratchSpace = stackalloc byte[AttributeNameStackAllocationThreshold]; - for (int i = 0; i < _keys.Length; i++) { SortKey key = _keys[i]; using (writer.PushSequence()) { - if (!string.IsNullOrEmpty(key.AttributeName)) - { - int octetStringLength = s_utf8Encoding.GetByteCount(key.AttributeName); - Span tmpValue = octetStringLength <= AttributeNameStackAllocationThreshold ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; - - s_utf8Encoding.GetBytes(key.AttributeName, tmpValue); - writer.WriteOctetString(tmpValue); - } - else - { - writer.WriteOctetString([]); - } - - if (!string.IsNullOrEmpty(key.MatchingRule)) - { - int octetStringLength = s_utf8Encoding.GetByteCount(key.MatchingRule); - Span tmpValue = octetStringLength <= AttributeNameStackAllocationThreshold ? scratchSpace.Slice(0, octetStringLength) : new byte[octetStringLength]; - - s_utf8Encoding.GetBytes(key.MatchingRule, tmpValue); - writer.WriteOctetString(tmpValue, s_orderingRuleTag); - } + writer.WriteLdapString(key.AttributeName, s_utf8Encoding, mandatory: true); + writer.WriteLdapString(key.MatchingRule, s_utf8Encoding, mandatory: false, tag: s_orderingRuleTag); if (key.ReverseOrder) { @@ -966,6 +900,8 @@ public override byte[] GetValue() public class SortResponseControl : DirectoryControl { + internal static readonly Asn1Tag AttributeNameTag = new(TagClass.ContextSpecific, 0); + internal SortResponseControl(ResultCode result, string attributeName, bool critical, byte[] value) : base("1.2.840.113556.1.4.474", value, critical, true) { Result = result; @@ -1101,8 +1037,7 @@ public byte[] ContextId public override byte[] GetValue() { - int sizeEstimate = 16 + (_target?.Length ?? 12) + (_context?.Length ?? 1); - AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.17. @@ -1196,8 +1131,7 @@ public QuotaControl(SecurityIdentifier querySid) : this() public override byte[] GetValue() { - int sizeEstimate = 4 + (_sid?.Length ?? 0); - AsnWriter writer = GetWriter(expectedSize: sizeEstimate); + AsnWriter writer = GetWriter(); writer.Reset(); /* This is as laid out in MS-ADTS, 3.1.1.3.4.1.19.