Skip to content

Fix incorrect definition of LSA_FOREST_TRUST_RECORD in System.DirectoryServices #68274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elinor-fung merged 2 commits into from
Apr 21, 2022
Merged

Fix incorrect definition of LSA_FOREST_TRUST_RECORD in System.DirectoryServices #68274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64931e3
Fix incorrect definition of LSA_FOREST_TRUST_RECORD in System.Directo…
elinor-fung Apr 20, 2022
5c3739b
PR feedback
elinor-fung Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions ...rvices/src/System/DirectoryServices/ActiveDirectory/ForestTrustRelationshipInformation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ public void Save()
{
throw ExceptionHelper.GetExceptionFromErrorCode(Marshal.GetLastWin32Error());
}
record.DomainInfo = new LSA_FOREST_TRUST_DOMAIN_INFO();
record.DomainInfo.sid = pSid;
sidList.Add(pSid);
record.DomainInfo.DNSNameBuffer = Marshal.StringToHGlobalUni(tmp.DnsName);
Expand All @@ -214,7 +213,6 @@ public void Save()
record.Time = (LARGE_INTEGER)_binaryDataTime[i]!;
record.Data.Length = ((byte[])_binaryData[i]!).Length;
record.ForestTrustType = (LSA_FOREST_TRUST_RECORD_TYPE)_binaryRecordType[i]!;
record.Data = new LSA_FOREST_TRUST_BINARY_DATA();
if (record.Data.Length == 0)
{
record.Data.Buffer = (IntPtr)0;
Expand Down Expand Up @@ -317,7 +315,7 @@ public void Save()
catch { throw; }
}

private void GetForestTrustInfoHelper()
private unsafe void GetForestTrustInfoHelper()
{
IntPtr forestTrustInfo = (IntPtr)0;
SafeLsaPolicyHandle? handle = null;
Expand Down Expand Up @@ -381,30 +379,30 @@ private void GetForestTrustInfoHelper()
if (record.ForestTrustType == LSA_FOREST_TRUST_RECORD_TYPE.ForestTrustTopLevelName)
{
IntPtr myPtr = IntPtr.Add(addr, 16);
Marshal.PtrToStructure(myPtr, record.TopLevelName);
record.TopLevelName = *(global::Interop.UNICODE_STRING*)myPtr;
TopLevelName TLN = new TopLevelName(record.Flags, record.TopLevelName, record.Time);
tmpTLNs.Add(TLN);
}
else if (record.ForestTrustType == LSA_FOREST_TRUST_RECORD_TYPE.ForestTrustTopLevelNameEx)
{
// get the excluded TLN and put it in our collection
IntPtr myPtr = IntPtr.Add(addr, 16);
Marshal.PtrToStructure(myPtr, record.TopLevelName);
record.TopLevelName = *(global::Interop.UNICODE_STRING*)myPtr;
string excludedName = Marshal.PtrToStringUni(record.TopLevelName.Buffer, record.TopLevelName.Length / 2);
tmpExcludedTLNs.Add(excludedName);
tmpExcludedNameTime.Add(excludedName, record.Time);
}
else if (record.ForestTrustType == LSA_FOREST_TRUST_RECORD_TYPE.ForestTrustDomainInfo)
{
IntPtr myPtr = IntPtr.Add(addr, 16);
Marshal.PtrToStructure(myPtr, record.DomainInfo!);
record.DomainInfo = *(LSA_FOREST_TRUST_DOMAIN_INFO*)myPtr;
ForestTrustDomainInformation dom = new ForestTrustDomainInformation(record.Flags, record.DomainInfo!, record.Time);
tmpDomainInformation.Add(dom);
}
else
{
IntPtr myPtr = IntPtr.Add(addr, 16);
Marshal.PtrToStructure(myPtr, record.Data);
record.Data = *(LSA_FOREST_TRUST_BINARY_DATA*)myPtr;
int length = record.Data.Length;
byte[] byteArray = new byte[length];
if ((record.Data.Buffer != (IntPtr)0) && (length != 0))
Expand Down
10 changes: 4 additions & 6 deletions ...tem.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/UnsafeNativeMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,9 @@ internal sealed class LSA_FOREST_TRUST_RECORD
[FieldOffset(16)]
public global::Interop.UNICODE_STRING TopLevelName;
[FieldOffset(16)]
public LSA_FOREST_TRUST_BINARY_DATA Data = null!;
public LSA_FOREST_TRUST_BINARY_DATA Data;
[FieldOffset(16)]
public LSA_FOREST_TRUST_DOMAIN_INFO? DomainInfo;
public LSA_FOREST_TRUST_DOMAIN_INFO DomainInfo;
}

[StructLayout(LayoutKind.Sequential)]
Expand All @@ -432,8 +432,7 @@ public LARGE_INTEGER()
}
}

[StructLayout(LayoutKind.Sequential)]
internal sealed class LSA_FOREST_TRUST_DOMAIN_INFO
internal struct LSA_FOREST_TRUST_DOMAIN_INFO
{
public IntPtr sid;
public short DNSNameLength;
Expand All @@ -444,8 +443,7 @@ internal sealed class LSA_FOREST_TRUST_DOMAIN_INFO
public IntPtr NetBIOSNameBuffer;
}

[StructLayout(LayoutKind.Sequential)]
internal sealed class LSA_FOREST_TRUST_BINARY_DATA
internal struct LSA_FOREST_TRUST_BINARY_DATA
{
public int Length;
public IntPtr Buffer;
Expand Down
8 changes: 8 additions & 0 deletions ...braries/System.DirectoryServices/tests/System/DirectoryServices/DirectoryServicesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections;
using Xunit;
using Xunit.Sdk;
using System.Reflection;

namespace System.DirectoryServices.Tests
{
Expand All @@ -14,6 +15,13 @@ public partial class DirectoryServicesTests
internal static bool IsLdapConfigurationExist => LdapConfiguration.Configuration != null;
internal static bool IsActiveDirectoryServer => IsLdapConfigurationExist && LdapConfiguration.Configuration.IsActiveDirectoryServer;

[Fact]
public void TestGetAllTypes()
{
Type[] allTypes = typeof(DirectoryEntry).Assembly.GetTypes();
Copy link
Member

@danmoseley danmoseley Apr 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it would be possible (later) to write a test that did this for each of our implementation assemblies, without actually pasting in the test into every one.

Something like - a common file that was included in all test assemblies, with a test that enumerated all loaded assemblies and did GetTypes on them.

Copy link
Member Author

@elinor-fung elinor-fung Apr 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #68341 to track trying to do something like this.

Assert.Contains(typeof(DirectoryEntry), allTypes);
}

[ConditionalFact(nameof(IsLdapConfigurationExist))]
public void TestOU() // adding and removing organization unit
{
Expand Down