diff --git a/src/Kestrel.Core/Features/IHttpMinRequestBodyDataRateFeature.cs b/src/Kestrel.Core/Features/IHttpMinRequestBodyDataRateFeature.cs index f80bd9977..0244621f0 100644 --- a/src/Kestrel.Core/Features/IHttpMinRequestBodyDataRateFeature.cs +++ b/src/Kestrel.Core/Features/IHttpMinRequestBodyDataRateFeature.cs @@ -5,6 +5,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Features { /// /// Feature to set the minimum data rate at which the the request body must be sent by the client. + /// This feature is not available for HTTP/2 requests. Instead, use + /// for server-wide configuration which applies to both HTTP/2 and HTTP/1.x. /// public interface IHttpMinRequestBodyDataRateFeature { @@ -12,6 +14,8 @@ public interface IHttpMinRequestBodyDataRateFeature /// The minimum data rate in bytes/second at which the request body must be sent by the client. /// Setting this property to null indicates no minimum data rate should be enforced. /// This limit has no effect on upgraded connections which are always unlimited. + /// This feature is not available for HTTP/2 requests. Instead, use + /// for server-wide configuration which applies to both HTTP/2 and HTTP/1.x. /// MinDataRate MinDataRate { get; set; } } diff --git a/src/Kestrel.Core/Features/IHttpMinResponseDataRateFeature.cs b/src/Kestrel.Core/Features/IHttpMinResponseDataRateFeature.cs index f901a338d..24c897ae8 100644 --- a/src/Kestrel.Core/Features/IHttpMinResponseDataRateFeature.cs +++ b/src/Kestrel.Core/Features/IHttpMinResponseDataRateFeature.cs @@ -5,6 +5,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Features { /// /// Feature to set the minimum data rate at which the response must be received by the client. + /// This feature is not available for HTTP/2 requests. Instead, use + /// for server-wide configuration which applies to both HTTP/2 and HTTP/1.x. /// public interface IHttpMinResponseDataRateFeature { @@ -12,6 +14,8 @@ public interface IHttpMinResponseDataRateFeature /// The minimum data rate in bytes/second at which the response must be received by the client. /// Setting this property to null indicates no minimum data rate should be enforced. /// This limit has no effect on upgraded connections which are always unlimited. + /// This feature is not available for HTTP/2 requests. Instead, use + /// for server-wide configuration which applies to both HTTP/2 and HTTP/1.x. /// MinDataRate MinDataRate { get; set; } } diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.FeatureCollection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.FeatureCollection.cs index 9264a75df..037f9d773 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.FeatureCollection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.FeatureCollection.cs @@ -11,7 +11,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { - public partial class Http1Connection : IHttpUpgradeFeature + public partial class Http1Connection : IHttpUpgradeFeature, + IHttpMinRequestBodyDataRateFeature, + IHttpMinResponseDataRateFeature { bool IHttpUpgradeFeature.IsUpgradableRequest => IsUpgradableRequest; @@ -44,5 +46,17 @@ async Task IHttpUpgradeFeature.UpgradeAsync() return _streams.Upgrade(); } + + MinDataRate IHttpMinRequestBodyDataRateFeature.MinDataRate + { + get => MinRequestBodyDataRate; + set => MinRequestBodyDataRate = value; + } + + MinDataRate IHttpMinResponseDataRateFeature.MinDataRate + { + get => MinResponseDataRate; + set => MinResponseDataRate = value; + } } } diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.cs index 26ec08808..145e1abfa 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.cs @@ -59,11 +59,14 @@ public Http1Connection(HttpConnectionContext context) public PipeReader Input => _context.Transport.Input; - public ITimeoutControl TimeoutControl => _context.TimeoutControl; public bool RequestTimedOut => _requestTimedOut; public override bool IsUpgradableRequest => _upgradeAvailable; + public MinDataRate MinRequestBodyDataRate { get; set; } + + public MinDataRate MinResponseDataRate { get; set; } + protected override void OnRequestProcessingEnded() { Input.Complete(); @@ -125,6 +128,12 @@ public void SendTimeoutResponse() public void HandleRequestHeadersTimeout() => SendTimeoutResponse(); + public void HandleReadDataRateTimeout() + { + Log.RequestBodyMinimumDataRateNotSatisfied(ConnectionId, TraceIdentifier, MinRequestBodyDataRate.BytesPerSecond); + SendTimeoutResponse(); + } + public void ParseRequest(ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { consumed = buffer.Start; @@ -423,13 +432,16 @@ private void ValidateNonOriginHostHeader(string hostText) protected override void OnReset() { - ResetIHttpUpgradeFeature(); + ResetHttp1Features(); _requestTimedOut = false; _requestTargetForm = HttpRequestTarget.Unknown; _absoluteRequestTarget = null; _remainingRequestHeadersBytesAllowed = ServerOptions.Limits.MaxRequestHeadersTotalSize + 2; _requestCount++; + + MinRequestBodyDataRate = ServerOptions.Limits.MinRequestBodyDataRate; + MinResponseDataRate = ServerOptions.Limits.MinResponseDataRate; } protected override void OnRequestProcessingEnding() diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index 97a81919c..15fb2dcb8 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -18,10 +18,9 @@ public abstract class Http1MessageBody : MessageBody private volatile bool _canceled; private Task _pumpTask; - private bool _timingReads; protected Http1MessageBody(Http1Connection context) - : base(context) + : base(context, context.MinRequestBodyDataRate) { _context = context; } @@ -39,8 +38,6 @@ private async Task PumpAsync() TryProduceContinue(); } - TryStartTimingReads(); - while (true) { var result = await awaitable; @@ -66,22 +63,7 @@ private async Task PumpAsync() bool done; done = Read(readableBuffer, _context.RequestBodyPipe.Writer, out consumed, out examined); - var writeAwaitable = _context.RequestBodyPipe.Writer.FlushAsync(); - var backpressure = false; - - if (!writeAwaitable.IsCompleted) - { - // Backpressure, stop controlling incoming data rate until data is read. - backpressure = true; - TryPauseTimingReads(); - } - - await writeAwaitable; - - if (backpressure) - { - TryResumeTimingReads(); - } + await _context.RequestBodyPipe.Writer.FlushAsync(); if (done) { @@ -109,7 +91,6 @@ private async Task PumpAsync() BadHttpRequestException.Throw(RequestRejectionReason.UnexpectedEndOfRequestContent); } - } finally { @@ -126,11 +107,10 @@ private async Task PumpAsync() finally { _context.RequestBodyPipe.Writer.Complete(error); - TryStopTimingReads(); } } - public override Task StopAsync() + protected override Task OnStopAsync() { if (!_context.HasStartedConsumingRequestBody) { @@ -219,8 +199,6 @@ private async Task OnConsumeAsyncAwaited() protected void Copy(ReadOnlySequence readableBuffer, PipeWriter writableBuffer) { - _context.TimeoutControl.BytesRead(readableBuffer.Length); - if (readableBuffer.IsSingleSegment) { writableBuffer.Write(readableBuffer.First.Span); @@ -244,53 +222,6 @@ protected virtual bool Read(ReadOnlySequence readableBuffer, PipeWriter wr throw new NotImplementedException(); } - private void TryStartTimingReads() - { - if (!RequestUpgrade) - { - Log.RequestBodyStart(_context.ConnectionIdFeature, _context.TraceIdentifier); - - // REVIEW: This makes it no longer effective to change the min rate after the app starts reading. - // Is this OK? Should we throw from the MinRequestBodyDataRate setter in this case? - var minRate = _context.MinRequestBodyDataRate; - - if (minRate != null) - { - _timingReads = true; - _context.TimeoutControl.StartTimingReads(minRate); - } - } - } - - private void TryPauseTimingReads() - { - if (_timingReads) - { - _context.TimeoutControl.PauseTimingReads(); - } - } - - private void TryResumeTimingReads() - { - if (_timingReads) - { - _context.TimeoutControl.ResumeTimingReads(); - } - } - - private void TryStopTimingReads() - { - if (!RequestUpgrade) - { - Log.RequestBodyDone(_context.ConnectionIdFeature, _context.TraceIdentifier); - - if (_timingReads) - { - _context.TimeoutControl.StopTimingReads(); - } - } - } - public static MessageBody For( HttpVersion httpVersion, HttpRequestHeaders headers, diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.FeatureCollection.cs index 51b5daf83..e947e708f 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -19,9 +19,7 @@ public partial class HttpProtocol : IHttpRequestFeature, IHttpRequestLifetimeFeature, IHttpRequestIdentifierFeature, IHttpBodyControlFeature, - IHttpMaxRequestBodySizeFeature, - IHttpMinRequestBodyDataRateFeature, - IHttpMinResponseDataRateFeature + IHttpMaxRequestBodySizeFeature { // NOTE: When feature interfaces are added to or removed from this HttpProtocol class implementation, // then the list of `implementedFeatures` in the generated code project MUST also be updated. @@ -192,21 +190,11 @@ bool IHttpBodyControlFeature.AllowSynchronousIO } } - MinDataRate IHttpMinRequestBodyDataRateFeature.MinDataRate - { - get => MinRequestBodyDataRate; - set => MinRequestBodyDataRate = value; - } - - MinDataRate IHttpMinResponseDataRateFeature.MinDataRate - { - get => MinResponseDataRate; - set => MinResponseDataRate = value; - } - - protected void ResetIHttpUpgradeFeature() + protected void ResetHttp1Features() { _currentIHttpUpgradeFeature = this; + _currentIHttpMinRequestBodyDataRateFeature = this; + _currentIHttpMinResponseDataRateFeature = this; } protected void ResetHttp2Features() diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.Generated.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.Generated.cs index af2b54096..c913c6adc 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.Generated.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.Generated.cs @@ -71,8 +71,6 @@ private void FastReset() _currentIHttpRequestLifetimeFeature = this; _currentIHttpConnectionFeature = this; _currentIHttpMaxRequestBodySizeFeature = this; - _currentIHttpMinRequestBodyDataRateFeature = this; - _currentIHttpMinResponseDataRateFeature = this; _currentIHttpBodyControlFeature = this; _currentIServiceProvidersFeature = null; @@ -87,6 +85,8 @@ private void FastReset() _currentITlsConnectionFeature = null; _currentIHttpWebSocketFeature = null; _currentISessionFeature = null; + _currentIHttpMinRequestBodyDataRateFeature = null; + _currentIHttpMinResponseDataRateFeature = null; _currentIHttpSendFileFeature = null; } diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index 88ab98a5f..7cb684417 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -83,6 +83,7 @@ public HttpProtocol(HttpConnectionContext context) public ServiceContext ServiceContext => _context.ServiceContext; private IPEndPoint LocalEndPoint => _context.LocalEndPoint; private IPEndPoint RemoteEndPoint => _context.RemoteEndPoint; + public ITimeoutControl TimeoutControl => _context.TimeoutControl; public IFeatureCollection ConnectionFeatures => _context.ConnectionFeatures; public IHttpOutputProducer Output { get; protected set; } @@ -275,10 +276,6 @@ public CancellationToken RequestAborted protected HttpResponseHeaders HttpResponseHeaders { get; } = new HttpResponseHeaders(); - public MinDataRate MinRequestBodyDataRate { get; set; } - - public MinDataRate MinResponseDataRate { get; set; } - public void InitializeStreams(MessageBody messageBody) { if (_streams == null) @@ -363,9 +360,6 @@ public void Reset() _responseBytesWritten = 0; - MinRequestBodyDataRate = ServerOptions.Limits.MinRequestBodyDataRate; - MinResponseDataRate = ServerOptions.Limits.MinResponseDataRate; - OnReset(); } @@ -628,7 +622,7 @@ private async Task ProcessRequests(IHttpApplication applicat { RequestBodyPipe.Reader.Complete(); - // Wait for MessageBody.PumpAsync() to call RequestBodyPipe.Writer.Complete(). + // Wait for Http1MessageBody.PumpAsync() to call RequestBodyPipe.Writer.Complete(). await messageBody.StopAsync(); } } diff --git a/src/Kestrel.Core/Internal/Http/MessageBody.cs b/src/Kestrel.Core/Internal/Http/MessageBody.cs index 2ef88d5f3..750a49079 100644 --- a/src/Kestrel.Core/Internal/Http/MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/MessageBody.cs @@ -4,6 +4,7 @@ using System; using System.Buffers; using System.IO; +using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -16,13 +17,20 @@ public abstract class MessageBody private static readonly MessageBody _zeroContentLengthKeepAlive = new ForZeroContentLength(keepAlive: true); private readonly HttpProtocol _context; + private readonly MinDataRate _minRequestBodyDataRate; private bool _send100Continue = true; private long _consumedBytes; + private bool _stopped; - protected MessageBody(HttpProtocol context) + private bool _timingEnabled; + private bool _backpressure; + private long _alreadyTimedBytes; + + protected MessageBody(HttpProtocol context, MinDataRate minRequestBodyDataRate) { _context = context; + _minRequestBodyDataRate = minRequestBodyDataRate; } public static MessageBody ZeroContentLengthClose => _zeroContentLengthClose; @@ -39,12 +47,15 @@ protected MessageBody(HttpProtocol context) public virtual async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default(CancellationToken)) { - TryInit(); + TryStart(); while (true) { - var result = await _context.RequestBodyPipe.Reader.ReadAsync(cancellationToken); + var result = await StartTimingReadAsync(cancellationToken); var readableBuffer = result.Buffer; + var readableBufferLength = readableBuffer.Length; + StopTimingRead(readableBufferLength); + var consumed = readableBuffer.End; var actual = 0; @@ -52,8 +63,12 @@ protected MessageBody(HttpProtocol context) { if (!readableBuffer.IsEmpty) { - // buffer.Count is int - actual = (int)Math.Min(readableBuffer.Length, buffer.Length); + // buffer.Length is int + actual = (int)Math.Min(readableBufferLength, buffer.Length); + + // Make sure we don't double-count bytes on the next read. + _alreadyTimedBytes = readableBufferLength - actual; + var slice = readableBuffer.Slice(0, actual); consumed = readableBuffer.GetPosition(actual); slice.CopyTo(buffer.Span); @@ -63,6 +78,7 @@ protected MessageBody(HttpProtocol context) if (result.IsCompleted) { + TryStop(); return 0; } } @@ -79,14 +95,14 @@ protected MessageBody(HttpProtocol context) public virtual async Task CopyToAsync(Stream destination, CancellationToken cancellationToken = default(CancellationToken)) { - TryInit(); + TryStart(); while (true) { - var result = await _context.RequestBodyPipe.Reader.ReadAsync(cancellationToken); + var result = await StartTimingReadAsync(cancellationToken); var readableBuffer = result.Buffer; - var consumed = readableBuffer.End; - var bytesRead = 0; + var readableBufferLength = readableBuffer.Length; + StopTimingRead(readableBufferLength); try { @@ -98,8 +114,6 @@ protected MessageBody(HttpProtocol context) // - The WriteAsync(ReadOnlyMemory) isn't overridden on the destination // - We change the Kestrel Memory Pool to not use pinned arrays but instead use native memory - bytesRead += memory.Length; - #if NETCOREAPP2_1 await destination.WriteAsync(memory, cancellationToken); #elif NETSTANDARD2_0 @@ -113,30 +127,38 @@ protected MessageBody(HttpProtocol context) if (result.IsCompleted) { + TryStop(); return; } } finally { - _context.RequestBodyPipe.Reader.AdvanceTo(consumed); + _context.RequestBodyPipe.Reader.AdvanceTo(readableBuffer.End); // Update the flow-control window after advancing the pipe reader, so we don't risk overfilling // the pipe despite the client being well-behaved. - OnDataRead(bytesRead); + OnDataRead(readableBufferLength); } } } public virtual Task ConsumeAsync() { - TryInit(); + TryStart(); return OnConsumeAsync(); } - protected abstract Task OnConsumeAsync(); + public virtual Task StopAsync() + { + TryStop(); + + return OnStopAsync(); + } + + protected virtual Task OnConsumeAsync() => Task.CompletedTask; - public abstract Task StopAsync(); + protected virtual Task OnStopAsync() => Task.CompletedTask; protected void TryProduceContinue() { @@ -147,13 +169,52 @@ protected void TryProduceContinue() } } - private void TryInit() + private void TryStart() + { + if (_context.HasStartedConsumingRequestBody) + { + return; + } + + OnReadStarting(); + _context.HasStartedConsumingRequestBody = true; + + if (!RequestUpgrade) + { + Log.RequestBodyStart(_context.ConnectionIdFeature, _context.TraceIdentifier); + + if (_minRequestBodyDataRate != null) + { + _timingEnabled = true; + _context.TimeoutControl.StartRequestBody(_minRequestBodyDataRate); + } + } + + OnReadStarted(); + } + + private void TryStop() { - if (!_context.HasStartedConsumingRequestBody) + if (_stopped) { - OnReadStarting(); - _context.HasStartedConsumingRequestBody = true; - OnReadStarted(); + return; + } + + _stopped = true; + + if (!RequestUpgrade) + { + Log.RequestBodyDone(_context.ConnectionIdFeature, _context.TraceIdentifier); + + if (_timingEnabled) + { + if (_backpressure) + { + _context.TimeoutControl.StopTimingRead(); + } + + _context.TimeoutControl.StopRequestBody(); + } } } @@ -165,7 +226,7 @@ protected virtual void OnReadStarted() { } - protected virtual void OnDataRead(int bytesRead) + protected virtual void OnDataRead(long bytesRead) { } @@ -179,10 +240,35 @@ protected void AddAndCheckConsumedBytes(long consumedBytes) } } + private ValueTask StartTimingReadAsync(CancellationToken cancellationToken) + { + var readAwaitable = _context.RequestBodyPipe.Reader.ReadAsync(cancellationToken); + + if (!readAwaitable.IsCompleted && _timingEnabled) + { + _backpressure = true; + _context.TimeoutControl.StartTimingRead(); + } + + return readAwaitable; + } + + private void StopTimingRead(long bytesRead) + { + _context.TimeoutControl.BytesRead(bytesRead - _alreadyTimedBytes); + _alreadyTimedBytes = 0; + + if (_backpressure) + { + _backpressure = false; + _context.TimeoutControl.StopTimingRead(); + } + } + private class ForZeroContentLength : MessageBody { public ForZeroContentLength(bool keepAlive) - : base(null) + : base(null, null) { RequestKeepAlive = keepAlive; } @@ -196,8 +282,6 @@ public ForZeroContentLength(bool keepAlive) public override Task ConsumeAsync() => Task.CompletedTask; public override Task StopAsync() => Task.CompletedTask; - - protected override Task OnConsumeAsync() => Task.CompletedTask; } } } diff --git a/src/Kestrel.Core/Internal/Http2/FlowControl/InputFlowControl.cs b/src/Kestrel.Core/Internal/Http2/FlowControl/InputFlowControl.cs index d4387fe8d..e43d290ce 100644 --- a/src/Kestrel.Core/Internal/Http2/FlowControl/InputFlowControl.cs +++ b/src/Kestrel.Core/Internal/Http2/FlowControl/InputFlowControl.cs @@ -24,6 +24,8 @@ public InputFlowControl(uint initialWindowSize, uint minWindowSizeIncrement) _minWindowSizeIncrement = (int)minWindowSizeIncrement; } + public bool IsAvailabilityLow => _flow.Available < _minWindowSizeIncrement; + public bool TryAdvance(int bytes) { lock (_flowLock) @@ -90,10 +92,7 @@ public bool TryUpdateWindow(int bytes, out int updateSize) public void StopWindowUpdates() { - lock (_flowLock) - { - _windowUpdatesDisabled = true; - } + _windowUpdatesDisabled = true; } public int Abort() diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index fcbb3f193..57bcb1eee 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -149,6 +149,12 @@ public void HandleRequestHeadersTimeout() Abort(new ConnectionAbortedException(CoreStrings.BadRequest_RequestHeadersTimeout)); } + public void HandleReadDataRateTimeout() + { + Log.RequestBodyMinimumDataRateNotSatisfied(ConnectionId, null, Limits.MinRequestBodyDataRate.BytesPerSecond); + Abort(new ConnectionAbortedException(CoreStrings.BadRequest_RequestBodyTimeout)); + } + public void StopProcessingNextRequest(bool sendGracefulGoAway = false) { lock (_stateLock) @@ -184,6 +190,8 @@ public async Task ProcessRequestsAsync(IHttpApplication appl try { ValidateTlsRequirements(); + + TimeoutControl.InitializeHttp2(_inputFlowControl); TimeoutControl.SetTimeout(Limits.KeepAliveTimeout.Ticks, TimeoutReason.KeepAlive); if (!await TryReadPrefaceAsync()) diff --git a/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs b/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs index f835ffa57..c2ef70a8c 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs @@ -3,6 +3,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { @@ -10,8 +11,8 @@ public class Http2MessageBody : MessageBody { private readonly Http2Stream _context; - private Http2MessageBody(Http2Stream context) - : base(context) + private Http2MessageBody(Http2Stream context, MinDataRate minRequestBodyDataRate) + : base(context, minRequestBodyDataRate) { _context = context; } @@ -34,26 +35,21 @@ protected override void OnReadStarted() } } - protected override void OnDataRead(int bytesRead) + protected override void OnDataRead(long bytesRead) { - _context.OnDataRead(bytesRead); + // The HTTP/2 flow control window cannot be larger than 2^31-1 which limits bytesRead. + _context.OnDataRead((int)bytesRead); AddAndCheckConsumedBytes(bytesRead); } - protected override Task OnConsumeAsync() => Task.CompletedTask; - - public override Task StopAsync() => Task.CompletedTask; - - public static MessageBody For( - HttpRequestHeaders headers, - Http2Stream context) + public static MessageBody For(Http2Stream context, MinDataRate minRequestBodyDataRate) { if (context.EndStreamReceived && !context.RequestBodyStarted) { return ZeroContentLengthClose; } - return new Http2MessageBody(context); + return new Http2MessageBody(context, minRequestBodyDataRate); } } } diff --git a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs index fa3121093..586ba04c0 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs @@ -24,6 +24,7 @@ public class Http2OutputProducer : IHttpOutputProducer, IHttpOutputAborter // This should only be accessed via the FrameWriter. The connection-level output flow control is protected by the // FrameWriter's connection-level write lock. private readonly StreamOutputFlowControl _flowControl; + private readonly MinDataRate _minResponseDataRate; private readonly Http2Stream _stream; private readonly object _dataWriterLock = new object(); private readonly Pipe _dataPipe; @@ -37,12 +38,14 @@ public Http2OutputProducer( Http2FrameWriter frameWriter, StreamOutputFlowControl flowControl, ITimeoutControl timeoutControl, + MinDataRate minResponseDataRate, MemoryPool pool, Http2Stream stream) { _streamId = streamId; _frameWriter = frameWriter; _flowControl = flowControl; + _minResponseDataRate = minResponseDataRate; _stream = stream; _dataPipe = CreateDataPipe(pool); _flusher = new TimingPipeFlusher(_dataPipe.Writer, timeoutControl); @@ -209,14 +212,14 @@ private async Task ProcessDataWrites() { if (readResult.Buffer.Length > 0) { - await _frameWriter.WriteDataAsync(_streamId, _flowControl, _stream.MinResponseDataRate, readResult.Buffer, endStream: false); + await _frameWriter.WriteDataAsync(_streamId, _flowControl, _minResponseDataRate, readResult.Buffer, endStream: false); } await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers); } else { - await _frameWriter.WriteDataAsync(_streamId, _flowControl, _stream.MinResponseDataRate, readResult.Buffer, endStream: readResult.IsCompleted); + await _frameWriter.WriteDataAsync(_streamId, _flowControl, _minResponseDataRate, readResult.Buffer, endStream: readResult.IsCompleted); } _dataPipe.Reader.AdvanceTo(readResult.Buffer.End); diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index 06f51fd09..4f2d54434 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -35,16 +35,26 @@ public Http2Stream(Http2StreamContext context) _context = context; _inputFlowControl = new StreamInputFlowControl( - _context.StreamId, - _context.FrameWriter, + context.StreamId, + context.FrameWriter, context.ConnectionInputFlowControl, - _context.ServerPeerSettings.InitialWindowSize, - _context.ServerPeerSettings.InitialWindowSize / 2); - - _outputFlowControl = new StreamOutputFlowControl(context.ConnectionOutputFlowControl, context.ClientPeerSettings.InitialWindowSize); - _http2Output = new Http2OutputProducer(context.StreamId, context.FrameWriter, _outputFlowControl, context.TimeoutControl, context.MemoryPool, this); - - RequestBodyPipe = CreateRequestBodyPipe(_context.ServerPeerSettings.InitialWindowSize); + context.ServerPeerSettings.InitialWindowSize, + context.ServerPeerSettings.InitialWindowSize / 2); + + _outputFlowControl = new StreamOutputFlowControl( + context.ConnectionOutputFlowControl, + context.ClientPeerSettings.InitialWindowSize); + + _http2Output = new Http2OutputProducer( + context.StreamId, + context.FrameWriter, + _outputFlowControl, + context.TimeoutControl, + context.ServiceContext.ServerOptions.Limits.MinResponseDataRate, + context.MemoryPool, + this); + + RequestBodyPipe = CreateRequestBodyPipe(context.ServerPeerSettings.InitialWindowSize); Output = _http2Output; } @@ -106,7 +116,7 @@ protected override string CreateRequestId() => StringUtilities.ConcatAsHexSuffix(ConnectionId, ':', (uint)StreamId); protected override MessageBody CreateMessageBody() - => Http2MessageBody.For(HttpRequestHeaders, this); + => Http2MessageBody.For(this, ServerOptions.Limits.MinRequestBodyDataRate); // Compare to Http1Connection.OnStartLine protected override bool TryParseRequest(ReadResult result, out bool endConnection) @@ -185,7 +195,7 @@ private bool TryValidatePseudoHeaders() // Approximate MaxRequestLineSize by totaling the required pseudo header field lengths. var requestLineLength = _methodText.Length + Scheme.Length + hostText.Length + path.Length; - if (requestLineLength > ServiceContext.ServerOptions.Limits.MaxRequestLineSize) + if (requestLineLength > ServerOptions.Limits.MaxRequestLineSize) { ResetAndAbort(new ConnectionAbortedException(CoreStrings.BadRequest_RequestLineTooLong), Http2ErrorCode.PROTOCOL_ERROR); return false; diff --git a/src/Kestrel.Core/Internal/HttpConnection.cs b/src/Kestrel.Core/Internal/HttpConnection.cs index fb347de2c..3b77900ff 100644 --- a/src/Kestrel.Core/Internal/HttpConnection.cs +++ b/src/Kestrel.Core/Internal/HttpConnection.cs @@ -384,8 +384,7 @@ public void OnTimeout(TimeoutReason reason) _requestProcessor.HandleRequestHeadersTimeout(); break; case TimeoutReason.ReadDataRate: - Log.RequestBodyMinimumDataRateNotSatisfied(_context.ConnectionId, _http1Connection.TraceIdentifier, _http1Connection.MinRequestBodyDataRate.BytesPerSecond); - _http1Connection.SendTimeoutResponse(); + _requestProcessor.HandleReadDataRateTimeout(); break; case TimeoutReason.WriteDataRate: Log.ResponseMinimumDataRateNotSatisfied(_context.ConnectionId, _http1Connection?.TraceIdentifier); diff --git a/src/Kestrel.Core/Internal/IRequestProcessor.cs b/src/Kestrel.Core/Internal/IRequestProcessor.cs index cf28d71ab..7e8e12a96 100644 --- a/src/Kestrel.Core/Internal/IRequestProcessor.cs +++ b/src/Kestrel.Core/Internal/IRequestProcessor.cs @@ -13,6 +13,7 @@ public interface IRequestProcessor Task ProcessRequestsAsync(IHttpApplication application); void StopProcessingNextRequest(); void HandleRequestHeadersTimeout(); + void HandleReadDataRateTimeout(); void OnInputOrOutputCompleted(); void Tick(DateTimeOffset now); void Abort(ConnectionAbortedException ex); diff --git a/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs index c1d169992..5a1a4e8c8 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; + namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { public interface ITimeoutControl @@ -11,10 +13,11 @@ public interface ITimeoutControl void ResetTimeout(long ticks, TimeoutReason timeoutReason); void CancelTimeout(); - void StartTimingReads(MinDataRate minRate); - void PauseTimingReads(); - void ResumeTimingReads(); - void StopTimingReads(); + void InitializeHttp2(InputFlowControl connectionInputFlowControl); + void StartRequestBody(MinDataRate minRate); + void StopRequestBody(); + void StartTimingRead(); + void StopTimingRead(); void BytesRead(long count); void StartTimingWrite(MinDataRate minRate, long size); diff --git a/src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs b/src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs index eec50242b..bb44c9e2b 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Threading; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { @@ -21,9 +22,13 @@ public class TimeoutControl : ITimeoutControl, IConnectionTimeoutFeature private bool _readTimingPauseRequested; private long _readTimingElapsedTicks; private long _readTimingBytesRead; + private InputFlowControl _connectionInputFlowControl; + // The following are always 0 or 1 for HTTP/1.x + private int _concurrentIncompleteRequestBodies; + private int _concurrentAwaitingReads; private readonly object _writeTimingLock = new object(); - private int _writeTimingWrites; + private int _cuncurrentAwaitingWrites; private long _writeTimingTimeoutTimestamp; public TimeoutControl(ITimeoutHandler timeoutHandler) @@ -76,6 +81,14 @@ private void CheckForReadDataRateTimeout(long timestamp) return; } + // Don't enforce the rate timeout if there is back pressure due to HTTP/2 connection-level input + // flow control. We don't consider stream-level flow control, because we wouldn't be timing a read + // for any stream that didn't have a completely empty stream-level flow control window. + if (_connectionInputFlowControl?.IsAvailabilityLow == true) + { + return; + } + lock (_readTimingLock) { if (!_readTimingEnabled) @@ -88,7 +101,7 @@ private void CheckForReadDataRateTimeout(long timestamp) if (_minReadRate.BytesPerSecond > 0 && _readTimingElapsedTicks > _minReadRate.GracePeriod.Ticks) { var elapsedSeconds = (double)_readTimingElapsedTicks / TimeSpan.TicksPerSecond; - var rate = Interlocked.Read(ref _readTimingBytesRead) / elapsedSeconds; + var rate = _readTimingBytesRead / elapsedSeconds; if (rate < _minReadRate.BytesPerSecond && !Debugger.IsAttached) { @@ -111,7 +124,7 @@ private void CheckForWriteDataRateTimeout(long timestamp) { lock (_writeTimingLock) { - if (_writeTimingWrites > 0 && timestamp > _writeTimingTimeoutTimestamp && !Debugger.IsAttached) + if (_cuncurrentAwaitingWrites > 0 && timestamp > _writeTimingTimeoutTimestamp && !Debugger.IsAttached) { _timeoutHandler.OnTimeout(TimeoutReason.WriteDataRate); } @@ -142,40 +155,64 @@ private void AssignTimeout(long ticks, TimeoutReason timeoutReason) TimerReason = timeoutReason; // Add Heartbeat.Interval since this can be called right before the next heartbeat. - Interlocked.Exchange(ref _timeoutTimestamp, _lastTimestamp + ticks + Heartbeat.Interval.Ticks); + Interlocked.Exchange(ref _timeoutTimestamp, Interlocked.Read(ref _lastTimestamp) + ticks + Heartbeat.Interval.Ticks); + } + + public void InitializeHttp2(InputFlowControl connectionInputFlowControl) + { + _connectionInputFlowControl = connectionInputFlowControl; } - public void StartTimingReads(MinDataRate minRate) + public void StartRequestBody(MinDataRate minRate) { lock (_readTimingLock) { + // minRate is always KestrelServerLimits.MinRequestBodyDataRate for HTTP/2 which is the only protocol that supports concurrent request bodies. + Debug.Assert(_concurrentIncompleteRequestBodies == 0 || minRate == _minReadRate, "Multiple simultaneous read data rates are not supported."); + _minReadRate = minRate; - _readTimingElapsedTicks = 0; - _readTimingBytesRead = 0; - _readTimingEnabled = true; + _concurrentIncompleteRequestBodies++; + + if (_concurrentIncompleteRequestBodies == 1) + { + _readTimingElapsedTicks = 0; + _readTimingBytesRead = 0; + } } } - public void StopTimingReads() + public void StopRequestBody() { lock (_readTimingLock) { - _readTimingEnabled = false; + _concurrentIncompleteRequestBodies--; + + if (_concurrentIncompleteRequestBodies == 0) + { + _readTimingEnabled = false; + } } } - public void PauseTimingReads() + public void StopTimingRead() { lock (_readTimingLock) { - _readTimingPauseRequested = true; + _concurrentAwaitingReads--; + + if (_concurrentAwaitingReads == 0) + { + _readTimingPauseRequested = true; + } } } - public void ResumeTimingReads() + public void StartTimingRead() { lock (_readTimingLock) { + _concurrentAwaitingReads++; + _readTimingEnabled = true; // In case pause and resume were both called between ticks @@ -185,7 +222,12 @@ public void ResumeTimingReads() public void BytesRead(long count) { - Interlocked.Add(ref _readTimingBytesRead, count); + Debug.Assert(count >= 0, "BytesRead count must not be negative."); + + lock (_readTimingLock) + { + _readTimingBytesRead += count; + } } public void StartTimingWrite(MinDataRate minRate, long size) @@ -193,7 +235,7 @@ public void StartTimingWrite(MinDataRate minRate, long size) lock (_writeTimingLock) { // Add Heartbeat.Interval since this can be called right before the next heartbeat. - var currentTimeUpperBound = _lastTimestamp + Heartbeat.Interval.Ticks; + var currentTimeUpperBound = Interlocked.Read(ref _lastTimestamp) + Heartbeat.Interval.Ticks; var ticksToCompleteWriteAtMinRate = TimeSpan.FromSeconds(size / minRate.BytesPerSecond).Ticks; // If ticksToCompleteWriteAtMinRate is less than the configured grace period, @@ -213,7 +255,7 @@ public void StartTimingWrite(MinDataRate minRate, long size) var accumulatedWriteTimeoutTimestamp = _writeTimingTimeoutTimestamp + ticksToCompleteWriteAtMinRate; _writeTimingTimeoutTimestamp = Math.Max(singleWriteTimeoutTimestamp, accumulatedWriteTimeoutTimestamp); - _writeTimingWrites++; + _cuncurrentAwaitingWrites++; } } @@ -221,7 +263,7 @@ public void StopTimingWrite() { lock (_writeTimingLock) { - _writeTimingWrites--; + _cuncurrentAwaitingWrites--; } } diff --git a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs b/test/Kestrel.Core.Tests/Http1ConnectionTests.cs index 39d29e5e5..d60303d62 100644 --- a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs +++ b/test/Kestrel.Core.Tests/Http1ConnectionTests.cs @@ -853,7 +853,7 @@ public async Task ConsumesRequestWhenApplicationDoesNotConsumeIt() var buffer = new byte[10]; await context.Response.Body.WriteAsync(buffer, 0, 10); }); - var mockMessageBody = new Mock(null); + var mockMessageBody = new Mock(null, null); _http1Connection.NextMessageBody = mockMessageBody.Object; var requestProcessingTask = _http1Connection.ProcessRequestsAsync(httpApplication); diff --git a/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs b/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs index 5f868bdee..b53866bbf 100644 --- a/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs +++ b/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs @@ -2,49 +2,34 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.IO.Pipelines; using System.Linq; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Moq; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { - public class HttpProtocolFeatureCollectionTests : IDisposable + public class HttpProtocolFeatureCollectionTests { - private readonly IDuplexPipe _transport; - private readonly IDuplexPipe _application; private readonly TestHttp1Connection _http1Connection; - private readonly ServiceContext _serviceContext; private readonly HttpConnectionContext _http1ConnectionContext; - private readonly MemoryPool _memoryPool; - private readonly IFeatureCollection _collection; public HttpProtocolFeatureCollectionTests() { - _memoryPool = KestrelMemoryPool.Create(); - var options = new PipeOptions(_memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); - var pair = DuplexPipe.CreateConnectionPair(options, options); - - _transport = pair.Transport; - _application = pair.Application; - - _serviceContext = new TestServiceContext(); _http1ConnectionContext = new HttpConnectionContext { - ServiceContext = _serviceContext, + ServiceContext = new TestServiceContext(), ConnectionFeatures = new FeatureCollection(), - MemoryPool = _memoryPool, TimeoutControl = Mock.Of(), - Transport = pair.Transport + Transport = Mock.Of(), }; _http1Connection = new TestHttp1Connection(_http1ConnectionContext); @@ -52,17 +37,6 @@ public HttpProtocolFeatureCollectionTests() _collection = _http1Connection; } - public void Dispose() - { - _transport.Input.Complete(); - _transport.Output.Complete(); - - _application.Input.Complete(); - _application.Output.Complete(); - - _memoryPool.Dispose(); - } - [Fact] public int FeaturesStartAsSelf() { @@ -166,6 +140,27 @@ public void FeaturesSetByGenericSameAsByType() EachHttpProtocolFeatureSetAndUnique(); } + [Fact] + public void Http2StreamFeatureCollectionDoesNotIncludeMinRateFeatures() + { + var http2Stream = new Http2Stream(new Http2StreamContext + { + ServiceContext = new TestServiceContext(), + ConnectionFeatures = new FeatureCollection(), + TimeoutControl = Mock.Of(), + Transport = Mock.Of(), + ServerPeerSettings = new Http2PeerSettings(), + ClientPeerSettings = new Http2PeerSettings(), + }); + var http2StreamCollection = (IFeatureCollection)http2Stream; + + Assert.Null(http2StreamCollection.Get()); + Assert.Null(http2StreamCollection.Get()); + + Assert.NotNull(_collection.Get()); + Assert.NotNull(_collection.Get()); + } + private void CompareGenericGetterToIndexer() { Assert.Same(_collection.Get(), _collection[typeof(IHttpRequestFeature)]); @@ -218,6 +213,6 @@ private int SetFeaturesToNonDefault() return featureCount; } - private HttpProtocol CreateHttp1Connection() => new TestHttp1Connection(_http1ConnectionContext); + private Http1Connection CreateHttp1Connection() => new TestHttp1Connection(_http1ConnectionContext); } } diff --git a/test/Kestrel.Core.Tests/HttpRequestStreamTests.cs b/test/Kestrel.Core.Tests/HttpRequestStreamTests.cs index 4c8e53a82..a9b0f9726 100644 --- a/test/Kestrel.Core.Tests/HttpRequestStreamTests.cs +++ b/test/Kestrel.Core.Tests/HttpRequestStreamTests.cs @@ -118,7 +118,7 @@ public async Task SynchronousReadsThrowIfDisallowedByIHttpBodyControlFeature() var allowSynchronousIO = false; var mockBodyControl = new Mock(); mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(() => allowSynchronousIO); - var mockMessageBody = new Mock((HttpProtocol)null); + var mockMessageBody = new Mock(null, null); mockMessageBody.Setup(m => m.ReadAsync(It.IsAny>(), CancellationToken.None)).Returns(new ValueTask(0)); var stream = new HttpRequestStream(mockBodyControl.Object); diff --git a/test/Kestrel.Core.Tests/MessageBodyTests.cs b/test/Kestrel.Core.Tests/MessageBodyTests.cs index 3ec3d2e2f..159f1ef14 100644 --- a/test/Kestrel.Core.Tests/MessageBodyTests.cs +++ b/test/Kestrel.Core.Tests/MessageBodyTests.cs @@ -699,10 +699,10 @@ public async Task LogsWhenStopsReadingRequestBody() input.Fin(); - await logEvent.Task.DefaultTimeout(); - input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); + + await logEvent.Task.DefaultTimeout(); } } @@ -717,46 +717,43 @@ public async Task PausesAndResumesRequestBodyTimeoutOnBackpressure() var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderContentLength = "12" }, input.Http1Connection); // Add some input and read it to start PumpAsync + var readTask1 = body.ReadAsync(new ArraySegment(new byte[6])); input.Add("hello,"); - Assert.Equal(6, await body.ReadAsync(new ArraySegment(new byte[6]))); + Assert.Equal(6, await readTask1); + var readTask2 = body.ReadAsync(new ArraySegment(new byte[6])); input.Add(" world"); - Assert.Equal(6, await body.ReadAsync(new ArraySegment(new byte[6]))); + Assert.Equal(6, await readTask2); // Due to the limits set on HttpProtocol.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)); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.StopTimingRead(), Times.Exactly(2)); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.StartTimingRead(), Times.Exactly(2)); } } [Fact] - public async Task OnlyEnforcesRequestBodyTimeoutAfterSending100Continue() + public async Task OnlyEnforcesRequestBodyTimeoutAfterFirstRead() { using (var input = new TestInput()) { - var produceContinueCalled = false; - var startTimingReadsCalledAfterProduceContinue = false; - - var mockHttpResponseControl = new Mock(); - mockHttpResponseControl - .Setup(httpResponseControl => httpResponseControl.ProduceContinue()) - .Callback(() => produceContinueCalled = true); - input.Http1Connection.HttpResponseControl = mockHttpResponseControl.Object; + var startRequestBodyCalled = false; var minReadRate = input.Http1Connection.MinRequestBodyDataRate; var mockTimeoutControl = new Mock(); mockTimeoutControl - .Setup(timeoutControl => timeoutControl.StartTimingReads(minReadRate)) - .Callback(() => startTimingReadsCalledAfterProduceContinue = produceContinueCalled); + .Setup(timeoutControl => timeoutControl.StartRequestBody(minReadRate)) + .Callback(() => startRequestBodyCalled = true); input.Http1ConnectionContext.TimeoutControl = mockTimeoutControl.Object; var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderContentLength = "5" }, input.Http1Connection); + Assert.False(startRequestBodyCalled); + // Add some input and read it to start PumpAsync var readTask = body.ReadAsync(new ArraySegment(new byte[1])); - Assert.True(startTimingReadsCalledAfterProduceContinue); + Assert.True(startRequestBodyCalled); input.Add("a"); await readTask; @@ -785,14 +782,14 @@ public async Task DoesNotEnforceRequestBodyTimeoutOnUpgradeRequests() Assert.Equal(0, await body.ReadAsync(new ArraySegment(new byte[1]))); - mockTimeoutControl.Verify(timeoutControl => timeoutControl.StartTimingReads(minReadRate), Times.Never); - mockTimeoutControl.Verify(timeoutControl => timeoutControl.StopTimingReads(), Times.Never); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.StartRequestBody(minReadRate), Times.Never); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.StopRequestBody(), Times.Never); // Due to the limits set on HttpProtocol.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); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.StopTimingRead(), Times.Never); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.StartTimingRead(), Times.Never); input.Http1Connection.RequestBodyPipe.Reader.Complete(); await body.StopAsync(); diff --git a/test/Kestrel.Core.Tests/StreamsTests.cs b/test/Kestrel.Core.Tests/StreamsTests.cs index e80b74564..673b1586e 100644 --- a/test/Kestrel.Core.Tests/StreamsTests.cs +++ b/test/Kestrel.Core.Tests/StreamsTests.cs @@ -72,10 +72,10 @@ public async Task StreamsThrowOnUpgradeAfterAbort() await upgrade.WriteAsync(new byte[1], 0, 1); } - private class MockMessageBody : Http1MessageBody + private class MockMessageBody : MessageBody { public MockMessageBody(bool upgradeable = false) - : base(null) + : base(null, null) { RequestUpgrade = upgradeable; } diff --git a/test/Kestrel.Core.Tests/TimeoutControlTests.cs b/test/Kestrel.Core.Tests/TimeoutControlTests.cs index 6468464af..cb4438759 100644 --- a/test/Kestrel.Core.Tests/TimeoutControlTests.cs +++ b/test/Kestrel.Core.Tests/TimeoutControlTests.cs @@ -66,7 +66,8 @@ public void RequestBodyMinimumDataRateNotEnforcedDuringGracePeriod() var now = DateTimeOffset.UtcNow; _timeoutControl.Tick(now); - _timeoutControl.StartTimingReads(minRate); + _timeoutControl.StartRequestBody(minRate); + _timeoutControl.StartTimingRead(); // Tick during grace period w/ low data rate now += TimeSpan.FromSeconds(1); @@ -95,7 +96,8 @@ public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() var now = DateTimeOffset.UtcNow; _timeoutControl.Tick(now); - _timeoutControl.StartTimingReads(minRate); + _timeoutControl.StartRequestBody(minRate); + _timeoutControl.StartTimingRead(); // Set base data rate to 200 bytes/second now += gracePeriod; @@ -153,7 +155,8 @@ public void RequestBodyDataRateNotComputedOnPausedTime() // Initialize timestamp _timeoutControl.Tick(systemClock.UtcNow); - _timeoutControl.StartTimingReads(minRate); + _timeoutControl.StartRequestBody(minRate); + _timeoutControl.StartTimingRead(); // Tick at 3s, expected counted time is 3s, expected data rate is 200 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(3); @@ -162,7 +165,7 @@ public void RequestBodyDataRateNotComputedOnPausedTime() // Pause at 3.5s systemClock.UtcNow += TimeSpan.FromSeconds(0.5); - _timeoutControl.PauseTimingReads(); + _timeoutControl.StopTimingRead(); // 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); @@ -177,7 +180,7 @@ public void RequestBodyDataRateNotComputedOnPausedTime() // Resume at 6.5s systemClock.UtcNow += TimeSpan.FromSeconds(0.5); - _timeoutControl.ResumeTimingReads(); + _timeoutControl.StartTimingRead(); // Tick at 9s, expected counted time is 6s, expected data rate is 100 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(1.5); @@ -203,7 +206,8 @@ public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() // Initialize timestamp _timeoutControl.Tick(systemClock.UtcNow); - _timeoutControl.StartTimingReads(minRate); + _timeoutControl.StartRequestBody(minRate); + _timeoutControl.StartTimingRead(); // Tick at 2s, expected counted time is 2s, expected data rate is 100 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(2); @@ -215,11 +219,11 @@ public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() // Pause at 2.25s systemClock.UtcNow += TimeSpan.FromSeconds(0.25); - _timeoutControl.PauseTimingReads(); + _timeoutControl.StopTimingRead(); // Resume at 2.5s systemClock.UtcNow += TimeSpan.FromSeconds(0.25); - _timeoutControl.ResumeTimingReads(); + _timeoutControl.StartTimingRead(); // Tick at 3s, expected counted time is 3s, expected data rate is 100 bytes/second systemClock.UtcNow += TimeSpan.FromSeconds(0.5); @@ -249,7 +253,8 @@ public void ReadTimingNotEnforcedWhenTimeoutIsSet() // Initialize timestamp _timeoutControl.Tick(startTime); - _timeoutControl.StartTimingReads(minRate); + _timeoutControl.StartRequestBody(minRate); + _timeoutControl.StartTimingRead(); _timeoutControl.SetTimeout(timeout.Ticks, TimeoutReason.RequestBodyDrain); @@ -394,7 +399,8 @@ private void TickBodyWithMinimumDataRate(int bytesPerSecond) var now = DateTimeOffset.UtcNow; _timeoutControl.Tick(now); - _timeoutControl.StartTimingReads(minRate); + _timeoutControl.StartRequestBody(minRate); + _timeoutControl.StartTimingRead(); // Tick after grace period w/ low data rate now += gracePeriod + TimeSpan.FromSeconds(1); diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs index e92f81d15..a54b38236 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -20,6 +20,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; @@ -1196,24 +1197,29 @@ public virtual void CancelTimeout() } - public virtual void StartTimingReads(MinDataRate minRate) + public virtual void InitializeHttp2(InputFlowControl connectionInputFlowControl) { - _realTimeoutControl.StartTimingReads(minRate); + _realTimeoutControl.InitializeHttp2(connectionInputFlowControl); } - public virtual void PauseTimingReads() + public virtual void StartRequestBody(MinDataRate minRate) { - _realTimeoutControl.PauseTimingReads(); + _realTimeoutControl.StartRequestBody(minRate); } - public virtual void ResumeTimingReads() + public virtual void StopTimingRead() { - _realTimeoutControl.ResumeTimingReads(); + _realTimeoutControl.StopTimingRead(); } - public virtual void StopTimingReads() + public virtual void StartTimingRead() { - _realTimeoutControl.StopTimingReads(); + _realTimeoutControl.StartTimingRead(); + } + + public virtual void StopRequestBody() + { + _realTimeoutControl.StopRequestBody(); } public virtual void BytesRead(long count) diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs index 563837bcf..b521b3993 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -40,6 +41,8 @@ public async Task Preamble_NotReceivedInitially_WithinKeepAliveTimeout_ClosesCon _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.KeepAlive), Times.Once); await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); + + _mockTimeoutHandler.VerifyNoOtherCalls(); } [Fact] @@ -63,6 +66,8 @@ public async Task HEADERS_NotReceivedInitially_WithinKeepAliveTimeout_ClosesConn _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.KeepAlive), Times.Once); await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); + + _mockTimeoutHandler.VerifyNoOtherCalls(); } [Fact] @@ -119,6 +124,8 @@ await ExpectAsync(Http2FrameType.DATA, _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.KeepAlive), Times.Once); await WaitForConnectionStopAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + + _mockTimeoutHandler.VerifyNoOtherCalls(); } [Fact] @@ -155,6 +162,9 @@ await WaitForConnectionErrorAsync( _mockConnectionContext.Verify(c =>c.Abort(It.Is(e => e.Message == CoreStrings.BadRequest_RequestHeadersTimeout)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); } [Fact] @@ -186,6 +196,9 @@ public async Task ResponseDrain_SlowerThanMinimumDataRate_AbortsConnection() _mockConnectionContext.Verify(c =>c.Abort(It.Is(e => e.Message == CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); } [Theory] @@ -249,6 +262,9 @@ public async Task DATA_Sent_TooSlowlyDueToSocketBackPressureOnSmallWrite_AbortsC var mockSystemClock = _serviceContext.MockSystemClock; var limits = _serviceContext.ServerOptions.Limits; + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinResponseDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + // Disable response buffering so "socket" backpressure is observed immediately. limits.MaxResponseBufferSize = 0; @@ -264,6 +280,9 @@ await ExpectAsync(Http2FrameType.HEADERS, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); + // Complete timing of the request body so we don't induce any unexpected request body rate timeouts. + _timeoutControl.Tick(mockSystemClock.UtcNow); + // Don't read data frame to induce "socket" backpressure. mockSystemClock.UtcNow += limits.MinResponseDataRate.GracePeriod + Heartbeat.Interval; _timeoutControl.Tick(mockSystemClock.UtcNow); @@ -289,6 +308,9 @@ await WaitForConnectionErrorAsync( _mockConnectionContext.Verify(c =>c.Abort(It.Is(e => e.Message == CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); } [Fact] @@ -297,6 +319,9 @@ public async Task DATA_Sent_TooSlowlyDueToSocketBackPressureOnLargeWrite_AbortsC var mockSystemClock = _serviceContext.MockSystemClock; var limits = _serviceContext.ServerOptions.Limits; + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinResponseDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + // Disable response buffering so "socket" backpressure is observed immediately. limits.MaxResponseBufferSize = 0; @@ -312,6 +337,9 @@ await ExpectAsync(Http2FrameType.HEADERS, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); + // Complete timing of the request body so we don't induce any unexpected request body rate timeouts. + _timeoutControl.Tick(mockSystemClock.UtcNow); + var timeToWriteMaxData = TimeSpan.FromSeconds(_maxData.Length / limits.MinResponseDataRate.BytesPerSecond); // Don't read data frame to induce "socket" backpressure. @@ -325,7 +353,7 @@ await ExpectAsync(Http2FrameType.HEADERS, _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.WriteDataRate), Times.Once); - // The "hello, world" bytes are buffered from before the timeout, but not an END_STREAM data frame. + // The _maxData bytes are buffered from before the timeout, but not an END_STREAM data frame. await ExpectAsync(Http2FrameType.DATA, withLength: _maxData.Length, withFlags: (byte)Http2DataFrameFlags.NONE, @@ -339,6 +367,9 @@ await WaitForConnectionErrorAsync( _mockConnectionContext.Verify(c => c.Abort(It.Is(e => e.Message == CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); } [Fact] @@ -347,6 +378,9 @@ public async Task DATA_Sent_TooSlowlyDueToFlowControlOnSmallWrite_AbortsConnecti var mockSystemClock = _serviceContext.MockSystemClock; var limits = _serviceContext.ServerOptions.Limits; + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinResponseDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + // This only affects the stream windows. The connection-level window is always initialized at 64KiB. _clientSettings.InitialWindowSize = 6; @@ -366,6 +400,9 @@ await ExpectAsync(Http2FrameType.DATA, withFlags: (byte)Http2DataFrameFlags.NONE, withStreamId: 1); + // Complete timing of the request body so we don't induce any unexpected request body rate timeouts. + _timeoutControl.Tick(mockSystemClock.UtcNow); + // Don't send WINDOW_UPDATE to induce flow-control backpressure mockSystemClock.UtcNow += limits.MinResponseDataRate.GracePeriod + Heartbeat.Interval; _timeoutControl.Tick(mockSystemClock.UtcNow); @@ -385,6 +422,9 @@ await WaitForConnectionErrorAsync( _mockConnectionContext.Verify(c => c.Abort(It.Is(e => e.Message == CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); } [Fact] @@ -393,6 +433,9 @@ public async Task DATA_Sent_TooSlowlyDueToOutputFlowControlOnLargeWrite_AbortsCo var mockSystemClock = _serviceContext.MockSystemClock; var limits = _serviceContext.ServerOptions.Limits; + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinResponseDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + // This only affects the stream windows. The connection-level window is always initialized at 64KiB. _clientSettings.InitialWindowSize = (uint)_maxData.Length - 1; @@ -412,6 +455,9 @@ await ExpectAsync(Http2FrameType.DATA, withFlags: (byte)Http2DataFrameFlags.NONE, withStreamId: 1); + // Complete timing of the request body so we don't induce any unexpected request body rate timeouts. + _timeoutControl.Tick(mockSystemClock.UtcNow); + var timeToWriteMaxData = TimeSpan.FromSeconds(_clientSettings.InitialWindowSize / limits.MinResponseDataRate.BytesPerSecond); // Don't send WINDOW_UPDATE to induce flow-control backpressure @@ -433,6 +479,9 @@ await WaitForConnectionErrorAsync( _mockConnectionContext.Verify(c => c.Abort(It.Is(e => e.Message == CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); } [Fact] @@ -441,6 +490,9 @@ public async Task DATA_Sent_TooSlowlyDueToOutputFlowControlOnMultipleStreams_Abo var mockSystemClock = _serviceContext.MockSystemClock; var limits = _serviceContext.ServerOptions.Limits; + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinResponseDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + // This only affects the stream windows. The connection-level window is always initialized at 64KiB. _clientSettings.InitialWindowSize = (uint)_maxData.Length - 1; @@ -472,6 +524,9 @@ await ExpectAsync(Http2FrameType.DATA, withFlags: (byte)Http2DataFrameFlags.NONE, withStreamId: 3); + // Complete timing of the request bodies so we don't induce any unexpected request body rate timeouts. + _timeoutControl.Tick(mockSystemClock.UtcNow); + var timeToWriteMaxData = TimeSpan.FromSeconds(_clientSettings.InitialWindowSize / limits.MinResponseDataRate.BytesPerSecond); // Double the timeout for the second stream. timeToWriteMaxData += timeToWriteMaxData; @@ -495,6 +550,353 @@ await WaitForConnectionErrorAsync( _mockConnectionContext.Verify(c => c.Abort(It.Is(e => e.Message == CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); + } + + [Fact] + public async Task DATA_Received_TooSlowlyOnSmallRead_AbortsConnectionAfterGracePeriod() + { + var mockSystemClock = _serviceContext.MockSystemClock; + var limits = _serviceContext.ServerOptions.Limits; + + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + + _timeoutControl.Initialize(mockSystemClock.UtcNow); + + await InitializeConnectionAsync(_echoApplication); + + // _helloWorldBytes is 12 bytes, and 12 bytes / 240 bytes/sec = .05 secs which is far below the grace period. + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + await SendDataAsync(1, _helloWorldBytes, endStream: false); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: _helloWorldBytes.Length, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + // Don't send any more data and advance just to and then past the grace period. + mockSystemClock.UtcNow += limits.MinRequestBodyDataRate.GracePeriod; + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + mockSystemClock.UtcNow += TimeSpan.FromTicks(1); + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 1, + Http2ErrorCode.INTERNAL_ERROR, + null); + + _mockConnectionContext.Verify(c => c.Abort(It.Is(e => + e.Message == CoreStrings.BadRequest_RequestBodyTimeout)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); + } + + [Fact] + public async Task DATA_Received_TooSlowlyOnLargeRead_AbortsConnectionAfterRateTimeout() + { + var mockSystemClock = _serviceContext.MockSystemClock; + var limits = _serviceContext.ServerOptions.Limits; + + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + + _timeoutControl.Initialize(mockSystemClock.UtcNow); + + await InitializeConnectionAsync(_echoApplication); + + // _maxData is 16 KiB, and 16 KiB / 240 bytes/sec ~= 68 secs which is far above the grace period. + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + await SendDataAsync(1, _maxData, endStream: false); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: _maxData.Length, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + // Due to the imprecision of floating point math and the fact that TimeoutControl derives rate from elapsed + // time for reads instead of vice versa like for writes, use a half-second instead of single-tick cushion. + var timeToReadMaxData = TimeSpan.FromSeconds(_maxData.Length / limits.MinRequestBodyDataRate.BytesPerSecond) - TimeSpan.FromSeconds(.5); + + // Don't send any more data and advance just to and then past the rate timeout. + mockSystemClock.UtcNow += timeToReadMaxData; + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + mockSystemClock.UtcNow += TimeSpan.FromSeconds(1); + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 1, + Http2ErrorCode.INTERNAL_ERROR, + null); + + _mockConnectionContext.Verify(c => c.Abort(It.Is(e => + e.Message == CoreStrings.BadRequest_RequestBodyTimeout)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); + } + + [Fact] + public async Task DATA_Received_TooSlowlyOnMultipleStreams_AbortsConnectionAfterAdditiveRateTimeout() + { + var mockSystemClock = _serviceContext.MockSystemClock; + var limits = _serviceContext.ServerOptions.Limits; + + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + + _timeoutControl.Initialize(mockSystemClock.UtcNow); + + await InitializeConnectionAsync(_echoApplication); + + // _maxData is 16 KiB, and 16 KiB / 240 bytes/sec ~= 68 secs which is far above the grace period. + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + await SendDataAsync(1, _maxData, endStream: false); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: _maxData.Length, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + await StartStreamAsync(3, _browserRequestHeaders, endStream: false); + await SendDataAsync(3, _maxData, endStream: false); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + await ExpectAsync(Http2FrameType.DATA, + withLength: _maxData.Length, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 3); + + var timeToReadMaxData = TimeSpan.FromSeconds(_maxData.Length / limits.MinRequestBodyDataRate.BytesPerSecond); + // Double the timeout for the second stream. + timeToReadMaxData += timeToReadMaxData; + + // Due to the imprecision of floating point math and the fact that TimeoutControl derives rate from elapsed + // time for reads instead of vice versa like for writes, use a half-second instead of single-tick cushion. + timeToReadMaxData -= TimeSpan.FromSeconds(.5); + + // Don't send any more data and advance just to and then past the rate timeout. + mockSystemClock.UtcNow += timeToReadMaxData; + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + mockSystemClock.UtcNow += TimeSpan.FromSeconds(1); + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 3, + Http2ErrorCode.INTERNAL_ERROR, + null); + + _mockConnectionContext.Verify(c => c.Abort(It.Is(e => + e.Message == CoreStrings.BadRequest_RequestBodyTimeout)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); + } + + [Fact] + public async Task DATA_Received_TooSlowlyOnSecondStream_AbortsConnectionAfterNonAdditiveRateTimeout() + { + var mockSystemClock = _serviceContext.MockSystemClock; + var limits = _serviceContext.ServerOptions.Limits; + + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + + _timeoutControl.Initialize(mockSystemClock.UtcNow); + + await InitializeConnectionAsync(_echoApplication); + + // _maxData is 16 KiB, and 16 KiB / 240 bytes/sec ~= 68 secs which is far above the grace period. + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + await SendDataAsync(1, _maxData, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: _maxData.Length, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + await StartStreamAsync(3, _browserRequestHeaders, endStream: false); + await SendDataAsync(3, _maxData, endStream: false); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + await ExpectAsync(Http2FrameType.DATA, + withLength: _maxData.Length, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 3); + + // Due to the imprecision of floating point math and the fact that TimeoutControl derives rate from elapsed + // time for reads instead of vice versa like for writes, use a half-second instead of single-tick cushion. + var timeToReadMaxData = TimeSpan.FromSeconds(_maxData.Length / limits.MinRequestBodyDataRate.BytesPerSecond) - TimeSpan.FromSeconds(.5); + + // Don't send any more data and advance just to and then past the rate timeout. + mockSystemClock.UtcNow += timeToReadMaxData; + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + mockSystemClock.UtcNow += TimeSpan.FromSeconds(1); + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 3, + Http2ErrorCode.INTERNAL_ERROR, + null); + + _mockConnectionContext.Verify(c => c.Abort(It.Is(e => + e.Message == CoreStrings.BadRequest_RequestBodyTimeout)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); + } + + [Fact] + public async Task DATA_Received_SlowlyDueToConnectionFlowControl_DoesNotAbortConnection() + { + var initialConnectionWindowSize = _serviceContext.ServerOptions.Limits.Http2.InitialConnectionWindowSize; + var framesConnectionInWindow = initialConnectionWindowSize / Http2PeerSettings.DefaultMaxFrameSize; + + var backpressureTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var mockSystemClock = _serviceContext.MockSystemClock; + var limits = _serviceContext.ServerOptions.Limits; + + // Use non-default value to ensure the min request and response rates aren't mixed up. + limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5)); + + _timeoutControl.Initialize(mockSystemClock.UtcNow); + + await InitializeConnectionAsync(async context => + { + var streamId = context.Features.Get().StreamId; + + if (streamId == 1) + { + await backpressureTcs.Task; + } + else + { + await _echoApplication(context); + } + }); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + for (var i = 0; i < framesConnectionInWindow / 2; i++) + { + await SendDataAsync(1, _maxData, endStream: false); + } + await SendDataAsync(1, _maxData, endStream: true); + + await StartStreamAsync(3, _browserRequestHeaders, endStream: false); + await SendDataAsync(3, _helloWorldBytes, endStream: false); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 3); + await ExpectAsync(Http2FrameType.DATA, + withLength: _helloWorldBytes.Length, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 3); + + // No matter how much time elapses there is no read timeout because the connection window is too small. + mockSystemClock.UtcNow += TimeSpan.FromDays(365); + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Opening the connection window starts the read rate timeout enforcement after that point. + backpressureTcs.SetResult(null); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 55, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.WINDOW_UPDATE, + withLength: 4, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 0); + + mockSystemClock.UtcNow += limits.MinRequestBodyDataRate.GracePeriod; + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + mockSystemClock.UtcNow += TimeSpan.FromTicks(1); + _timeoutControl.Tick(mockSystemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 3, + Http2ErrorCode.INTERNAL_ERROR, + null); + + _mockConnectionContext.Verify(c => c.Abort(It.Is(e => + e.Message == CoreStrings.BadRequest_RequestBodyTimeout)), Times.Once); + + _mockTimeoutHandler.VerifyNoOtherCalls(); + _mockConnectionContext.VerifyNoOtherCalls(); } } } diff --git a/tools/CodeGenerator/HttpProtocolFeatureCollection.cs b/tools/CodeGenerator/HttpProtocolFeatureCollection.cs index 85e41da0f..790ceb8fa 100644 --- a/tools/CodeGenerator/HttpProtocolFeatureCollection.cs +++ b/tools/CodeGenerator/HttpProtocolFeatureCollection.cs @@ -63,8 +63,6 @@ public static string GenerateFile() "IHttpRequestLifetimeFeature", "IHttpConnectionFeature", "IHttpMaxRequestBodySizeFeature", - "IHttpMinRequestBodyDataRateFeature", - "IHttpMinResponseDataRateFeature", "IHttpBodyControlFeature", };