Remove most unsafe code from SocketAddress (#61006)

Part of the overall "reduce unsafe" effort org-wide.

Author: GrabYourPitchforks

Diff for: src/Servers/HttpSys/src/NativeMethods.txt

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

Diff for: src/Servers/IIS/IIS/src/NativeMethods.txt

@@ -14,3 +14,5 @@ HTTP_REQUEST_PROPERTY_SNI_HOST_MAX_LENGTH
 HTTP_REQUEST_V1
 HTTP_REQUEST_V2
 HTTP_SSL_PROTOCOL_INFO
+SOCKADDR_IN
+SOCKADDR_IN6

Diff for: src/Shared/HttpSys/NativeInterop/SocketAddress.cs

@@ -1,96 +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((short)_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)
-    {
-        var addressFamily = *((ushort*)address);
-        if (addressFamily == (ushort)AddressFamily.InterNetwork)
+        internal SocketAddressIPv6(in SOCKADDR_IN6 sockaddr)
         {
-            var v4address = new SocketAddress(AddressFamily.InterNetwork, IPv4AddressSize);
-            fixed (byte* pBuffer = v4address._buffer)
-            {
-                for (var index = 2; index < IPv4AddressSize; index++)
-                {
-                    pBuffer[index] = ((byte*)address)[index];
-                }
-            }
-            return v4address;
+            _sockaddr = sockaddr;
         }
-        if (addressFamily == (ushort)AddressFamily.InterNetworkV6)
+
+        internal override int GetPort()
         {
-            var v6address = new SocketAddress(AddressFamily.InterNetworkV6, IPv6AddressSize);
-            fixed (byte* pBuffer = v6address._buffer)
-            {
-                for (var index = 2; index < IPv6AddressSize; index++)
-                {
-                    pBuffer[index] = ((byte*)address)[index];
-                }
-            }
-            return v6address;
+            // sin6_port is network byte order
+            return IPAddress.NetworkToHostOrder((short)_sockaddr.sin6_port);
         }
 
-        return null;
+        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?
+        }
     }
 }

Diff for: src/Shared/HttpSys/RequestProcessing/NativeRequestContext.cs

@@ -17,6 +17,7 @@
 using Microsoft.Extensions.Primitives;
 using Windows.Win32;
 using Windows.Win32.Networking.HttpServer;
+using Windows.Win32.Networking.WinSock;
 
 namespace Microsoft.AspNetCore.HttpSys.Internal;
 
@@ -656,7 +657,8 @@ private void GetUnknownHeadersHelper(IDictionary<string, StringValues> unknownHe
         {
             return null;
         }
-        var address = (IntPtr)(pMemoryBlob + _bufferAlignment - (byte*)_originalBufferAddress + source);
+
+        var address = (SOCKADDR*)(pMemoryBlob + _bufferAlignment - (byte*)_originalBufferAddress + source);
         return SocketAddress.CopyOutAddress(address);
     }
 

Diff for: src/Shared/test/Shared.Tests/NativeMethods.txt

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