diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 2d92a7b74e9a1..8d55e073efebe 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -67,6 +67,7 @@ internal sealed partial class HttpConnection : HttpConnectionBase 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 @@ private void Dispose(bool disposing) /// Prepare an idle connection to be used for a new request. /// Indicates whether the coming request will be sync or async. - /// True if connection can be used, false if it is invalid due to receiving EOF or unexpected data. + /// True if connection can be used, false if it is invalid due to a timeout or receiving EOF or unexpected data. 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 @@ public bool PrepareForReuse(bool async) } /// Check whether a currently idle connection is still usable, or should be scavenged. - /// True if connection can be used, false if it is invalid due to receiving EOF or unexpected data. + /// True if connection can be used, false if it is invalid due to a timeout or receiving EOF or unexpected data. 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 @@ async ValueTask ReadAheadWithZeroByteReadAsync() } } + 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? ConsumeReadAheadTask() { if (Interlocked.CompareExchange(ref _readAheadTaskLock, 1, 0) == 0) @@ -646,6 +665,11 @@ public async Task SendAsyncCore(HttpRequestMessage request, 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 @@ private static void ParseHeaderNameValue(HttpConnection connection, ReadOnlySpan } } + private void ProcessHttp10KeepAliveHeader(HttpResponseMessage response) + { + if (response.Headers.NonValidated.TryGetValues(KnownHeaders.KeepAlive.Name, out HeaderStringValues keepAliveValues)) + { + string keepAlive = keepAliveValues.ToString(); + var parsedValues = new UnvalidatedObjectCollection(); + + 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 source) { Debug.Assert(source.Length <= _writeBuffer.Length - _writeOffset); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index ebc5c612d9e63..e560915288e56 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -2324,7 +2324,7 @@ static bool IsUsableConnection(HttpConnectionBase connection, long nowTicks, Tim 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 index 0000000000000..75ed4219e1783 --- /dev/null +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http1KeepAlive.cs @@ -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(() => 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(() => 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(); + }); + }); + } + } +} diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj index 6c05b3cc0f35b..8eb14c5c99c8a 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj @@ -160,6 +160,7 @@ +