Fix #41618 Correct marshalling of SortKey objects on Linux (#65548)

lscorcia · web-flow · commit b6c630448c8c · 2022-03-22T09:41:33.000-07:00
diff --git a/src/libraries/Common/tests/System/DirectoryServices/LDAP.Configuration.xml b/src/libraries/Common/tests/System/DirectoryServices/LDAP.Configuration.xml
@@ -94,6 +94,7 @@ Note:
         <User>cn=admin,dc=example,dc=com</User>
         <Password>password</Password>
         <AuthenticationTypes>ServerBind,None</AuthenticationTypes>
+        <SupportsServerSideSort>True</SupportsServerSideSort>
     </Connection>
     <Connection Name="SLAPD OPENLDAP SERVER">
         <ServerName>localhost</ServerName>
@@ -102,6 +103,7 @@ Note:
         <User>cn=admin,dc=example,dc=com</User>
         <Password>password</Password>
         <AuthenticationTypes>ServerBind,None</AuthenticationTypes>
+        <SupportsServerSideSort>False</SupportsServerSideSort>
     </Connection>
     <Connection Name="ACTIVE DIRECTORY SERVER">
         <ServerName>danmose-ldap.danmose-domain.com</ServerName>
@@ -110,6 +112,7 @@ Note:
         <User>danmose-domain\Administrator</User>
         <Password>%TESTPASSWORD%</Password>
         <AuthenticationTypes>ServerBind,None</AuthenticationTypes>
+        <SupportsServerSideSort>True</SupportsServerSideSort>
     </Connection>
     <Connection Name="SLAPD OPENLDAP SERVER TLS">
         <ServerName>ldap.local</ServerName>
@@ -119,6 +122,7 @@ Note:
         <Password>password</Password>
         <AuthenticationTypes>ServerBind,None</AuthenticationTypes>
         <UseTls>true</UseTls>
+        <SupportsServerSideSort>False</SupportsServerSideSort>
     </Connection>
 
 </Configuration>
