Skip to content
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

Fix ber scanf/printf. #51205

Merged
merged 5 commits into from
Apr 28, 2021
Merged

Fix ber scanf/printf. #51205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3215d25
Reenable the tests.
Apr 1, 2021
552d9dd
Use __arglist for windows.
Apr 13, 2021
46908e6
Use ber_put_*/ber_get_* for Unix
Apr 14, 2021
0711327
fix tag
Apr 16, 2021
f3fd828
fix review.
Apr 19, 2021
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
194 changes: 178 additions & 16 deletions src/libraries/Common/src/Interop/Linux/OpenLdap/Interop.Ber.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
using System;
using System.Runtime.InteropServices;
using System.DirectoryServices.Protocols;
using System.Diagnostics;

internal static partial class Interop
{
internal static partial class Ldap
{
private const int ber_default_successful_return_code = 0;

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_alloc_t", CharSet = CharSet.Ansi)]
public static extern IntPtr ber_alloc(int option);

Expand All @@ -18,17 +21,105 @@ internal static partial class Ldap
[DllImport(Libraries.OpenLdap, EntryPoint = "ber_free", CharSet = CharSet.Ansi)]
public static extern IntPtr ber_free([In] IntPtr berelement, int option);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_printf", CharSet = CharSet.Ansi)]
public static extern int ber_printf_emptyarg(SafeBerHandle berElement, string format);
public static int ber_printf_emptyarg(SafeBerHandle berElement, string format, int tag)
{
if (format == "{")
{
return ber_start_seq(berElement, tag);
}
else if (format == "}")
{
return ber_put_seq(berElement, tag);
}
else if (format == "[")
{
return ber_start_set(berElement, tag);
}
else if (format == "]")
{
return ber_put_set(berElement, tag);
}
else
{
Debug.Assert(format == "n");
return ber_put_null(berElement, tag);
}
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_start_seq", CharSet = CharSet.Ansi)]
public static extern int ber_start_seq(SafeBerHandle berElement, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_start_set", CharSet = CharSet.Ansi)]
public static extern int ber_start_set(SafeBerHandle berElement, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_seq", CharSet = CharSet.Ansi)]
public static extern int ber_put_seq(SafeBerHandle berElement, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_set", CharSet = CharSet.Ansi)]
public static extern int ber_put_set(SafeBerHandle berElement, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_null", CharSet = CharSet.Ansi)]
public static extern int ber_put_null(SafeBerHandle berElement, int tag);


public static int ber_printf_tag(SafeBerHandle berElement, string format, int tag)
{
// Ber Linux tags are passed with the values that they affect, like `ber_printf_int(.., tag)`.
// So this function does nothing on Linux.
return ber_default_successful_return_code;
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a comment here saying why we are not supporting this? Or I wonder if for this scenario we should falk back to use the varargs func instead so that it works as expected on platforms where they are supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a comment:
// Ber Linux tags are passed with the values that they affect, so this function does nothing.

}

public static int ber_printf_int(SafeBerHandle berElement, string format, int value, int tag)
{
if (format == "i")
joperezr marked this conversation as resolved.
Show resolved Hide resolved
{
return ber_put_int(berElement, value, tag);
}
else if (format == "e")
{
return ber_put_enum(berElement, value, tag);
}
else
{
Debug.Assert(format == "b");
return ber_put_boolean(berElement, value, tag);
}
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_int", CharSet = CharSet.Ansi)]
public static extern int ber_put_int(SafeBerHandle berElement, int value, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_enum", CharSet = CharSet.Ansi)]
public static extern int ber_put_enum(SafeBerHandle berElement, int value, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_printf", CharSet = CharSet.Ansi)]
public static extern int ber_printf_int(SafeBerHandle berElement, string format, int value);
[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_boolean", CharSet = CharSet.Ansi)]
public static extern int ber_put_boolean(SafeBerHandle berElement, int value, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_printf", CharSet = CharSet.Ansi)]
public static extern int ber_printf_bytearray(SafeBerHandle berElement, string format, HGlobalMemHandle value, int length);
public static int ber_printf_bytearray(SafeBerHandle berElement, string format, HGlobalMemHandle value, int length, int tag)
{
if (format == "o")
{
return ber_put_ostring(berElement, value, length, tag);
}
else if (format == "s")
{
return ber_put_string(berElement, value, tag);
}
else
{
Debug.Assert(format == "X");
return ber_put_bitstring(berElement, value, length, tag);
}
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_printf", CharSet = CharSet.Ansi)]
public static extern int ber_printf_berarray(SafeBerHandle berElement, string format, IntPtr value);
[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_ostring", CharSet = CharSet.Ansi)]
private static extern int ber_put_ostring(SafeBerHandle berElement, HGlobalMemHandle value, int length, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_string", CharSet = CharSet.Ansi)]
private static extern int ber_put_string(SafeBerHandle berElement, HGlobalMemHandle value, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_put_bitstring", CharSet = CharSet.Ansi)]
private static extern int ber_put_bitstring(SafeBerHandle berElement, HGlobalMemHandle value, int length, int tag);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_flatten", CharSet = CharSet.Ansi)]
public static extern int ber_flatten(SafeBerHandle berElement, ref IntPtr value);
Expand All @@ -39,16 +130,87 @@ internal static partial class Ldap
[DllImport(Libraries.OpenLdap, EntryPoint = "ber_bvecfree", CharSet = CharSet.Ansi)]
public static extern int ber_bvecfree(IntPtr value);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_scanf", CharSet = CharSet.Ansi)]
public static extern int ber_scanf(SafeBerHandle berElement, string format);
public static int ber_scanf_emptyarg(SafeBerHandle berElement, string format)
{
Debug.Assert(format == "{" || format == "}" || format == "[" || format == "]" || format == "n" || format == "x");
if (format == "{" || format == "[")
{
int len = 0;
return ber_skip_tag(berElement, ref len);
}
else if (format == "]" || format == "}")
{
return ber_default_successful_return_code;
}
else
{
Debug.Assert(format == "n" || format == "x");
return ber_get_null(berElement);
}
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_skip_tag", CharSet = CharSet.Ansi)]
private static extern int ber_skip_tag(SafeBerHandle berElement, ref int len);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_get_null", CharSet = CharSet.Ansi)]
private static extern int ber_get_null(SafeBerHandle berElement);

public static int ber_scanf_int(SafeBerHandle berElement, string format, ref int value)
{
if (format == "i")
{
return ber_get_int(berElement, ref value);
}
else if (format == "e")
{
return ber_get_enum(berElement, ref value);
}
else
{
Debug.Assert(format == "b");
return ber_get_boolean(berElement, ref value);
}
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_get_int", CharSet = CharSet.Ansi)]
private static extern int ber_get_int(SafeBerHandle berElement, ref int value);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_get_enum", CharSet = CharSet.Ansi)]
private static extern int ber_get_enum(SafeBerHandle berElement, ref int value);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_get_boolean", CharSet = CharSet.Ansi)]
private static extern int ber_get_boolean(SafeBerHandle berElement, ref int value);

public static int ber_scanf_bitstring(SafeBerHandle berElement, string format, ref IntPtr value, ref int bitLength)
{
Debug.Assert(format == "B");
return ber_get_stringb(berElement, ref value, ref bitLength);
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_get_stringb", CharSet = CharSet.Ansi)]
private static extern int ber_get_stringb(SafeBerHandle berElement, ref IntPtr value, ref int bitLength);

public static int ber_scanf_ptr(SafeBerHandle berElement, string format, ref IntPtr value)
{
Debug.Assert(format == "O");
return ber_get_stringal(berElement, ref value);
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_scanf", CharSet = CharSet.Ansi)]
public static extern int ber_scanf_int(SafeBerHandle berElement, string format, ref int value);
[DllImport(Libraries.OpenLdap, EntryPoint = "ber_get_stringal", CharSet = CharSet.Ansi)]
private static extern int ber_get_stringal(SafeBerHandle berElement, ref IntPtr value);

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_scanf", CharSet = CharSet.Ansi)]
public static extern int ber_scanf_bitstring(SafeBerHandle berElement, string format, ref IntPtr value, ref int bitLength);
public static int ber_printf_berarray(SafeBerHandle berElement, string format, IntPtr value, int tag)
{
Debug.Assert(format == "v" || format == "V");
Copy link
Member

Choose a reason for hiding this comment

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

Same here, could we just fall back to call the old printf in this case? I know it means that this won't work in the new Macs, but that means we won't regress existing runtimes in case support is added in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have supported 'v' on Linux (and changed the Windows version to be consistent) by converting one 'v' call to several 'o' calls:

runtime/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/common/BerConverter.cs

Line 121 in 0711327

error = EncodingByteArrayHelper(berElement, tempValue, 'o', tag);

it does not work for 'V' because I have not found an API that takes 'BerVal'.
The definition is:

V

Several octet strings. A null-terminated array of struct berval *'s is supplied. Note that a construct like '{V}' is required to get an actual SEQUENCE OF octet strings.

and there is no API that takes a single A null-terminated struct berval *'s

so the only thing that regressed is 'V' encoding for Linux. 't' (encoding, decoding), 'v'(encoding) are supported'; 'V' and 'v' decoding were not supported before these changes.

// V and v are not supported on Unix yet.
return -1;
}

[DllImport(Libraries.OpenLdap, EntryPoint = "ber_scanf", CharSet = CharSet.Ansi)]
public static extern int ber_scanf_ptr(SafeBerHandle berElement, string format, ref IntPtr value);
public static int ber_scanf_multibytearray(SafeBerHandle berElement, string format, ref IntPtr value)
{
Debug.Assert(format == "v" || format == "V");
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decode for vv and VV was not supported previously so we don't regress anything here:

runtime/src/libraries/System.DirectoryServices.Protocols/tests/BerConverterTests.cs

Line 124 in acff8f3

if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) // vv and VV formats are not supported yet in Linux

Copy link
Member

Choose a reason for hiding this comment

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

yeah I understand that, but what I mean is imagine that in a future version of libldap (if it ever ships) they add support for vv or VV, then the old code would have just worked as the scanf calls would just light up and start working; whereas if we keep it the way you have it we would have to basically make the change manually. TBH I doubt libldap will ship a new future major version with this support so I'm fine keeping it as you have it but just wanted to raise the comment to get your opinion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this comment means that "vv" and "VV" are not supported in System.DirectoryServices.Protocols or in OpenLDAP? I thought it was about "System.DirectoryServices.Protocols". I guess it was not supported exactly because vararg calling convention for passing such struct was different so we could not call it even on x64 Unix.

// V and v are not supported on Unix yet.
return -1;
}
}
}
22 changes: 2 additions & 20 deletions src/libraries/Common/src/Interop/Windows/Wldap32/Interop.Ber.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,7 @@ internal static partial class Ldap
public static extern IntPtr ber_alloc(int option);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_printf", CharSet = CharSet.Unicode)]
public static extern int ber_printf_emptyarg(SafeBerHandle berElement, string format);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_printf", CharSet = CharSet.Unicode)]
public static extern int ber_printf_int(SafeBerHandle berElement, string format, int value);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_printf", CharSet = CharSet.Unicode)]
public static extern int ber_printf_bytearray(SafeBerHandle berElement, string format, HGlobalMemHandle value, int length);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_printf", CharSet = CharSet.Unicode)]
public static extern int ber_printf_berarray(SafeBerHandle berElement, string format, IntPtr value);
public static extern int ber_printf(SafeBerHandle berElement, string format, __arglist);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_flatten", CharSet = CharSet.Unicode)]
public static extern int ber_flatten(SafeBerHandle berElement, ref IntPtr value);
Expand All @@ -34,16 +25,7 @@ internal static partial class Ldap
public static extern IntPtr ber_init(berval value);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_scanf", CharSet = CharSet.Unicode)]
public static extern int ber_scanf(SafeBerHandle berElement, string format);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_scanf", CharSet = CharSet.Unicode)]
public static extern int ber_scanf_int(SafeBerHandle berElement, string format, ref int value);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_scanf", CharSet = CharSet.Unicode)]
public static extern int ber_scanf_ptr(SafeBerHandle berElement, string format, ref IntPtr value);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_scanf", CharSet = CharSet.Unicode)]
public static extern int ber_scanf_bitstring(SafeBerHandle berElement, string format, ref IntPtr value, ref int bitLength);
public static extern int ber_scanf(SafeBerHandle berElement, string format, __arglist);

[DllImport(Libraries.Wldap32, CallingConvention = CallingConvention.Cdecl, EntryPoint = "ber_bvfree", CharSet = CharSet.Unicode)]
public static extern int ber_bvfree(IntPtr value);
Expand Down
14 changes: 9 additions & 5 deletions ...irectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/BerPal.Linux.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,26 @@ internal static class BerPal

internal static int FlattenBerElement(SafeBerHandle berElement, ref IntPtr flattenptr) => Interop.Ldap.ber_flatten(berElement, ref flattenptr);

internal static int PrintBerArray(SafeBerHandle berElement, string format, IntPtr value) => Interop.Ldap.ber_printf_berarray(berElement, format, value);
internal static int PrintBerArray(SafeBerHandle berElement, string format, IntPtr value, int tag) => Interop.Ldap.ber_printf_berarray(berElement, format, value, tag);

internal static int PrintByteArray(SafeBerHandle berElement, string format, HGlobalMemHandle value, int length) => Interop.Ldap.ber_printf_bytearray(berElement, format, value, length);
internal static int PrintByteArray(SafeBerHandle berElement, string format, HGlobalMemHandle value, int length, int tag) => Interop.Ldap.ber_printf_bytearray(berElement, format, value, length, tag);

internal static int PrintEmptyArgument(SafeBerHandle berElement, string format) => Interop.Ldap.ber_printf_emptyarg(berElement, format);
internal static int PrintEmptyArgument(SafeBerHandle berElement, string format, int tag) => Interop.Ldap.ber_printf_emptyarg(berElement, format, tag);

internal static int PrintInt(SafeBerHandle berElement, string format, int value) => Interop.Ldap.ber_printf_int(berElement, format, value);
internal static int PrintInt(SafeBerHandle berElement, string format, int value, int tag) => Interop.Ldap.ber_printf_int(berElement, format, value, tag);

internal static int ScanNext(SafeBerHandle berElement, string format) => Interop.Ldap.ber_scanf(berElement, format);
internal static int PrintTag(SafeBerHandle berElement, string format, int tag) => Interop.Ldap.ber_printf_tag(berElement, format, tag);

internal static int ScanNext(SafeBerHandle berElement, string format) => Interop.Ldap.ber_scanf_emptyarg(berElement, format);

internal static int ScanNextBitString(SafeBerHandle berElement, string format, ref IntPtr ptrResult, ref int bitLength) => Interop.Ldap.ber_scanf_bitstring(berElement, format, ref ptrResult, ref bitLength);

internal static int ScanNextInt(SafeBerHandle berElement, string format, ref int result) => Interop.Ldap.ber_scanf_int(berElement, format, ref result);

internal static int ScanNextPtr(SafeBerHandle berElement, string format, ref IntPtr value) => Interop.Ldap.ber_scanf_ptr(berElement, format, ref value);

internal static int ScanNextMultiByteArray(SafeBerHandle berElement, string format, ref IntPtr value) => Interop.Ldap.ber_scanf_multibytearray(berElement, format, ref value);

internal static bool IsBerDecodeError(int errorCode) => errorCode == -1;
}
}
20 changes: 12 additions & 8 deletions ...ectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/BerPal.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,25 @@ internal static class BerPal

internal static int FlattenBerElement(SafeBerHandle berElement, ref IntPtr flattenptr) => Interop.Ldap.ber_flatten(berElement, ref flattenptr);

internal static int PrintBerArray(SafeBerHandle berElement, string format, IntPtr value) => Interop.Ldap.ber_printf_berarray(berElement, format, value);
internal static int PrintBerArray(SafeBerHandle berElement, string format, IntPtr value, int tag) => Interop.Ldap.ber_printf(berElement, format, __arglist(value));

internal static int PrintByteArray(SafeBerHandle berElement, string format, HGlobalMemHandle value, int length) => Interop.Ldap.ber_printf_bytearray(berElement, format, value, length);
internal static int PrintByteArray(SafeBerHandle berElement, string format, HGlobalMemHandle value, int length, int tag) => Interop.Ldap.ber_printf(berElement, format, __arglist(value, length));

internal static int PrintEmptyArgument(SafeBerHandle berElement, string format) => Interop.Ldap.ber_printf_emptyarg(berElement, format);
internal static int PrintEmptyArgument(SafeBerHandle berElement, string format, int tag) => Interop.Ldap.ber_printf(berElement, format, __arglist());

internal static int PrintInt(SafeBerHandle berElement, string format, int value) => Interop.Ldap.ber_printf_int(berElement, format, value);
internal static int PrintInt(SafeBerHandle berElement, string format, int value, int tag) => Interop.Ldap.ber_printf(berElement, format, __arglist(value));

internal static int ScanNext(SafeBerHandle berElement, string format) => Interop.Ldap.ber_scanf(berElement, format);
internal static int PrintTag(SafeBerHandle berElement, string format, int tag) => Interop.Ldap.ber_printf(berElement, format, __arglist(tag));

internal static int ScanNextBitString(SafeBerHandle berElement, string format, ref IntPtr ptrResult, ref int bitLength) => Interop.Ldap.ber_scanf_bitstring(berElement, format, ref ptrResult, ref bitLength);
internal static int ScanNext(SafeBerHandle berElement, string format) => Interop.Ldap.ber_scanf(berElement, format, __arglist());

internal static int ScanNextInt(SafeBerHandle berElement, string format, ref int result) => Interop.Ldap.ber_scanf_int(berElement, format, ref result);
internal static int ScanNextBitString(SafeBerHandle berElement, string format, ref IntPtr ptrResult, ref int bitLength) => Interop.Ldap.ber_scanf(berElement, format, __arglist(ref ptrResult, ref bitLength));

internal static int ScanNextPtr(SafeBerHandle berElement, string format, ref IntPtr value) => Interop.Ldap.ber_scanf_ptr(berElement, format, ref value);
internal static int ScanNextInt(SafeBerHandle berElement, string format, ref int result) => Interop.Ldap.ber_scanf(berElement, format, __arglist(ref result));

internal static int ScanNextPtr(SafeBerHandle berElement, string format, ref IntPtr value) => Interop.Ldap.ber_scanf(berElement, format, __arglist(ref value));

internal static int ScanNextMultiByteArray(SafeBerHandle berElement, string format, ref IntPtr value) => Interop.Ldap.ber_scanf(berElement, format, __arglist(ref value));

internal static bool IsBerDecodeError(int errorCode) => errorCode != 0;
}
Expand Down
Loading