From d5a57a9091a388b9779033e39053df6a047ec6bf Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 15 Apr 2024 17:34:14 +0200 Subject: [PATCH 1/7] Check for HttpRequestError.HttpProtocolError when expecting HTTP3 protocol errors in test --- .../System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs | 3 ++- .../tests/FunctionalTests/HttpClientHandlerTest.Http3.cs | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index 64ba4089d5dbca..72e4f29f65ed6a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -278,7 +278,8 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null) { // we close the connection, propagate the AbortException - throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, _connection.AbortException); + HttpRequestError httpRequestError = _connection.AbortException is HttpProtocolException ? HttpRequestError.HttpProtocolError : HttpRequestError.Unknown; + throw new HttpRequestException(httpRequestError, SR.net_http_client_execution_error, _connection.AbortException); } // It is possible for user's Content code to throw an unexpected OperationCanceledException. catch (OperationCanceledException ex) when (ex.CancellationToken == _requestBodyCancellationSource.Token || ex.CancellationToken == cancellationToken) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs index 16abc260386b42..dc9df7f8f0fc3e 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs @@ -35,7 +35,10 @@ private async Task AssertProtocolErrorAsync(long errorCode, Func task) { Exception outerEx = await Assert.ThrowsAnyAsync(task); _output.WriteLine(outerEx.ToString()); - Assert.IsType(outerEx); + + HttpRequestException httpReqException = Assert.IsType(outerEx); + Assert.Equal(HttpRequestError.HttpProtocolError, httpReqException.HttpRequestError); + HttpProtocolException protocolEx = Assert.IsType(outerEx.InnerException); Assert.Equal(errorCode, protocolEx.ErrorCode); } From 16be6cbbef242ba493683aa0b18ea1bc32b116d6 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 16 Apr 2024 12:34:16 +0200 Subject: [PATCH 2/7] WIP --- .../Net/Http/SocketsHttpHandler/Http3RequestStream.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index 72e4f29f65ed6a..86c1d6c18dee4a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -277,8 +277,15 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s } catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null) { + HttpRequestError httpRequestError = HttpRequestError.Unknown; + + // TODO: there are still some races + if (_connection.AbortException is HttpProtocolException) + { + httpRequestError = HttpRequestError.HttpProtocolError; + } + // we close the connection, propagate the AbortException - HttpRequestError httpRequestError = _connection.AbortException is HttpProtocolException ? HttpRequestError.HttpProtocolError : HttpRequestError.Unknown; throw new HttpRequestException(httpRequestError, SR.net_http_client_execution_error, _connection.AbortException); } // It is possible for user's Content code to throw an unexpected OperationCanceledException. From 89e5caea6d3b1155b006b46661bd8a9a3ea21898 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 17 Apr 2024 15:42:26 +0200 Subject: [PATCH 3/7] Fix data race in ServerClosesOutboundControlStream_ClientClosesConnection --- .../tests/FunctionalTests/HttpClientHandlerTest.Http3.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs index dc9df7f8f0fc3e..5baccdd3785543 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs @@ -1762,7 +1762,14 @@ public async Task ServerClosesOutboundControlStream_ClientClosesConnection(Close return; } await connection.OutboundControlStream.DisposeAsync(); - await connection.EstablishControlStreamAsync(Array.Empty()); + try + { + await connection.EstablishControlStreamAsync(Array.Empty()); + } + catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted && ex.ApplicationErrorCode == Http3LoopbackConnection.H3_CLOSED_CRITICAL_STREAM) + { + // Data race, connection closed between WaitAsync and EstablishControlStreamAsync. Ignore this. + } await Task.Delay(100); } } From 0eb8069e92b7201e76083b188533995d740a5003 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 17 Apr 2024 17:06:49 +0200 Subject: [PATCH 4/7] Handle more exceptions explicitly in Http3RequestStream --- .../aspnetcore/Http3/Frames/Http3ErrorCode.cs | 15 +++++++++++++++ .../Http/SocketsHttpHandler/Http3RequestStream.cs | 12 ++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/Frames/Http3ErrorCode.cs b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/Frames/Http3ErrorCode.cs index fdfe04cee1580b..ebaa96f12cdc46 100644 --- a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/Frames/Http3ErrorCode.cs +++ b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/Frames/Http3ErrorCode.cs @@ -91,5 +91,20 @@ internal enum Http3ErrorCode : long /// The requested operation cannot be served over HTTP/3. The peer should retry over HTTP/1.1. /// VersionFallback = 0x110, + /// + /// H3_QPACK_DECOMPRESSION_FAILED (0x200): + /// The decoder failed to interpret an encoded field section and is not able to continue decoding that field section. + /// + QPackDecompressionFailed = 0x200, + /// + /// H3_QPACK_ENCODER_STREAM_ERROR (0x201): + /// The decoder failed to interpret an encoder instruction received on the encoder stream. + /// + QPackEncoderStreamError = 0x201, + /// + /// H3_QPACK_DECODER_STREAM_ERROR (0x202): + /// The encoder failed to interpret an decoder instruction received on the decoder stream. + /// + QPackDecoderStreamError = 0x202, } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index 86c1d6c18dee4a..94775343d70aa1 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -308,6 +308,16 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s _connection.Abort(ex); throw new HttpRequestException(ex.HttpRequestError, SR.net_http_client_execution_error, ex); } + catch (QPackDecodingException ex) + { + Exception abortException = _connection.Abort(HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.QPackDecompressionFailed)); + throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response, ex); + } + catch (QPackEncodingException ex) + { + _stream.Abort(QuicAbortDirection.Write, (long)Http3ErrorCode.InternalError); + throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, ex); + } catch (Exception ex) { _stream.Abort(QuicAbortDirection.Write, (long)Http3ErrorCode.InternalError); @@ -315,6 +325,8 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s { throw; } + // all exceptions should be already handled above + Debug.Fail($"Unexpected exception type in Http3RequestStream.SendAsync: {ex}"); throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, ex); } finally From 3e440e52529435f9db2cf2e321fe2799a17cad53 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 18 Apr 2024 09:37:59 +0200 Subject: [PATCH 5/7] WIP --- .../Net/Http/SocketsHttpHandler/Http3RequestStream.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index 94775343d70aa1..12420f3a3217ad 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -277,15 +277,17 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s } catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null) { + // we closed the connection already, propagate the AbortException HttpRequestError httpRequestError = HttpRequestError.Unknown; - // TODO: there are still some races if (_connection.AbortException is HttpProtocolException) { httpRequestError = HttpRequestError.HttpProtocolError; } - // we close the connection, propagate the AbortException + // TODO: there are still some races? + Debug.Assert(httpRequestError != HttpRequestError.Unknown); + throw new HttpRequestException(httpRequestError, SR.net_http_client_execution_error, _connection.AbortException); } // It is possible for user's Content code to throw an unexpected OperationCanceledException. From dbfa1634ea50a374c9837e6572dc748fdeddc5b3 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 15 May 2024 14:58:28 +0200 Subject: [PATCH 6/7] Remove debug guard --- .../Http/SocketsHttpHandler/Http3RequestStream.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index 12420f3a3217ad..dd3cc2bc85f7a1 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -278,15 +278,9 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null) { // we closed the connection already, propagate the AbortException - HttpRequestError httpRequestError = HttpRequestError.Unknown; - - if (_connection.AbortException is HttpProtocolException) - { - httpRequestError = HttpRequestError.HttpProtocolError; - } - - // TODO: there are still some races? - Debug.Assert(httpRequestError != HttpRequestError.Unknown); + HttpRequestError httpRequestError = _connection.AbortException is HttpProtocolException + ? HttpRequestError.HttpProtocolError + : HttpRequestError.Unknown; throw new HttpRequestException(httpRequestError, SR.net_http_client_execution_error, _connection.AbortException); } @@ -327,6 +321,7 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s { throw; } + // all exceptions should be already handled above Debug.Fail($"Unexpected exception type in Http3RequestStream.SendAsync: {ex}"); throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, ex); From 768b28e8f818f4ed0d9f08cb0e8af2547b039b77 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 15 May 2024 18:19:12 +0200 Subject: [PATCH 7/7] Fix #102202 --- .../Net/Http/SocketsHttpHandler/Http3RequestStream.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index dd3cc2bc85f7a1..ef2532b2b22d08 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -1270,6 +1270,13 @@ private void HandleReadResponseContentException(Exception ex, CancellationToken _connection.Abort(exception); throw exception; + case QuicException e when (e.QuicError == QuicError.OperationAborted && _connection.AbortException != null): + // we closed the connection already, propagate the AbortException + HttpRequestError httpRequestError = _connection.AbortException is HttpProtocolException + ? HttpRequestError.HttpProtocolError + : HttpRequestError.Unknown; + throw new HttpRequestException(httpRequestError, SR.net_http_client_execution_error, _connection.AbortException); + case HttpIOException: _connection.Abort(ex); ExceptionDispatchInfo.Throw(ex); // Rethrow.