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

Commit 122ea86

Browse files
author
Cesar Blum Silveira
committed
Throw if connection has error when checking for FIN.
1 parent 255e0ba commit 122ea86

File tree

5 files changed

+88
-19
lines changed

5 files changed

+88
-19
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public override async Task RequestProcessingAsync()
3434
{
3535
while (!_requestProcessingStopping && TakeStartLine(SocketInput) != RequestLineStatus.Done)
3636
{
37-
if (SocketInput.RemoteIntakeFin)
37+
if (SocketInput.CheckFinOrThrow())
3838
{
3939
// We need to attempt to consume start lines and headers even after
4040
// SocketInput.RemoteIntakeFin is set to true to ensure we don't close a
@@ -62,7 +62,7 @@ public override async Task RequestProcessingAsync()
6262

6363
while (!_requestProcessingStopping && !TakeMessageHeaders(SocketInput, FrameRequestHeaders))
6464
{
65-
if (SocketInput.RemoteIntakeFin)
65+
if (SocketInput.CheckFinOrThrow())
6666
{
6767
// We need to attempt to consume start lines and headers even after
6868
// SocketInput.RemoteIntakeFin is set to true to ensure we don't close a

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ private async Task<int> ReadStateMachineAsync(SocketInput input, ArraySegment<by
239239
{
240240
while (_mode == Mode.Prefix)
241241
{
242-
var fin = input.RemoteIntakeFin;
242+
var fin = input.CheckFinOrThrow();
243243

244244
ParseChunkedPrefix(input);
245245

@@ -257,7 +257,7 @@ private async Task<int> ReadStateMachineAsync(SocketInput input, ArraySegment<by
257257

258258
while (_mode == Mode.Extension)
259259
{
260-
var fin = input.RemoteIntakeFin;
260+
var fin = input.CheckFinOrThrow();
261261

262262
ParseExtension(input);
263263

@@ -275,7 +275,7 @@ private async Task<int> ReadStateMachineAsync(SocketInput input, ArraySegment<by
275275

276276
while (_mode == Mode.Data)
277277
{
278-
var fin = input.RemoteIntakeFin;
278+
var fin = input.CheckFinOrThrow();
279279

280280
int actual = ReadChunkedData(input, buffer.Array, buffer.Offset, buffer.Count);
281281

@@ -297,7 +297,7 @@ private async Task<int> ReadStateMachineAsync(SocketInput input, ArraySegment<by
297297

298298
while (_mode == Mode.Suffix)
299299
{
300-
var fin = input.RemoteIntakeFin;
300+
var fin = input.CheckFinOrThrow();
301301

302302
ParseChunkedSuffix(input);
303303

@@ -317,7 +317,7 @@ private async Task<int> ReadStateMachineAsync(SocketInput input, ArraySegment<by
317317
// Chunks finished, parse trailers
318318
while (_mode == Mode.Trailer)
319319
{
320-
var fin = input.RemoteIntakeFin;
320+
var fin = input.CheckFinOrThrow();
321321

322322
ParseChunkedTrailer(input);
323323

@@ -337,7 +337,7 @@ private async Task<int> ReadStateMachineAsync(SocketInput input, ArraySegment<by
337337
{
338338
while (!_context.TakeMessageHeaders(input, _requestHeaders))
339339
{
340-
if (input.RemoteIntakeFin)
340+
if (input.CheckFinOrThrow())
341341
{
342342
if (_context.TakeMessageHeaders(input, _requestHeaders))
343343
{

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@ public SocketInput(MemoryPool memory, IThreadPool threadPool, IBufferSizeControl
4545

4646
public bool IsCompleted => ReferenceEquals(_awaitableState, _awaitableIsCompleted);
4747

48+
public bool CheckFinOrThrow()
49+
{
50+
lock (_sync)
51+
{
52+
if (_awaitableError != null)
53+
{
54+
ThrowOnConnectionError();
55+
}
56+
57+
return RemoteIntakeFin;
58+
}
59+
}
60+
4861
public MemoryPoolBlock IncomingStart()
4962
{
5063
const int minimumSize = 2048;
@@ -303,14 +316,10 @@ public void GetResult()
303316
{
304317
_manualResetEvent.Wait();
305318
}
306-
var error = _awaitableError;
307-
if (error != null)
319+
320+
if (_awaitableError != null)
308321
{
309-
if (error is TaskCanceledException || error is InvalidOperationException)
310-
{
311-
throw error;
312-
}
313-
throw new IOException(error.Message, error);
322+
ThrowOnConnectionError();
314323
}
315324
}
316325

@@ -340,5 +349,15 @@ private static void ReturnBlocks(MemoryPoolBlock block, MemoryPoolBlock end)
340349
returnBlock.Pool.Return(returnBlock);
341350
}
342351
}
352+
353+
private void ThrowOnConnectionError()
354+
{
355+
var error = _awaitableError;
356+
if (error is TaskCanceledException || error is InvalidOperationException)
357+
{
358+
throw error;
359+
}
360+
throw new IOException(error.Message, error);
361+
}
343362
}
344363
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public static ValueTask<int> ReadAsync(this SocketInput input, byte[] buffer, in
1111
{
1212
while (input.IsCompleted)
1313
{
14-
var fin = input.RemoteIntakeFin;
14+
var fin = input.CheckFinOrThrow();
1515

1616
var begin = input.ConsumingStart();
1717
int actual;
@@ -37,7 +37,7 @@ private static async Task<int> ReadAsyncAwaited(this SocketInput input, byte[] b
3737
{
3838
await input;
3939

40-
var fin = input.RemoteIntakeFin;
40+
var fin = input.CheckFinOrThrow();
4141

4242
var begin = input.ConsumingStart();
4343
int actual;

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

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ public async Task ConnectionResetAbortsRequest()
193193
const int connectionErrorEventId = 14; // from KestrelTrace.cs
194194

195195
var testSink = new TestSink(write => write.EventId.Id == connectionErrorEventId);
196-
var requestAbortedAcquired = new CancellationTokenSource();
197196
var builder = new WebHostBuilder()
198197
.UseLoggerFactory(new TestLoggerFactory(testSink, true))
199198
.UseKestrel()
@@ -217,7 +216,6 @@ public async Task ConnectionResetAbortsRequest()
217216
var filteredWrites = testSink.Writes.Where(write => write.EventId.Id == connectionErrorEventId);
218217
for (var i = 0; i < 10; i++)
219218
{
220-
System.Console.WriteLine(i);
221219
if (filteredWrites.Any())
222220
{
223221
break;
@@ -232,6 +230,58 @@ public async Task ConnectionResetAbortsRequest()
232230
}
233231
}
234232

233+
[Fact]
234+
public async Task ThrowsOnReadAfterConnectionError_RequestBody()
235+
{
236+
var requestStarted = new SemaphoreSlim(0);
237+
var connectionReset = new SemaphoreSlim(0);
238+
var appDone = new SemaphoreSlim(0);
239+
var ioExceptionThrown = false;
240+
241+
var builder = new WebHostBuilder()
242+
.UseKestrel()
243+
.UseUrls($"http://127.0.0.1:0")
244+
.Configure(app => app.Run(async context =>
245+
{
246+
requestStarted.Release();
247+
await connectionReset.WaitAsync();
248+
249+
try
250+
{
251+
await context.Request.Body.ReadAsync(new byte[1], 0, 1);
252+
}
253+
catch (BadHttpRequestException)
254+
{
255+
// We need this here because BadHttpRequestException derives from IOException,
256+
// and we're looking for an actual IOException.
257+
}
258+
catch (IOException)
259+
{
260+
ioExceptionThrown = true;
261+
}
262+
263+
appDone.Release(1);
264+
}));
265+
266+
using (var host = builder.Build())
267+
{
268+
host.Start();
269+
270+
using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
271+
{
272+
socket.Connect(new IPEndPoint(IPAddress.Loopback, host.GetPort()));
273+
socket.LingerState = new LingerOption(true, 0);
274+
socket.Send(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\nContent-Length: 1\r\n\r\n"));
275+
await requestStarted.WaitAsync();
276+
}
277+
278+
connectionReset.Release(1);
279+
280+
await appDone.WaitAsync();
281+
Assert.True(ioExceptionThrown);
282+
}
283+
}
284+
235285
private async Task TestRemoteIPAddress(string registerAddress, string requestAddress, string expectAddress)
236286
{
237287
var builder = new WebHostBuilder()

0 commit comments

Comments
 (0)