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.

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 marshalling of StringBuilder errors #533

Merged
merged 7 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions SharpPcap/LibPcap/CaptureFileReaderDevice.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@
/// </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 @@
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 @@
);
}

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

Check warning on line 99 in SharpPcap/LibPcap/CaptureFileReaderDevice.cs

View check run for this annotation

Codecov / codecov/patch

SharpPcap/LibPcap/CaptureFileReaderDevice.cs#L99

Added line #L99 was not covered by tests
}

// handle error
Expand Down
4 changes: 2 additions & 2 deletions SharpPcap/LibPcap/CaptureHandleReaderDevice.cs
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
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
// 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 @@
{
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 @@
(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 @@
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();

Check warning on line 283 in SharpPcap/LibPcap/LibPcapLiveDevice.cs

View check run for this annotation

Codecov / codecov/patch

SharpPcap/LibPcap/LibPcapLiveDevice.cs#L283

Added line #L283 was not covered by tests
throw new PcapException(err);
}

Expand All @@ -294,18 +292,16 @@
{
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();

Check warning on line 304 in SharpPcap/LibPcap/LibPcapLiveDevice.cs

View check run for this annotation

Codecov / codecov/patch

SharpPcap/LibPcap/LibPcapLiveDevice.cs#L304

Added line #L304 was not covered by tests
throw new PcapException(err);
}
}
Expand Down
48 changes: 25 additions & 23 deletions SharpPcap/LibPcap/LibPcapSafeNativeMethods.Encoding.cs
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,14 @@
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);

Check warning on line 183 in SharpPcap/LibPcap/PcapInterface.cs

View check run for this annotation

Codecov / codecov/patch

SharpPcap/LibPcap/PcapInterface.cs#L183

Added line #L183 was not covered by tests
if (result < 0)
{
throw new PcapException(errorBuffer.ToString());
throw new PcapException(errbuf.ToString());

Check warning on line 186 in SharpPcap/LibPcap/PcapInterface.cs

View check run for this annotation

Codecov / codecov/patch

SharpPcap/LibPcap/PcapInterface.cs#L186

Added line #L186 was not covered by tests
}
}
catch (TypeLoadException ex)
Expand All @@ -206,12 +205,11 @@
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());

Check warning on line 212 in SharpPcap/LibPcap/PcapInterface.cs

View check run for this annotation

Codecov / codecov/patch

SharpPcap/LibPcap/PcapInterface.cs#L212

Added line #L212 was not covered by tests
}
var pcapInterfaces = GetAllPcapInterfaces(devicePtr, null);

Expand Down Expand Up @@ -260,8 +258,7 @@
{
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
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
Loading