wrap exceptions from callbacks in QuicError.CallbackError (#88614)

* wrap exceptions from callbacks in QuicError.CallbackError

* feedback

diff --git a/src/libraries/System.Net.Quic/src/Resources/Strings.resx b/src/libraries/System.Net.Quic/src/Resources/Strings.resx
index 0aff3df..dc8b015 100644 (file)
--- a/src/libraries/System.Net.Quic/src/Resources/Strings.resx
+++ b/src/libraries/System.Net.Quic/src/Resources/Strings.resx
@@ -195,21 +195,12 @@
   <data name="net_quic_accept_not_allowed" xml:space="preserve">
     <value>QuicConnection is configured to not accept any streams.</value>
   </data>
-  <data name="net_quic_address_in_use" xml:space="preserve">
-    <value>The local address is already in use.</value>
-  </data>
-  <data name="net_quic_host_unreachable" xml:space="preserve">
-    <value>The server is currently unreachable.</value>
-  </data>
   <data name="net_quic_connection_refused" xml:space="preserve">
     <value>The server refused the connection.</value>
   </data>
   <data name="net_quic_protocol_error" xml:space="preserve">
     <value>A QUIC protocol error was encountered</value>
   </data>
-  <data name="net_quic_alpn_in_use" xml:space="preserve">
-    <value>Another QUIC listener is already listening on one of the requested application protocols on the same port.</value>
-  </data>
   <data name="net_quic_ver_neg_error" xml:space="preserve">
     <value>A version negotiation error was encountered.</value>
   </data>
@@ -219,9 +210,6 @@
   <data name="net_quic_connection_idle" xml:space="preserve">
     <value>The connection timed out from inactivity.</value>
   </data>
-  <data name="net_quic_invalid_address" xml:space="preserve">
-    <value>Binding to socket failed, likely caused by a family mismatch between local and remote address.</value>
-  </data>
   <data name="net_quic_auth" xml:space="preserve">
     <value>Authentication failed: {0}.</value>
   </data>
@@ -239,5 +227,8 @@
   <data name="net_InvalidSocketAddressSize" xml:space="preserve">
     <value>The supplied {0} is an invalid size for the {1} end point.</value>
   </data>
+  <data name="net_quic_callback_error" xml:space="preserve">
+    <value>User configured callback failed.</value>
+  </data>
 </root>
 

diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.SslConnectionOptions.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.SslConnectionOptions.cs
index 333ef43..dad23bf 100644 (file)
--- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.SslConnectionOptions.cs
+++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.SslConnectionOptions.cs
@@ -68,6 +68,7 @@ public partial class QuicConnection
             SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None;
             IntPtr certificateBuffer = 0;
             int certificateLength = 0;
+            bool wrapException = false;
 
             X509Chain? chain = null;
             X509Certificate2? result = null;
@@ -130,8 +131,10 @@ public partial class QuicConnection
                 int status = QUIC_STATUS_SUCCESS;
                 if (_validationCallback is not null)
                 {
+                    wrapException = true;
                     if (!_validationCallback(_connection, result, chain, sslPolicyErrors))
                     {
+                        wrapException = false;
                         if (_isClient)
                         {
                             throw new AuthenticationException(SR.net_quic_cert_custom_validation);
@@ -153,9 +156,14 @@ public partial class QuicConnection
                 certificate = result;
                 return status;
             }
-            catch
+            catch (Exception ex)
             {
                 result?.Dispose();
+                if (wrapException)
+                {
+                    throw new QuicException(QuicError.CallbackError, null, SR.net_quic_callback_error, ex);
+                }
+
                 throw;
             }
             finally

diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs
index 7b4c661..eac3218 100644 (file)
--- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs
+++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs
@@ -209,13 +209,16 @@ public sealed partial class QuicListener : IAsyncDisposable
     /// <param name="clientHello">The TLS ClientHello data.</param>
     private async void StartConnectionHandshake(QuicConnection connection, SslClientHelloInfo clientHello)
     {
+        bool wrapException = false;
         CancellationToken cancellationToken = default;
         try
         {
             using CancellationTokenSource linkedCts = CancellationTokenSource.CreateLinkedTokenSource(_disposeCts.Token);
             linkedCts.CancelAfter(QuicDefaults.HandshakeTimeout);
             cancellationToken = linkedCts.Token;
+            wrapException = true;
             QuicServerConnectionOptions options = await _connectionOptionsCallback(connection, clientHello, cancellationToken).ConfigureAwait(false);
+            wrapException = false;
             options.Validate(nameof(options)); // Validate and fill in defaults for the options.
             await connection.FinishHandshakeAsync(options, clientHello.ServerName, cancellationToken).ConfigureAwait(false);
             if (!_acceptQueue.Writer.TryWrite(connection))
@@ -267,7 +270,10 @@ public sealed partial class QuicListener : IAsyncDisposable
             }
 
             await connection.DisposeAsync().ConfigureAwait(false);
-            if (!_acceptQueue.Writer.TryWrite(ex))
+            if (!_acceptQueue.Writer.TryWrite(
+                    wrapException ?
+                        ExceptionDispatchInfo.SetCurrentStackTrace(new QuicException(QuicError.CallbackError, null, SR.net_quic_callback_error, ex)) :
+                        ex))
             {
                 // Channel has been closed, connection is already disposed, do nothing.
             }

diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTests.cs
index 1a15e3f..0471fdb 100644 (file)
--- a/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTests.cs
+++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTests.cs
@@ -384,7 +384,8 @@ namespace System.Net.Quic.Tests
 
             clientOptions.ClientAuthenticationOptions.TargetHost = "foobar1";
 
-            await Assert.ThrowsAsync<ArithmeticException>(() => CreateQuicConnection(clientOptions).AsTask());
+            Exception exception = await AssertThrowsQuicExceptionAsync(QuicError.CallbackError, async () => await CreateQuicConnection(clientOptions));
+            Assert.True(exception.InnerException is ArithmeticException);
             await Assert.ThrowsAsync<AuthenticationException>(async () => await listener.AcceptConnectionAsync());
 
             // Make sure the listener is still usable and there is no lingering bad connection

diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicListenerTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicListenerTests.cs
index 68790da..68509bf 100644 (file)
--- a/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicListenerTests.cs
+++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/QuicListenerTests.cs
@@ -96,8 +96,42 @@ namespace System.Net.Quic.Tests
             await using QuicListener listener = await CreateQuicListener(listenerOptions);
 
             ValueTask<QuicConnection> connectTask = CreateQuicConnection(listener.LocalEndPoint);
-            Exception exception = await Assert.ThrowsAsync<Exception>(async () => await listener.AcceptConnectionAsync());
-            Assert.Equal(expectedMessage, exception.Message);
+
+            Exception exception = await AssertThrowsQuicExceptionAsync(QuicError.CallbackError, async () => await listener.AcceptConnectionAsync());
+            Assert.NotNull(exception.InnerException);
+            Assert.Equal(expectedMessage, exception.InnerException.Message);
+            await Assert.ThrowsAsync<AuthenticationException>(() => connectTask.AsTask());
+        }
+
+        [Fact]
+        public async Task AcceptConnectionAsync_ThrowingCallbackOde_KeepRunning()
+        {
+            bool firstRun = true;
+
+            QuicListenerOptions listenerOptions = CreateQuicListenerOptions();
+            // Throw an exception, which should throw the same from accept.
+            listenerOptions.ConnectionOptionsCallback = (_, _, _) =>
+            {
+                if (firstRun)
+                {
+                    firstRun = false;
+                    throw new ObjectDisposedException("failed");
+                }
+
+                return ValueTask.FromResult(CreateQuicServerOptions());
+            };
+            await using QuicListener listener = await CreateQuicListener(listenerOptions);
+
+            ValueTask<QuicConnection> connectTask = CreateQuicConnection(listener.LocalEndPoint);
+
+            Exception exception = await AssertThrowsQuicExceptionAsync(QuicError.CallbackError, async () => await listener.AcceptConnectionAsync());
+            Assert.True(exception.InnerException is ObjectDisposedException);
+            await Assert.ThrowsAsync<AuthenticationException>(() => connectTask.AsTask());
+
+            // Throwing ODE in callback should keep Listener running
+            connectTask = CreateQuicConnection(listener.LocalEndPoint);
+            await using QuicConnection serverConnection = await listener.AcceptConnectionAsync();
+            await using QuicConnection clientConnection = await connectTask;
         }
 
         [Theory]