Skip to content

Commit 428ef31

Browse files
MihaZupanMihaZupan
authored
Fix HTTP/2 extended connect hang (#80066)
* Fix HTTP/2 extended connect hang * Remove short test timeout * Fix tests * Avoid reusing the same exception instance
1 parent 5625fee commit 428ef31

File tree

5 files changed

+176
-65
lines changed

5 files changed

+176
-65
lines changed

‎src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ public async Task<HeadersFrame> ReadRequestHeaderFrameAsync(bool expectEndOfStre
402402
return (HeadersFrame)frame;
403403
}
404404

405-
public async Task<Frame> ReadDataFrameAsync()
405+
public async Task<DataFrame> ReadDataFrameAsync()
406406
{
407407
// Receive DATA frame for request.
408408
Frame frame = await ReadFrameAsync(_timeout).ConfigureAwait(false);
@@ -412,7 +412,7 @@ public async Task<Frame> ReadDataFrameAsync()
412412
}
413413

414414
Assert.Equal(FrameType.Data, frame.Type);
415-
return frame;
415+
return Assert.IsType<DataFrame>(frame);
416416
}
417417

418418
private static (int bytesConsumed, int value) DecodeInteger(ReadOnlySpan<byte> headerBlock, byte prefixMask)

‎src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,9 +512,12 @@ private async Task ProcessIncomingFramesAsync()
512512

513513
// Process the initial SETTINGS frame. This will send an ACK.
514514
ProcessSettingsFrame(frameHeader, initialFrame: true);
515+
516+
Debug.Assert(InitialSettingsReceived.Task.IsCompleted);
515517
}
516-
catch (IOException e)
518+
catch (Exception e)
517519
{
520+
InitialSettingsReceived.TrySetException(new IOException(SR.net_http_http2_connection_not_established, e));
518521
throw new IOException(SR.net_http_http2_connection_not_established, e);
519522
}
520523

‎src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -2516,68 +2516,6 @@ public async Task PostAsyncDuplex_ServerSendsEndStream_Success()
25162516
}
25172517
}
25182518

