From 793e19df44d78b15b795f844cca2da4153c76620 Mon Sep 17 00:00:00 2001 From: yykkibbb Date: Fri, 20 Feb 2026 22:56:23 +0900 Subject: [PATCH 1/3] Propagate _abortException in Http2Connection.SetupAsync (#119022) When ProcessIncomingFramesAsync detects a server-initiated disconnect and calls Abort(), the original failure reason is stored in _abortException. However, SetupAsync's catch block was wrapping the uninformative ObjectDisposedException instead of the actual cause. This change uses _abortException (when available) as the inner exception, consistent with the existing pattern in PerformWriteAsync and AddStream. Fixes #119022 --- .../SocketsHttpHandler/Http2Connection.cs | 6 ++-- .../HttpClientHandlerTest.Http2.cs | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 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 6fde1077122f64..143021d56f7732 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 @@ -255,8 +255,10 @@ public async ValueTask SetupAsync(CancellationToken cancellationToken) throw; } - // TODO: Review this case! - throw new IOException(SR.net_http_http2_connection_not_established, e); + // Use _abortException if available, as it contains the real reason for the connection failure. + // For example, when ProcessIncomingFramesAsync detects a server-initiated disconnect and calls Abort(), + // _abortException will have the original IOException, while 'e' here may be an uninformative ObjectDisposedException. + throw new IOException(SR.net_http_http2_connection_not_established, _abortException ?? e); } // Avoid capturing the initial request's ExecutionContext for the entire lifetime of the new connection. 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 416cd342435a69..99f8688553b72a 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -687,6 +687,35 @@ public async Task GetAsync_ServerSendSettingsWithoutMaxConcurrentStreams_ClientA } } + [ConditionalFact(nameof(SupportsAlpn))] + public async Task ServerDisconnectDuringSetup_PropagatesMeaningfulException() + { + using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) + using (HttpClient client = CreateHttpClient()) + { + Task sendTask = client.GetAsync(server.Address); + + // Accept connection and read client preface, but do NOT send SETTINGS. + Http2LoopbackConnection connection = await server.AcceptConnectionAsync(); + + // Immediately shut down the connection without sending SETTINGS. + // This simulates a server-side disconnect during HTTP/2 setup. + await connection.ShutdownSendAsync(); + + // The client should throw HttpRequestException. + HttpRequestException ex = await Assert.ThrowsAsync(() => sendTask); + + // The inner exception chain should NOT have ObjectDisposedException as the root cause. + // Instead, it should contain a meaningful exception about the connection failure. + Exception inner = ex.InnerException; + while (inner?.InnerException != null) + { + inner = inner.InnerException; + } + Assert.IsNotType(inner); + } + } + // This test is based on RFC 7540 section 6.1: // "If a DATA frame is received whose stream identifier field is 0x0, the recipient MUST // respond with a connection error (Section 5.4.1) of type PROTOCOL_ERROR." From 689bfe79e1ff6ed5fde05a94ee4cc56bda006262 Mon Sep 17 00:00:00 2001 From: yykkibbb Date: Fri, 20 Feb 2026 23:19:40 +0900 Subject: [PATCH 2/3] Assert expected exception shape in ServerDisconnectDuringSetup test Replace negative assertion (Assert.IsNotType) with positive assertions verifying the expected exception chain: HttpRequestException(InvalidResponse) -> HttpIOException(InvalidResponse) -> HttpIOException(ResponseEnded). --- .../HttpClientHandlerTest.Http2.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 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 99f8688553b72a..5d79738dbe2983 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -702,17 +702,15 @@ public async Task ServerDisconnectDuringSetup_PropagatesMeaningfulException() // This simulates a server-side disconnect during HTTP/2 setup. await connection.ShutdownSendAsync(); - // The client should throw HttpRequestException. + // The client should throw HttpRequestException(InvalidResponse) wrapping + // HttpIOException(InvalidResponse) -> HttpIOException(ResponseEnded), + // indicating the server disconnected before sending SETTINGS. HttpRequestException ex = await Assert.ThrowsAsync(() => sendTask); - - // The inner exception chain should NOT have ObjectDisposedException as the root cause. - // Instead, it should contain a meaningful exception about the connection failure. - Exception inner = ex.InnerException; - while (inner?.InnerException != null) - { - inner = inner.InnerException; - } - Assert.IsNotType(inner); + Assert.Equal(HttpRequestError.InvalidResponse, ex.HttpRequestError); + HttpIOException httpIoEx = Assert.IsAssignableFrom(ex.InnerException); + Assert.Equal(HttpRequestError.InvalidResponse, httpIoEx.HttpRequestError); + HttpIOException innerHttpIoEx = Assert.IsAssignableFrom(httpIoEx.InnerException); + Assert.Equal(HttpRequestError.ResponseEnded, innerHttpIoEx.HttpRequestError); } } From 366ca460670cf7a5e3092704c7020673d58b9b66 Mon Sep 17 00:00:00 2001 From: yykkibbb Date: Sat, 21 Feb 2026 00:04:53 +0900 Subject: [PATCH 3/3] Add GOAWAY test case to ServerDisconnectDuringSetup test Convert test from ConditionalFact to ConditionalTheory with bool sendGoAway parameter to cover both server disconnect scenarios during HTTP/2 setup: - sendGoAway=false: server disconnects without SETTINGS (HttpIOException) - sendGoAway=true: server sends GOAWAY instead of SETTINGS (HttpProtocolException) --- .../HttpClientHandlerTest.Http2.cs | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 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 5d79738dbe2983..36d1a46e717a6f 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -687,8 +687,10 @@ public async Task GetAsync_ServerSendSettingsWithoutMaxConcurrentStreams_ClientA } } - [ConditionalFact(nameof(SupportsAlpn))] - public async Task ServerDisconnectDuringSetup_PropagatesMeaningfulException() + [ConditionalTheory(nameof(SupportsAlpn))] + [InlineData(false)] // server disconnects without sending SETTINGS + [InlineData(true)] // server sends GOAWAY instead of SETTINGS + public async Task ServerDisconnectDuringSetup_PropagatesMeaningfulException(bool sendGoAway) { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) using (HttpClient client = CreateHttpClient()) @@ -698,19 +700,32 @@ public async Task ServerDisconnectDuringSetup_PropagatesMeaningfulException() // Accept connection and read client preface, but do NOT send SETTINGS. Http2LoopbackConnection connection = await server.AcceptConnectionAsync(); - // Immediately shut down the connection without sending SETTINGS. - // This simulates a server-side disconnect during HTTP/2 setup. - await connection.ShutdownSendAsync(); + if (sendGoAway) + { + // Send GOAWAY instead of SETTINGS, then shut down. + await connection.SendGoAway(0, ProtocolErrors.ENHANCE_YOUR_CALM); + await connection.ShutdownSendAsync(); - // The client should throw HttpRequestException(InvalidResponse) wrapping - // HttpIOException(InvalidResponse) -> HttpIOException(ResponseEnded), - // indicating the server disconnected before sending SETTINGS. - HttpRequestException ex = await Assert.ThrowsAsync(() => sendTask); - Assert.Equal(HttpRequestError.InvalidResponse, ex.HttpRequestError); - HttpIOException httpIoEx = Assert.IsAssignableFrom(ex.InnerException); - Assert.Equal(HttpRequestError.InvalidResponse, httpIoEx.HttpRequestError); - HttpIOException innerHttpIoEx = Assert.IsAssignableFrom(httpIoEx.InnerException); - Assert.Equal(HttpRequestError.ResponseEnded, innerHttpIoEx.HttpRequestError); + // The client should throw HttpRequestException(HttpProtocolError) wrapping + // HttpProtocolException with the GOAWAY error code. + await AssertProtocolErrorAsync(sendTask, ProtocolErrors.ENHANCE_YOUR_CALM); + } + else + { + // Immediately shut down the connection without sending SETTINGS. + // This simulates a server-side disconnect during HTTP/2 setup. + await connection.ShutdownSendAsync(); + + // The client should throw HttpRequestException(InvalidResponse) wrapping + // HttpIOException(InvalidResponse) -> HttpIOException(ResponseEnded), + // indicating the server disconnected before sending SETTINGS. + HttpRequestException ex = await Assert.ThrowsAsync(() => sendTask); + Assert.Equal(HttpRequestError.InvalidResponse, ex.HttpRequestError); + HttpIOException httpIoEx = Assert.IsAssignableFrom(ex.InnerException); + Assert.Equal(HttpRequestError.InvalidResponse, httpIoEx.HttpRequestError); + HttpIOException innerHttpIoEx = Assert.IsAssignableFrom(httpIoEx.InnerException); + Assert.Equal(HttpRequestError.ResponseEnded, innerHttpIoEx.HttpRequestError); + } } }