Skip to content

Remove a few unsafe bits in S.P.Uri, S.N.Ping, System.Transactions.Local #116281

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

Merged
merged 3 commits into from
Jun 5, 2025
Merged
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
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ public partial class Ping
private const int IpV6HeaderLengthInBytes = 40;
private static readonly ushort DontFragment = OperatingSystem.IsFreeBSD() ? (ushort)IPAddress.HostToNetworkOrder((short)0x4000) : (ushort)0x4000;

private static unsafe SocketConfig GetSocketConfig(IPAddress address, byte[] buffer, int timeout, PingOptions? options)
private static SocketConfig GetSocketConfig(IPAddress address, byte[] buffer, int timeout, PingOptions? options)
{
// Use a random value as the identifier. This doesn't need to be perfectly random
// or very unpredictable, rather just good enough to avoid unexpected conflicts.
@@ -35,7 +35,10 @@ private static unsafe SocketConfig GetSocketConfig(IPAddress address, byte[] buf
if (sendIpHeader)
{
iph.VersionAndLength = 0x45;
totalLength = sizeof(IpHeader) + checked(sizeof(IcmpHeader) + buffer.Length);
unsafe
{
totalLength = sizeof(IpHeader) + checked(sizeof(IcmpHeader) + buffer.Length);
}
// On OSX this strangely must be host byte order.
iph.TotalLength = OperatingSystem.IsFreeBSD() ? (ushort)IPAddress.HostToNetworkOrder((short)totalLength) : (ushort)totalLength;
iph.Protocol = 1; // ICMP
@@ -103,19 +106,16 @@ private static Socket GetRawSocket(SocketConfig socketConfig)
{
// If it is not multicast, use Connect to scope responses only to the target address.
socket.Connect(socketConfig.EndPoint);
unsafe
int opt = 1;
if (ipv4)
{
int opt = 1;
if (ipv4)
{
// setsockopt(fd, IPPROTO_IP, IP_RECVERR, &value, sizeof(int))
socket.SetRawSocketOption(0, 11, new ReadOnlySpan<byte>(&opt, sizeof(int)));
}
else
{
// setsockopt(fd, IPPROTO_IPV6, IPV6_RECVERR, &value, sizeof(int))
socket.SetRawSocketOption(41, 25, new ReadOnlySpan<byte>(&opt, sizeof(int)));
}
// setsockopt(fd, IPPROTO_IP, IP_RECVERR, &value, sizeof(int))
socket.SetRawSocketOption(0, 11, MemoryMarshal.AsBytes(new ReadOnlySpan<int>(in opt)));
}
else
{
// setsockopt(fd, IPPROTO_IPV6, IPV6_RECVERR, &value, sizeof(int))
socket.SetRawSocketOption(41, 25, MemoryMarshal.AsBytes(new ReadOnlySpan<int>(in opt)));
}
}
#pragma warning restore 618
@@ -247,7 +247,7 @@ private static bool TryGetPingReply(
return true;
}

private static unsafe PingReply SendIcmpEchoRequestOverRawSocket(IPAddress address, byte[] buffer, int timeout, PingOptions? options)
private static PingReply SendIcmpEchoRequestOverRawSocket(IPAddress address, byte[] buffer, int timeout, PingOptions? options)
{
SocketConfig socketConfig = GetSocketConfig(address, buffer, timeout, options);
using (Socket socket = GetRawSocket(socketConfig))
61 changes: 27 additions & 34 deletions src/libraries/System.Private.Uri/src/System/IPv4AddressHelper.cs
Original file line number Diff line number Diff line change
@@ -13,50 +13,43 @@ internal static partial class IPv4AddressHelper
// Parse and canonicalize
internal static string ParseCanonicalName(string str, int start, int end, ref bool isLoopback)
{
// end includes ports, so changedEnd may be different from end
int changedEnd = end;
long result;

unsafe
{
byte* numbers = stackalloc byte[NumberOfLabels];
isLoopback = Parse(str, numbers, start, end);

Span<char> stackSpace = stackalloc char[NumberOfLabels * 3 + 3];
int totalChars = 0, charsWritten;
for (int i = 0; i < 3; i++)
fixed (char* ipString = str)
{
numbers[i].TryFormat(stackSpace.Slice(totalChars), out charsWritten);
int periodPos = totalChars + charsWritten;
stackSpace[periodPos] = '.';
totalChars = periodPos + 1;
result = ParseNonCanonical(ipString, start, ref changedEnd, true);
}
numbers[3].TryFormat(stackSpace.Slice(totalChars), out charsWritten);
return new string(stackSpace.Slice(0, totalChars + charsWritten));
}
}

//
// Parse
//
// Convert this IPv4 address into a sequence of 4 8-bit numbers
//
private static unsafe bool Parse(string name, byte* numbers, int start, int end)
{
fixed (char* ipString = name)
{
// end includes ports, so changedEnd may be different from end
int changedEnd = end;
long result = IPv4AddressHelper.ParseNonCanonical(ipString, start, ref changedEnd, true);
Debug.Assert(result != Invalid, $"Failed to parse after already validated: {str}");

Debug.Assert(result != Invalid, $"Failed to parse after already validated: {name}");
Span<byte> numbers =
unchecked(
[
(byte)(result >> 24),
(byte)(result >> 16),
(byte)(result >> 8),
(byte)(result)
]);

unchecked
{
numbers[0] = (byte)(result >> 24);
numbers[1] = (byte)(result >> 16);
numbers[2] = (byte)(result >> 8);
numbers[3] = (byte)(result);
}
isLoopback = numbers[0] == 127;

Span<char> stackSpace = stackalloc char[NumberOfLabels * 3 + 3];
int totalChars = 0, charsWritten;
for (int i = 0; i < NumberOfLabels - 1; i++)
{
numbers[i].TryFormat(stackSpace.Slice(totalChars), out charsWritten);
int periodPos = totalChars + charsWritten;
stackSpace[periodPos] = '.';
totalChars = periodPos + 1;
}

return numbers[0] == 127;
numbers[3].TryFormat(stackSpace.Slice(totalChars), out charsWritten);
return new string(stackSpace.Slice(0, totalChars + charsWritten));
}
}
}
2 changes: 1 addition & 1 deletion src/libraries/System.Private.Uri/src/System/UriExt.cs
Original file line number Diff line number Diff line change
@@ -891,7 +891,7 @@ private Uri(Flags flags, UriParser? uriParser, string uri)
return null;
}

private unsafe string GetRelativeSerializationString(UriFormat format)
private string GetRelativeSerializationString(UriFormat format)
{
if (format == UriFormat.UriEscaped)
{
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
using System.Text;
@@ -24,27 +25,33 @@ internal Xactopt(uint ulTimeout, string szDescription)
[CustomMarshaller(typeof(Xactopt), MarshalMode.UnmanagedToManagedIn, typeof(Marshaller))]
internal static class Marshaller
{
internal unsafe struct XactoptNative
internal struct XactoptNative
{
public uint UlTimeout;

public fixed byte SzDescription[40];
public SzDescription SzDescription;
}

public static unsafe XactoptNative ConvertToUnmanaged(Xactopt managed)
[InlineArray(40)]
internal struct SzDescription
{
private byte _element0;
}

public static XactoptNative ConvertToUnmanaged(Xactopt managed)
{
XactoptNative native = new()
{
UlTimeout = managed.UlTimeout,
};

// Usage of Xactopt never passes non-ASCII chars, so we can ignore them.
Encoding.ASCII.TryGetBytes(managed.SzDescription, new Span<byte>(native.SzDescription, 40), out _);
Encoding.ASCII.TryGetBytes(managed.SzDescription, native.SzDescription, out _);

return native;
}

public static unsafe Xactopt ConvertToManaged(XactoptNative unmanaged)
=> new(unmanaged.UlTimeout, Encoding.ASCII.GetString(unmanaged.SzDescription, 40));
public static Xactopt ConvertToManaged(XactoptNative unmanaged)
=> new(unmanaged.UlTimeout, Encoding.ASCII.GetString(unmanaged.SzDescription));
}
}
Loading
Oops, something went wrong.