Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Commit cedbe76

Browse files
author
Cesar Blum Silveira
committed
Abort request on client FIN (#1139).
1 parent 51ecbd7 commit cedbe76

20 files changed

+422
-288
lines changed

‎KestrelHttpServer.sln

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "shared", "shared", "{0EF2AC
4242
test\shared\MockSocketOutput.cs = test\shared\MockSocketOutput.cs
4343
test\shared\MockSystemClock.cs = test\shared\MockSystemClock.cs
4444
test\shared\SocketInputExtensions.cs = test\shared\SocketInputExtensions.cs
45+
test\shared\TestApp.cs = test\shared\TestApp.cs
4546
test\shared\TestApplicationErrorLogger.cs = test\shared\TestApplicationErrorLogger.cs
4647
test\shared\TestConnection.cs = test\shared\TestConnection.cs
4748
test\shared\TestFrame.cs = test\shared\TestFrame.cs

‎src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using Microsoft.AspNetCore.Server.Kestrel.Filter.Internal;
1111
using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
1212
using Microsoft.AspNetCore.Server.Kestrel.Internal.Networking;
13+
using Microsoft.Extensions.Internal;
1314
using Microsoft.Extensions.Logging;
1415

1516
namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
@@ -200,7 +201,11 @@ private void ApplyConnectionFilter()
200201
_frame.Input = _filteredStreamAdapter.SocketInput;
201202
_frame.Output = _filteredStreamAdapter.SocketOutput;
202203

203-
_readInputTask = _filteredStreamAdapter.ReadInputAsync();
204+
// Don't attempt to read input if connection has already closed.
205+
// This can happen if a client opens a connection and immediately closes it.
206+
_readInputTask = _socketClosedTcs.Task.Status == TaskStatus.WaitingForActivation ?
207+
_filteredStreamAdapter.ReadInputAsync() :
208+
TaskCache.CompletedTask;
204209
}
205210

206211
_frame.PrepareRequest = _filterContext.PrepareRequest;
@@ -278,7 +283,7 @@ private void OnRead(UvStreamHandle handle, int status)
278283

279284
Input.IncomingComplete(readCount, error);
280285

281-
if (errorDone)
286+
if (!normalRead)
282287
{
283288
Abort(error);
284289
}

‎src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,7 @@ public void Write(ArraySegment<byte> data)
564564
}
565565
else
566566
{
567+
CheckLastWrite();
567568
Output.Write(data);
568569
}
569570
}
@@ -599,6 +600,7 @@ public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationTo
599600
}
600601
else
601602
{
603+
CheckLastWrite();
602604
return Output.WriteAsync(data, cancellationToken: cancellationToken);
603605
}
604606
}
@@ -631,6 +633,7 @@ public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken c
631633
}
632634
else
633635
{
636+
CheckLastWrite();
634637
await Output.WriteAsync(data, cancellationToken: cancellationToken);
635638
}
636639
}
@@ -658,6 +661,24 @@ private void VerifyAndUpdateWrite(int count)
658661
_responseBytesWritten += count;
659662
}
660663