2519-
[Fact]
2520-
public async Task ConnectAsync_ReadWriteWebSocketStream()
2521-
{
2522-
var clientMessage = new byte[] { 1, 2, 3 };
2523-
var serverMessage = new byte[] { 4, 5, 6, 7 };
2524-
2525-
using Http2LoopbackServer server = Http2LoopbackServer.CreateServer();
2526-
Http2LoopbackConnection connection = null;
2527-
2528-
Task serverTask = Task.Run(async () =>
2529-
{
2530-
connection = await server.EstablishConnectionAsync(new SettingsEntry { SettingId = SettingId.EnableConnect, Value = 1 });
2531-
2532-
// read request headers
2533-
(int streamId, _) = await connection.ReadAndParseRequestHeaderAsync(readBody: false);
2534-
2535-
// send response headers
2536-
await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false);
2537-
2538-
// send reply
2539-
await connection.SendResponseDataAsync(streamId, serverMessage, endStream: false);
2540-
2541-
// send server EOS
2542-
await connection.SendResponseDataAsync(streamId, Array.Empty<byte>(), endStream: true);
2543-
});
2544-
2545-
StreamingHttpContent requestContent = new StreamingHttpContent();
2546-
2547-
using var handler = CreateSocketsHttpHandler(allowAllCertificates: true);
2548-
using HttpClient client = new HttpClient(handler);
2549-
2550-
HttpRequestMessage request = new(HttpMethod.Connect, server.Address);
2551-
request.Version = HttpVersion.Version20;
2552-
request.VersionPolicy = HttpVersionPolicy.RequestVersionExact;
2553-
request.Headers.Protocol = "websocket";
2554-
2555-
// initiate request
2556-
var responseTask = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);
2557-
2558-
using HttpResponseMessage response = await responseTask.WaitAsync(TimeSpan.FromSeconds(10));
2559-
2560-
await serverTask.WaitAsync(TimeSpan.FromSeconds(60));
2561-
2562-
var responseStream = await response.Content.ReadAsStreamAsync();
2563-
2564-
// receive data
2565-
var readBuffer = new byte[10];
2566-
int bytesRead = await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10));
2567-
Assert.Equal(bytesRead, serverMessage.Length);
2568-
Assert.Equal(serverMessage, readBuffer[..bytesRead]);
2569-
2570-
await responseStream.WriteAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10));
2571-
2572-
// Send client's EOS
2573-
requestContent.CompleteStream();
2574-
// Receive server's EOS
2575-
Assert.Equal(0, await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10)));
2576-
2577-
Assert.NotNull(connection);
2578-
await connection.DisposeAsync();
2579-
}
2580-
25812519
[Fact]
25822520
public async Task PostAsyncDuplex_RequestContentException_ResetsStream()
25832521
{

‎src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2ExtendedConnect.cs‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Collections.Generic;
5+
using System.IO;
6+
using System.Net.Test.Common;
7+
using System.Threading.Tasks;
8+
using Xunit;
9+
using Xunit.Abstractions;
10+
11+
namespace System.Net.Http.Functional.Tests
12+
{
13+
[ConditionalClass(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))]
14+
public sealed class SocketsHttpHandler_Http2ExtendedConnect_Test : HttpClientHandlerTestBase
15+
{
16+
public SocketsHttpHandler_Http2ExtendedConnect_Test(ITestOutputHelper output) : base(output) { }
17+
18+
protected override Version UseVersion => HttpVersion.Version20;
19+
20+
public static IEnumerable<object[]> UseSsl_MemberData()
21+
{
22+
yield return new object[] { false };
23+
24+
if (PlatformDetection.SupportsAlpn)
25+
{
26+
yield return new object[] { true };
27+
}
28+
}
29+
30+
[Theory]
31+
[MemberData(nameof(UseSsl_MemberData))]
32+
public async Task Connect_ReadWriteResponseStream(bool useSsl)
33+
{
34+
byte[] clientMessage = new byte[] { 1, 2, 3 };
35+
byte[] serverMessage = new byte[] { 4, 5, 6, 7 };
36+
37+
TaskCompletionSource clientCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously);
38+
39+
await Http2LoopbackServerFactory.Singleton.CreateClientAndServerAsync(async uri =>
40+
{
41+
using HttpClient client = CreateHttpClient();
42+
43+
HttpRequestMessage request = CreateRequest(HttpMethod.Connect, uri, UseVersion, exactVersion: true);
44+
request.Headers.Protocol = "foo";
45+
46+
using HttpResponseMessage response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);
47+
48+
using Stream responseStream = await response.Content.ReadAsStreamAsync();
49+
50+
await responseStream.WriteAsync(clientMessage);
51+
await responseStream.FlushAsync();
52+
53+
byte[] readBuffer = new byte[serverMessage.Length];
54+
await responseStream.ReadExactlyAsync(readBuffer);
55+
Assert.Equal(serverMessage, readBuffer);
56+
57+
// Receive server's EOS
58+
Assert.Equal(0, await responseStream.ReadAsync(readBuffer));
59+
60+
clientCompleted.SetResult();
61+
},
62+
async server =>
63+
{
64+
await using Http2LoopbackConnection connection = await ((Http2LoopbackServer)server).EstablishConnectionAsync(new SettingsEntry { SettingId = SettingId.EnableConnect, Value = 1 });
65+
66+
(int streamId, _) = await connection.ReadAndParseRequestHeaderAsync(readBody: false);
67+
68+
await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false);
69+
70+
DataFrame dataFrame = await connection.ReadDataFrameAsync();
71+
Assert.Equal(clientMessage, dataFrame.Data.ToArray());
72+
73+
await connection.SendResponseDataAsync(streamId, serverMessage, endStream: true);
74+
75+
await clientCompleted.Task.WaitAsync(TestHelper.PassingTestTimeout);
76+
}, options: new GenericLoopbackOptions { UseSsl = useSsl });
77+
}
78+
79+
[Theory]
80+
[MemberData(nameof(UseSsl_MemberData))]
81+
public async Task Connect_ServerDoesNotSupportExtendedConnect_ClientIncludesExceptionData(bool useSsl)
82+
{
83+
TaskCompletionSource clientCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously);
84+
85+
await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
86+
{
87+
using HttpClient client = CreateHttpClient();
88+
89+
HttpRequestMessage request = CreateRequest(HttpMethod.Connect, uri, UseVersion, exactVersion: true);
90+
request.Headers.Protocol = "foo";
91+
92+
HttpRequestException ex = await Assert.ThrowsAsync<HttpRequestException>(() => client.SendAsync(request));
93+
94+
Assert.Equal(false, ex.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"]);
95+
96+
clientCompleted.SetResult();
97+
},
98+
async server =>
99+
{
100+
try
101+
{
102+
await server.AcceptConnectionAsync(async connection =>
103+
{
104+
await clientCompleted.Task.WaitAsync(TestHelper.PassingTestTimeout);
105+
});
106+
}
107+
catch (Exception ex)
108+
{
109+
_output.WriteLine($"Ignoring exception {ex}");
110+
}
111+
}, options: new GenericLoopbackOptions { UseSsl = useSsl });
112+
}
113+
114+
[Theory]
115+
[InlineData(true)]
116+
[InlineData(false)]
117+
public async Task Connect_Http11Endpoint_Throws(bool useSsl)
118+
{
119+
using var server = new LoopbackServer(new LoopbackServer.Options
120+
{
121+
UseSsl = useSsl
122+
});
123+
124+
await server.ListenAsync();
125+
126+
TaskCompletionSource clientCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously);
127+
128+
Task serverTask = Task.Run(async () =>
129+
{
130+
try
131+
{
132+
await server.AcceptConnectionAsync(async connection =>
133+
{
134+
if (!useSsl)
135+
{
136+
byte[] http2GoAwayHttp11RequiredBytes = new byte[17] { 0, 0, 8, 7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 13 };
137+
138+
await connection.SendResponseAsync(http2GoAwayHttp11RequiredBytes);
139+
140+
await clientCompleted.Task.WaitAsync(TestHelper.PassingTestTimeout);
141+
}
142+
});
143+
}
144+
catch (Exception ex)
145+
{
146+
_output.WriteLine($"Ignoring exception {ex}");
147+
}
148+
});
149+
150+
Task clientTask = Task.Run(async () =>
151+
{
152+
using HttpClient client = CreateHttpClient();
153+
154+
HttpRequestMessage request = CreateRequest(HttpMethod.Connect, server.Address, UseVersion, exactVersion: true);
155+
request.Headers.Protocol = "foo";
156+
157+
Exception ex = await Assert.ThrowsAnyAsync<Exception>(() => client.SendAsync(request));
158+
clientCompleted.SetResult();
159+
160+
if (useSsl)
161+
{
162+
Assert.Equal(false, ex.Data["HTTP2_ENABLED"]);
163+
}
164+
});
165+
166+
await new[] { serverTask, clientTask }.WhenAllOrAnyFailed().WaitAsync(TestHelper.PassingTestTimeout);
167+
}
168+
}
169+
}

‎src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@
155155
<Compile Include="HttpClientHandlerTest.AltSvc.cs" />
156156
<Compile Include="SocketsHttpHandlerTest.Cancellation.cs" />
157157
<Compile Include="SocketsHttpHandlerTest.Http1KeepAlive.cs" />
158+
<Compile Include="SocketsHttpHandlerTest.Http2ExtendedConnect.cs" />
158159
<Compile Include="SocketsHttpHandlerTest.Http2FlowControl.cs" />
159160
<Compile Include="SocketsHttpHandlerTest.Http2KeepAlivePing.cs" />
160161
<Compile Include="HttpClientHandlerTest.Connect.cs" />

0 commit comments

Comments
 (0)