Skip to content

Commit

Permalink
Fix marshalling of StringBuilder errors (#533)
Browse files Browse the repository at this point in the history 
* Fix marshalling of StringBuilder errors

* Move StringBuilder logic to PcapStringBuilderMarshaler

* Drop ICustomMarshaler for error string, use ErrorBuffer class instead.ss

* Revert csproj changes

* Fix build

* Cleanup
  • Loading branch information
@kayoub5
kayoub5 authored Oct 8, 2024
1 parent 2fff830 commit 5bf70ce
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 66 deletions.
7 changes: 3 additions & 4 deletions SharpPcap/LibPcap/CaptureFileReaderDevice.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ public CaptureFileReaderDevice(string captureFilename)
/// </summary>
public override void Open(DeviceConfiguration configuration)
{
// holds errors
StringBuilder errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors
ErrorBuffer errbuf; //will hold errors

PcapHandle adapterHandle;

Expand All @@ -82,7 +81,7 @@ public override void Open(DeviceConfiguration configuration)
var resolution = configuration.TimestampResolution ?? TimestampResolution.Microsecond;
if (has_offline_with_tstamp_precision_support)
{
adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline_with_tstamp_precision(m_pcapFile, (uint)resolution, errbuf);
adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline_with_tstamp_precision(m_pcapFile, (uint)resolution, out errbuf);
}
else
{
Expand All @@ -97,7 +96,7 @@ public override void Open(DeviceConfiguration configuration)
);
}

adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline(m_pcapFile, errbuf);
adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline(m_pcapFile, out errbuf);
}

// handle error
Expand Down
4 changes: 2 additions & 2 deletions SharpPcap/LibPcap/CaptureHandleReaderDevice.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ public CaptureHandleReaderDevice(SafeHandle handle)
public override void Open(DeviceConfiguration configuration)
{
// holds errors
StringBuilder errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE);
ErrorBuffer errbuf;

var resolution = configuration.TimestampResolution ?? TimestampResolution.Microsecond;
PcapHandle adapterHandle;
try
{
adapterHandle = LibPcapSafeNativeMethods.pcap_open_handle_offline_with_tstamp_precision(
FileHandle, (uint)resolution, errbuf);
FileHandle, (uint)resolution, out errbuf);
}
catch (TypeLoadException ex)
{
Expand Down
18 changes: 7 additions & 11 deletions SharpPcap/LibPcap/LibPcapLiveDevice.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public override void Open(DeviceConfiguration configuration)
// See https://www.tcpdump.org/manpages/pcap_set_immediate_mode.3pcap.html
var mintocopy_supported = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);

var errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors
ErrorBuffer errbuf; //will hold errors

// set the StopCaptureTimeout value to twice the read timeout to ensure that
// we wait long enough before considering the capture thread to be stuck when stopping
Expand Down Expand Up @@ -131,7 +131,7 @@ public override void Open(DeviceConfiguration configuration)
{
Handle = LibPcapSafeNativeMethods.pcap_create(
Name, // name of the device
errbuf); // error buffer
out errbuf); // error buffer

