Skip to content

Commit

Permalink
Respect the Keep-Alive response header on HTTP/1.0 (#73020)
Browse files Browse the repository at this point in the history
* Respect the Keep-Alive response header on HTTP/1.0

* Remove TimeoutOffset

* Update Trace message

* Update tests

* Adjust test timeouts
  • Loading branch information
MihaZupan authored Aug 2, 2022
1 parent 8762a02 commit 3b706dd
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -148,9 +149,14 @@ private void Dispose(bool disposing)

/// <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)
Expand Down Expand Up @@ -196,9 +202,14 @@ public bool PrepareForReuse(bool async)
}

/// <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();
Expand All @@ -223,6 +234,14 @@ async ValueTask<int> 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<int>? ConsumeReadAheadTask()
{
if (Interlocked.CompareExchange(ref _readAheadTaskLock, 1, 0) == 0)
Expand Down Expand Up @@ -646,6 +665,11 @@ public async Task<HttpResponseMessage> 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)
Expand Down Expand Up @@ -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<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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
<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" />
Expand Down

0 comments on commit 3b706dd

Please sign in to comment.