diff --git a/src/libraries/Common/tests/System/DirectoryServices/LdapConfiguration.cs b/src/libraries/Common/tests/System/DirectoryServices/LdapConfiguration.cs
@@ -10,7 +10,7 @@ namespace System.DirectoryServices.Tests
 {
     internal class LdapConfiguration
     {
-        private LdapConfiguration(string serverName, string searchDn, string userName, string password, string port, AuthenticationTypes at, bool useTls)
+        private LdapConfiguration(string serverName, string searchDn, string userName, string password, string port, AuthenticationTypes at, bool useTls, bool supportsServerSideSort)
         {
             ServerName = serverName;
             SearchDn = searchDn;
@@ -19,6 +19,7 @@ private LdapConfiguration(string serverName, string searchDn, string userName, s
             Port = port;
             AuthenticationTypes = at;
             UseTls = useTls;
+            SupportsServerSideSort = supportsServerSideSort;
         }
 
         private static LdapConfiguration s_ldapConfiguration = GetConfiguration("LDAP.Configuration.xml");
@@ -32,6 +33,7 @@ private LdapConfiguration(string serverName, string searchDn, string userName, s
         internal string SearchDn { get; set; }
         internal AuthenticationTypes AuthenticationTypes { get; set; }
         internal bool UseTls { get; set; }
+        internal bool SupportsServerSideSort { get; set; }
         internal string LdapPath => string.IsNullOrEmpty(Port) ? $"LDAP://{ServerName}/{SearchDn}" : $"LDAP://{ServerName}:{Port}/{SearchDn}";
         internal string RootDSEPath => string.IsNullOrEmpty(Port) ? $"LDAP://{ServerName}/rootDSE" : $"LDAP://{ServerName}:{Port}/rootDSE";
         internal string UserNameWithNoDomain
@@ -107,6 +109,7 @@ internal static LdapConfiguration GetConfiguration(string configFile)
                 string password = "";
                 AuthenticationTypes at = AuthenticationTypes.None;
                 bool useTls = false;
+                bool supportsServerSideSort = false;
 
                 XElement child = connection.Element("ServerName");
                 if (child != null)
@@ -141,6 +144,12 @@ internal static LdapConfiguration GetConfiguration(string configFile)
                     useTls = bool.Parse(child.Value);
                 }
 
+                child = connection.Element("SupportsServerSideSort");
+                if (child != null)
+                {
+                    supportsServerSideSort = bool.Parse(child.Value);
+                }
+
                 child = connection.Element("AuthenticationTypes");
                 if (child != null)
                 {
@@ -170,7 +179,7 @@ internal static LdapConfiguration GetConfiguration(string configFile)
                             at |= AuthenticationTypes.Signing;
                     }
 
-                    ldapConfig = new LdapConfiguration(serverName, searchDn, user, password, port, at, useTls);
+                    ldapConfig = new LdapConfiguration(serverName, searchDn, user, password, port, at, useTls, supportsServerSideSort);
                 }
             }
             catch (Exception ex)
diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj b/src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj
@@ -42,6 +42,7 @@
     <Compile Include="System\DirectoryServices\Protocols\ldap\LdapPartialResultsProcessor.cs" />
     <Compile Include="System\DirectoryServices\Protocols\ldap\LdapSessionOptions.cs" />
     <Compile Include="System\DirectoryServices\Protocols\Interop\SafeHandles.cs" />
+    <Compile Include="System\DirectoryServices\Protocols\Interop\SortKeyInterop.cs" />
     <Compile Include="$(CommonPath)DisableRuntimeMarshalling.cs"
       Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">
       <Link>Common\DisableRuntimeMarshalling.cs</Link>
@@ -58,6 +59,7 @@
     <Compile Include="System\DirectoryServices\Protocols\common\QuotaControl.Windows.cs" />
     <Compile Include="System\DirectoryServices\Protocols\Interop\LdapPal.Windows.cs" />
     <Compile Include="System\DirectoryServices\Protocols\Interop\BerPal.Windows.cs" />
+    <Compile Include="System\DirectoryServices\Protocols\Interop\SortKeyInterop.Windows.cs" />
     <Compile Include="System\DirectoryServices\Protocols\ldap\LdapConnection.Windows.cs" />
     <Compile Include="System\DirectoryServices\Protocols\ldap\LdapSessionOptions.Windows.cs" />
     <Compile Include="System\DirectoryServices\Protocols\Interop\SafeHandles.Windows.cs" />
@@ -76,6 +78,7 @@
     <Compile Include="System\DirectoryServices\Protocols\common\QuotaControl.Linux.cs" />
     <Compile Include="System\DirectoryServices\Protocols\Interop\LdapPal.Linux.cs" />
     <Compile Include="System\DirectoryServices\Protocols\Interop\BerPal.Linux.cs" />
+    <Compile Include="System\DirectoryServices\Protocols\Interop\SortKeyInterop.Linux.cs" />
     <Compile Include="System\DirectoryServices\Protocols\ldap\LdapConnection.Linux.cs" />
     <Compile Include="System\DirectoryServices\Protocols\ldap\LdapSessionOptions.Linux.cs" />
     <Compile Include="System\DirectoryServices\Protocols\ldap\LocalAppContextSwitches.cs" />
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
@@ -0,0 +1,18 @@
+// 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.Text;
+using System.Runtime.InteropServices;
+using System.Security.Principal;
+using System.Runtime.Versioning;
+
+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
@@ -0,0 +1,18 @@
+// 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.Text;
+using System.Runtime.InteropServices;
+using System.Security.Principal;
+using System.Runtime.Versioning;
+
+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
@@ -0,0 +1,35 @@
+// 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.Text;
+using System.Runtime.InteropServices;
+using System.Security.Principal;
+using System.Runtime.Versioning;
+
+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
@@ -703,9 +703,15 @@ public SortKey[] SortKeys
 
         public override byte[] GetValue()
         {
+            SortKeyInterop[] nativeSortKeys = new SortKeyInterop[_keys.Length];
+            for (int i = 0; i < _keys.Length; ++i)
+            {
+                nativeSortKeys[i] = new SortKeyInterop(_keys[i]);
+            }
+
             IntPtr control = IntPtr.Zero;
-            int structSize = Marshal.SizeOf(typeof(SortKey));
-            int keyCount = _keys.Length;
+            int structSize = Marshal.SizeOf(typeof(SortKeyInterop));
+            int keyCount = nativeSortKeys.Length;
             IntPtr memHandle = Utility.AllocHGlobalIntPtrArray(keyCount + 1);
 
             try
@@ -716,7 +722,7 @@ public override byte[] GetValue()
                 for (i = 0; i < keyCount; i++)
                 {
                     sortPtr = Marshal.AllocHGlobal(structSize);
-                    Marshal.StructureToPtr(_keys[i], sortPtr, false);
+                    Marshal.StructureToPtr(nativeSortKeys[i], sortPtr, false);
                     tempPtr = (IntPtr)((long)memHandle + IntPtr.Size * i);
                     Marshal.WriteIntPtr(tempPtr, sortPtr);
                 }
diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs
@@ -15,6 +15,8 @@ public partial class DirectoryServicesProtocolsTests
         internal static bool IsLdapConfigurationExist => LdapConfiguration.Configuration != null;
         internal static bool IsActiveDirectoryServer => IsLdapConfigurationExist && LdapConfiguration.Configuration.IsActiveDirectoryServer;
 
+        internal static bool IsServerSideSortSupported => IsLdapConfigurationExist && LdapConfiguration.Configuration.SupportsServerSideSort;
+
         [ConditionalFact(nameof(IsLdapConfigurationExist))]
         public void TestInvalidFilter()
         {
@@ -533,6 +535,58 @@ public void TestPageRequests()
             }
         }
 
+        [ConditionalFact(nameof(IsServerSideSortSupported))]
+        public void TestSortedSearch()
+        {
+            using (LdapConnection connection = GetConnection())
+            {
+                string ouName = "ProtocolsGroup10";
+                string dn = "ou=" + ouName;
+
+                try
+                {
+                    for (int i=0; i<10; i++)
+                    {
+                        DeleteEntry(connection, "ou=ProtocolsSubGroup10." + i + "," + dn);
+                    }
+                    DeleteEntry(connection, dn);
+
+                    AddOrganizationalUnit(connection, dn);
+                    SearchResultEntry sre = SearchOrganizationalUnit(connection, LdapConfiguration.Configuration.SearchDn, ouName);
+                    Assert.NotNull(sre);
+
+                    for (int i=0; i<10; i++)
+                    {
+                        AddOrganizationalUnit(connection, "ou=ProtocolsSubGroup10." + i + "," + dn);
+                    }
+
+                    string filter = "(objectClass=*)";
+                    SearchRequest searchRequest = new SearchRequest(
+                                                        dn + "," + LdapConfiguration.Configuration.SearchDn,
+                                                        filter,
+                                                        SearchScope.Subtree,
+                                                        null);
+
+                    var sortRequestControl = new SortRequestControl("ou", true);
+                    searchRequest.Controls.Add(sortRequestControl);
+
+                    SearchResponse searchResponse = (SearchResponse) connection.SendRequest(searchRequest);
+                    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);
+                }
+                finally
+                {
+                    for (int i=0; i<20; i++)
+                    {
+                        DeleteEntry(connection, "ou=ProtocolsSubGroup10." + i + "," + dn);
+                    }
+                    DeleteEntry(connection, dn);
+                }
+            }
+        }
+        
         private void DeleteAttribute(LdapConnection connection, string entryDn, string attributeName)
         {
             string dn = entryDn + "," + LdapConfiguration.Configuration.SearchDn;
diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/SortRequestControlTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/SortRequestControlTests.cs
@@ -32,11 +32,15 @@ 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,
                 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} :
