Respect the Keep-Alive response header on HTTP/1.0 (#73020)
authorMiha Zupan <mihazupan.zupan1@gmail.com>
Tue, 2 Aug 2022 15:58:47 +0000 (17:58 +0200)
committerGitHub <noreply@github.com>
Tue, 2 Aug 2022 15:58:47 +0000 (08:58 -0700)
* Respect the Keep-Alive response header on HTTP/1.0

* Remove TimeoutOffset

* Update Trace message

* Update tests

* Adjust test timeouts

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http1KeepAlive.cs [new file with mode: 0644]
src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj

index 2d92a7b..8d55e07 100644 (file)
@@ -67,6 +67,7 @@ namespace System.Net.Http
         private int _readLength;
 
         private long _idleSinceTickCount;
+        private int _keepAliveTimeoutSeconds; // 0 == no timeout
         private bool _inUse;
         private bool _detachedFromPool;
         private bool _canRetry;
@@ -148,9 +149,14 @@ namespace System.Net.Http
 
         /// <summary>Prepare an idle connection to be used for a new request.</summary>
         /// <param name="async">Indicates whether the coming request will be sync or async.</param>
-        /// <returns>True if connection can be used, false if it is invalid due to receiving EOF or unexpected data.</returns>
+        /// <returns>True if connection can be used, false if it is invalid due to a timeout or receiving EOF or unexpected data.</returns>
         public bool PrepareForReuse(bool async)
         {
+            if (CheckKeepAliveTimeoutExceeded())
+            {
+                return false;
+            }
+
             // We may already have a read-ahead task if we did a previous scavenge and haven't used the connection since.
             // If the read-ahead task is completed, then we've received either EOF or erroneous data the connection, so it's not usable.
             if (_readAheadTask is not null)
@@ -196,9 +202,14 @@ namespace System.Net.Http
         }
 
         /// <summary>Check whether a currently idle connection is still usable, or should be scavenged.</summary>
-        /// <returns>True if connection can be used, false if it is invalid due to receiving EOF or unexpected data.</returns>
+        /// <returns>True if connection can be used, false if it is invalid due to a timeout or receiving EOF or unexpected data.</returns>
         public override bool CheckUsabilityOnScavenge()
         {
+            if (CheckKeepAliveTimeoutExceeded())
+            {
+                return false;
+            }
+
             // We may already have a read-ahead task if we did a previous scavenge and haven't used the connection since.
 #pragma warning disable CA2012 // we're very careful to ensure the ValueTask is only consumed once, even though it's stored into a field
             _readAheadTask ??= ReadAheadWithZeroByteReadAsync();
@@ -223,6 +234,14 @@ namespace System.Net.Http
             }
         }
 
+        private bool CheckKeepAliveTimeoutExceeded()
+        {
+            // We only honor a Keep-Alive timeout on HTTP/1.0 responses.
+            // If _keepAliveTimeoutSeconds is 0, no timeout has been set.
+            return _keepAliveTimeoutSeconds != 0 &&
+                GetIdleTicks(Environment.TickCount64) >= _keepAliveTimeoutSeconds * 1000;
+        }
+
         private ValueTask<int>? ConsumeReadAheadTask()
         {
             if (Interlocked.CompareExchange(ref _readAheadTaskLock, 1, 0) == 0)
@@ -646,6 +665,11 @@ namespace System.Net.Http
                     ParseHeaderNameValue(this, line.Span, response, isFromTrailer: false);
                 }
 
+                if (response.Version.Minor == 0)
+                {
+                    ProcessHttp10KeepAliveHeader(response);
+                }
+
                 if (HttpTelemetry.Log.IsEnabled()) HttpTelemetry.Log.ResponseHeadersStop();
 
                 if (allowExpect100ToContinue != null)
@@ -1108,6 +1132,45 @@ namespace System.Net.Http
             }
         }
 
