diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx b/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx index 72d2a8aa5..13454c512 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx @@ -333,4 +333,7 @@ Value must be a non-negative TimeSpan. + + The request body rate enforcement grace period must be greater than {heartbeatInterval} second. + diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/MinimumDataRate.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/MinimumDataRate.cs index c73b5deb3..a7985171e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/MinimumDataRate.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/MinimumDataRate.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Core { @@ -11,17 +12,18 @@ public class MinimumDataRate /// Creates a new instance of . /// /// The minimum rate in bytes/second at which data should be processed. - /// The amount of time to delay enforcement of . + /// The amount of time to delay enforcement of , + /// starting at the time data is first read or written. public MinimumDataRate(double bytesPerSecond, TimeSpan gracePeriod) { - if (bytesPerSecond <= 0) + if (bytesPerSecond < 0) { - throw new ArgumentOutOfRangeException(nameof(bytesPerSecond), CoreStrings.PositiveNumberRequired); + throw new ArgumentOutOfRangeException(nameof(bytesPerSecond), CoreStrings.NonNegativeNumberRequired); } - if (gracePeriod < TimeSpan.Zero) + if (gracePeriod <= Heartbeat.Interval) { - throw new ArgumentOutOfRangeException(nameof(gracePeriod), CoreStrings.NonNegativeTimeSpanRequired); + throw new ArgumentOutOfRangeException(nameof(gracePeriod), CoreStrings.FormatMinimumGracePeriodRequired(Heartbeat.Interval.TotalSeconds)); } BytesPerSecond = bytesPerSecond; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs index ce955e41d..5fb571f55 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -1018,6 +1018,20 @@ internal static string NonNegativeTimeSpanRequired internal static string FormatNonNegativeTimeSpanRequired() => GetString("NonNegativeTimeSpanRequired"); + /// + /// The request body rate enforcement grace period must be greater than {heartbeatInterval} seconds. + /// + internal static string MinimumGracePeriodRequired + { + get => GetString("MinimumGracePeriodRequired"); + } + + /// + /// The request body rate enforcement grace period must be greater than {heartbeatInterval} seconds. + /// + internal static string FormatMinimumGracePeriodRequired(object heartbeatInterval) + => string.Format(CultureInfo.CurrentCulture, GetString("MinimumGracePeriodRequired", "heartbeatInterval"), heartbeatInterval); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionTests.cs index 7968c03e4..163f47240 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionTests.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; -using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Internal.System.IO.Pipelines; @@ -128,7 +127,7 @@ public void RequestBodyMinimumDataRateNotEnforcedDuringGracePeriod() public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() { var bytesPerSecond = 100; - var gracePeriod = TimeSpan.FromSeconds(1); + var gracePeriod = TimeSpan.FromSeconds(2); _frameConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = new MinimumDataRate(bytesPerSecond: bytesPerSecond, gracePeriod: gracePeriod); @@ -145,14 +144,14 @@ public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() _frameConnection.StartTimingReads(); - // Tick after grace period to start enforcing minimum data rate + // Set base data rate to 200 bytes/second now += gracePeriod; - _frameConnection.BytesRead(100); + _frameConnection.BytesRead(400); _frameConnection.Tick(now); // Data rate: 200 bytes/second now += TimeSpan.FromSeconds(1); - _frameConnection.BytesRead(300); + _frameConnection.BytesRead(200); _frameConnection.Tick(now); // Not timed out @@ -162,7 +161,7 @@ public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() // Data rate: 150 bytes/second now += TimeSpan.FromSeconds(1); - _frameConnection.BytesRead(50); + _frameConnection.BytesRead(0); _frameConnection.Tick(now); // Not timed out @@ -170,9 +169,9 @@ public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); - // Data rate: 115 bytes/second + // Data rate: 120 bytes/second now += TimeSpan.FromSeconds(1); - _frameConnection.BytesRead(10); + _frameConnection.BytesRead(0); _frameConnection.Tick(now); // Not timed out @@ -180,9 +179,19 @@ public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); - // Data rate: 50 bytes/second - now += TimeSpan.FromSeconds(6); - _frameConnection.BytesRead(40); + // Data rate: 100 bytes/second + now += TimeSpan.FromSeconds(1); + _frameConnection.BytesRead(0); + _frameConnection.Tick(now); + + // Not timed out + Assert.False(_frameConnection.TimedOut); + mockLogger.Verify(logger => + logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); + + // Data rate: ~85 bytes/second + now += TimeSpan.FromSeconds(1); + _frameConnection.BytesRead(0); _frameConnection.Tick(now); // Timed out @@ -194,11 +203,10 @@ public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() [Fact] public void RequestBodyDataRateNotComputedOnPausedTime() { - var requestBodyTimeout = TimeSpan.FromSeconds(5); var systemClock = new MockSystemClock(); _frameConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinimumDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.Zero); + new MinimumDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); _frameConnectionContext.ServiceContext.SystemClock = systemClock; var mockLogger = new Mock(); @@ -212,21 +220,21 @@ public void RequestBodyDataRateNotComputedOnPausedTime() _frameConnection.StartTimingReads(); - // Tick at 1s, expected counted time is 1s, expected data rate is 400 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(1); - _frameConnection.BytesRead(400); + // Tick at 3s, expected counted time is 3s, expected data rate is 200 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(3); + _frameConnection.BytesRead(600); _frameConnection.Tick(systemClock.UtcNow); - // Pause at 1.5s + // Pause at 3.5s systemClock.UtcNow += TimeSpan.FromSeconds(0.5); _frameConnection.PauseTimingReads(); - // Tick at 2s, expected counted time is 2s, expected data rate is 400 bytes/second + // Tick at 4s, expected counted time is 4s (first tick after pause goes through), expected data rate is 150 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(0.5); _frameConnection.Tick(systemClock.UtcNow); - // Tick at 6s, expected counted time is 2s, expected data rate is 400 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(4); + // Tick at 6s, expected counted time is 4s, expected data rate is 150 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(2); _frameConnection.Tick(systemClock.UtcNow); // Not timed out @@ -239,7 +247,7 @@ public void RequestBodyDataRateNotComputedOnPausedTime() systemClock.UtcNow += TimeSpan.FromSeconds(0.5); _frameConnection.ResumeTimingReads(); - // Tick at 8s, expected counted time is 4s, expected data rate is 100 bytes/second + // Tick at 9s, expected counted time is 6s, expected data rate is 100 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(1.5); _frameConnection.Tick(systemClock.UtcNow); @@ -249,7 +257,7 @@ public void RequestBodyDataRateNotComputedOnPausedTime() logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - // Tick at 9s, expected counted time is 9s, expected data rate drops below 100 bytes/second + // Tick at 10s, expected counted time is 7s, expected data rate drops below 100 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(1); _frameConnection.Tick(systemClock.UtcNow); @@ -266,7 +274,7 @@ public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() var systemClock = new MockSystemClock(); _frameConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinimumDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.Zero); + new MinimumDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); _frameConnectionContext.ServiceContext.SystemClock = systemClock; var mockLogger = new Mock(); @@ -280,9 +288,9 @@ public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() _frameConnection.StartTimingReads(); - // Tick at 1s, expected counted time is 1s, expected data rate is 100 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(1); - _frameConnection.BytesRead(100); + // Tick at 2s, expected counted time is 2s, expected data rate is 100 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(2); + _frameConnection.BytesRead(200); _frameConnection.Tick(systemClock.UtcNow); // Not timed out @@ -291,15 +299,15 @@ public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - // Pause at 1.25s + // Pause at 2.25s systemClock.UtcNow += TimeSpan.FromSeconds(0.25); _frameConnection.PauseTimingReads(); - // Resume at 1.5s + // Resume at 2.5s systemClock.UtcNow += TimeSpan.FromSeconds(0.25); _frameConnection.ResumeTimingReads(); - // Tick at 2s, expected counted time is 2s, expected data rate is 100 bytes/second + // Tick at 3s, expected counted time is 3s, expected data rate is 100 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(0.5); _frameConnection.BytesRead(100); _frameConnection.Tick(systemClock.UtcNow); @@ -310,7 +318,7 @@ public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - // Tick at 3s, expected counted time is 3s, expected data rate drops below 100 bytes/second + // Tick at 4s, expected counted time is 4s, expected data rate drops below 100 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(1); _frameConnection.Tick(systemClock.UtcNow); diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameTests.cs index 161d4c593..19072e91a 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameTests.cs @@ -143,7 +143,7 @@ public void ResetResetsTraceIdentifier() [Fact] public void ResetResetsMinRequestBodyDataRate() { - _frame.MinRequestBodyDataRate = new MinimumDataRate(bytesPerSecond: 1, gracePeriod: TimeSpan.Zero); + _frame.MinRequestBodyDataRate = new MinimumDataRate(bytesPerSecond: 1, gracePeriod: TimeSpan.MaxValue); _frame.Reset(); @@ -881,7 +881,7 @@ public static TheoryData QueryStringWithNullCharData public static TheoryData MinRequestBodyDataRateData => new TheoryData { null, - new MinimumDataRate(bytesPerSecond: 1, gracePeriod: TimeSpan.Zero) + new MinimumDataRate(bytesPerSecond: 1, gracePeriod: TimeSpan.MaxValue) }; private class RequestHeadersWrapper : IHeaderDictionary diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/HeartbeatTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/HeartbeatTests.cs index 1101f56ef..6cd3b2700 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/HeartbeatTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/HeartbeatTests.cs @@ -15,6 +15,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { public class HeartbeatTests { + [Fact] + public void HeartbeatIntervalIsOneSecond() + { + Assert.Equal(TimeSpan.FromSeconds(1), Heartbeat.Interval); + } + [Fact] public void BlockedHeartbeatDoesntCauseOverlapsAndIsLoggedAsError() { diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MinimumDataRateTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MinimumDataRateTests.cs index 7e8a89d5e..20c4b55f3 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MinimumDataRateTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/MinimumDataRateTests.cs @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using Microsoft.AspNetCore.Server.Kestrel.Core.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests @@ -10,22 +10,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public class MinimumDataRateTests { [Theory] + [InlineData(0)] [InlineData(double.Epsilon)] [InlineData(double.MaxValue)] public void BytesPerSecondValid(double value) { - Assert.Equal(value, new MinimumDataRate(bytesPerSecond: value, gracePeriod: TimeSpan.Zero).BytesPerSecond); + Assert.Equal(value, new MinimumDataRate(bytesPerSecond: value, gracePeriod: TimeSpan.MaxValue).BytesPerSecond); } [Theory] [InlineData(double.MinValue)] - [InlineData(0)] + [InlineData(-double.Epsilon)] public void BytesPerSecondInvalid(double value) { - var exception = Assert.Throws(() => new MinimumDataRate(bytesPerSecond: value, gracePeriod: TimeSpan.Zero)); + var exception = Assert.Throws(() => new MinimumDataRate(bytesPerSecond: value, gracePeriod: TimeSpan.MaxValue)); Assert.Equal("bytesPerSecond", exception.ParamName); - Assert.StartsWith(CoreStrings.PositiveNumberRequired, exception.Message); + Assert.StartsWith(CoreStrings.NonNegativeNumberRequired, exception.Message); } [Theory] @@ -42,20 +43,21 @@ public void GracePeriodInvalid(TimeSpan value) var exception = Assert.Throws(() => new MinimumDataRate(bytesPerSecond: 1, gracePeriod: value)); Assert.Equal("gracePeriod", exception.ParamName); - Assert.StartsWith(CoreStrings.NonNegativeTimeSpanRequired, exception.Message); + Assert.StartsWith(CoreStrings.FormatMinimumGracePeriodRequired(Heartbeat.Interval.TotalSeconds), exception.Message); } public static TheoryData GracePeriodValidData => new TheoryData { - TimeSpan.Zero, - TimeSpan.FromTicks(1), + Heartbeat.Interval + TimeSpan.FromTicks(1), TimeSpan.MaxValue }; public static TheoryData GracePeriodInvalidData => new TheoryData { TimeSpan.MinValue, - TimeSpan.FromTicks(-1) + TimeSpan.FromTicks(-1), + TimeSpan.Zero, + Heartbeat.Interval }; } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs index 5882cd559..cd608be55 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs @@ -1429,7 +1429,8 @@ public async Task DoesNotEnforceRequestBodyMinimumDataRateOnUpgradedRequest() using (var server = new TestServer(async context => { - context.Features.Get().MinimumDataRate = new MinimumDataRate(bytesPerSecond: double.MaxValue, gracePeriod: TimeSpan.Zero); + context.Features.Get().MinimumDataRate = + new MinimumDataRate(bytesPerSecond: double.MaxValue, gracePeriod: Heartbeat.Interval + TimeSpan.FromTicks(1)); using (var stream = await context.Features.Get().UpgradeAsync()) {