Further reduce use of unsafe code

Author: GrabYourPitchforks

src/Servers/HttpSys/src/NativeMethods.txt

@@ -57,3 +57,5 @@ HttpSetRequestQueueProperty
 HttpSetUrlGroupProperty
 HttpSetUrlGroupProperty
 SetFileCompletionNotificationModes
+SOCKADDR_IN
+SOCKADDR_IN6

src/Shared/HttpSys/NativeInterop/SocketAddress.cs

@@ -1,85 +1,73 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Diagnostics.Contracts;
 using System.Net;
-using System.Net.Sockets;
+using Windows.Win32.Networking.WinSock;
 
 namespace Microsoft.AspNetCore.HttpSys.Internal;
 
-internal sealed class SocketAddress
+internal abstract class SocketAddress
 {
-    private const int NumberOfIPv6Labels = 8;
-    private const int IPv6AddressSize = 28;
-    private const int IPv4AddressSize = 16;
-    private const int WriteableOffset = 2;
+    internal abstract int GetPort();
 
-    private readonly byte[] _buffer;
-    private readonly int _size;
+    internal abstract IPAddress? GetIPAddress();
 
-    private SocketAddress(AddressFamily family, int size)
+    internal static unsafe SocketAddress? CopyOutAddress(SOCKADDR* pSockaddr)
     {
-        ArgumentOutOfRangeException.ThrowIfLessThan(size, WriteableOffset);
-        Family = family;
-        _size = size;
-        // Sized to match the native structure
-        _buffer = new byte[((size / IntPtr.Size) + 2) * IntPtr.Size]; // sizeof DWORD
+        // Per https://learn.microsoft.com/windows/win32/api/ws2def/ns-ws2def-sockaddr,
+        // use the SOCKADDR* pointer only to read the address family, then cast the pointer to
+        // the appropriate family type and continue processing.
+        return pSockaddr->sa_family switch
+        {
+            ADDRESS_FAMILY.AF_INET => new SocketAddressIPv4(*(SOCKADDR_IN*)pSockaddr),
+            ADDRESS_FAMILY.AF_INET6 => new SocketAddressIPv6(*(SOCKADDR_IN6*)pSockaddr),
+            _ => null
+        };
     }
 
-    internal AddressFamily Family { get; }
-
-    internal int GetPort()
+    private sealed class SocketAddressIPv4 : SocketAddress
     {
-        return (_buffer[2] << 8 & 0xFF00) | (_buffer[3]);
-    }
+        private readonly SOCKADDR_IN _sockaddr;
 
-    internal IPAddress? GetIPAddress()
-    {
-        if (Family == AddressFamily.InterNetworkV6)
+        internal SocketAddressIPv4(in SOCKADDR_IN sockaddr)
         {
-            return GetIpv6Address();
+            _sockaddr = sockaddr;
         }
-        else if (Family == AddressFamily.InterNetwork)
+
+        internal override int GetPort()
         {
-            return GetIPv4Address();
+            // sin_port is network byte order
+            return IPAddress.NetworkToHostOrder(_sockaddr.sin_port);
         }
-        else
+
+        internal override IPAddress? GetIPAddress()
         {
-            return null;
+            // address is network byte order
+            return new IPAddress(_sockaddr.sin_addr.S_un.S_addr);
         }
     }
 
-    private IPAddress GetIpv6Address()
-    {
-        Contract.Assert(_size >= IPv6AddressSize);
-        var bytes = new byte[NumberOfIPv6Labels * 2];
-        Array.Copy(_buffer, 8, bytes, 0, NumberOfIPv6Labels * 2);
-        return new IPAddress(bytes); // TODO: Does scope id matter?
-    }
-
-    private IPAddress GetIPv4Address()
+    private sealed class SocketAddressIPv6 : SocketAddress
     {
-        Contract.Assert(_size >= IPv4AddressSize);
-        return new IPAddress(new byte[] { _buffer[4], _buffer[5], _buffer[6], _buffer[7] });
-    }
+        private readonly SOCKADDR_IN6 _sockaddr;
 
-    internal static unsafe SocketAddress? CopyOutAddress(IntPtr address)
-    {
-        AddressFamily family = (AddressFamily)(*((ushort*)address));
-        int? addressSize = family switch
+        internal SocketAddressIPv6(in SOCKADDR_IN6 sockaddr)
         {
-            AddressFamily.InterNetwork => IPv4AddressSize,
-            AddressFamily.InterNetworkV6 => IPv6AddressSize,
-            _ => null
-        };
+            _sockaddr = sockaddr;
+        }
 
-        if (!addressSize.HasValue)
+        internal override int GetPort()
         {
-            return null;
+            // sin6_port is network byte order
+            return IPAddress.NetworkToHostOrder(_sockaddr.sin6_port);
         }
 
-        var sockAddress = new SocketAddress(family, addressSize.Value);
-        new ReadOnlySpan<byte>((byte*)address, addressSize.Value).Slice(sizeof(ushort)).CopyTo(sockAddress._buffer.AsSpan(sizeof(ushort)));
-        return sockAddress;
+        internal override IPAddress? GetIPAddress()
+        {
+            // address is network byte order
+            // when CsWin32 gets support for inline arrays, remove 'AsReadOnlySpan' call below.
+            // https://github.com/microsoft/CsWin32/issues/1086
+            return new IPAddress(_sockaddr.sin6_addr.u.Byte.AsReadOnlySpan()); // TODO: Does scope id matter?
+        }
     }
 }

src/Shared/HttpSys/RequestProcessing/NativeRequestContext.cs

@@ -650,14 +650,14 @@ private void GetUnknownHeadersHelper(IDictionary<string, StringValues> unknownHe
 
     private SocketAddress? GetEndPointHelper(bool localEndpoint, HTTP_REQUEST_V1* request, byte* pMemoryBlob)
     {
-        var source = localEndpoint ? (byte*)request->Address.pLocalAddress : (byte*)request->Address.pRemoteAddress;
+        var source = localEndpoint ? request->Address.pLocalAddress : request->Address.pRemoteAddress;
 
         if (source == null)
         {
             return null;
         }
-        var address = (IntPtr)(pMemoryBlob + _bufferAlignment - (byte*)_originalBufferAddress + source);
-        return SocketAddress.CopyOutAddress(address);
+
+        return SocketAddress.CopyOutAddress(source);
     }
 
     internal uint GetChunks(ref int dataChunkIndex, ref uint dataChunkOffset, byte[] buffer, int offset, int size)

src/Shared/test/Shared.Tests/NativeMethods.txt

@@ -1,2 +1,4 @@
 // https://github.com/microsoft/cswin32
 Windows.Win32.Networking.HttpServer
+SOCKADDR_IN
+SOCKADDR_IN6