diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/MessageBody.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/MessageBody.cs index eb987d9590..2bc4ff6e5a 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/MessageBody.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/MessageBody.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Internal.System.IO.Pipelines; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { @@ -92,14 +93,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { // Backpressure, stop controlling incoming data rate until data is read. backpressure = true; - _context.TimeoutControl.PauseTimingReads(); + TryPauseTimingReads(); } await writeAwaitable; if (backpressure) { - _context.TimeoutControl.ResumeTimingReads(); + TryResumeTimingReads(); } if (done) @@ -266,6 +267,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } + private void TryPauseTimingReads() + { + if (!RequestUpgrade) + { + _context.TimeoutControl.PauseTimingReads(); + } + } + + private void TryResumeTimingReads() + { + if (!RequestUpgrade) + { + _context.TimeoutControl.ResumeTimingReads(); + } + } + private void TryStopTimingReads() { if (!RequestUpgrade) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MessageBodyTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MessageBodyTests.cs index 2c8b3b7f8b..3b1cf10a92 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MessageBodyTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MessageBodyTests.cs @@ -647,6 +647,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } } + [Fact] + public async Task PausesAndResumesRequestBodyTimeoutOnBackpressure() + { + using (var input = new TestInput()) + { + var mockTimeoutControl = new Mock(); + input.FrameContext.TimeoutControl = mockTimeoutControl.Object; + + var body = MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders { HeaderContentLength = "12" }, input.Frame); + + // Add some input and read it to start PumpAsync + input.Add("hello,"); + Assert.Equal(6, await body.ReadAsync(new ArraySegment(new byte[6]))); + + input.Add(" world"); + Assert.Equal(6, await body.ReadAsync(new ArraySegment(new byte[6]))); + + // Due to the limits set on Frame.RequestBodyPipe, backpressure should be triggered on every write to that pipe. + mockTimeoutControl.Verify(timeoutControl => timeoutControl.PauseTimingReads(), Times.Exactly(2)); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.ResumeTimingReads(), Times.Exactly(2)); + } + } + [Fact] public async Task OnlyEnforcesRequestBodyTimeoutAfterSending100Continue() { @@ -700,6 +723,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests mockTimeoutControl.Verify(timeoutControl => timeoutControl.StartTimingReads(), Times.Never); mockTimeoutControl.Verify(timeoutControl => timeoutControl.StopTimingReads(), Times.Never); + + // Due to the limits set on Frame.RequestBodyPipe, backpressure should be triggered on every + // write to that pipe. Verify that read timing pause and resume are not called on upgrade + // requests. + mockTimeoutControl.Verify(timeoutControl => timeoutControl.PauseTimingReads(), Times.Never); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.ResumeTimingReads(), Times.Never); } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs index 8bf0ce3831..650095e84c 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs @@ -17,7 +17,9 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions; using Microsoft.AspNetCore.Testing; using Microsoft.AspNetCore.Testing.xunit; @@ -1415,6 +1417,68 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } + [Fact] + public async Task DoesNotEnforceRequestBodyMinimumDataRateOnUpgradedRequest() + { + var appEvent = new ManualResetEventSlim(); + var delayEvent = new ManualResetEventSlim(); + var serviceContext = new TestServiceContext + { + SystemClock = new SystemClock() + }; + + using (var server = new TestServer(async context => + { + context.Features.Get().MinimumDataRate = new MinimumDataRate(rate: double.MaxValue, gracePeriod: TimeSpan.Zero); + + using (var stream = await context.Features.Get().UpgradeAsync()) + { + appEvent.Set(); + + // Read once to go through one set of TryPauseTimingReads()/TryResumeTimingReads() calls + await stream.ReadAsync(new byte[1], 0, 1); + + delayEvent.Wait(); + + // Read again to check that the connection is still alive + await stream.ReadAsync(new byte[1], 0, 1); + + // Send a response to distinguish from the timeout case where the 101 is still received, but without any content + var response = Encoding.ASCII.GetBytes("hello"); + await stream.WriteAsync(response, 0, response.Length); + } + }, serviceContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "Connection: upgrade", + "", + "a"); + + Assert.True(appEvent.Wait(TimeSpan.FromSeconds(10))); + + await Task.Delay(TimeSpan.FromSeconds(5)); + + delayEvent.Set(); + + await connection.Send("b"); + + await connection.Receive( + "HTTP/1.1 101 Switching Protocols", + "Connection: Upgrade", + ""); + await connection.ReceiveStartsWith( + $"Date: "); + await connection.ReceiveForcedEnd( + "", + "hello"); + } + } + } + private async Task TestRemoteIPAddress(string registerAddress, string requestAddress, string expectAddress) { var builder = new WebHostBuilder()