From 5eff2eb60b7a2fc7e245f77c8468135e0b7e2f94 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 13 Jul 2022 14:09:38 +0200 Subject: [PATCH 1/9] Replace Http3ProtocolException with HttpProtocolException --- .../src/Resources/Strings.resx | 3 + .../src/System.Net.Http.csproj | 2 - .../System/Net/Http/HttpProtocolException.cs | 35 ++++++++++++ .../SocketsHttpHandler/Http3Connection.cs | 38 ++++++------- .../Http3ConnectionException.cs | 24 -------- .../Http3ProtocolException.cs | 56 ------------------- .../SocketsHttpHandler/Http3RequestStream.cs | 18 +++--- .../src/System/Net/Quic/QuicException.cs | 3 + 8 files changed, 69 insertions(+), 110 deletions(-) delete mode 100644 src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ConnectionException.cs delete mode 100644 src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ProtocolException.cs diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 3ac957f5d8107c..be79102d91d2a9 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -411,6 +411,9 @@ The HTTP/2 server reset the stream. HTTP/2 error code '{0}' (0x{1}). + + The HTTP/3 server reset the stream. HTTP/3 error code '{0}' (0x{1}). + An HTTP/2 connection could not be established because the server did not complete the HTTP/2 handshake. diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index f27128545761c6..3819a658d78bae 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -181,8 +181,6 @@ - - diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs index 5d3e914aeac347..90de2b179f99b5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs @@ -47,6 +47,18 @@ internal static HttpProtocolException CreateHttp2ConnectionException(Http2Protoc return new HttpProtocolException((long)protocolError, message, null); } + internal static HttpProtocolException CreateHttp3StreamException(Http3ErrorCode protocolError) + { + string message = SR.Format(SR.net_http_http3_stream_error, GetName(protocolError), ((int)protocolError).ToString("x")); + return new HttpProtocolException((long)protocolError, message, null); + } + + internal static HttpProtocolException CreateHttp3ConnectionException(Http3ErrorCode protocolError) + { + string message = SR.Format(SR.net_http_http3_connection_error, GetName(protocolError), ((int)protocolError).ToString("x")); + return new HttpProtocolException((long)protocolError, message, null); + } + private static string GetName(Http2ProtocolErrorCode code) => // These strings are the names used in the HTTP2 spec and should not be localized. code switch @@ -67,6 +79,29 @@ private static string GetName(Http2ProtocolErrorCode code) => Http2ProtocolErrorCode.Http11Required => "HTTP_1_1_REQUIRED", _ => "(unknown error)", }; + + private static string GetName(Http3ErrorCode code) => + // These strings come from the H3 spec and should not be localized. + code switch + { + Http3ErrorCode.NoError => "H3_NO_ERROR", + Http3ErrorCode.ProtocolError => "H3_GENERAL_PROTOCOL_ERROR", + Http3ErrorCode.InternalError => "H3_INTERNAL_ERROR", + Http3ErrorCode.StreamCreationError => "H3_STREAM_CREATION_ERROR", + Http3ErrorCode.ClosedCriticalStream => "H3_CLOSED_CRITICAL_STREAM", + Http3ErrorCode.UnexpectedFrame => "H3_FRAME_UNEXPECTED", + Http3ErrorCode.FrameError => "H3_FRAME_ERROR", + Http3ErrorCode.ExcessiveLoad => "H3_EXCESSIVE_LOAD", + Http3ErrorCode.IdError => "H3_ID_ERROR", + Http3ErrorCode.SettingsError => "H3_SETTINGS_ERROR", + Http3ErrorCode.MissingSettings => "H3_MISSING_SETTINGS", + Http3ErrorCode.RequestRejected => "H3_REQUEST_REJECTED", + Http3ErrorCode.RequestCancelled => "H3_REQUEST_CANCELLED", + Http3ErrorCode.RequestIncomplete => "H3_REQUEST_INCOMPLETE", + Http3ErrorCode.ConnectError => "H3_CONNECT_ERROR", + Http3ErrorCode.VersionFallback => "H3_VERSION_FALLBACK", + _ => "(unknown error)" + }; #endif } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index eabd710dc676e5..c0c5ddb582609b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -271,7 +271,7 @@ internal Exception Abort(Exception abortException) // Stop sending requests to this connection. _pool.InvalidateHttp3Connection(this); - Http3ErrorCode connectionResetErrorCode = (abortException as Http3ProtocolException)?.ErrorCode ?? Http3ErrorCode.InternalError; + long connectionResetErrorCode = (abortException as HttpProtocolException)?.ErrorCode ?? (long)Http3ErrorCode.InternalError; lock (SyncObj) { @@ -443,7 +443,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) if (stream.CanWrite) { // Server initiated bidirectional streams are either push streams or extensions, and we support neither. - throw new Http3ConnectionException(Http3ErrorCode.StreamCreationError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.StreamCreationError); } buffer = new ArrayBuffer(initialSize: 32, usePool: true); @@ -478,7 +478,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) if (Interlocked.Exchange(ref _haveServerControlStream, 1) != 0) { // A second control stream has been received. - throw new Http3ConnectionException(Http3ErrorCode.StreamCreationError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.StreamCreationError); } // Discard the stream type header. @@ -494,7 +494,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) if (Interlocked.Exchange(ref _haveServerQpackDecodeStream, 1) != 0) { // A second QPack decode stream has been received. - throw new Http3ConnectionException(Http3ErrorCode.StreamCreationError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.StreamCreationError); } // The stream must not be closed, but we aren't using QPACK right now -- ignore. @@ -505,7 +505,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) if (Interlocked.Exchange(ref _haveServerQpackEncodeStream, 1) != 0) { // A second QPack encode stream has been received. - throw new Http3ConnectionException(Http3ErrorCode.StreamCreationError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.StreamCreationError); } // We haven't enabled QPack in our SETTINGS frame, so we shouldn't receive any meaningful data here. @@ -516,7 +516,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) case (byte)Http3StreamType.Push: // We don't support push streams. // Because no maximum push stream ID was negotiated via a MAX_PUSH_ID frame, server should not have sent this. Abort the connection with H3_ID_ERROR. - throw new Http3ConnectionException(Http3ErrorCode.IdError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.IdError); default: // Unknown stream type. Per spec, these must be ignored and aborted but not be considered a connection-level error. @@ -573,12 +573,12 @@ private async Task ProcessServerControlStreamAsync(QuicStream stream, ArrayBuffe if (frameType == null) { // Connection closed prematurely, expected SETTINGS frame. - throw new Http3ConnectionException(Http3ErrorCode.ClosedCriticalStream); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream); } if (frameType != Http3FrameType.Settings) { - throw new Http3ConnectionException(Http3ErrorCode.MissingSettings); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.MissingSettings); } await ProcessSettingsFrameAsync(payloadLength).ConfigureAwait(false); @@ -596,7 +596,7 @@ private async Task ProcessServerControlStreamAsync(QuicStream stream, ArrayBuffe break; case Http3FrameType.Settings: // If an endpoint receives a second SETTINGS frame on the control stream, the endpoint MUST respond with a connection error of type H3_FRAME_UNEXPECTED. - throw new Http3ConnectionException(Http3ErrorCode.UnexpectedFrame); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame); case Http3FrameType.Headers: // Servers should not send these frames to a control stream. case Http3FrameType.Data: case Http3FrameType.MaxPushId: @@ -604,11 +604,11 @@ private async Task ProcessServerControlStreamAsync(QuicStream stream, ArrayBuffe case Http3FrameType.ReservedHttp2Ping: case Http3FrameType.ReservedHttp2WindowUpdate: case Http3FrameType.ReservedHttp2Continuation: - throw new Http3ConnectionException(Http3ErrorCode.UnexpectedFrame); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame); case Http3FrameType.PushPromise: case Http3FrameType.CancelPush: // Because we haven't sent any MAX_PUSH_ID frame, it is invalid to receive any push-related frames as they will all reference a too-large ID. - throw new Http3ConnectionException(Http3ErrorCode.IdError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.IdError); case null: // End of stream reached. If we're shutting down, stop looping. Otherwise, this is an error (this stream should not be closed for life of connection). bool shuttingDown; @@ -618,7 +618,7 @@ private async Task ProcessServerControlStreamAsync(QuicStream stream, ArrayBuffe } if (!shuttingDown) { - throw new Http3ConnectionException(Http3ErrorCode.ClosedCriticalStream); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream); } return; default: @@ -650,7 +650,7 @@ private async Task ProcessServerControlStreamAsync(QuicStream stream, ArrayBuffe else { // Our buffer has partial frame data in it but not enough to complete the read: bail out. - throw new Http3ConnectionException(Http3ErrorCode.FrameError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.FrameError); } } @@ -678,7 +678,7 @@ async ValueTask ProcessSettingsFrameAsync(long settingsPayloadLength) else { // Our buffer has partial frame data in it but not enough to complete the read: bail out. - throw new Http3ConnectionException(Http3ErrorCode.FrameError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.FrameError); } } @@ -688,7 +688,7 @@ async ValueTask ProcessSettingsFrameAsync(long settingsPayloadLength) { // An integer was encoded past the payload length. // A frame payload that contains additional bytes after the identified fields or a frame payload that terminates before the end of the identified fields MUST be treated as a connection error of type H3_FRAME_ERROR. - throw new Http3ConnectionException(Http3ErrorCode.FrameError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.FrameError); } buffer.Discard(bytesRead); @@ -704,7 +704,7 @@ async ValueTask ProcessSettingsFrameAsync(long settingsPayloadLength) case Http3SettingType.ReservedHttp2MaxFrameSize: // Per https://tools.ietf.org/html/draft-ietf-quic-http-31#section-7.2.4.1 // these settings IDs are reserved and must never be sent. - throw new Http3ConnectionException(Http3ErrorCode.SettingsError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.SettingsError); } } } @@ -726,7 +726,7 @@ async ValueTask ProcessGoAwayFrameAsync(long goawayPayloadLength) else { // Our buffer has partial frame data in it but not enough to complete the read: bail out. - throw new Http3ConnectionException(Http3ErrorCode.FrameError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.FrameError); } } @@ -734,7 +734,7 @@ async ValueTask ProcessGoAwayFrameAsync(long goawayPayloadLength) if (bytesRead != goawayPayloadLength) { // Frame contains unknown extra data after the integer. - throw new Http3ConnectionException(Http3ErrorCode.FrameError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.FrameError); } OnServerGoAway(firstRejectedStreamId); @@ -755,7 +755,7 @@ async ValueTask SkipUnknownPayloadAsync(Http3FrameType frameType, long payloadLe else { // Our buffer has partial frame data in it but not enough to complete the read: bail out. - throw new Http3ConnectionException(Http3ErrorCode.FrameError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.FrameError); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ConnectionException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ConnectionException.cs deleted file mode 100644 index dbb22d36a79c5e..00000000000000 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ConnectionException.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.Serialization; -using System.Runtime.Versioning; - -namespace System.Net.Http -{ - [Serializable] - [SupportedOSPlatform("windows")] - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macos")] - internal sealed class Http3ConnectionException : Http3ProtocolException - { - public Http3ConnectionException(Http3ErrorCode errorCode) - : base(SR.Format(SR.net_http_http3_connection_error, GetName(errorCode), ((long)errorCode).ToString("x")), errorCode) - { - } - - private Http3ConnectionException(SerializationInfo info, StreamingContext context) : base(info, context) - { - } - } -} diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ProtocolException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ProtocolException.cs deleted file mode 100644 index c3731643a238c1..00000000000000 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ProtocolException.cs +++ /dev/null @@ -1,56 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.Serialization; -using System.Runtime.Versioning; - -namespace System.Net.Http -{ - [Serializable] - [SupportedOSPlatform("windows")] - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macos")] - internal class Http3ProtocolException : Exception - { - public Http3ErrorCode ErrorCode { get; } - - protected Http3ProtocolException(string message, Http3ErrorCode errorCode) : base(message) - { - ErrorCode = errorCode; - } - - protected Http3ProtocolException(SerializationInfo info, StreamingContext context) : base(info, context) - { - ErrorCode = (Http3ErrorCode)info.GetUInt32(nameof(ErrorCode)); - } - - public override void GetObjectData(SerializationInfo info, StreamingContext context) - { - info.AddValue(nameof(ErrorCode), (uint)ErrorCode); - base.GetObjectData(info, context); - } - - protected static string GetName(Http3ErrorCode errorCode) => - // These strings come from the H3 spec and should not be localized. - errorCode switch - { - Http3ErrorCode.NoError => "H3_NO_ERROR (0x100)", - Http3ErrorCode.ProtocolError => "H3_GENERAL_PROTOCOL_ERROR (0x101)", - Http3ErrorCode.InternalError => "H3_INTERNAL_ERROR (0x102)", - Http3ErrorCode.StreamCreationError => "H3_STREAM_CREATION_ERROR (0x103)", - Http3ErrorCode.ClosedCriticalStream => "H3_CLOSED_CRITICAL_STREAM (0x104)", - Http3ErrorCode.UnexpectedFrame => "H3_FRAME_UNEXPECTED (0x105)", - Http3ErrorCode.FrameError => "H3_FRAME_ERROR (0x106)", - Http3ErrorCode.ExcessiveLoad => "H3_EXCESSIVE_LOAD (0x107)", - Http3ErrorCode.IdError => "H3_ID_ERROR (0x108)", - Http3ErrorCode.SettingsError => "H3_SETTINGS_ERROR (0x109)", - Http3ErrorCode.MissingSettings => "H3_MISSING_SETTINGS (0x10A)", - Http3ErrorCode.RequestRejected => "H3_REQUEST_REJECTED (0x10B)", - Http3ErrorCode.RequestCancelled => "H3_REQUEST_CANCELLED (0x10C)", - Http3ErrorCode.RequestIncomplete => "H3_REQUEST_INCOMPLETE (0x10D)", - Http3ErrorCode.ConnectError => "H3_CONNECT_ERROR (0x10F)", - Http3ErrorCode.VersionFallback => "H3_VERSION_FALLBACK (0x110)", - _ => "(unknown error)" - }; - } -} 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 58a2159d3a0073..8e93e81a426faf 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 @@ -269,7 +269,7 @@ await Task.WhenAny(sendContentTask, readResponseTask).ConfigureAwait(false) == s throw new HttpRequestException(SR.net_http_request_aborted, ex, RequestRetryType.RetryOnConnectionFailure); } } - catch (Http3ConnectionException ex) + catch (HttpProtocolException ex) { // A connection-level protocol error has occurred on our stream. _connection.Abort(ex); @@ -794,12 +794,12 @@ private void BufferBytes(ReadOnlySpan span) case Http3FrameType.ReservedHttp2Ping: case Http3FrameType.ReservedHttp2WindowUpdate: case Http3FrameType.ReservedHttp2Continuation: - throw new Http3ConnectionException(Http3ErrorCode.UnexpectedFrame); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame); case Http3FrameType.PushPromise: case Http3FrameType.CancelPush: // Because we haven't sent any MAX_PUSH_ID frames, any of these push-related // frames that the server sends will have an out-of-range push ID. - throw new Http3ConnectionException(Http3ErrorCode.IdError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.IdError); default: // Unknown frame types should be skipped. await SkipUnknownPayloadAsync(payloadLength, cancellationToken).ConfigureAwait(false); @@ -883,7 +883,7 @@ private void GetStaticQPackHeader(int index, out HeaderDescriptor descriptor, ou if (!HeaderDescriptor.TryGetStaticQPackHeader(index, out descriptor, out knownValue)) { if (NetEventSource.Log.IsEnabled()) Trace($"Response contains invalid static header index '{index}'."); - throw new Http3ConnectionException(Http3ErrorCode.ProtocolError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ProtocolError); } } @@ -899,13 +899,13 @@ private void OnHeader(int? staticIndex, HeaderDescriptor descriptor, string? sta if (!descriptor.Equals(KnownHeaders.PseudoStatus)) { if (NetEventSource.Log.IsEnabled()) Trace($"Received unknown pseudo-header '{descriptor.Name}'."); - throw new Http3ConnectionException(Http3ErrorCode.ProtocolError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ProtocolError); } if (_headerState != HeaderState.StatusHeader) { if (NetEventSource.Log.IsEnabled()) Trace("Received extra status header."); - throw new Http3ConnectionException(Http3ErrorCode.ProtocolError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ProtocolError); } int statusCode; @@ -996,7 +996,7 @@ int ParseStatusCode(int? index, string value) { case HeaderState.StatusHeader: if (NetEventSource.Log.IsEnabled()) Trace($"Received headers without :status."); - throw new Http3ConnectionException(Http3ErrorCode.ProtocolError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ProtocolError); case HeaderState.ResponseHeaders when descriptor.HeaderType.HasFlag(HttpHeaderType.Content): _response!.Content!.Headers.TryAddWithoutValidation(descriptor, headerValue); break; @@ -1034,7 +1034,7 @@ private async ValueTask SkipUnknownPayloadAsync(long payloadLength, Cancellation else { // Our buffer has partial frame data in it but not enough to complete the read: bail out. - throw new Http3ConnectionException(Http3ErrorCode.FrameError); + throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.FrameError); } } @@ -1196,7 +1196,7 @@ private void HandleReadResponseContentException(Exception ex, CancellationToken // Our connection was reset. Start aborting the connection. Exception abortException = _connection.Abort(ex); throw new IOException(SR.net_http_client_execution_error, new HttpRequestException(SR.net_http_client_execution_error, abortException)); - case Http3ConnectionException: + case HttpProtocolException: // A connection-level protocol error has occurred on our stream. _connection.Abort(ex); throw new IOException(SR.net_http_client_execution_error, new HttpRequestException(SR.net_http_client_execution_error, ex)); diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicException.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicException.cs index 40b8d38df25fe5..f254c9b2f7a86b 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicException.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicException.cs @@ -13,6 +13,9 @@ public sealed class QuicException : IOException /// /// Initializes a new instance of the class. /// + /// The error associated with the exception. + /// The application protocol error code associated with the error. + /// The message for the exception. public QuicException(QuicError error, long? applicationErrorCode, string message) : base(message) { From 3bbea56e6848b6c7db27fc6c42980ff5269bc785 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 13 Jul 2022 15:25:46 +0200 Subject: [PATCH 2/9] Correctly process incoming protocol errors --- .../src/Resources/Strings.resx | 6 +++++ .../System/Net/Http/HttpProtocolException.cs | 8 +++--- .../SocketsHttpHandler/Http2Connection.cs | 25 ++++++++++--------- .../SocketsHttpHandler/Http3Connection.cs | 10 +++----- .../SocketsHttpHandler/Http3RequestStream.cs | 11 +++++--- 5 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index be79102d91d2a9..5a250aaa58488d 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -408,6 +408,9 @@ The HTTP/2 server sent invalid data on the connection. HTTP/2 error code '{0}' (0x{1}). + + The HTTP/2 server closed the connection. HTTP/2 error code '{0}' (0x{1}). + The HTTP/2 server reset the stream. HTTP/2 error code '{0}' (0x{1}). @@ -480,6 +483,9 @@ The HTTP/3 server sent invalid data on the connection. HTTP/3 error code '{0}' (0x{1}). + + The HTTP/3 server closed the connection. HTTP/3 error code '{0}' (0x{1}). + The server is unable to process the request using the current HTTP version and indicates the request should be retried on an older HTTP version. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs index 90de2b179f99b5..15d9eae82b00c4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpProtocolException.cs @@ -41,9 +41,9 @@ internal static HttpProtocolException CreateHttp2StreamException(Http2ProtocolEr return new HttpProtocolException((long)protocolError, message, null); } - internal static HttpProtocolException CreateHttp2ConnectionException(Http2ProtocolErrorCode protocolError) + internal static HttpProtocolException CreateHttp2ConnectionException(Http2ProtocolErrorCode protocolError, string? message = null) { - string message = SR.Format(SR.net_http_http2_connection_error, GetName(protocolError), ((int)protocolError).ToString("x")); + message = SR.Format(message ?? SR.net_http_http2_connection_error, GetName(protocolError), ((int)protocolError).ToString("x")); return new HttpProtocolException((long)protocolError, message, null); } @@ -53,9 +53,9 @@ internal static HttpProtocolException CreateHttp3StreamException(Http3ErrorCode return new HttpProtocolException((long)protocolError, message, null); } - internal static HttpProtocolException CreateHttp3ConnectionException(Http3ErrorCode protocolError) + internal static HttpProtocolException CreateHttp3ConnectionException(Http3ErrorCode protocolError, string? message = null) { - string message = SR.Format(SR.net_http_http3_connection_error, GetName(protocolError), ((int)protocolError).ToString("x")); + message = SR.Format(message ?? SR.net_http_http3_connection_error, GetName(protocolError), ((int)protocolError).ToString("x")); return new HttpProtocolException((long)protocolError, message, null); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 30c8761de4eeb4..b4b6aeaabc8e2b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -170,7 +170,8 @@ public Http2Connection(HttpConnectionPool pool, Stream stream) if (NetEventSource.Log.IsEnabled()) TraceConnection(_stream); - static long TimeSpanToMs(TimeSpan value) { + static long TimeSpanToMs(TimeSpan value) + { double milliseconds = value.TotalMilliseconds; return (long)(milliseconds > int.MaxValue ? int.MaxValue : milliseconds); } @@ -486,7 +487,7 @@ private async Task ProcessIncomingFramesAsync() if (frameHeader.Type == FrameType.GoAway) { var (_, errorCode) = ReadGoAwayFrame(frameHeader); - ThrowProtocolError(errorCode); + ThrowProtocolError(errorCode, SR.net_http_http2_connection_close); } else { @@ -1045,7 +1046,7 @@ private void ProcessGoAwayFrame(FrameHeader frameHeader) var (lastStreamId, errorCode) = ReadGoAwayFrame(frameHeader); Debug.Assert(lastStreamId >= 0); - Exception resetException = HttpProtocolException.CreateHttp2ConnectionException(errorCode); + Exception resetException = HttpProtocolException.CreateHttp2ConnectionException(errorCode, SR.net_http_http2_connection_close); // There is no point sending more PING frames for RTT estimation: _rttEstimator.OnGoAwayReceived(); @@ -1258,7 +1259,7 @@ private Task SendPingAsync(long pingContent, bool isAck = false) => Debug.Assert(sizeof(long) == FrameHeader.PingLength); Span span = writeBuffer.Span; - FrameHeader.WriteTo(span, FrameHeader.PingLength, FrameType.Ping, state.isAck ? FrameFlags.Ack: FrameFlags.None, streamId: 0); + FrameHeader.WriteTo(span, FrameHeader.PingLength, FrameType.Ping, state.isAck ? FrameFlags.Ack : FrameFlags.None, streamId: 0); BinaryPrimitives.WriteInt64BigEndian(span.Slice(FrameHeader.Size), state.pingContent); return true; @@ -1923,13 +1924,13 @@ private enum FrameFlags : byte // Some frame types define bits differently. Define them all here for simplicity. - EndStream = 0b00000001, - Ack = 0b00000001, - EndHeaders = 0b00000100, - Padded = 0b00001000, - Priority = 0b00100000, + EndStream = 0b00000001, + Ack = 0b00000001, + EndHeaders = 0b00000100, + Padded = 0b00001000, + Priority = 0b00100000, - ValidBits = 0b00101101 + ValidBits = 0b00101101 } private enum SettingId : ushort @@ -2140,7 +2141,7 @@ private static void ThrowProtocolError() => ThrowProtocolError(Http2ProtocolErrorCode.ProtocolError); [DoesNotReturn] - private static void ThrowProtocolError(Http2ProtocolErrorCode errorCode) => - throw HttpProtocolException.CreateHttp2ConnectionException(errorCode); + private static void ThrowProtocolError(Http2ProtocolErrorCode errorCode, string? message = null) => + throw HttpProtocolException.CreateHttp2ConnectionException(errorCode, message); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index c0c5ddb582609b..9f2d35ff4ac908 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -232,13 +232,11 @@ public async Task SendAsync(HttpRequestMessage request, lon return await responseTask.ConfigureAwait(false); } - catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted) + catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted) { - Debug.Assert(ex.ApplicationErrorCode.HasValue); - - // This will happen if we aborted _connection somewhere. - Abort(ex); - throw new HttpRequestException(SR.Format(SR.net_http_http3_connection_error, ex.ApplicationErrorCode.Value), ex, RequestRetryType.RetryOnConnectionFailure); + // This will happen if we aborted _connection somewhere and we have pending OpenOutboundStreamAsync call. + Debug.Assert(_abortException is not null); + throw new HttpRequestException(SR.net_http_client_execution_error, _abortException, RequestRetryType.RetryOnConnectionFailure); } finally { 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 8e93e81a426faf..dea8fc9df7a28a 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 @@ -231,8 +231,9 @@ await Task.WhenAny(sendContentTask, readResponseTask).ConfigureAwait(false) == s catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted) { Debug.Assert(ex.ApplicationErrorCode.HasValue); + Http3ErrorCode code = (Http3ErrorCode)ex.ApplicationErrorCode.Value; - switch ((Http3ErrorCode)ex.ApplicationErrorCode.Value) + switch (code) { case Http3ErrorCode.VersionFallback: // The server is requesting us fall back to an older HTTP version. @@ -244,14 +245,16 @@ await Task.WhenAny(sendContentTask, readResponseTask).ConfigureAwait(false) == s default: // Our stream was reset. - Exception? abortException = _connection.AbortException; - throw new HttpRequestException(SR.net_http_client_execution_error, abortException ?? ex); + throw new HttpRequestException(SR.net_http_client_execution_error, HttpProtocolException.CreateHttp3StreamException(code)); } } catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted) { // Our connection was reset. Start shutting down the connection. - Exception abortException = _connection.Abort(ex); + Debug.Assert(ex.ApplicationErrorCode.HasValue); + Http3ErrorCode code = (Http3ErrorCode)ex.ApplicationErrorCode.Value; + + Exception abortException = _connection.Abort(HttpProtocolException.CreateHttp3ConnectionException(code, SR.net_http_http3_connection_close)); throw new HttpRequestException(SR.net_http_client_execution_error, abortException); } // It is possible for user's Content code to throw an unexpected OperationCanceledException. From a3af68c710c905c84348a831e4e95dfc15fa2e29 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 13 Jul 2022 15:31:16 +0200 Subject: [PATCH 3/9] Fix HTTP3 stress --- .../HttpStress/ClientOperations.cs | 31 ++++--------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/StressTests/HttpStress/ClientOperations.cs b/src/libraries/System.Net.Http/tests/StressTests/HttpStress/ClientOperations.cs index 0f8a249a7665d7..ab706fe1a0f80f 100644 --- a/src/libraries/System.Net.Http/tests/StressTests/HttpStress/ClientOperations.cs +++ b/src/libraries/System.Net.Http/tests/StressTests/HttpStress/ClientOperations.cs @@ -318,30 +318,11 @@ public static (string name, Func operation)[] Operations = } } - if (ctx.HttpVersion == HttpVersion.Version30) + if (ctx.HttpVersion == HttpVersion.Version30 && + e is HttpProtocolException protocolException && + protocolException.ErrorCode == 258) // 258 = H3_INTERNAL_ERROR (0x102) { - // HTTP/3 exception nesting: - // HttpRequestException->IOException->HttpRequestException->QuicStreamAbortedException - // HttpRequestException->QuicStreamAbortedException - - if (e is IOException && e.InnerException is HttpRequestException) - { - e = e.InnerException; - } - - if (e is HttpRequestException) - { - string? name = e.InnerException?.GetType().Name; - switch (name) - { - case "QuicStreamAbortedException": - if (e.InnerException?.Message?.Equals("Stream aborted by peer (258).") ?? false) // 258 = H3_INTERNAL_ERROR (0x102) - { - return; - } - break; - } - } + return; } throw; @@ -521,9 +502,9 @@ private static void ValidateContent(string expectedContent, string actualContent int divergentIndex = Enumerable .Zip(actualContent, expectedContent) - .Select((x,i) => (x.First, x.Second, i)) + .Select((x, i) => (x.First, x.Second, i)) .Where(x => x.First != x.Second) - .Select(x => (int?) x.i) + .Select(x => (int?)x.i) .FirstOrDefault() .GetValueOrDefault(Math.Min(actualContent.Length, expectedContent.Length)); From b97cf41761c2708d583bfed17325f754a88e168d Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 13 Jul 2022 17:18:47 +0200 Subject: [PATCH 4/9] Add some tests --- .../Net/Http/Http3LoopbackConnection.cs | 5 +- .../SocketsHttpHandler/Http3Connection.cs | 7 ++ .../HttpClientHandlerTest.Http3.cs | 76 ++++++++++++++++++- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs index 8554c4f14bfdbe..3ba00388eb6d6a 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs @@ -84,10 +84,7 @@ public override void Dispose() #endif } - public async Task CloseAsync(long errorCode) - { - await _connection.CloseAsync(errorCode).ConfigureAwait(false); - } + public Task CloseAsync(long errorCode) => _connection.CloseAsync(errorCode).AsTask(); public async ValueTask OpenUnidirectionalStreamAsync() { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index 9f2d35ff4ac908..5c9413b66f02c5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -421,6 +421,13 @@ private async Task AcceptStreamsAsync() { // Shutdown initiated by us, no need to abort. } + catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted) + { + Debug.Assert(ex.ApplicationErrorCode.HasValue); + Http3ErrorCode code = (Http3ErrorCode)ex.ApplicationErrorCode.Value; + + Abort(HttpProtocolException.CreateHttp3ConnectionException(code, SR.net_http_http3_connection_close)); + } catch (Exception ex) { Abort(ex); 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 de5c0d6c9d17ac..66f2dbf187c3a2 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs @@ -31,6 +31,14 @@ public HttpClientHandlerTest_Http3(ITestOutputHelper output) : base(output) { } + private async Task AssertProtocolErrorAsync(long errorCode, Func task) + { + HttpRequestException outerEx = await Assert.ThrowsAsync(task); + _output.WriteLine(outerEx.InnerException.Message); + HttpProtocolException protocolEx = Assert.IsType(outerEx.InnerException); + Assert.Equal(errorCode, protocolEx.ErrorCode); + } + [Theory] [InlineData(10)] // 2 bytes settings value. [InlineData(100)] // 4 bytes settings value. @@ -298,12 +306,78 @@ public async Task ReservedFrameType_Throws() VersionPolicy = HttpVersionPolicy.RequestVersionExact }; - await Assert.ThrowsAsync(async () => await client.SendAsync(request)); + await AssertProtocolErrorAsync(UnexpectedFrameErrorCode, () => client.SendAsync(request)); + }); + + await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000); + } + + [Fact] + public async Task ServerClosesConnection_ThrowsHttpProtocolException() + { + const long GeneralProtocolError = 0x101; + + using Http3LoopbackServer server = CreateHttp3LoopbackServer(); + + Task serverTask = Task.Run(async () => + { + using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); + using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); + + await connection.CloseAsync(GeneralProtocolError); + }); + + Task clientTask = Task.Run(async () => + { + using HttpClient client = CreateHttpClient(); + using HttpRequestMessage request = new() + { + Method = HttpMethod.Get, + RequestUri = server.Address, + Version = HttpVersion30, + VersionPolicy = HttpVersionPolicy.RequestVersionExact + }; + + await AssertProtocolErrorAsync(GeneralProtocolError, () => client.SendAsync(request)); + }); + + await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000); + } + + [Fact] + public async Task ServerClosesStream_ThrowsHttpProtocolException() + { + // normally, the server should not use this code when resetting the stream, but we should still check if we behave sanely... + const long GeneralProtocolError = 0x101; + + using Http3LoopbackServer server = CreateHttp3LoopbackServer(); + + Task serverTask = Task.Run(async () => + { + using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); + using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); + + await stream.AbortAndWaitForShutdownAsync(GeneralProtocolError); + }); + + Task clientTask = Task.Run(async () => + { + using HttpClient client = CreateHttpClient(); + using HttpRequestMessage request = new() + { + Method = HttpMethod.Get, + RequestUri = server.Address, + Version = HttpVersion30, + VersionPolicy = HttpVersionPolicy.RequestVersionExact + }; + + await AssertProtocolErrorAsync(GeneralProtocolError, () => client.SendAsync(request)); }); await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000); } + [Fact] public async Task RequestSentResponseDisposed_ThrowsOnServer() { From 047b2ce1c99f721540b82300a77c076ec59b4a84 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 14 Jul 2022 13:36:48 +0200 Subject: [PATCH 5/9] Remove formating changes --- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index b4b6aeaabc8e2b..e026419270f4a1 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1924,13 +1924,13 @@ private enum FrameFlags : byte // Some frame types define bits differently. Define them all here for simplicity. - EndStream = 0b00000001, - Ack = 0b00000001, - EndHeaders = 0b00000100, - Padded = 0b00001000, - Priority = 0b00100000, + EndStream = 0b00000001, + Ack = 0b00000001, + EndHeaders = 0b00000100, + Padded = 0b00001000, + Priority = 0b00100000, - ValidBits = 0b00101101 + ValidBits = 0b00101101 } private enum SettingId : ushort From 9e4324b306786b0a75f58e3542022469fed4843b Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 14 Jul 2022 15:50:10 +0200 Subject: [PATCH 6/9] Code review feedback --- .../Http/SocketsHttpHandler/Http3RequestStream.cs | 2 +- .../FunctionalTests/HttpClientHandlerTest.Http3.cs | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 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 f4b06796a76220..dc9d2fbba84248 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 @@ -245,7 +245,7 @@ await Task.WhenAny(sendContentTask, readResponseTask).ConfigureAwait(false) == s default: // Our stream was reset. - throw new HttpRequestException(SR.net_http_client_execution_error, HttpProtocolException.CreateHttp3StreamException(code)); + throw new HttpRequestException(SR.net_http_client_execution_error, _connection.AbortException ?? HttpProtocolException.CreateHttp3StreamException(code)); } } catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted) 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 1292fe43c48050..ea8e74675bc50a 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs @@ -33,9 +33,11 @@ public HttpClientHandlerTest_Http3(ITestOutputHelper output) : base(output) private async Task AssertProtocolErrorAsync(long errorCode, Func task) { - HttpRequestException outerEx = await Assert.ThrowsAsync(task); - _output.WriteLine(outerEx.InnerException.Message); + Exception outerEx = await Assert.ThrowsAnyAsync(task); + _output.WriteLine(outerEx.ToString()); + Assert.IsType(outerEx.InnerException); HttpProtocolException protocolEx = Assert.IsType(outerEx.InnerException); + _output.WriteLine(protocolEx.Message); Assert.Equal(errorCode, protocolEx.ErrorCode); } @@ -321,8 +323,8 @@ public async Task ServerClosesConnection_ThrowsHttpProtocolException() Task serverTask = Task.Run(async () => { - using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); - using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); + await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); + await using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); await connection.CloseAsync(GeneralProtocolError); }); @@ -354,8 +356,8 @@ public async Task ServerClosesStream_ThrowsHttpProtocolException() Task serverTask = Task.Run(async () => { - using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); - using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); + await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); + await using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); await stream.AbortAndWaitForShutdownAsync(GeneralProtocolError); }); From 030a14a2ec575eee81f27fc63cd710a1d2e6805e Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 14 Jul 2022 16:36:26 +0200 Subject: [PATCH 7/9] Fix build --- .../tests/FunctionalTests/HttpClientHandlerTest.Http3.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 ea8e74675bc50a..2c1cf3fa312d99 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,7 @@ private async Task AssertProtocolErrorAsync(long errorCode, Func task) { Exception outerEx = await Assert.ThrowsAnyAsync(task); _output.WriteLine(outerEx.ToString()); - Assert.IsType(outerEx.InnerException); + Assert.IsType(outerEx); HttpProtocolException protocolEx = Assert.IsType(outerEx.InnerException); _output.WriteLine(protocolEx.Message); Assert.Equal(errorCode, protocolEx.ErrorCode); @@ -354,12 +354,14 @@ public async Task ServerClosesStream_ThrowsHttpProtocolException() using Http3LoopbackServer server = CreateHttp3LoopbackServer(); + SemaphoreSlim semaphore = new SemaphoreSlim(0); Task serverTask = Task.Run(async () => { await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); await using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); - await stream.AbortAndWaitForShutdownAsync(GeneralProtocolError); + stream.Abort(GeneralProtocolError); + await semaphore.WaitAsync(); }); Task clientTask = Task.Run(async () => @@ -374,6 +376,7 @@ public async Task ServerClosesStream_ThrowsHttpProtocolException() }; await AssertProtocolErrorAsync(GeneralProtocolError, () => client.SendAsync(request)); + semaphore.Release(); }); await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000); From 4caa3d222899be19d12244a92019c892e1cb1b9c Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 14 Jul 2022 17:25:32 +0200 Subject: [PATCH 8/9] Throw HttpProtocolException from content stream as well --- .../SocketsHttpHandler/Http3RequestStream.cs | 21 +++++++++++++------ .../HttpClientHandlerTest.Http3.cs | 2 -- 2 files changed, 15 insertions(+), 8 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 dc9d2fbba84248..d5a4e7596f3e22 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 @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Net.Http.Headers; using System.Net.Quic; @@ -1188,21 +1189,29 @@ private async ValueTask ReadResponseContentAsync(HttpResponseMessage respon } } + [DoesNotReturn] private void HandleReadResponseContentException(Exception ex, CancellationToken cancellationToken) { switch (ex) { - case QuicException e when (e.QuicError == QuicError.StreamAborted || e.QuicError == QuicError.OperationAborted): - // Peer or user aborted the stream - throw new IOException(SR.net_http_client_execution_error, new HttpRequestException(SR.net_http_client_execution_error, ex)); + case QuicException e when (e.QuicError == QuicError.StreamAborted): + // Peer aborted the stream + Debug.Assert(e.ApplicationErrorCode.HasValue); + throw HttpProtocolException.CreateHttp3StreamException((Http3ErrorCode)e.ApplicationErrorCode.Value); + case QuicException e when (e.QuicError == QuicError.ConnectionAborted): // Our connection was reset. Start aborting the connection. - Exception abortException = _connection.Abort(ex); - throw new IOException(SR.net_http_client_execution_error, new HttpRequestException(SR.net_http_client_execution_error, abortException)); + Debug.Assert(e.ApplicationErrorCode.HasValue); + HttpProtocolException exception = HttpProtocolException.CreateHttp3ConnectionException((Http3ErrorCode)e.ApplicationErrorCode.Value, SR.net_http_http3_connection_close); + _connection.Abort(exception); + throw exception; + case HttpProtocolException: // A connection-level protocol error has occurred on our stream. _connection.Abort(ex); - throw new IOException(SR.net_http_client_execution_error, new HttpRequestException(SR.net_http_client_execution_error, ex)); + ExceptionDispatchInfo.Throw(ex); // Rethrow. + return; // Never reached. + case OperationCanceledException oce when oce.CancellationToken == cancellationToken: _stream.Abort(QuicAbortDirection.Read, (long)Http3ErrorCode.RequestCancelled); ExceptionDispatchInfo.Throw(ex); // Rethrow. 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 2c1cf3fa312d99..252797e5415e03 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs @@ -37,7 +37,6 @@ private async Task AssertProtocolErrorAsync(long errorCode, Func task) _output.WriteLine(outerEx.ToString()); Assert.IsType(outerEx); HttpProtocolException protocolEx = Assert.IsType(outerEx.InnerException); - _output.WriteLine(protocolEx.Message); Assert.Equal(errorCode, protocolEx.ErrorCode); } @@ -382,7 +381,6 @@ public async Task ServerClosesStream_ThrowsHttpProtocolException() await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000); } - [Fact] public async Task RequestSentResponseDisposed_ThrowsOnServer() { From ace9da896d84aebfadb7e1190dfaa5d2cec058ca Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 14 Jul 2022 17:59:09 +0200 Subject: [PATCH 9/9] Add test for throwing when reading the content stream --- .../HttpClientHandlerTest.Http2.cs | 4 +- .../HttpClientHandlerTest.Http3.cs | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs index 41543b32f7348c..dfe7f2e386c43a 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -36,7 +36,7 @@ private async Task AssertProtocolErrorAsync(Task task, ProtocolErrors errorCode) Assert.Equal(errorCode, (ProtocolErrors)protocolEx.ErrorCode); } - private async Task AssertProtocolErrorForIOExceptionAsync(Task task, ProtocolErrors errorCode) + private async Task AssertHttpProtocolException(Task task, ProtocolErrors errorCode) { HttpProtocolException protocolEx = await Assert.ThrowsAsync(() => task); Assert.Equal(errorCode, (ProtocolErrors)protocolEx.ErrorCode); @@ -2838,7 +2838,7 @@ public async Task PostAsyncDuplex_ServerResetsStream_Throws() await connection.WriteFrameAsync(new RstStreamFrame(FrameFlags.None, (int)ProtocolErrors.ENHANCE_YOUR_CALM, streamId)); // Trying to read on the response stream should fail now, and client should ignore any data received - await AssertProtocolErrorForIOExceptionAsync(SendAndReceiveResponseDataAsync(contentBytes, responseStream, connection, streamId), ProtocolErrors.ENHANCE_YOUR_CALM); + await AssertHttpProtocolException(SendAndReceiveResponseDataAsync(contentBytes, responseStream, connection, streamId), ProtocolErrors.ENHANCE_YOUR_CALM); // Attempting to write on the request body should now fail with IOException. Exception e = await Assert.ThrowsAnyAsync(async () => { await SendAndReceiveRequestDataAsync(contentBytes, requestStream, connection, streamId); }); 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 252797e5415e03..fa8eec764998b0 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs @@ -381,6 +381,48 @@ public async Task ServerClosesStream_ThrowsHttpProtocolException() await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000); } + [Fact] + public async Task ServerClosesConnection_ResponseContentStream_ThrowsHttpProtocolException() + { + const long GeneralProtocolError = 0x101; + + using Http3LoopbackServer server = CreateHttp3LoopbackServer(); + + SemaphoreSlim semaphore = new SemaphoreSlim(0); + Task serverTask = Task.Run(async () => + { + await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync(); + await using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); + + await stream.ReadRequestBodyAsync(); + await stream.SendResponseHeadersAsync(); + await stream.SendDataFrameAsync(new byte[1024]); + await semaphore.WaitAsync(); + await connection.CloseAsync(GeneralProtocolError); + }); + + Task clientTask = Task.Run(async () => + { + using HttpClient client = CreateHttpClient(); + using HttpRequestMessage request = new() + { + Method = HttpMethod.Get, + RequestUri = server.Address, + Version = HttpVersion30, + VersionPolicy = HttpVersionPolicy.RequestVersionExact + }; + + var response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); + var stream = await response.Content.ReadAsStreamAsync(); + await stream.ReadAsync(new byte[1024]); + semaphore.Release(); + var ex = await Assert.ThrowsAsync(async () => await stream.ReadAsync(new byte[1024])); + Assert.Equal(GeneralProtocolError, ex.ErrorCode); + }); + + await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000); + } + [Fact] public async Task RequestSentResponseDisposed_ThrowsOnServer() {