+        private void ProcessHttp10KeepAliveHeader(HttpResponseMessage response)
+        {
+            if (response.Headers.NonValidated.TryGetValues(KnownHeaders.KeepAlive.Name, out HeaderStringValues keepAliveValues))
+            {
+                string keepAlive = keepAliveValues.ToString();
+                var parsedValues = new UnvalidatedObjectCollection<NameValueHeaderValue>();
+
+                if (NameValueHeaderValue.GetNameValueListLength(keepAlive, 0, ',', parsedValues) == keepAlive.Length)
+                {
+                    foreach (NameValueHeaderValue nameValue in parsedValues)
+                    {
+                        if (string.Equals(nameValue.Name, "timeout", StringComparison.OrdinalIgnoreCase))
+                        {
+                            if (!string.IsNullOrEmpty(nameValue.Value) &&
+                                HeaderUtilities.TryParseInt32(nameValue.Value, out int timeout) &&
+                                timeout >= 0)
+                            {
+                                if (timeout == 0)
+                                {
+                                    _connectionClose = true;
+                                }
+                                else
+                                {
+                                    _keepAliveTimeoutSeconds = timeout;
+                                }
+                            }
+                        }
+                        else if (string.Equals(nameValue.Name, "max", StringComparison.OrdinalIgnoreCase))
+                        {
+                            if (nameValue.Value == "0")
+                            {
+                                _connectionClose = true;
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
         private void WriteToBuffer(ReadOnlySpan<byte> source)
         {
             Debug.Assert(source.Length <= _writeBuffer.Length - _writeOffset);
index ebc5c61..e560915 100644 (file)
@@ -2324,7 +2324,7 @@ namespace System.Net.Http
 
                 if (!connection.CheckUsabilityOnScavenge())
                 {
-                    if (NetEventSource.Log.IsEnabled()) connection.Trace($"Scavenging connection. Unexpected data or EOF received.");
+                    if (NetEventSource.Log.IsEnabled()) connection.Trace($"Scavenging connection. Keep-Alive timeout exceeded, unexpected data or EOF received.");
                     return false;
                 }
 
diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http1KeepAlive.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http1KeepAlive.cs
new file mode 100644 (file)
index 0000000..75ed421
--- /dev/null
@@ -0,0 +1,150 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+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_Http1KeepAlive_Test : HttpClientHandlerTestBase
+    {
+        public SocketsHttpHandler_Http1KeepAlive_Test(ITestOutputHelper output) : base(output) { }
+
+        [Fact]
+        public async Task Http10Response_ConnectionIsReusedFor10And11()
+        {
+            await LoopbackServer.CreateClientAndServerAsync(async uri =>
+            {
+                using HttpClient client = CreateHttpClient();
+
+                await client.SendAsync(CreateRequest(HttpMethod.Get, uri, HttpVersion.Version10, exactVersion: true));
+                await client.SendAsync(CreateRequest(HttpMethod.Get, uri, HttpVersion.Version11, exactVersion: true));
+                await client.SendAsync(CreateRequest(HttpMethod.Get, uri, HttpVersion.Version10, exactVersion: true));
+            },
+            server => server.AcceptConnectionAsync(async connection =>
+            {
+                HttpRequestData request = await connection.ReadRequestDataAsync();
+                Assert.Equal(0, request.Version.Minor);
+                await connection.WriteStringAsync("HTTP/1.0 200 OK\r\nContent-Length: 1\r\n\r\n1");
+                connection.CompleteRequestProcessing();
+
+                request = await connection.ReadRequestDataAsync();
+                Assert.Equal(1, request.Version.Minor);
+                await connection.WriteStringAsync("HTTP/1.0 200 OK\r\nContent-Length: 1\r\n\r\n2");
+                connection.CompleteRequestProcessing();
+
+                request = await connection.ReadRequestDataAsync();
+                Assert.Equal(0, request.Version.Minor);
+                await connection.WriteStringAsync("HTTP/1.0 200 OK\r\nContent-Length: 1\r\n\r\n3");
+            }));
+        }
+
+        [OuterLoop("Uses Task.Delay")]
+        [Fact]
+        public async Task Http10ResponseWithKeepAliveTimeout_ConnectionRecycledAfterTimeout()
+        {
+            await LoopbackServer.CreateClientAndServerAsync(async uri =>
+            {
+                using HttpClient client = CreateHttpClient();
+
+                await client.GetAsync(uri);
+
+                await Task.Delay(2000);
+                await client.GetAsync(uri);
+            },
+            async server =>
+            {
+                await server.AcceptConnectionAsync(async connection =>
+                {
+                    await connection.ReadRequestDataAsync();
+                    await connection.WriteStringAsync("HTTP/1.0 200 OK\r\nKeep-Alive: timeout=1\r\nContent-Length: 1\r\n\r\n1");
+                    connection.CompleteRequestProcessing();
+
+                    await Assert.ThrowsAnyAsync<Exception>(() => connection.ReadRequestDataAsync());
+                });
+
+                await server.AcceptConnectionSendResponseAndCloseAsync();
+            });
+        }
+
+        [Theory]
+        [InlineData("timeout=1000", true)]
+        [InlineData("timeout=30", true)]
+        [InlineData("timeout=0", false)]
+        [InlineData("foo, bar=baz, timeout=30", true)]
+        [InlineData("foo, bar=baz, timeout=0", false)]
+        [InlineData("timeout=-1", true)]
+        [InlineData("timeout=abc", true)]
+        [InlineData("max=1", true)]
+        [InlineData("max=0", false)]
+        [InlineData("max=-1", true)]
+        [InlineData("max=abc", true)]
+        [InlineData("timeout=30, max=1", true)]
+        [InlineData("timeout=30, max=0", false)]
+        [InlineData("timeout=0, max=1", false)]
+        [InlineData("timeout=0, max=0", false)]
+        public async Task Http10ResponseWithKeepAlive_ConnectionNotReusedForShortTimeoutOrMax0(string keepAlive, bool shouldReuseConnection)
+        {
+            await LoopbackServer.CreateClientAndServerAsync(async uri =>
+            {
+                using HttpClient client = CreateHttpClient();
+
+                await client.GetAsync(uri);
+                await client.GetAsync(uri);
+            },
+            async server =>
+            {
+                await server.AcceptConnectionAsync(async connection =>
+                {
+                    await connection.ReadRequestDataAsync();
+                    await connection.WriteStringAsync($"HTTP/1.0 200 OK\r\nKeep-Alive: {keepAlive}\r\nContent-Length: 1\r\n\r\n1");
+                    connection.CompleteRequestProcessing();
+
+                    if (shouldReuseConnection)
+                    {
+                        await connection.HandleRequestAsync();
+                    }
+                    else
+                    {
+                        await Assert.ThrowsAnyAsync<Exception>(() => connection.ReadRequestDataAsync());
+                    }
+                });
+
+                if (!shouldReuseConnection)
+                {
+                    await server.AcceptConnectionSendResponseAndCloseAsync();
+                }
+            });
+        }
+
+        [Theory]
+        [InlineData("timeout=1")]
+        [InlineData("timeout=0")]
+        [InlineData("max=1")]
+        [InlineData("max=0")]
+        public async Task Http11ResponseWithKeepAlive_KeepAliveIsIgnored(string keepAlive)
+        {
+            await LoopbackServer.CreateClientAndServerAsync(async uri =>
+            {
+                using HttpClient client = CreateHttpClient();
+
+                await client.GetAsync(uri);
+                await client.GetAsync(uri);
+            },
+            async server =>
+            {
+                await server.AcceptConnectionAsync(async connection =>
+                {
+                    await connection.ReadRequestDataAsync();
+                    await connection.WriteStringAsync($"HTTP/1.1 200 OK\r\nKeep-Alive: {keepAlive}\r\nContent-Length: 1\r\n\r\n1");
+                    connection.CompleteRequestProcessing();
+
+                    await connection.HandleRequestAsync();
+                });
+            });
+        }
+    }
+}
index 6c05b3c..8eb14c5 100644 (file)
     <Compile Include="HttpClientHandlerTest.AltSvc.cs" />
     <Compile Include="SocketsHttpHandlerTest.Cancellation.cs" />
     <Compile Include="SocketsHttpHandlerTest.Cancellation.NonParallel.cs" />
+    <Compile Include="SocketsHttpHandlerTest.Http1KeepAlive.cs" />
     <Compile Include="SocketsHttpHandlerTest.Http2FlowControl.cs" />
     <Compile Include="SocketsHttpHandlerTest.Http2KeepAlivePing.cs" />
     <Compile Include="HttpClientHandlerTest.Connect.cs" />