From 1d3090f056f0e17b02750bea3bbadf14aecf20a1 Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Wed, 17 Oct 2018 15:53:57 -0700 Subject: [PATCH] Only reset the request body pipe for HTTP/1 #3006 --- .../Internal/Http/Http1MessageBody.cs | 18 +++++++- .../Internal/Http/HttpProtocol.cs | 3 -- test/Kestrel.Core.Tests/MessageBodyTests.cs | 46 +++++++++---------- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index 249180d231..97a81919cd 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -137,9 +137,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return Task.CompletedTask; } + // PumpTask catches all Exceptions internally. + if (_pumpTask.IsCompleted) + { + // At this point both the request body pipe reader and writer should be completed. + _context.RequestBodyPipe.Reset(); + return Task.CompletedTask; + } + + return StopAsyncAwaited(); + } + + private async Task StopAsyncAwaited() + { _canceled = true; _context.Input.CancelPendingRead(); - return _pumpTask; + await _pumpTask; + + // At this point both the request body pipe reader and writer should be completed. + _context.RequestBodyPipe.Reset(); } protected override Task OnConsumeAsync() diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index ebcd74a0b2..88ab98a5f0 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -630,9 +630,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // Wait for MessageBody.PumpAsync() to call RequestBodyPipe.Writer.Complete(). await messageBody.StopAsync(); - - // At this point both the request body pipe reader and writer should be completed. - RequestBodyPipe.Reset(); } } } diff --git a/test/Kestrel.Core.Tests/MessageBodyTests.cs b/test/Kestrel.Core.Tests/MessageBodyTests.cs index da210fa84c..3ec3d2e2fe 100644 --- a/test/Kestrel.Core.Tests/MessageBodyTests.cs +++ b/test/Kestrel.Core.Tests/MessageBodyTests.cs @@ -47,6 +47,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests count = stream.Read(buffer, 0, buffer.Length); Assert.Equal(0, count); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -73,6 +74,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests count = await stream.ReadAsync(buffer, 0, buffer.Length); Assert.Equal(0, count); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -101,6 +103,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests count = stream.Read(buffer, 0, buffer.Length); Assert.Equal(0, count); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -127,6 +130,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests count = await stream.ReadAsync(buffer, 0, buffer.Length); Assert.Equal(0, count); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -152,6 +156,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(5, await readTask.DefaultTimeout()); Assert.Equal(0, await stream.ReadAsync(buffer, 0, buffer.Length)); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -173,6 +178,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.IsType(ex.InnerException); Assert.Equal(CoreStrings.BadRequest_BadChunkSizeData, ex.Message); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -194,6 +200,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(CoreStrings.BadRequest_BadChunkSizeData, ex.Message); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -221,6 +228,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Fin(); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -246,6 +254,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Fin(); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -316,6 +325,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(8197, requestArray.Length); AssertASCII(largeInput + "Hello", new ArraySegment(requestArray, 0, requestArray.Length)); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -381,6 +391,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(0, await body.ReadAsync(new ArraySegment(new byte[1]))); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -398,6 +409,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(0, await body.ReadAsync(new ArraySegment(new byte[1]))); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -494,6 +506,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Fin(); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -520,6 +533,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Fin(); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -543,34 +557,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Add("b"); Assert.Equal(1, await stream.ReadAsync(new byte[1], 0, 1)); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } - [Fact] - public async Task StopAsyncPreventsFurtherDataConsumption() - { - using (var input = new TestInput()) - { - var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderContentLength = "2" }, input.Http1Connection); - var stream = new HttpRequestStream(Mock.Of()); - stream.StartAcceptingReads(body); - - // Add some input and consume it to ensure PumpAsync is running - input.Add("a"); - Assert.Equal(1, await stream.ReadAsync(new byte[1], 0, 1)); - - await body.StopAsync(); - - // Add some more data. Checking for cancellation and exiting the loop - // should take priority over reading this data. - input.Add("b"); - - // There shouldn't be any additional data available - Assert.Equal(0, await stream.ReadAsync(new byte[1], 0, 1)); - } - } - [Fact] public async Task ReadAsyncThrowsOnTimeout() { @@ -592,6 +583,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var exception = await Assert.ThrowsAsync(async () => await body.ReadAsync(new Memory(new byte[1]))); Assert.Equal(StatusCodes.Status408RequestTimeout, exception.StatusCode); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -622,6 +614,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests It.IsAny(), It.Is(ex => ex.Reason == RequestRejectionReason.RequestBodyTimeout))); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -650,6 +643,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(StatusCodes.Status408RequestTimeout, exception.StatusCode); } + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -676,6 +670,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Fin(); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -706,6 +701,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await logEvent.Task.DefaultTimeout(); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -765,6 +761,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Add("a"); await readTask; + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } } @@ -797,6 +794,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests mockTimeoutControl.Verify(timeoutControl => timeoutControl.PauseTimingReads(), Times.Never); mockTimeoutControl.Verify(timeoutControl => timeoutControl.ResumeTimingReads(), Times.Never); + input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); } }