-                new byte[] { 48, 19, 48, 9, 4, 1, 110, 128, 1, 114, 129, 1, 255, 48, 6, 4, 1, 110, 128, 1, 114 };
+                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, 
+                114, 117, 108, 101, 49, 129, 1, 255, 48, 14, 4, 5, 110, 97, 109, 
+                101, 50, 128, 5, 114, 117, 108, 101, 50 };
             Assert.Equal(expected, control.GetValue());
         }
 

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ namespace System.DirectoryServices.Tests`
`10`	`10`	`{`
`11`	`11`	`internal class LdapConfiguration`
`12`	`12`	`{`
`13`		`- private LdapConfiguration(string serverName, string searchDn, string userName, string password, string port, AuthenticationTypes at, bool useTls)`
	`13`	`+ private LdapConfiguration(string serverName, string searchDn, string userName, string password, string port, AuthenticationTypes at, bool useTls, bool supportsServerSideSort)`
`14`	`14`	`{`
`15`	`15`	`ServerName = serverName;`
`16`	`16`	`SearchDn = searchDn;`
`@@ -19,6 +19,7 @@ private LdapConfiguration(string serverName, string searchDn, string userName, s`
`19`	`19`	`Port = port;`
`20`	`20`	`AuthenticationTypes = at;`
`21`	`21`	`UseTls = useTls;`
	`22`	`+ SupportsServerSideSort = supportsServerSideSort;`
`22`	`23`	`}`
`23`	`24`
`24`	`25`	`private static LdapConfiguration s_ldapConfiguration = GetConfiguration("LDAP.Configuration.xml");`
`@@ -32,6 +33,7 @@ private LdapConfiguration(string serverName, string searchDn, string userName, s`
`32`	`33`	`internal string SearchDn { get; set; }`
`33`	`34`	`internal AuthenticationTypes AuthenticationTypes { get; set; }`
`34`	`35`	`internal bool UseTls { get; set; }`
	`36`	`+ internal bool SupportsServerSideSort { get; set; }`
`35`	`37`	`internal string LdapPath => string.IsNullOrEmpty(Port) ? $"LDAP://{ServerName}/{SearchDn}" : $"LDAP://{ServerName}:{Port}/{SearchDn}";`
`36`	`38`	`internal string RootDSEPath => string.IsNullOrEmpty(Port) ? $"LDAP://{ServerName}/rootDSE" : $"LDAP://{ServerName}:{Port}/rootDSE";`
`37`	`39`	`internal string UserNameWithNoDomain`
`@@ -107,6 +109,7 @@ internal static LdapConfiguration GetConfiguration(string configFile)`
`107`	`109`	`string password = "";`
`108`	`110`	`AuthenticationTypes at = AuthenticationTypes.None;`
`109`	`111`	`bool useTls = false;`
	`112`	`+ bool supportsServerSideSort = false;`
`110`	`113`
`111`	`114`	`XElement child = connection.Element("ServerName");`
`112`	`115`	`if (child != null)`
`@@ -141,6 +144,12 @@ internal static LdapConfiguration GetConfiguration(string configFile)`
`141`	`144`	`useTls = bool.Parse(child.Value);`
`142`	`145`	`}`
`143`	`146`
	`147`	`+ child = connection.Element("SupportsServerSideSort");`
	`148`	`+ if (child != null)`
	`149`	`+ {`
	`150`	`+ supportsServerSideSort = bool.Parse(child.Value);`
	`151`	`+ }`
	`152`	`+`
`144`	`153`	`child = connection.Element("AuthenticationTypes");`
`145`	`154`	`if (child != null)`
`146`	`155`	`{`
`@@ -170,7 +179,7 @@ internal static LdapConfiguration GetConfiguration(string configFile)`
`170`	`179`	`at \|= AuthenticationTypes.Signing;`
`171`	`180`	`}`
`172`	`181`
`173`		`- ldapConfig = new LdapConfiguration(serverName, searchDn, user, password, port, at, useTls);`
	`182`	`+ ldapConfig = new LdapConfiguration(serverName, searchDn, user, password, port, at, useTls, supportsServerSideSort);`
`174`	`183`	`}`
`175`	`184`	`}`
`176`	`185`	`catch (Exception ex)`