From 9fd77453ce9f5fab3e69082cbe5b1e56ce54851d Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 30 Dec 2022 11:29:01 +0100 Subject: [PATCH 1/4] Fix HTTP/2 extended connect hang --- .../Net/Http/Http2LoopbackConnection.cs | 4 +- .../SocketsHttpHandler/Http2Connection.cs | 8 +- .../HttpClientHandlerTest.Http2.cs | 62 ------- ...etsHttpHandlerTest.Http2ExtendedConnect.cs | 159 ++++++++++++++++++ .../System.Net.Http.Functional.Tests.csproj | 1 + 5 files changed, 168 insertions(+), 66 deletions(-) create mode 100644 src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index ed5af827da852..973ef6931eb48 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -402,7 +402,7 @@ public async Task ReadRequestHeaderFrameAsync(bool expectEndOfStre return (HeadersFrame)frame; } - public async Task ReadDataFrameAsync() + public async Task ReadDataFrameAsync() { // Receive DATA frame for request. Frame frame = await ReadFrameAsync(_timeout).ConfigureAwait(false); @@ -412,7 +412,7 @@ public async Task ReadDataFrameAsync() } Assert.Equal(FrameType.Data, frame.Type); - return frame; + return Assert.IsType(frame); } private static (int bytesConsumed, int value) DecodeInteger(ReadOnlySpan headerBlock, byte prefixMask) 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 e513bce81b0af..38ed705fd9008 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 @@ -500,10 +500,14 @@ private async Task ProcessIncomingFramesAsync() // Process the initial SETTINGS frame. This will send an ACK. ProcessSettingsFrame(frameHeader, initialFrame: true); + + Debug.Assert(InitialSettingsReceived.Task.IsCompleted); } - catch (IOException e) + catch (Exception e) { - throw new IOException(SR.net_http_http2_connection_not_established, e); + e = new IOException(SR.net_http_http2_connection_not_established, e); + InitialSettingsReceived.TrySetException(e); + throw e; } // Keep processing frames as they arrive. 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 7b14f99393c28..224c116adbcb7 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -2516,68 +2516,6 @@ public async Task PostAsyncDuplex_ServerSendsEndStream_Success() } } - [Fact] - public async Task ConnectAsync_ReadWriteWebSocketStream() - { - var clientMessage = new byte[] { 1, 2, 3 }; - var serverMessage = new byte[] { 4, 5, 6, 7 }; - - using Http2LoopbackServer server = Http2LoopbackServer.CreateServer(); - Http2LoopbackConnection connection = null; - - Task serverTask = Task.Run(async () => - { - connection = await server.EstablishConnectionAsync(new SettingsEntry { SettingId = SettingId.EnableConnect, Value = 1 }); - - // read request headers - (int streamId, _) = await connection.ReadAndParseRequestHeaderAsync(readBody: false); - - // send response headers - await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false); - - // send reply - await connection.SendResponseDataAsync(streamId, serverMessage, endStream: false); - - // send server EOS - await connection.SendResponseDataAsync(streamId, Array.Empty(), endStream: true); - }); - - StreamingHttpContent requestContent = new StreamingHttpContent(); - - using var handler = CreateSocketsHttpHandler(allowAllCertificates: true); - using HttpClient client = new HttpClient(handler); - - HttpRequestMessage request = new(HttpMethod.Connect, server.Address); - request.Version = HttpVersion.Version20; - request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; - request.Headers.Protocol = "websocket"; - - // initiate request - var responseTask = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); - - using HttpResponseMessage response = await responseTask.WaitAsync(TimeSpan.FromSeconds(10)); - - await serverTask.WaitAsync(TimeSpan.FromSeconds(60)); - - var responseStream = await response.Content.ReadAsStreamAsync(); - - // receive data - var readBuffer = new byte[10]; - int bytesRead = await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10)); - Assert.Equal(bytesRead, serverMessage.Length); - Assert.Equal(serverMessage, readBuffer[..bytesRead]); - - await responseStream.WriteAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10)); - - // Send client's EOS - requestContent.CompleteStream(); - // Receive server's EOS - Assert.Equal(0, await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10))); - - Assert.NotNull(connection); - await connection.DisposeAsync(); - } - [Fact] public async Task PostAsyncDuplex_RequestContentException_ResetsStream() { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs new file mode 100644 index 0000000000000..1ebee01e7221c --- /dev/null +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs @@ -0,0 +1,159 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.Net.Test.Common; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace System.Net.Http.Functional.Tests +{ + [ConditionalClass(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))] + public sealed class SocketsHttpHandler_Http2ExtendedConnect_Test : HttpClientHandlerTestBase + { + public SocketsHttpHandler_Http2ExtendedConnect_Test(ITestOutputHelper output) : base(output) { } + + protected override Version UseVersion => HttpVersion.Version20; + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Connect_ReadWriteResponseStream(bool useSsl) + { + byte[] clientMessage = new byte[] { 1, 2, 3 }; + byte[] serverMessage = new byte[] { 4, 5, 6, 7 }; + + TaskCompletionSource clientCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); + + await Http2LoopbackServerFactory.Singleton.CreateClientAndServerAsync(async uri => + { + using HttpClient client = CreateHttpClient(); + + HttpRequestMessage request = CreateRequest(HttpMethod.Connect, uri, UseVersion, exactVersion: true); + request.Headers.Protocol = "foo"; + + using HttpResponseMessage response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead).WaitAsync(TimeSpan.FromSeconds(10)); + + using Stream responseStream = await response.Content.ReadAsStreamAsync(); + + await responseStream.WriteAsync(clientMessage); + + byte[] readBuffer = new byte[serverMessage.Length]; + await responseStream.ReadExactlyAsync(readBuffer); + Assert.Equal(serverMessage, readBuffer); + + // Receive server's EOS + Assert.Equal(0, await responseStream.ReadAsync(readBuffer)); + + clientCompleted.SetResult(); + }, + async server => + { + await using Http2LoopbackConnection connection = await ((Http2LoopbackServer)server).EstablishConnectionAsync(new SettingsEntry { SettingId = SettingId.EnableConnect, Value = 1 }); + + (int streamId, _) = await connection.ReadAndParseRequestHeaderAsync(readBody: false); + + await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false); + + DataFrame dataFrame = await connection.ReadDataFrameAsync(); + Assert.Equal(clientMessage, dataFrame.Data.ToArray()); + + await connection.SendResponseDataAsync(streamId, serverMessage, endStream: true); + + await clientCompleted.Task.WaitAsync(TestHelper.PassingTestTimeout); + }, options: new GenericLoopbackOptions { UseSsl = useSsl }); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Connect_ServerDoesNotSupportExtendedConnect_ClientIncludesExceptionData(bool useSsl) + { + TaskCompletionSource clientCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); + + await LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + using HttpClient client = CreateHttpClient(); + + HttpRequestMessage request = CreateRequest(HttpMethod.Connect, uri, UseVersion, exactVersion: true); + request.Headers.Protocol = "foo"; + + HttpRequestException ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + + Assert.Equal(false, ex.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"]); + + clientCompleted.SetResult(); + }, + async server => + { + try + { + await server.AcceptConnectionAsync(async connection => + { + await clientCompleted.Task.WaitAsync(TestHelper.PassingTestTimeout); + }); + } + catch (Exception ex) + { + _output.WriteLine($"Ignoring exception {ex}"); + } + }, options: new GenericLoopbackOptions { UseSsl = useSsl }); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Connect_Http11Endpoint_Throws(bool useSsl) + { + using var server = new LoopbackServer(new LoopbackServer.Options + { + UseSsl = useSsl + }); + + await server.ListenAsync(); + + TaskCompletionSource clientCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); + + Task serverTask = Task.Run(async () => + { + try + { + await server.AcceptConnectionAsync(async connection => + { + if (!useSsl) + { + byte[] http2GoAwayHttp11RequiredBytes = new byte[17] { 0, 0, 8, 7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 13 }; + + await connection.SendResponseAsync(http2GoAwayHttp11RequiredBytes); + + await clientCompleted.Task.WaitAsync(TestHelper.PassingTestTimeout); + } + }); + } + catch (Exception ex) + { + _output.WriteLine($"Ignoring exception {ex}"); + } + }); + + Task clientTask = Task.Run(async () => + { + using HttpClient client = CreateHttpClient(); + + HttpRequestMessage request = CreateRequest(HttpMethod.Connect, server.Address, UseVersion, exactVersion: true); + request.Headers.Protocol = "foo"; + + Exception ex = await Assert.ThrowsAnyAsync(() => client.SendAsync(request)); + clientCompleted.SetResult(); + + if (useSsl) + { + Assert.Equal(false, ex.Data["HTTP2_ENABLED"]); + } + }); + + await new[] { serverTask, clientTask }.WhenAllOrAnyFailed().WaitAsync(TestHelper.PassingTestTimeout); + } + } +} diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj index 416a0fae6a17f..8b70380ab45f3 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj @@ -155,6 +155,7 @@ + From f569a194aaf752667713638d777ee17cf9f2b49f Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 30 Dec 2022 12:46:56 +0100 Subject: [PATCH 2/4] Remove short test timeout --- .../SocketsHttpHandlerTest.Http2ExtendedConnect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs index 1ebee01e7221c..6f38ddb24fac5 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs @@ -33,7 +33,7 @@ await Http2LoopbackServerFactory.Singleton.CreateClientAndServerAsync(async uri HttpRequestMessage request = CreateRequest(HttpMethod.Connect, uri, UseVersion, exactVersion: true); request.Headers.Protocol = "foo"; - using HttpResponseMessage response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead).WaitAsync(TimeSpan.FromSeconds(10)); + using HttpResponseMessage response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); using Stream responseStream = await response.Content.ReadAsStreamAsync(); From ffb3d8d49c4d3d71ec4a0a7b595e0d1a233a0a74 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Sat, 31 Dec 2022 09:08:46 +0100 Subject: [PATCH 3/4] Fix tests --- ...ketsHttpHandlerTest.Http2ExtendedConnect.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs index 6f38ddb24fac5..207e7ecf6f2ba 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.IO; using System.Net.Test.Common; using System.Threading.Tasks; @@ -16,9 +17,18 @@ public SocketsHttpHandler_Http2ExtendedConnect_Test(ITestOutputHelper output) : protected override Version UseVersion => HttpVersion.Version20; + public static IEnumerable UseSsl_MemberData() + { + yield return new object[] { false }; + + if (PlatformDetection.SupportsAlpn) + { + yield return new object[] { true }; + } + } + [Theory] - [InlineData(true)] - [InlineData(false)] + [MemberData(nameof(UseSsl_MemberData))] public async Task Connect_ReadWriteResponseStream(bool useSsl) { byte[] clientMessage = new byte[] { 1, 2, 3 }; @@ -38,6 +48,7 @@ await Http2LoopbackServerFactory.Singleton.CreateClientAndServerAsync(async uri using Stream responseStream = await response.Content.ReadAsStreamAsync(); await responseStream.WriteAsync(clientMessage); + await responseStream.FlushAsync(); byte[] readBuffer = new byte[serverMessage.Length]; await responseStream.ReadExactlyAsync(readBuffer); @@ -66,8 +77,7 @@ await Http2LoopbackServerFactory.Singleton.CreateClientAndServerAsync(async uri } [Theory] - [InlineData(true)] - [InlineData(false)] + [MemberData(nameof(UseSsl_MemberData))] public async Task Connect_ServerDoesNotSupportExtendedConnect_ClientIncludesExceptionData(bool useSsl) { TaskCompletionSource clientCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); From 7d7e4a5abe88a990e956316f72f00039e6175ea2 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Wed, 4 Jan 2023 12:11:21 -0800 Subject: [PATCH 4/4] Avoid reusing the same exception instance --- .../System/Net/Http/SocketsHttpHandler/Http2Connection.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 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 38ed705fd9008..8807cac59106d 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 @@ -505,9 +505,8 @@ private async Task ProcessIncomingFramesAsync() } catch (Exception e) { - e = new IOException(SR.net_http_http2_connection_not_established, e); - InitialSettingsReceived.TrySetException(e); - throw e; + InitialSettingsReceived.TrySetException(new IOException(SR.net_http_http2_connection_not_established, e)); + throw new IOException(SR.net_http_http2_connection_not_established, e); } // Keep processing frames as they arrive.