Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Servers/HttpSys/src/HttpSysOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ public class HttpSysOptions
private long? _maxRequestBodySize = DefaultMaxRequestBodySize;
private string? _requestQueueName;

private const string RespectHttp10KeepAliveSwitch = "Microsoft.AspNetCore.Server.HttpSys.RespectHttp10KeepAlive";

// Internal for testing
internal bool RespectHttp10KeepAlive = AppContext.TryGetSwitch(RespectHttp10KeepAliveSwitch, out var enabled) && enabled;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might say "// Internal for testing".


/// <summary>
/// Initializes a new <see cref="HttpSysOptions"/>.
/// </summary>
Expand Down
18 changes: 17 additions & 1 deletion src/Servers/HttpSys/src/RequestProcessing/Response.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ internal sealed class Response
private BoundaryType _boundaryType;
private HttpApiTypes.HTTP_RESPONSE_V2 _nativeResponse;
private HeaderCollection? _trailers;
private readonly bool _respectHttp10KeepAlive;

internal Response(RequestContext requestContext)
{
Expand All @@ -51,6 +52,7 @@ internal Response(RequestContext requestContext)
_nativeStream = null;
_cacheTtl = null;
_authChallenges = RequestContext.Server.Options.Authentication.Schemes;
_respectHttp10KeepAlive = RequestContext.Server.Options.RespectHttp10KeepAlive;
}

private enum ResponseState
Expand Down Expand Up @@ -390,6 +392,7 @@ internal HttpApiTypes.HTTP_FLAGS ComputeHeaders(long writeCount, bool endOfReque
var requestConnectionString = Request.Headers[HeaderNames.Connection];
var isHeadRequest = Request.IsHeadMethod;
var requestCloseSet = Matches(Constants.Close, requestConnectionString);
var requestConnectionKeepAliveSet = Matches(Constants.KeepAlive, requestConnectionString);

// Gather everything the app may have set on the response:
// Http.Sys does not allow us to specify the response protocol version, assume this is a HTTP/1.1 response when making decisions.
Expand All @@ -402,12 +405,25 @@ internal HttpApiTypes.HTTP_FLAGS ComputeHeaders(long writeCount, bool endOfReque

// Determine if the connection will be kept alive or closed.
var keepConnectionAlive = true;
if (requestVersion <= Constants.V1_0 // Http.Sys does not support "Keep-Alive: true" or "Connection: Keep-Alive"

if (requestVersion < Constants.V1_0
|| (requestVersion == Constants.V1_1 && requestCloseSet)
|| responseCloseSet)
{
keepConnectionAlive = false;
}
else if (requestVersion == Constants.V1_0)
{
// In .NET 9, we updated the behavior for 1.0 clients here to match
// RFC 2068. The new behavior is available down-level behind an
// AppContext switch.

// An HTTP/1.1 server may also establish persistent connections with
// HTTP/1.0 clients upon receipt of a Keep-Alive connection token.
// However, a persistent connection with an HTTP/1.0 client cannot make
// use of the chunked transfer-coding. From: https://www.rfc-editor.org/rfc/rfc2068#section-19.7.1
keepConnectionAlive = _respectHttp10KeepAlive && requestConnectionKeepAliveSet && !responseChunkedSet;
Copy link
Member

@Tratcher Tratcher Aug 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

responseChunkedSet is only for cases where the application sets the chunked header and does the chunking themselves, it doesn't cover the default case where the server does chunking automatically. See _boundaryType = BoundaryType.Chunked; vs _boundaryType = BoundaryType.PassThrough; below.

edit nevermind, the automatic case only happens for 1.1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

}

// Determine the body format. If the user asks to do something, let them, otherwise choose a good default for the scenario.
if (responseContentLength.HasValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Sockets;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Testing;
Expand Down Expand Up @@ -207,20 +208,83 @@ public async Task ResponseHeaders_11HeadRequestStatusCodeWithoutBody_NoContentLe
}

[ConditionalFact]
public async Task ResponseHeaders_HTTP10KeepAliveRequest_Gets11Close()
public async Task ResponseHeaders_HTTP10KeepAliveRequest_KeepAliveHeader_RespectsSwitch()
{
string address;
using (var server = Utilities.CreateHttpServer(out address, respectHttp10KeepAlive: true))
{
// Track the number of times ConnectCallback is invoked to ensure the underlying socket wasn't closed.
int connectCallbackInvocations = 0;
var handler = new SocketsHttpHandler();
handler.ConnectCallback = (context, cancellationToken) =>
{
Interlocked.Increment(ref connectCallbackInvocations);
return ConnectCallback(context, cancellationToken);
};

using (var client = new HttpClient(handler))
{
// Send the first request
Task<HttpResponseMessage> responseTask = SendRequestAsync(address, usehttp11: false, sendKeepAlive: true, httpClient: client);
var context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask);
context.Dispose();

HttpResponseMessage response = await responseTask;
response.EnsureSuccessStatusCode();
Assert.Equal(new Version(1, 1), response.Version);
Assert.Null(response.Headers.ConnectionClose);

// Send the second request
responseTask = SendRequestAsync(address, usehttp11: false, sendKeepAlive: true, httpClient: client);
context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask);
context.Dispose();

response = await responseTask;
response.EnsureSuccessStatusCode();
Assert.Equal(new Version(1, 1), response.Version);
Assert.Null(response.Headers.ConnectionClose);
}

// Verify that ConnectCallback was only called once
Assert.Equal(1, connectCallbackInvocations);
}

using (var server = Utilities.CreateHttpServer(out address, respectHttp10KeepAlive: false))
{
var handler = new SocketsHttpHandler();
using (var client = new HttpClient(handler))
{
// Send the first request
Task<HttpResponseMessage> responseTask = SendRequestAsync(address, usehttp11: false, sendKeepAlive: true, httpClient: client);
var context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask);
context.Dispose();

HttpResponseMessage response = await responseTask;
response.EnsureSuccessStatusCode();
Assert.Equal(new Version(1, 1), response.Version);
Assert.True(response.Headers.ConnectionClose.Value);
}
}
}

