[8.0] Optionally respect HTTP/1.0 keep-Alive for HTTP.sys (#57182)

* Optionally respect HTTP/1.0 Keep-Alive for HTTP.sys.

* Update test to account for switch.

* Simplify.

* Remove extraneous parens.

* Move setting to HttpSysOptions for improved testability.

* Add comment about internal field

Author: adityamandaleeka

src/Servers/HttpSys/src/HttpSysOptions.cs

@@ -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;
+
     /// <summary>
     /// Initializes a new <see cref="HttpSysOptions"/>.
     /// </summary>

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

@@ -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)
     {
@@ -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
@@ -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.
@@ -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;
+        }
 
         // Determine the body format. If the user asks to do something, let them, otherwise choose a good default for the scenario.
         if (responseContentLength.HasValue)

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

@@ -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;
@@ -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);
         }
     }
@@ -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)
         {
@@ -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)
@@ -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;
+        }
+    }
 }

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

@@ -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)
@@ -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)
         {
@@ -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
                 {

src/Shared/HttpSys/Constants.cs

@@ -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";