Code review feedback

src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.NativeMethods.cs

@@ -376,13 +376,47 @@ public int StreamReceiveSetEnabled(MsQuicSafeHandle stream, byte enabled)
         }
     }
 
-    public int ConnectionCertificateValidationComplete(MsQuicSafeHandle connection, bool result, QUIC_TLS_ALERT_CODES alert)
+    public int DatagramSend(MsQuicSafeHandle connection, QUIC_BUFFER* buffers, uint buffersCount, QUIC_SEND_FLAGS flags, void* context)
     {
         bool success = false;
         try
         {
             connection.DangerousAddRef(ref success);
-            return ApiTable->ConnectionCertificateValidationComplete(connection.QuicHandle, (byte)(result ? 1 : 0), alert);
+            return ApiTable->DatagramSend(connection.QuicHandle, buffers, buffersCount, flags, context);
+        }
+        finally
+        {
+            if (success)
+            {
+                connection.DangerousRelease();
+            }
+        }
+    }
+
+    public int ConnectionResumptionTicketValidationComplete(MsQuicSafeHandle connection, byte result)
+    {
+        bool success = false;
+        try
+        {
+            connection.DangerousAddRef(ref success);
+            return ApiTable->ConnectionResumptionTicketValidationComplete(connection.QuicHandle, result);
+        }
+        finally
+        {
+            if (success)
+            {
+                connection.DangerousRelease();
+            }
+        }
+    }
+
+    public int ConnectionCertificateValidationComplete(MsQuicSafeHandle connection, byte result, QUIC_TLS_ALERT_CODES alert)
+    {
+        bool success = false;
+        try
+        {
+            connection.DangerousAddRef(ref success);
+            return ApiTable->ConnectionCertificateValidationComplete(connection.QuicHandle, result, alert);
         }
         finally
         {

src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.SslConnectionOptions.cs

@@ -1,10 +1,13 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Buffers;
+using System.Diagnostics;
 using System.Net.Security;
 using System.Security.Authentication;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
+using System.Threading.Tasks;
 using Microsoft.Quic;
 using static Microsoft.Quic.MsQuic;
 
@@ -63,7 +66,102 @@ public SslConnectionOptions(QuicConnection connection, bool isClient,
             _certificateChainPolicy = certificateChainPolicy;
         }
 
-        public unsafe QUIC_TLS_ALERT_CODES ValidateCertificate(X509Certificate2? certificate, Span<byte> certData, Span<byte> chainData)
+        internal unsafe void StartAsyncCertificateValidation(void* certificatePtr, void* chainPtr)
+        {
+            //
+            // the provided data pointers are valid only while still inside this function, so they need to be
+            // copied to separate buffers which are then handed off to threadpool.
+            //
+
+            X509Certificate2? certificate = null;
+
+            byte[]? certDataRented = null;
+            Memory<byte> certData = default;
+            byte[]? chainDataRented = null;
+            Memory<byte> chainData = default;
+
+            if (certificatePtr != null)
+            {
+                if (MsQuicApi.UsesSChannelBackend)
+                {
+                    certificate = new X509Certificate2((IntPtr)certificatePtr);
+                    // TODO: what about chainPtr?
+                }
+                else
+                {
+                    // on non-SChannel backends we specify USE_PORTABLE_CERTIFICATES and the content is buffers
+                    // with DER encoded cert and chain
+                    QUIC_BUFFER* certificateBuffer = (QUIC_BUFFER*)certificatePtr;
+                    QUIC_BUFFER* chainBuffer = (QUIC_BUFFER*)chainPtr;
+
+                    if (certificateBuffer->Length > 0)
+                    {
+                        certDataRented = ArrayPool<byte>.Shared.Rent((int)certificateBuffer->Length);
+                        certData = certDataRented.AsMemory(0, (int)certificateBuffer->Length);
+                        certificateBuffer->Span.CopyTo(certData.Span);
+                    }
+
+                    if (chainBuffer->Length > 0)
+                    {
+                        chainDataRented = ArrayPool<byte>.Shared.Rent((int)chainBuffer->Length);
+                        chainData = chainDataRented.AsMemory(0, (int)chainBuffer->Length);
+                        chainBuffer->Span.CopyTo(chainData.Span);
+                    }
+                }
+            }
+
+            // hand-off rest of the work to the threadpool, certificatePtr and chainPtr are invalid inside the lambda
+            QuicConnection thisConnection = _connection; // cannot use "this" inside lambda since SslConnectionOptions is struct
+            _ = Task.Run(() =>
+            {
+
+                QUIC_TLS_ALERT_CODES result;
+                try
+                {
+                    if (certData.Length > 0)
+                    {
+                        Debug.Assert(certificate == null);
+                        certificate = new X509Certificate2(certData.Span);
+                    }
+
+                    result = thisConnection._sslConnectionOptions.ValidateCertificate(certificate, certData.Span, chainData.Span);
+                    thisConnection._remoteCertificate = certificate;
+                }
+                catch (Exception ex)
+                {
+                    certificate?.Dispose();
+                    thisConnection._connectedTcs.TrySetException(ex);
+                    result = QUIC_TLS_ALERT_CODES.USER_CANCELED;
+                }
+                finally
+                {
+                    if (certDataRented != null)
+                    {
+                        ArrayPool<byte>.Shared.Return(certDataRented);
+                    }
+
+                    if (chainDataRented != null)
+                    {
+                        ArrayPool<byte>.Shared.Return(chainDataRented);
+                    }
+                }
+
+                int status = MsQuicApi.Api.ConnectionCertificateValidationComplete(
+                    thisConnection._handle,
+                    result == QUIC_TLS_ALERT_CODES.SUCCESS ? (byte)1 : (byte)0,
+                    result);
+
+                if (MsQuic.StatusFailed(status))
+                {
+                    if (NetEventSource.Log.IsEnabled())
+                    {
+                        NetEventSource.Error(thisConnection, $"{thisConnection} ConnectionCertificateValidationComplete failed with {ThrowHelper.GetErrorMessageForStatus(status)}");
+                    }
+                }
+            });
+        }
+
+        private QUIC_TLS_ALERT_CODES ValidateCertificate(X509Certificate2? certificate, Span<byte> certData, Span<byte> chainData)
         {
             SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None;
             bool wrapException = false;

src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Buffers;
 using System.Diagnostics;
 using System.Net.Security;
 using System.Net.Sockets;
@@ -578,88 +577,8 @@ private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEI
         // also prevents potential user RemoteCertificateValidationCallback from blocking MsQuic
         // worker threads.
         //
-        // the provided data pointers are valid only while still inside the callback so they need to be
-        // copied before handing them to threadpool.
-        //
-
-        X509Certificate2? certificate = null;
-
-        byte[]? certDataRented = null;
-        Memory<byte> certData = default;
-        byte[]? chainDataRented = null;
-        Memory<byte> chainData = default;
-
-        if (data.Certificate != null)
-        {
-            if (MsQuicApi.UsesSChannelBackend)
-            {
-                certificate = new X509Certificate2((IntPtr)data.Certificate);
-                // TODO: what about chainPtr?
-            }
-            else
-            {
-                // on non-SChannel backends we specify USE_PORTABLE_CERTIFICATES and the content is buffers
-                // with DER encoded cert and chain
-                QUIC_BUFFER* certificatePtr = (QUIC_BUFFER*)data.Certificate;
-                QUIC_BUFFER* chainPtr = (QUIC_BUFFER*)data.Chain;
-
-                if (certificatePtr->Length > 0)
-                {
-                    certDataRented = ArrayPool<byte>.Shared.Rent((int)certificatePtr->Length);
-                    certData = certDataRented.AsMemory(0, (int)certificatePtr->Length);
-                    certificatePtr->Span.CopyTo(certData.Span);
-                }
-
-                if (chainPtr->Length > 0)
-                {
-                    chainDataRented = ArrayPool<byte>.Shared.Rent((int)chainPtr->Length);
-                    chainData = chainDataRented.AsMemory(0, (int)chainPtr->Length);
-                    chainPtr->Span.CopyTo(chainData.Span);
-                }
-            }
-        }
-
-        _ = Task.Run(() =>
-        {
-            QUIC_TLS_ALERT_CODES result;
-            try
-            {
-                if (certData.Length > 0)
-                {
-                    Debug.Assert(certificate == null);
-                    certificate = new X509Certificate2(certData.Span);
-                }
-
-                result = _sslConnectionOptions.ValidateCertificate(certificate, certData.Span, chainData.Span);
-                _remoteCertificate = certificate;
-            }
-            catch (Exception ex)
-            {
-                certificate?.Dispose();
-                _connectedTcs.TrySetException(ex);
-                result = QUIC_TLS_ALERT_CODES.USER_CANCELED;
-            }
-            finally
-            {
-                if (certDataRented != null)
-                {
-                    ArrayPool<byte>.Shared.Return(certDataRented);
-                }
-
-                if (chainDataRented != null)
-                {
-                    ArrayPool<byte>.Shared.Return(chainDataRented);
-                }
-            }
-
-            int status = MsQuicApi.Api.ConnectionCertificateValidationComplete(
-                _handle,
-                result == QUIC_TLS_ALERT_CODES.SUCCESS,
-                result);
-
-            Debug.Assert(MsQuic.StatusSucceeded(status), $"ConnectionCertificateValidationComplete failed with 0x{status:X}");
-        });
 
+        _sslConnectionOptions.StartAsyncCertificateValidation(data.Certificate, data.Chain);
         return QUIC_STATUS_PENDING;
     }
 
@@ -677,7 +596,6 @@ private unsafe int HandleConnectionEvent(ref QUIC_CONNECTION_EVENT connectionEve
             _ => QUIC_STATUS_SUCCESS,
         };
 
-
 #pragma warning disable CS3016
     [UnmanagedCallersOnly(CallConvs = new Type[] { typeof(CallConvCdecl) })]
 #pragma warning restore CS3016