[ConditionalFact]
public async Task ResponseHeaders_HTTP10KeepAliveRequest_ChunkedTransferEncoding_Gets11Close()
{
string address;
using (var server = Utilities.CreateHttpServer(out address))
{
// Http.Sys does not support 1.0 keep-alives.
Task<HttpResponseMessage> responseTask = SendRequestAsync(address, usehttp11: false, sendKeepAlive: true);

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

HttpResponseMessage response = await responseTask;
response.EnsureSuccessStatusCode();
Assert.Equal(new Version(1, 1), response.Version);
Assert.True(response.Headers.TransferEncodingChunked.HasValue, "Chunked");
Assert.True(response.Headers.ConnectionClose.Value);
}
}
Expand Down Expand Up @@ -289,8 +353,9 @@ public async Task AddingControlCharactersToHeadersThrows(string key, string valu
}
}

private async Task<HttpResponseMessage> SendRequestAsync(string uri, bool usehttp11 = true, bool sendKeepAlive = false)
private async Task<HttpResponseMessage> SendRequestAsync(string uri, bool usehttp11 = true, bool sendKeepAlive = false, HttpClient httpClient = null)
{
httpClient ??= _client;
var request = new HttpRequestMessage(HttpMethod.Get, uri);
if (!usehttp11)
{
Expand All @@ -300,7 +365,7 @@ private async Task<HttpResponseMessage> SendRequestAsync(string uri, bool usehtt
{
request.Headers.Add("Connection", "Keep-Alive");
}
return await _client.SendAsync(request);
return await httpClient.SendAsync(request);
}

private async Task<HttpResponseMessage> SendHeadRequestAsync(string uri, bool usehttp11 = true)
Expand All @@ -312,4 +377,19 @@ private async Task<HttpResponseMessage> SendHeadRequestAsync(string uri, bool us
}
return await _client.SendAsync(request);
}

private static async ValueTask<Stream> ConnectCallback(SocketsHttpConnectionContext connectContext, CancellationToken ct)
{
var s = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
try
{
await s.ConnectAsync(connectContext.DnsEndPoint, ct);
return new NetworkStream(s, ownsSocket: true);
}
catch
{
s.Dispose();
throw;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ internal static class Utilities

internal static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(15);

internal static HttpSysListener CreateHttpServer(out string baseAddress)
internal static HttpSysListener CreateHttpServer(out string baseAddress, bool respectHttp10KeepAlive = false)
{
string root;
return CreateDynamicHttpServer(string.Empty, out root, out baseAddress);
return CreateDynamicHttpServer(string.Empty, out root, out baseAddress, respectHttp10KeepAlive);
}

internal static HttpSysListener CreateHttpServerReturnRoot(string path, out string root)
Expand All @@ -34,7 +34,7 @@ internal static HttpSysListener CreateHttpServerReturnRoot(string path, out stri
return CreateDynamicHttpServer(path, out root, out baseAddress);
}

internal static HttpSysListener CreateDynamicHttpServer(string basePath, out string root, out string baseAddress)
internal static HttpSysListener CreateDynamicHttpServer(string basePath, out string root, out string baseAddress, bool respectHttp10KeepAlive = false)
{
lock (PortLock)
{
Expand All @@ -47,6 +47,7 @@ internal static HttpSysListener CreateDynamicHttpServer(string basePath, out str
var options = new HttpSysOptions();
options.UrlPrefixes.Add(prefix);
options.RequestQueueName = prefix.Port; // Convention for use with CreateServerOnExistingQueue
options.RespectHttp10KeepAlive = respectHttp10KeepAlive;
var listener = new HttpSysListener(options, new LoggerFactory());
try
{
Expand Down
1 change: 1 addition & 0 deletions src/Shared/HttpSys/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ internal static class Constants
internal const string HttpsScheme = "https";
internal const string Chunked = "chunked";
internal const string Close = "close";
internal const string KeepAlive = "keep-alive";
internal const string Zero = "0";
internal const string SchemeDelimiter = "://";
internal const string DefaultServerAddress = "http://localhost:5000";
Expand Down