if (Handle.IsInvalid)
{
Expand Down Expand Up @@ -171,7 +171,7 @@ public override void Open(DeviceConfiguration configuration)
(short)mode, // flags
(short)configuration.ReadTimeout, // read timeout
ref auth, // authentication
errbuf); // error buffer
out errbuf); // error buffer
}
catch (TypeLoadException)
{
Expand Down Expand Up @@ -275,14 +275,12 @@ public bool NonBlockingMode
get
{
ThrowIfNotOpen("Can't get blocking mode, the device is closed");

var errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors
int ret = LibPcapSafeNativeMethods.pcap_getnonblock(Handle, errbuf);
int ret = LibPcapSafeNativeMethods.pcap_getnonblock(Handle, out var errbuf);

// Errorbuf is only filled when ret = -1
if (ret == -1)
{
string err = "Unable to set get blocking" + errbuf.ToString();
string err = "Unable to get blocking mode. " + errbuf.ToString();
throw new PcapException(err);
}

Expand All @@ -294,18 +292,16 @@ public bool NonBlockingMode
{
ThrowIfNotOpen("Can't set blocking mode, the device is closed");

var errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors

int block = disableBlocking;
if (value)
block = enableBlocking;

int ret = LibPcapSafeNativeMethods.pcap_setnonblock(Handle, block, errbuf);
int ret = LibPcapSafeNativeMethods.pcap_setnonblock(Handle, block, out var errbuf);

// Errorbuf is only filled when ret = -1
if (ret == -1)
{
string err = "Unable to set non blocking" + errbuf.ToString();
string err = "Unable to set blocking mode. " + errbuf.ToString();
throw new PcapException(err);
}
}
Expand Down
48 changes: 25 additions & 23 deletions SharpPcap/LibPcap/LibPcapSafeNativeMethods.Encoding.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ internal static partial class LibPcapSafeNativeMethods
{

/// <summary>
/// This defaul is good enough for .NET Framework and .NET Core on non Windows with Libpcap default config
/// This default is good enough for .NET Framework and .NET Core on non Windows with Libpcap default config
/// </summary>
private static readonly Encoding StringEncoding = Encoding.Default;
internal static readonly Encoding StringEncoding = Encoding.Default;

private static Encoding ConfigureStringEncoding()
{
Expand All @@ -29,9 +29,8 @@ private static Encoding ConfigureStringEncoding()
try
{
// Try to change Libpcap to UTF-8 mode
var errorBuffer = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE);
const uint PCAP_CHAR_ENC_UTF_8 = 1;
var res = pcap_init(PCAP_CHAR_ENC_UTF_8, errorBuffer);
var res = pcap_init(PCAP_CHAR_ENC_UTF_8, out _);
if (res == 0)
{
// We made it
Expand Down Expand Up @@ -90,25 +89,11 @@ public IntPtr MarshalManagedToNative(object managedObj)
{
return IntPtr.Zero;
}
byte[] bytes = null;
var byteCount = 0;
if (managedObj is string str)
{
bytes = StringEncoding.GetBytes(str);
byteCount = bytes.Length + 1;
}

if (managedObj is StringBuilder builder)
{
bytes = StringEncoding.GetBytes(builder.ToString());
byteCount = StringEncoding.GetMaxByteCount(builder.Capacity) + 1;
}

if (bytes is null)
{
throw new ArgumentException("The input argument is not a supported type.");
}
var ptr = Marshal.AllocHGlobal(byteCount);
var str = (string)managedObj;
var bytes = StringEncoding.GetBytes(str);
// The problem is that we need a reference to the StringBuilder in MarshalNativeToManaged
// So we get a pointer to it with GCHandle, and put it as prefix of the pointer we return
var ptr = Marshal.AllocHGlobal(bytes.Length + 1);
Marshal.Copy(bytes, 0, ptr, bytes.Length);
// Put zero string termination
Marshal.WriteByte(ptr + bytes.Length, 0);
Expand All @@ -131,4 +116,21 @@ public unsafe object MarshalNativeToManaged(IntPtr nativeData)
}
}
}

[StructLayout(LayoutKind.Sequential)]
internal struct ErrorBuffer
{
[MarshalAs(UnmanagedType.ByValArray, SizeConst = 256)]
internal byte[] Data;

public override string ToString()
{
var nbBytes = 0;
while (Data[nbBytes] != 0)
{
nbBytes++;
}
return LibPcapSafeNativeMethods.StringEncoding.GetString(Data, 0, nbBytes);
}
}
}
25 changes: 11 additions & 14 deletions SharpPcap/LibPcap/LibPcapSafeNativeMethods.Interop.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@
// SPDX-License-Identifier: MIT

using System;
using System.IO;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Security;
using System.Text;
using static SharpPcap.LibPcap.PcapUnmanagedStructures;

namespace SharpPcap.LibPcap
Expand All @@ -30,21 +27,21 @@ internal static partial class LibPcapSafeNativeMethods
[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
internal extern static int pcap_init(
uint opts,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);

[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
internal extern static int pcap_findalldevs(
ref IntPtr /* pcap_if_t** */ alldevs,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);

[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
internal extern static int pcap_findalldevs_ex(
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string /*char **/source,
ref pcap_rmtauth /*pcap_rmtauth **/auth,
ref IntPtr /*pcap_if_t ** */alldevs,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /*char * */errbuf
out ErrorBuffer /* char* */ errbuf
);

[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
Expand All @@ -57,19 +54,19 @@ internal extern static int pcap_findalldevs_ex(
int flags,
int read_timeout,
ref pcap_rmtauth rmtauth,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder errbuf
out ErrorBuffer /* char* */ errbuf
);

[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
internal extern static PcapHandle /* pcap_t* */ pcap_create(
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string dev,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder errbuf
out ErrorBuffer /* char* */ errbuf
);

[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
internal extern static PcapHandle /* pcap_t* */ pcap_open_offline(
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string/*const char* */ fname,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder/* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);

[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
Expand Down Expand Up @@ -187,7 +184,7 @@ internal extern static int pcap_compile(
internal extern static int pcap_setnonblock(
PcapHandle /* pcap_if_t** */ adaptHandle,
int nonblock,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);

/// <summary>
Expand All @@ -196,7 +193,7 @@ internal extern static int pcap_setnonblock(
[DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)]
internal extern static int pcap_getnonblock(
PcapHandle /* pcap_if_t** */ adaptHandle,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);

/// <summary>
Expand Down Expand Up @@ -422,7 +419,7 @@ internal extern static int pcap_tstamp_type_name_to_val(
internal extern static PcapHandle /* pcap_t* */ pcap_open_offline_with_tstamp_precision(
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string /* const char* */ fname,
uint precision,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);

/// <summary>
Expand Down Expand Up @@ -488,7 +485,7 @@ internal extern static int pcap_tstamp_type_name_to_val(
internal extern static IntPtr /* pcap_t* */ _pcap_hopen_offline_with_tstamp_precision(
SafeHandle handle,
uint precision,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);

/// <summary>
Expand All @@ -503,7 +500,7 @@ internal extern static int pcap_tstamp_type_name_to_val(
internal extern static IntPtr /* pcap_t* */ _pcap_fopen_offline_with_tstamp_precision(
SafeHandle fileObject,
uint precision,
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf
out ErrorBuffer /* char* */ errbuf
);
}
}
6 changes: 3 additions & 3 deletions SharpPcap/LibPcap/LibPcapSafeNativeMethods.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ internal static int pcap_get_tstamp_precision(PcapHandle /* pcap_t* p */ adapter
/// <param name="errbuf">Buffer that will receive an error description if an error occurs.</param>
/// <returns></returns>
internal static PcapHandle pcap_open_handle_offline_with_tstamp_precision(
SafeHandle handle, uint precision, StringBuilder errbuf)
SafeHandle handle, uint precision, out ErrorBuffer errbuf)
{
var pointer = RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
? _pcap_hopen_offline_with_tstamp_precision(handle, precision, errbuf)
: _pcap_fopen_offline_with_tstamp_precision(handle, precision, errbuf);
? _pcap_hopen_offline_with_tstamp_precision(handle, precision, out errbuf)
: _pcap_fopen_offline_with_tstamp_precision(handle, precision, out errbuf);
if (pointer == IntPtr.Zero)
{
return PcapHandle.Invalid;
Expand Down
13 changes: 5 additions & 8 deletions SharpPcap/LibPcap/PcapInterface.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,14 @@ static public IReadOnlyList<PcapInterface> GetAllPcapInterfaces(IPEndPoint sourc
static public IReadOnlyList<PcapInterface> GetAllPcapInterfaces(string source, RemoteAuthentication credentials)
{
var devicePtr = IntPtr.Zero;
var errorBuffer = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE);
var auth = RemoteAuthentication.CreateAuth(credentials);

try
{
var result = LibPcapSafeNativeMethods.pcap_findalldevs_ex(source, ref auth, ref devicePtr, errorBuffer);
var result = LibPcapSafeNativeMethods.pcap_findalldevs_ex(source, ref auth, ref devicePtr, out var errbuf);
if (result < 0)
{
throw new PcapException(errorBuffer.ToString());
throw new PcapException(errbuf.ToString());
}
}
catch (TypeLoadException ex)
Expand All @@ -206,12 +205,11 @@ static public IReadOnlyList<PcapInterface> GetAllPcapInterfaces(string source, R
static public IReadOnlyList<PcapInterface> GetAllPcapInterfaces()
{
var devicePtr = IntPtr.Zero;
var errorBuffer = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE);

int result = LibPcapSafeNativeMethods.pcap_findalldevs(ref devicePtr, errorBuffer);
int result = LibPcapSafeNativeMethods.pcap_findalldevs(ref devicePtr, out var errbuf);
if (result < 0)
{
throw new PcapException(errorBuffer.ToString());
throw new PcapException(errbuf.ToString());
}
var pcapInterfaces = GetAllPcapInterfaces(devicePtr, null);

Expand Down Expand Up @@ -260,8 +258,7 @@ public System.Collections.Generic.List<PcapClock> TimestampsSupported
{
get
{
StringBuilder errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors
using (var handle = LibPcapSafeNativeMethods.pcap_create(Name, errbuf))
using (var handle = LibPcapSafeNativeMethods.pcap_create(Name, out var errbuf))
{

IntPtr typePtr = IntPtr.Zero;
Expand Down
1 change: 0 additions & 1 deletion SharpPcap/Pcap.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public class Pcap
/* interface is loopback */
internal const uint PCAP_IF_LOOPBACK = 0x00000001;
internal const int MAX_PACKET_SIZE = 65536;
internal const int PCAP_ERRBUF_SIZE = 256;

// Constants for address families
// These are set in a Pcap static initializer because the values
Expand Down

0 comments on commit 5bf70ce

Please sign in to comment.