Skip to content

Commit 6548eba

Browse files
authored
Do not close connection for http1.0 when client request connection keep-alive (#56558)
1 parent ec15944 commit 6548eba

File tree

3 files changed

+76
-5
lines changed

3 files changed

+76
-5
lines changed

src/Servers/HttpSys/src/RequestProcessing/Response.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ internal uint ComputeHeaders(long writeCount, bool endOfRequest = false)
391391
var requestConnectionString = Request.Headers[HeaderNames.Connection];
392392
var isHeadRequest = Request.IsHeadMethod;
393393
var requestCloseSet = Matches(Constants.Close, requestConnectionString);
394+
var requestConnectionKeepAliveSet = Matches(Constants.KeepAlive, requestConnectionString);
394395

395396
// Gather everything the app may have set on the response:
396397
// Http.Sys does not allow us to specify the response protocol version, assume this is a HTTP/1.1 response when making decisions.
@@ -403,7 +404,13 @@ internal uint ComputeHeaders(long writeCount, bool endOfRequest = false)
403404

404405
// Determine if the connection will be kept alive or closed.
405406
var keepConnectionAlive = true;
406-
if (requestVersion <= Constants.V1_0 // Http.Sys does not support "Keep-Alive: true" or "Connection: Keep-Alive"
407+
408+
// An HTTP/1.1 server may also establish persistent connections with
409+
// HTTP/1.0 clients upon receipt of a Keep-Alive connection token.
410+
// However, a persistent connection with an HTTP/1.0 client cannot make
411+
// use of the chunked transfer-coding. From: https://www.rfc-editor.org/rfc/rfc2068#section-19.7.1
412+
if (requestVersion < Constants.V1_0
413+
|| (requestVersion == Constants.V1_0 && (!requestConnectionKeepAliveSet || responseChunkedSet))
407414
|| (requestVersion == Constants.V1_1 && requestCloseSet)
408415
|| responseCloseSet)
409416
{

src/Servers/HttpSys/test/FunctionalTests/Listener/ResponseHeaderTests.cs

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Linq;
77
using System.Net;
88
using System.Net.Http;
9+
using System.Net.Sockets;
910
using System.Text;
1011
using System.Threading.Tasks;
1112
using Microsoft.AspNetCore.InternalTesting;
@@ -207,20 +208,66 @@ public async Task ResponseHeaders_11HeadRequestStatusCodeWithoutBody_NoContentLe
207208
}
208209

209210
[ConditionalFact]
210-
public async Task ResponseHeaders_HTTP10KeepAliveRequest_Gets11Close()
211+
public async Task ResponseHeaders_HTTP10KeepAliveRequest_KeepAliveHeader_Gets11NoClose()
212+
{
213+
string address;
214+
using (var server = Utilities.CreateHttpServer(out address))
215+
{
216+
// Track the number of times ConnectCallback is invoked to ensure the underlying socket wasn't closed.
217+
int connectCallbackInvocations = 0;
218+
var handler = new SocketsHttpHandler();
219+
handler.ConnectCallback = (context, cancellationToken) =>
220+
{
221+
Interlocked.Increment(ref connectCallbackInvocations);
222+
return ConnectCallback(context, cancellationToken);
223+
};
224+
225+
using (var client = new HttpClient(handler))
226+
{
227+
// Send the first request
228+
Task<HttpResponseMessage> responseTask = SendRequestAsync(address, usehttp11: false, sendKeepAlive: true, httpClient: client);
229+
var context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask);
230+
context.Dispose();
231+
232+
HttpResponseMessage response = await responseTask;
233+
response.EnsureSuccessStatusCode();
234+
Assert.Equal(new Version(1, 1), response.Version);
235+
Assert.Null(response.Headers.ConnectionClose);
236+
237+
// Send the second request
238+
responseTask = SendRequestAsync(address, usehttp11: false, sendKeepAlive: true, httpClient: client);
239+
context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask);
240+
context.Dispose();
241+
242+
response = await responseTask;
243+
response.EnsureSuccessStatusCode();
244+
Assert.Equal(new Version(1, 1), response.Version);
245+
Assert.Null(response.Headers.ConnectionClose);
246+
}
247+
248+
// Verify that ConnectCallback was only called once
249+
Assert.Equal(1, connectCallbackInvocations);
250+
}
251+
}
252+
253+
[ConditionalFact]
254+
public async Task ResponseHeaders_HTTP10KeepAliveRequest_ChunkedTransferEncoding_Gets11Close()
211255
{
212256
string address;
213257
using (var server = Utilities.CreateHttpServer(out address))
214258
{
215-
// Http.Sys does not support 1.0 keep-alives.
216259
Task<HttpResponseMessage> responseTask = SendRequestAsync(address, usehttp11: false, sendKeepAlive: true);
217260

218261
var context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask);
262+
context.Response.Headers["Transfer-Encoding"] = new string[] { "chunked" };
263+
var responseBytes = Encoding.ASCII.GetBytes("10\r\nManually Chunked\r\n0\r\n\r\n");
264+
await context.Response.Body.WriteAsync(responseBytes, 0, responseBytes.Length);
219265
context.Dispose();
220266

221267
HttpResponseMessage response = await responseTask;
222268
response.EnsureSuccessStatusCode();
223269
Assert.Equal(new Version(1, 1), response.Version);
270+
Assert.True(response.Headers.TransferEncodingChunked.HasValue, "Chunked");
224271
Assert.True(response.Headers.ConnectionClose.Value);
225272
}
226273
}
@@ -289,8 +336,9 @@ public async Task AddingControlCharactersToHeadersThrows(string key, string valu
289336
}
290337
}
291338

292-
private async Task<HttpResponseMessage> SendRequestAsync(string uri, bool usehttp11 = true, bool sendKeepAlive = false)
339+
private async Task<HttpResponseMessage> SendRequestAsync(string uri, bool usehttp11 = true, bool sendKeepAlive = false, HttpClient httpClient = null)
293340
{
341+
httpClient ??= _client;
294342
var request = new HttpRequestMessage(HttpMethod.Get, uri);
295343
if (!usehttp11)
296344
{
@@ -300,7 +348,7 @@ private async Task<HttpResponseMessage> SendRequestAsync(string uri, bool usehtt
300348
{
301349
request.Headers.Add("Connection", "Keep-Alive");
302350
}
303-
return await _client.SendAsync(request);
351+
return await httpClient.SendAsync(request);
304352
}
305353

306354
private async Task<HttpResponseMessage> SendHeadRequestAsync(string uri, bool usehttp11 = true)
@@ -312,4 +360,19 @@ private async Task<HttpResponseMessage> SendHeadRequestAsync(string uri, bool us
312360
}
313361
return await _client.SendAsync(request);
314362
}
363+
364+
private static async ValueTask<Stream> ConnectCallback(SocketsHttpConnectionContext connectContext, CancellationToken ct)
365+
{
366+
var s = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
367+
try
368+
{
369+
await s.ConnectAsync(connectContext.DnsEndPoint, ct);
370+
return new NetworkStream(s, ownsSocket: true);
371+
}
372+
catch
373+
{
374+
s.Dispose();
375+
throw;
376+
}
377+
}
315378
}

src/Shared/HttpSys/Constants.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ internal static class Constants
99
internal const string HttpsScheme = "https";
1010
internal const string Chunked = "chunked";
1111
internal const string Close = "close";
12+
internal const string KeepAlive = "keep-alive";
1213
internal const string Zero = "0";
1314
internal const string SchemeDelimiter = "://";
1415
internal const string DefaultServerAddress = "http://localhost:5000";

0 commit comments

Comments
 (0)