664+
private void CheckLastWrite()
665+
{
666+
var responseHeaders = FrameResponseHeaders;
667+
668+
// Prevent firing request aborted token if this is the last write, to avoid
669+
// aborting the request if the app is still running when the client receives
670+
// the final bytes of the response and gracefully closes the connection.
671+
//
672+
// Called after VerifyAndUpdateWrite(), so _responseBytesWritten has already been updated.
673+
if (responseHeaders != null &&
674+
!responseHeaders.HasTransferEncoding &&
675+
responseHeaders.HasContentLength &&
676+
_responseBytesWritten == responseHeaders.HeaderContentLengthValue.Value)
677+
{
678+
_abortedCts = null;
679+
}
680+
}
681+
661682
protected void VerifyResponseContentLength()
662683
{
663684
var responseHeaders = FrameResponseHeaders;
@@ -838,6 +859,9 @@ private Task WriteSuffix()
838859

839860
private async Task WriteAutoChunkSuffixAwaited()
840861
{
862+
// For the same reason we call CheckLastWrite() in Content-Length responses.
863+
_abortedCts = null;
864+
841865
await WriteChunkedResponseSuffix();
842866

843867
if (_keepAlive)

‎test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ public async Task LargeUpload(long? maxRequestBufferSize, bool ssl, bool expectP
103103
}
104104

105105
Assert.Equal(data.Length, bytesWritten);
106-
socket.Shutdown(SocketShutdown.Send);
107106
clientFinishedSendingRequestBody.Set();
108107
};
109108

‎test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestLineSizeTests.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,11 @@ public class MaxRequestLineSizeTests
2525
[InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 1027)]
2626
public async Task ServerAcceptsRequestLineWithinLimit(string request, int limit)
2727
{
28-
var maxRequestLineSize = limit;
29-
3028
using (var server = CreateServer(limit))
3129
{
3230
using (var connection = new TestConnection(server.Port))
3331
{
34-
await connection.SendEnd(request);
32+
await connection.Send(request);
3533
await connection.ReceiveEnd(
3634
"HTTP/1.1 200 OK",
3735
$"Date: {server.Context.DateHeaderValue}",
@@ -57,8 +55,8 @@ public async Task ServerRejectsRequestLineExceedingLimit(string requestLine)
5755
{
5856
using (var connection = new TestConnection(server.Port))
5957
{
60-
await connection.SendAllTryEnd($"{requestLine}\r\n");
61-
await connection.Receive(
58+
await connection.SendAll(requestLine);
59+
await connection.ReceiveForcedEnd(
6260
"HTTP/1.1 414 URI Too Long",
6361
"Connection: close",
6462
$"Date: {server.Context.DateHeaderValue}",

‎test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestHeaderLimitsTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public async Task ServerAcceptsRequestWithHeaderTotalSizeWithinLimit(int headerC
2828
{
2929
using (var connection = new TestConnection(server.Port))
3030
{
31-
await connection.SendEnd($"GET / HTTP/1.1\r\n{headers}\r\n");
31+
await connection.Send($"GET / HTTP/1.1\r\n{headers}\r\n");
3232
await connection.ReceiveEnd(
3333
"HTTP/1.1 200 OK",
3434
$"Date: {server.Context.DateHeaderValue}",
@@ -60,7 +60,7 @@ public async Task ServerAcceptsRequestWithHeaderCountWithinLimit(int headerCount
6060
{
6161
using (var connection = new TestConnection(server.Port))
6262
{
63-
await connection.SendEnd($"GET / HTTP/1.1\r\n{headers}\r\n");
63+
await connection.Send($"GET / HTTP/1.1\r\n{headers}\r\n");
6464
await connection.ReceiveEnd(
6565
"HTTP/1.1 200 OK",
6666
$"Date: {server.Context.DateHeaderValue}",
@@ -86,7 +86,7 @@ public async Task ServerRejectsRequestWithHeaderTotalSizeOverLimit(int headerCou
8686
{
8787
using (var connection = new TestConnection(server.Port))
8888
{
89-
await connection.SendAllTryEnd($"GET / HTTP/1.1\r\n{headers}\r\n");
89+
await connection.SendAll($"GET / HTTP/1.1\r\n{headers}\r\n");
9090
await connection.ReceiveForcedEnd(
9191
"HTTP/1.1 431 Request Header Fields Too Large",
9292
"Connection: close",
@@ -110,7 +110,7 @@ public async Task ServerRejectsRequestWithHeaderCountOverLimit(int headerCount,
110110
{
111111
using (var connection = new TestConnection(server.Port))
112112
{
113-
await connection.SendAllTryEnd($"GET / HTTP/1.1\r\n{headers}\r\n");
113+
await connection.SendAll($"GET / HTTP/1.1\r\n{headers}\r\n");
114114
await connection.ReceiveForcedEnd(
115115
"HTTP/1.1 431 Request Header Fields Too Large",
116116
"Connection: close",

‎test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using Microsoft.AspNetCore.Http;
1717
using Microsoft.AspNetCore.Http.Features;
1818
using Microsoft.AspNetCore.Server.Kestrel.Internal.Networking;
19+
using Microsoft.AspNetCore.Testing;
1920
using Microsoft.AspNetCore.Testing.xunit;
2021
using Microsoft.Extensions.Logging;
2122
using Microsoft.Extensions.Logging.Testing;
@@ -420,6 +421,38 @@ public async Task ThrowsOnReadAfterConnectionError()
420421
}
421422
}
422423

424+
[Fact]
425+
public async Task RequestAbortedTokenFiredOnClientFIN()
426+
{
427+
var appStarted = new SemaphoreSlim(0);
428+
var requestAborted = new SemaphoreSlim(0);
429+
var builder = new WebHostBuilder()
430+
.UseKestrel()
431+
.UseUrls($"http://127.0.0.1:0")
432+
.Configure(app => app.Run(async context =>
433+
{
434+
appStarted.Release();
435+
436+
var token = context.RequestAborted;
437+
token.Register(() => requestAborted.Release(2));
438+
await requestAborted.WaitAsync().TimeoutAfter(TimeSpan.FromSeconds(10));
439+
}));
440+
441+
using (var host = builder.Build())
442+
{
443+
host.Start();
444+
445+
using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
446+
{
447+
socket.Connect(new IPEndPoint(IPAddress.Loopback, host.GetPort()));
448+
socket.Send(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n\r\n"));
449+
await appStarted.WaitAsync();
450+
socket.Shutdown(SocketShutdown.Send);
451+
await requestAborted.WaitAsync().TimeoutAfter(TimeSpan.FromSeconds(10));
452+
}
453+
}
454+
}
455+
423456
private async Task TestRemoteIPAddress(string registerAddress, string requestAddress, string expectAddress)
424457
{
425458
var builder = new WebHostBuilder()

0 commit comments

Comments
 (0)