diff --git a/benchmarks/Kestrel.Performance/Http1ConnectionBenchmark.cs b/benchmarks/Kestrel.Performance/Http1ConnectionBenchmark.cs index 0443a43b5e..638eb70cb3 100644 --- a/benchmarks/Kestrel.Performance/Http1ConnectionBenchmark.cs +++ b/benchmarks/Kestrel.Performance/Http1ConnectionBenchmark.cs @@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance HttpParser = NullParser.Instance }; - var http1Connection = new Http1Connection(context: new Http1ConnectionContext + var http1Connection = new Http1Connection(context: new HttpConnectionContext { ServiceContext = serviceContext, ConnectionFeatures = new FeatureCollection(), diff --git a/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs b/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs index c7d3df7702..31e8d54aa3 100644 --- a/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs +++ b/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs @@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance HttpParser = NullParser.Instance }; - var http1Connection = new Http1Connection(new Http1ConnectionContext + var http1Connection = new Http1Connection(new HttpConnectionContext { ServiceContext = serviceContext, ConnectionFeatures = new FeatureCollection(), diff --git a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs index 1125eb47b2..a71d067437 100644 --- a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs @@ -108,7 +108,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance HttpParser = new HttpParser() }; - var http1Connection = new TestHttp1Connection(new Http1ConnectionContext + var http1Connection = new TestHttp1Connection(new HttpConnectionContext { ServiceContext = serviceContext, ConnectionFeatures = new FeatureCollection(), diff --git a/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs b/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs index 6498d49842..2e4d6a6742 100644 --- a/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs +++ b/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs @@ -91,7 +91,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance HttpParser = new HttpParser() }; - var http1Connection = new Http1Connection(new Http1ConnectionContext + var http1Connection = new Http1Connection(new HttpConnectionContext { ServiceContext = serviceContext, ConnectionFeatures = new FeatureCollection(), diff --git a/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs b/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs index 879c86bab6..70565511ff 100644 --- a/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs +++ b/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs @@ -1,6 +1,7 @@ // 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; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Performance.Mocks @@ -11,15 +12,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance.Mocks { } - public void ResetTimeout(long ticks, TimeoutAction timeoutAction) + public void ResetTimeout(long ticks, TimeoutReason timeoutReason) { } - public void SetTimeout(long ticks, TimeoutAction timeoutAction) + public void SetTimeout(long ticks, TimeoutReason timeoutReason) { } - public void StartTimingReads() + public void StartTimingReads(MinDataRate minRate) { } @@ -39,7 +40,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance.Mocks { } - public void StartTimingWrite(long size) + public void StartTimingWrite(MinDataRate rate, long size) { } diff --git a/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs b/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs index c03f8c7284..b5664bb9f6 100644 --- a/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs @@ -36,7 +36,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance HttpParser = new HttpParser() }; - var http1Connection = new Http1Connection(new Http1ConnectionContext + var http1Connection = new Http1Connection(new HttpConnectionContext { ServiceContext = serviceContext, ConnectionFeatures = new FeatureCollection(), diff --git a/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs b/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs index 84ee6f477a..6b06f6aac9 100644 --- a/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs +++ b/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs @@ -184,7 +184,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance HttpParser = new HttpParser() }; - var http1Connection = new Http1Connection(new Http1ConnectionContext + var http1Connection = new Http1Connection(new HttpConnectionContext { ServiceContext = serviceContext, ConnectionFeatures = new FeatureCollection(), diff --git a/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs b/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs index 1bb5f54258..be53f22bf9 100644 --- a/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs @@ -127,7 +127,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance HttpParser = new HttpParser() }; - var http1Connection = new TestHttp1Connection(new Http1ConnectionContext + var http1Connection = new TestHttp1Connection(new HttpConnectionContext { ServiceContext = serviceContext, ConnectionFeatures = new FeatureCollection(), diff --git a/src/Connections.Abstractions/ConnectionContext.cs b/src/Connections.Abstractions/ConnectionContext.cs index 381780b32e..680762d680 100644 --- a/src/Connections.Abstractions/ConnectionContext.cs +++ b/src/Connections.Abstractions/ConnectionContext.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.IO.Pipelines; -using System.Threading; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index e3b66fe44a..635d4e3e49 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -584,4 +584,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l The decoded integer exceeds the maximum value of Int32.MaxValue. + + The client closed the connection. + \ No newline at end of file diff --git a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs index a3b47a709f..f0c90a4388 100644 --- a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs +++ b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs @@ -6,9 +6,7 @@ using System.Buffers; using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Connections; -using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.Extensions.Logging; @@ -84,12 +82,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } finally { - Log.ConnectionStop(connectionContext.ConnectionId); - KestrelEventSource.Log.ConnectionStop(connectionContext); - connection.Complete(); _serviceContext.ConnectionManager.RemoveConnection(id); + + Log.ConnectionStop(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStop(connectionContext); } } diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.cs index c2655344b8..486af7fb00 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.cs @@ -21,8 +21,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private const byte ByteForwardSlash = (byte)'/'; private const string Asterisk = "*"; - private readonly Http1ConnectionContext _context; + private readonly HttpConnectionContext _context; private readonly IHttpParser _parser; + private readonly Http1OutputProducer _http1Output; protected readonly long _keepAliveTicks; private readonly long _requestHeadersTimeoutTicks; @@ -35,7 +36,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private int _remainingRequestHeadersBytesAllowed; - public Http1Connection(Http1ConnectionContext context) + public Http1Connection(HttpConnectionContext context) : base(context) { _context = context; @@ -44,13 +45,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _requestHeadersTimeoutTicks = ServerOptions.Limits.RequestHeadersTimeout.Ticks; RequestBodyPipe = CreateRequestBodyPipe(); - Output = new Http1OutputProducer( + + _http1Output = new Http1OutputProducer( _context.Transport.Output, _context.ConnectionId, _context.ConnectionContext, _context.ServiceContext.Log, _context.TimeoutControl, - _context.ConnectionFeatures.Get()); + this); + + Output = _http1Output; } public PipeReader Input => _context.Transport.Input; @@ -60,6 +64,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http public override bool IsUpgradableRequest => _upgradeAvailable; + protected override void OnRequestProcessingEnded() + { + Input.Complete(); + + TimeoutControl.StartDrainTimeout(MinResponseDataRate, ServerOptions.Limits.MaxResponseBufferSize); + + // Prevent RequestAborted from firing. Free up unneeded feature references. + Reset(); + + _http1Output.Dispose(); + } + + public void OnInputOrOutputCompleted() + { + _http1Output.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient)); + AbortRequest(); + } + /// /// Immediately kill the connection and poison the request body stream with an error. /// @@ -70,11 +92,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return; } - // Abort output prior to calling OnIOCompleted() to give the transport the chance to complete the input - // with the correct error and message. - Output.Abort(abortReason); + _http1Output.Abort(abortReason); - OnInputOrOutputCompleted(); + AbortRequest(); PoisonRequestBodyStream(abortReason); } @@ -115,7 +135,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http break; } - TimeoutControl.ResetTimeout(_requestHeadersTimeoutTicks, TimeoutAction.SendTimeoutResponse); + TimeoutControl.ResetTimeout(_requestHeadersTimeoutTicks, TimeoutReason.RequestHeaders); _requestProcessingStatus = RequestProcessingStatus.ParsingRequestLine; goto case RequestProcessingStatus.ParsingRequestLine; @@ -411,7 +431,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected override void OnRequestProcessingEnding() { - Input.Complete(); } protected override string CreateRequestId() @@ -424,7 +443,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { // Reset the features and timeout. Reset(); - TimeoutControl.SetTimeout(_keepAliveTicks, TimeoutAction.StopProcessingNextRequest); + TimeoutControl.SetTimeout(_keepAliveTicks, TimeoutReason.KeepAlive); } protected override bool BeginRead(out ValueTask awaitable) diff --git a/src/Kestrel.Core/Internal/Http/Http1ConnectionContext.cs b/src/Kestrel.Core/Internal/Http/Http1ConnectionContext.cs deleted file mode 100644 index 9ad2e9e9b3..0000000000 --- a/src/Kestrel.Core/Internal/Http/Http1ConnectionContext.cs +++ /dev/null @@ -1,25 +0,0 @@ -// 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 System.Buffers; -using System.IO.Pipelines; -using System.Net; -using Microsoft.AspNetCore.Connections; -using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http -{ - public class Http1ConnectionContext : IHttpProtocolContext - { - public string ConnectionId { get; set; } - public ServiceContext ServiceContext { get; set; } - public ConnectionContext ConnectionContext { get; set; } - public IFeatureCollection ConnectionFeatures { get; set; } - public MemoryPool MemoryPool { get; set; } - public IPEndPoint RemoteEndPoint { get; set; } - public IPEndPoint LocalEndPoint { get; set; } - public ITimeoutControl TimeoutControl { get; set; } - public IDuplexPipe Transport { get; set; } - } -} diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index c0461f9eb8..5319e515b0 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -18,6 +18,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private volatile bool _canceled; private Task _pumpTask; + private bool _timingReads; protected Http1MessageBody(Http1Connection context) : base(context) @@ -175,7 +176,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { Log.RequestBodyNotEntirelyRead(_context.ConnectionIdFeature, _context.TraceIdentifier); - _context.TimeoutControl.SetTimeout(Constants.RequestBodyDrainTimeout.Ticks, TimeoutAction.AbortConnection); + _context.TimeoutControl.SetTimeout(Constants.RequestBodyDrainTimeout.Ticks, TimeoutReason.RequestBodyDrain); try { @@ -232,13 +233,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http if (!RequestUpgrade) { Log.RequestBodyStart(_context.ConnectionIdFeature, _context.TraceIdentifier); - _context.TimeoutControl.StartTimingReads(); + + // 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 (!RequestUpgrade) + if (_timingReads) { _context.TimeoutControl.PauseTimingReads(); } @@ -246,7 +256,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private void TryResumeTimingReads() { - if (!RequestUpgrade) + if (_timingReads) { _context.TimeoutControl.ResumeTimingReads(); } @@ -257,7 +267,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http if (!RequestUpgrade) { Log.RequestBodyDone(_context.ConnectionIdFeature, _context.TraceIdentifier); - _context.TimeoutControl.StopTimingReads(); + + if (_timingReads) + { + _context.TimeoutControl.StopTimingReads(); + } } } diff --git a/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs b/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs index 5acff95b16..202676621c 100644 --- a/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs @@ -8,12 +8,12 @@ using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { - public class Http1OutputProducer : IHttpOutputProducer + public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDisposable { private static readonly ReadOnlyMemory _continueBytes = new ReadOnlyMemory(Encoding.ASCII.GetBytes("HTTP/1.1 100 Continue\r\n\r\n")); private static readonly byte[] _bytesHttpVersion11 = Encoding.ASCII.GetBytes("HTTP/1.1 "); @@ -22,10 +22,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private readonly string _connectionId; private readonly ConnectionContext _connectionContext; - private readonly ITimeoutControl _timeoutControl; private readonly IKestrelTrace _log; - private readonly IBytesWrittenFeature _transportBytesWrittenFeature; - private readonly StreamSafePipeFlusher _flusher; + private readonly IHttpMinResponseDataRateFeature _minResponseDataRateFeature; + private readonly TimingPipeFlusher _flusher; // This locks access to to all of the below fields private readonly object _contextLock = new object(); @@ -33,24 +32,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private bool _completed = false; private bool _aborted; private long _unflushedBytes; - private long _totalBytesCommitted; private readonly PipeWriter _pipeWriter; + public Http1OutputProducer( PipeWriter pipeWriter, string connectionId, ConnectionContext connectionContext, IKestrelTrace log, ITimeoutControl timeoutControl, - IBytesWrittenFeature transportBytesWrittenFeature) + IHttpMinResponseDataRateFeature minResponseDataRateFeature) { _pipeWriter = pipeWriter; _connectionId = connectionId; _connectionContext = connectionContext; - _timeoutControl = timeoutControl; _log = log; - _transportBytesWrittenFeature = transportBytesWrittenFeature; - _flusher = new StreamSafePipeFlusher(pipeWriter, timeoutControl); + _minResponseDataRateFeature = minResponseDataRateFeature; + _flusher = new TimingPipeFlusher(pipeWriter, timeoutControl); } public Task WriteDataAsync(ReadOnlySpan buffer, CancellationToken cancellationToken = default) @@ -85,7 +83,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var buffer = _pipeWriter; var bytesCommitted = callback(buffer, state); _unflushedBytes += bytesCommitted; - _totalBytesCommitted += bytesCommitted; } return FlushAsync(cancellationToken); @@ -112,7 +109,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http writer.Commit(); _unflushedBytes += writer.BytesCommitted; - _totalBytesCommitted += writer.BytesCommitted; } } @@ -128,15 +124,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _log.ConnectionDisconnect(_connectionId); _completed = true; _pipeWriter.Complete(); - - var unsentBytes = _totalBytesCommitted - _transportBytesWrittenFeature.TotalBytesWritten; - - if (unsentBytes > 0) - { - // unsentBytes should never be over 64KB in the default configuration. - _timeoutControl.StartTimingWrite((int)Math.Min(unsentBytes, int.MaxValue)); - _pipeWriter.OnReaderCompleted((ex, state) => ((ITimeoutControl)state).StopTimingWrite(), _timeoutControl); - } } } @@ -154,13 +141,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _aborted = true; _connectionContext.Abort(error); - - if (!_completed) - { - _log.ConnectionDisconnect(_connectionId); - _completed = true; - _pipeWriter.Complete(); - } + Dispose(); } } @@ -186,14 +167,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http writer.Write(buffer); _unflushedBytes += buffer.Length; - _totalBytesCommitted += buffer.Length; } writer.Commit(); var bytesWritten = _unflushedBytes; _unflushedBytes = 0; - return _flusher.FlushAsync(bytesWritten, this, cancellationToken); + return _flusher.FlushAsync( + _minResponseDataRateFeature.MinDataRate, + bytesWritten, + this, + cancellationToken); } } } diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index 2e80c87bae..07b3be55de 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -21,8 +21,6 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; -// ReSharper disable AccessToModifiedClosure - namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { public abstract partial class HttpProtocol : IHttpResponseControl @@ -60,12 +58,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private long _responseBytesWritten; - private readonly IHttpProtocolContext _context; + private readonly HttpConnectionContext _context; protected string _methodText = null; private string _scheme = null; - public HttpProtocol(IHttpProtocolContext context) + public HttpProtocol(HttpConnectionContext context) { _context = context; @@ -412,7 +410,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } - public void OnInputOrOutputCompleted() + protected void AbortRequest() { if (Interlocked.Exchange(ref _ioCompleted, 1) != 0) { @@ -421,8 +419,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _keepAlive = false; - Output.Dispose(); - // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); } @@ -476,13 +472,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { try { - OnRequestProcessingEnding(); await TryProduceInvalidRequestResponse(); - - // Prevent RequestAborted from firing. - Reset(); - - Output.Dispose(); } catch (Exception ex) { diff --git a/src/Kestrel.Core/Internal/Http/IHttpOutputAborter.cs b/src/Kestrel.Core/Internal/Http/IHttpOutputAborter.cs new file mode 100644 index 0000000000..989fe91ce8 --- /dev/null +++ b/src/Kestrel.Core/Internal/Http/IHttpOutputAborter.cs @@ -0,0 +1,12 @@ +// 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.Connections; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http +{ + public interface IHttpOutputAborter + { + void Abort(ConnectionAbortedException abortReason); + } +} diff --git a/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs b/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs index 25f3af7012..1259a18f5f 100644 --- a/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs @@ -5,20 +5,16 @@ using System; using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Connections; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { - public interface IHttpOutputProducer : IDisposable + public interface IHttpOutputProducer { - void Abort(ConnectionAbortedException abortReason); Task WriteAsync(Func callback, T state, CancellationToken cancellationToken); Task FlushAsync(CancellationToken cancellationToken); Task Write100ContinueAsync(); void WriteResponseHeaders(int statusCode, string ReasonPhrase, HttpResponseHeaders responseHeaders); - // The reason this is ReadOnlySpan and not ReadOnlyMemory is because writes are always - // synchronous. Flushing to get back pressure is the only time we truly go async but - // that's after the buffer is copied + // This takes ReadOnlySpan instead of ReadOnlyMemory because it always synchronously copies data before flushing. Task WriteDataAsync(ReadOnlySpan data, CancellationToken cancellationToken); Task WriteStreamSuffixAsync(); } diff --git a/src/Kestrel.Core/Internal/Http/IHttpProtocolContext.cs b/src/Kestrel.Core/Internal/Http/IHttpProtocolContext.cs deleted file mode 100644 index 2aa526b867..0000000000 --- a/src/Kestrel.Core/Internal/Http/IHttpProtocolContext.cs +++ /dev/null @@ -1,19 +0,0 @@ -// 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 System.Buffers; -using System.Net; -using Microsoft.AspNetCore.Http.Features; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http -{ - public interface IHttpProtocolContext - { - string ConnectionId { get; set; } - ServiceContext ServiceContext { get; set; } - IFeatureCollection ConnectionFeatures { get; set; } - MemoryPool MemoryPool { get; set; } - IPEndPoint RemoteEndPoint { get; set; } - IPEndPoint LocalEndPoint { get; set; } - } -} diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 756b564351..86133de3ce 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -23,7 +23,7 @@ using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { - public class Http2Connection : ITimeoutControl, IHttp2StreamLifetimeHandler, IHttpHeadersHandler, IRequestProcessor + public class Http2Connection : IHttp2StreamLifetimeHandler, IHttpHeadersHandler, IRequestProcessor { private enum RequestHeaderParsingState { @@ -60,7 +60,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private static readonly byte[] _trailersBytes = Encoding.ASCII.GetBytes("trailers"); private static readonly byte[] _connectBytes = Encoding.ASCII.GetBytes("CONNECT"); - private readonly Http2ConnectionContext _context; + private readonly HttpConnectionContext _context; private readonly Http2FrameWriter _frameWriter; private readonly HPackDecoder _hpackDecoder; private readonly InputFlowControl _inputFlowControl; @@ -84,13 +84,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private readonly ConcurrentDictionary _streams = new ConcurrentDictionary(); - public Http2Connection(Http2ConnectionContext context) + public Http2Connection(HttpConnectionContext context) { var httpLimits = context.ServiceContext.ServerOptions.Limits; var http2Limits = httpLimits.Http2; _context = context; - _frameWriter = new Http2FrameWriter(context.Transport.Output, context.ConnectionContext, _outputFlowControl, this, context.ConnectionId, context.ServiceContext.Log); + _frameWriter = new Http2FrameWriter(context.Transport.Output, context.ConnectionContext, _outputFlowControl, context.TimeoutControl, context.ConnectionId, context.ServiceContext.Log); _serverSettings.MaxConcurrentStreams = (uint)http2Limits.MaxStreamsPerConnection; _serverSettings.MaxFrameSize = (uint)http2Limits.MaxFrameSize; _serverSettings.HeaderTableSize = (uint)http2Limits.HeaderTableSize; @@ -102,12 +102,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } public string ConnectionId => _context.ConnectionId; - public PipeReader Input => _context.Transport.Input; - public IKestrelTrace Log => _context.ServiceContext.Log; - public IFeatureCollection ConnectionFeatures => _context.ConnectionFeatures; + public KestrelServerOptions ServerOptions => _context.ServiceContext.ServerOptions; internal Http2PeerSettings ServerSettings => _serverSettings; @@ -117,12 +115,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { if (_state != Http2ConnectionState.Closed) { - _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR); UpdateState(Http2ConnectionState.Closed); } } - _frameWriter.Complete(); + _frameWriter.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient)); } public void Abort(ConnectionAbortedException ex) @@ -300,6 +297,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 await _streamsCompleted.Task; + _context.TimeoutControl.StartDrainTimeout(ServerOptions.Limits.MinResponseDataRate, ServerOptions.Limits.MaxResponseBufferSize); + _frameWriter.Complete(); } catch @@ -556,7 +555,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 FrameWriter = _frameWriter, ConnectionInputFlowControl = _inputFlowControl, ConnectionOutputFlowControl = _outputFlowControl, - TimeoutControl = this, + TimeoutControl = _context.TimeoutControl, }); _currentHeadersStream.Reset(); @@ -1137,45 +1136,5 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 Log.Http2ConnectionClosed(_context.ConnectionId, _highestOpenedStreamId); } } - - void ITimeoutControl.SetTimeout(long ticks, TimeoutAction timeoutAction) - { - } - - void ITimeoutControl.ResetTimeout(long ticks, TimeoutAction timeoutAction) - { - } - - void ITimeoutControl.CancelTimeout() - { - } - - void ITimeoutControl.StartTimingReads() - { - } - - void ITimeoutControl.PauseTimingReads() - { - } - - void ITimeoutControl.ResumeTimingReads() - { - } - - void ITimeoutControl.StopTimingReads() - { - } - - void ITimeoutControl.BytesRead(long count) - { - } - - void ITimeoutControl.StartTimingWrite(long size) - { - } - - void ITimeoutControl.StopTimingWrite() - { - } } } diff --git a/src/Kestrel.Core/Internal/Http2/Http2ConnectionContext.cs b/src/Kestrel.Core/Internal/Http2/Http2ConnectionContext.cs deleted file mode 100644 index 04a7ab8ca6..0000000000 --- a/src/Kestrel.Core/Internal/Http2/Http2ConnectionContext.cs +++ /dev/null @@ -1,23 +0,0 @@ -// 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 System.Buffers; -using System.IO.Pipelines; -using System.Net; -using Microsoft.AspNetCore.Connections; -using Microsoft.AspNetCore.Http.Features; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 -{ - public class Http2ConnectionContext - { - public string ConnectionId { get; set; } - public ConnectionContext ConnectionContext { get; set; } - public ServiceContext ServiceContext { get; set; } - public IFeatureCollection ConnectionFeatures { get; set; } - public MemoryPool MemoryPool { get; set; } - public IPEndPoint LocalEndPoint { get; set; } - public IPEndPoint RemoteEndPoint { get; set; } - public IDuplexPipe Transport { get; set; } - } -} diff --git a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs index c71ee387f0..f66f162707 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private readonly OutputFlowControl _connectionOutputFlowControl; private readonly string _connectionId; private readonly IKestrelTrace _log; - private readonly StreamSafePipeFlusher _flusher; + private readonly TimingPipeFlusher _flusher; private bool _completed; @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _connectionOutputFlowControl = connectionOutputFlowControl; _connectionId = connectionId; _log = log; - _flusher = new StreamSafePipeFlusher(_outputWriter, timeoutControl); + _flusher = new TimingPipeFlusher(_outputWriter, timeoutControl); _outgoingFrame = new Http2Frame(); _headerEncodingBuffer = new byte[_maxFrameSize]; } @@ -99,7 +99,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - public Task FlushAsync(IHttpOutputProducer outputProducer, CancellationToken cancellationToken) + public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { lock (_writeLock) { @@ -108,7 +108,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - return _flusher.FlushAsync(0, outputProducer, cancellationToken); + return _flusher.FlushAsync(outputAborter, cancellationToken); } } diff --git a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs index fe97317ff0..812a2d9fca 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs @@ -15,11 +15,11 @@ using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { - public class Http2OutputProducer : IHttpOutputProducer + public class Http2OutputProducer : IHttpOutputProducer, IHttpOutputAborter { private readonly int _streamId; private readonly Http2FrameWriter _frameWriter; - private readonly StreamSafePipeFlusher _flusher; + private readonly TimingPipeFlusher _flusher; // This should only be accessed via the FrameWriter. The connection-level output flow control is protected by the // FrameWriter's connection-level write lock. @@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _frameWriter = frameWriter; _flowControl = flowControl; _dataPipe = CreateDataPipe(pool); - _flusher = new StreamSafePipeFlusher(_dataPipe.Writer, timeoutControl); + _flusher = new TimingPipeFlusher(_dataPipe.Writer, timeoutControl); _dataWriteProcessingTask = ProcessDataWrites(); } @@ -65,7 +65,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 // Complete with an exception to prevent an end of stream data frame from being sent without an // explicit call to WriteStreamSuffixAsync. ConnectionAbortedExceptions are swallowed, so the // message doesn't matter - _dataPipe.Writer.Complete(new ConnectionAbortedException()); + _dataPipe.Writer.Complete(new OperationCanceledException()); } _frameWriter.AbortPendingStreamDataWrites(_flowControl); @@ -100,7 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { // If there's already been response data written to the stream, just wait for that. Any header // should be in front of the data frames in the connection pipe. Trailers could change things. - return _flusher.FlushAsync(0, this, cancellationToken); + return _flusher.FlushAsync(this, cancellationToken); } else { @@ -158,7 +158,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _startedWritingDataFrames = true; _dataPipe.Writer.Write(data); - return _flusher.FlushAsync(data.Length, this, cancellationToken); + return _flusher.FlushAsync(this, cancellationToken); } } @@ -173,9 +173,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _completed = true; - // Even if there's no actual data, completing the writer gracefully sends an END_STREAM DATA frame. - _startedWritingDataFrames = true; - _dataPipe.Writer.Complete(); return _dataWriteProcessingTask; } @@ -208,9 +205,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _dataPipe.Reader.AdvanceTo(readResult.Buffer.End); } while (!readResult.IsCompleted); } - catch (ConnectionAbortedException) + catch (OperationCanceledException) { - // Writes should not throw for aborted connections. + // Writes should not throw for aborted streams/connections. } catch (Exception ex) { diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index dca23f399f..2f5566084d 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -67,11 +67,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 try { + _http2Output.Dispose(); + RequestBodyPipe.Reader.Complete(); // The app can no longer read any more of the request body, so return any bytes that weren't read to the // connection's flow-control window. _inputFlowControl.Abort(); + + Reset(); } finally { @@ -435,9 +439,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private void AbortCore(Exception abortReason) { - // Call OnIOCompleted() which closes the output prior to poisoning the request body stream or pipe to + // Call _http2Output.Dispose() prior to poisoning the request body stream or pipe to // ensure that an app that completes early due to the abort doesn't result in header frames being sent. - OnInputOrOutputCompleted(); + _http2Output.Dispose(); + + AbortRequest(); // Unblock the request body. PoisonRequestBodyStream(abortReason); diff --git a/src/Kestrel.Core/Internal/Http2/Http2StreamContext.cs b/src/Kestrel.Core/Internal/Http2/Http2StreamContext.cs index 71db771a2b..7cb87da4f8 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2StreamContext.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2StreamContext.cs @@ -1,30 +1,18 @@ // 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 System.Buffers; -using System.Net; -using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { - public class Http2StreamContext : IHttpProtocolContext + public class Http2StreamContext : HttpConnectionContext { - public string ConnectionId { get; set; } public int StreamId { get; set; } - public ServiceContext ServiceContext { get; set; } - public IFeatureCollection ConnectionFeatures { get; set; } - public MemoryPool MemoryPool { get; set; } - public IPEndPoint RemoteEndPoint { get; set; } - public IPEndPoint LocalEndPoint { get; set; } public IHttp2StreamLifetimeHandler StreamLifetimeHandler { get; set; } public Http2PeerSettings ClientPeerSettings { get; set; } public Http2PeerSettings ServerPeerSettings { get; set; } public Http2FrameWriter FrameWriter { get; set; } public InputFlowControl ConnectionInputFlowControl { get; set; } public OutputFlowControl ConnectionOutputFlowControl { get; set; } - public ITimeoutControl TimeoutControl { get; set; } } } diff --git a/src/Kestrel.Core/Internal/HttpConnection.cs b/src/Kestrel.Core/Internal/HttpConnection.cs index 9fc64acb68..e1eb1278d7 100644 --- a/src/Kestrel.Core/Internal/HttpConnection.cs +++ b/src/Kestrel.Core/Internal/HttpConnection.cs @@ -8,7 +8,6 @@ using System.Diagnostics; using System.IO; using System.IO.Pipelines; using System.Net; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; @@ -24,12 +23,13 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { - public class HttpConnection : ITimeoutControl, IConnectionTimeoutFeature + public class HttpConnection : ITimeoutHandler { private static readonly ReadOnlyMemory Http2Id = new[] { (byte)'h', (byte)'2' }; private readonly HttpConnectionContext _context; private readonly ISystemClock _systemClock; + private readonly TimeoutControl _timeoutControl; private IList _adaptedConnections; private IDuplexPipe _adaptedTransport; @@ -39,33 +39,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal private IRequestProcessor _requestProcessor; private Http1Connection _http1Connection; - private long _lastTimestamp; - private long _timeoutTimestamp = long.MaxValue; - private TimeoutAction _timeoutAction; - - private readonly object _readTimingLock = new object(); - private bool _readTimingEnabled; - private bool _readTimingPauseRequested; - private long _readTimingElapsedTicks; - private long _readTimingBytesRead; - - private readonly object _writeTimingLock = new object(); - private int _writeTimingWrites; - private long _writeTimingTimeoutTimestamp; - public HttpConnection(HttpConnectionContext context) { _context = context; _systemClock = _context.ServiceContext.SystemClock; + + _timeoutControl = new TimeoutControl(this); } // For testing internal HttpProtocol Http1Connection => _http1Connection; internal IDebugger Debugger { get; set; } = DebuggerWrapper.Singleton; - // For testing - internal bool RequestTimedOut { get; private set; } - public string ConnectionId => _context.ConnectionId; public IPEndPoint LocalEndPoint => _context.LocalEndPoint; public IPEndPoint RemoteEndPoint => _context.RemoteEndPoint; @@ -131,9 +116,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal using (connectionLifetimeNotificationFeature?.ConnectionClosedRequested.Register(state => ((HttpConnection)state).StopProcessingNextRequest(), this)) { - _lastTimestamp = _context.ServiceContext.SystemClock.UtcNow.Ticks; + // Ensure TimeoutControl._lastTimestamp is intialized before anything that could set timeouts runs. + _timeoutControl.Initialize(_systemClock.UtcNow); - _context.ConnectionFeatures.Set(this); + _context.ConnectionFeatures.Set(_timeoutControl); if (adaptedPipeline != null) { @@ -149,18 +135,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal // Ensure that the connection hasn't already been stopped. if (_protocolSelectionState == ProtocolSelectionState.Initializing) { + var derivedContext = CreateDerivedContext(_adaptedTransport); + switch (SelectProtocol()) { case HttpProtocols.Http1: // _http1Connection must be initialized before adding the connection to the connection manager - requestProcessor = _http1Connection = CreateHttp1Connection(_adaptedTransport); + requestProcessor = _http1Connection = new Http1Connection(derivedContext); _protocolSelectionState = ProtocolSelectionState.Selected; break; case HttpProtocols.Http2: // _http2Connection must be initialized before yielding control to the transport thread, // to prevent a race condition where _http2Connection.Abort() is called just as // _http2Connection is about to be initialized. - requestProcessor = CreateHttp2Connection(_adaptedTransport); + requestProcessor = new Http2Connection(derivedContext); _protocolSelectionState = ProtocolSelectionState.Selected; break; case HttpProtocols.None: @@ -210,39 +198,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal // For testing only internal void Initialize(IDuplexPipe transport) { - _requestProcessor = _http1Connection = CreateHttp1Connection(transport); + _requestProcessor = _http1Connection = new Http1Connection(CreateDerivedContext(transport)); _protocolSelectionState = ProtocolSelectionState.Selected; } - private Http1Connection CreateHttp1Connection(IDuplexPipe transport) + private HttpConnectionContext CreateDerivedContext(IDuplexPipe transport) { - return new Http1Connection(new Http1ConnectionContext + return new HttpConnectionContext { ConnectionId = _context.ConnectionId, ConnectionFeatures = _context.ConnectionFeatures, - MemoryPool = MemoryPool, - LocalEndPoint = LocalEndPoint, - RemoteEndPoint = RemoteEndPoint, + MemoryPool = _context.MemoryPool, + LocalEndPoint = _context.LocalEndPoint, + RemoteEndPoint = _context.RemoteEndPoint, ServiceContext = _context.ServiceContext, ConnectionContext = _context.ConnectionContext, - TimeoutControl = this, + TimeoutControl = _timeoutControl, Transport = transport - }); - } - - private Http2Connection CreateHttp2Connection(IDuplexPipe transport) - { - return new Http2Connection(new Http2ConnectionContext - { - ConnectionId = _context.ConnectionId, - ConnectionContext = _context.ConnectionContext, - ServiceContext = _context.ServiceContext, - ConnectionFeatures = _context.ConnectionFeatures, - MemoryPool = MemoryPool, - LocalEndPoint = LocalEndPoint, - RemoteEndPoint = RemoteEndPoint, - Transport = transport - }); + }; } private void StopProcessingNextRequest() @@ -378,11 +351,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } private void Tick() - { - Tick(_systemClock.UtcNow); - } - - public void Tick(DateTimeOffset now) { if (_protocolSelectionState == ProtocolSelectionState.Aborted) { @@ -391,246 +359,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal return; } - var timestamp = now.Ticks; - - CheckForTimeout(timestamp); - - // HTTP/2 rate timeouts are not yet supported. - if (_http1Connection != null) - { - CheckForReadDataRateTimeout(timestamp); - CheckForWriteDataRateTimeout(timestamp); - } - - Interlocked.Exchange(ref _lastTimestamp, timestamp); - } - - private void CheckForTimeout(long timestamp) - { - // TODO: Use PlatformApis.VolatileRead equivalent again - if (timestamp > Interlocked.Read(ref _timeoutTimestamp)) - { - if (!Debugger.IsAttached) - { - CancelTimeout(); - - switch (_timeoutAction) - { - case TimeoutAction.StopProcessingNextRequest: - // Http/2 keep-alive timeouts are not yet supported. - _http1Connection?.StopProcessingNextRequest(); - break; - case TimeoutAction.SendTimeoutResponse: - // HTTP/2 timeout responses are not yet supported. - if (_http1Connection != null) - { - RequestTimedOut = true; - _http1Connection.SendTimeoutResponse(); - } - break; - case TimeoutAction.AbortConnection: - // This is actually supported with HTTP/2! - Abort(new ConnectionAbortedException(CoreStrings.ConnectionTimedOutByServer)); - break; - } - } - } - } - - private void CheckForReadDataRateTimeout(long timestamp) - { - Debug.Assert(_http1Connection != null); - - // The only time when both a timeout is set and the read data rate could be enforced is - // when draining the request body. Since there's already a (short) timeout set for draining, - // it's safe to not check the data rate at this point. - if (Interlocked.Read(ref _timeoutTimestamp) != long.MaxValue) - { - return; - } - - lock (_readTimingLock) - { - if (_readTimingEnabled) - { - // Reference in local var to avoid torn reads in case the min rate is changed via IHttpMinRequestBodyDataRateFeature - var minRequestBodyDataRate = _http1Connection.MinRequestBodyDataRate; - - _readTimingElapsedTicks += timestamp - _lastTimestamp; - - if (minRequestBodyDataRate?.BytesPerSecond > 0 && _readTimingElapsedTicks > minRequestBodyDataRate.GracePeriod.Ticks) - { - var elapsedSeconds = (double)_readTimingElapsedTicks / TimeSpan.TicksPerSecond; - var rate = Interlocked.Read(ref _readTimingBytesRead) / elapsedSeconds; - - if (rate < minRequestBodyDataRate.BytesPerSecond && !Debugger.IsAttached) - { - Log.RequestBodyMinimumDataRateNotSatisfied(_context.ConnectionId, _http1Connection.TraceIdentifier, minRequestBodyDataRate.BytesPerSecond); - RequestTimedOut = true; - _http1Connection.SendTimeoutResponse(); - } - } - - // PauseTimingReads() cannot just set _timingReads to false. It needs to go through at least one tick - // before pausing, otherwise _readTimingElapsed might never be updated if PauseTimingReads() is always - // called before the next tick. - if (_readTimingPauseRequested) - { - _readTimingEnabled = false; - _readTimingPauseRequested = false; - } - } - } - } - - private void CheckForWriteDataRateTimeout(long timestamp) - { - Debug.Assert(_http1Connection != null); - - lock (_writeTimingLock) - { - if (_writeTimingWrites > 0 && timestamp > _writeTimingTimeoutTimestamp && !Debugger.IsAttached) - { - RequestTimedOut = true; - Log.ResponseMinimumDataRateNotSatisfied(_http1Connection.ConnectionIdFeature, _http1Connection.TraceIdentifier); - Abort(new ConnectionAbortedException(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)); - } - } - } - - public void SetTimeout(long ticks, TimeoutAction timeoutAction) - { - Debug.Assert(_timeoutTimestamp == long.MaxValue, "Concurrent timeouts are not supported"); - - AssignTimeout(ticks, timeoutAction); - } - - public void ResetTimeout(long ticks, TimeoutAction timeoutAction) - { - AssignTimeout(ticks, timeoutAction); - } - - public void CancelTimeout() - { - Interlocked.Exchange(ref _timeoutTimestamp, long.MaxValue); - } - - private void AssignTimeout(long ticks, TimeoutAction timeoutAction) - { - _timeoutAction = timeoutAction; - - // Add Heartbeat.Interval since this can be called right before the next heartbeat. - Interlocked.Exchange(ref _timeoutTimestamp, _lastTimestamp + ticks + Heartbeat.Interval.Ticks); - } - - public void StartTimingReads() - { - lock (_readTimingLock) - { - _readTimingElapsedTicks = 0; - _readTimingBytesRead = 0; - _readTimingEnabled = true; - } - } - - public void StopTimingReads() - { - lock (_readTimingLock) - { - _readTimingEnabled = false; - } - } - - public void PauseTimingReads() - { - lock (_readTimingLock) - { - _readTimingPauseRequested = true; - } - } - - public void ResumeTimingReads() - { - lock (_readTimingLock) - { - _readTimingEnabled = true; - - // In case pause and resume were both called between ticks - _readTimingPauseRequested = false; - } - } - - public void BytesRead(long count) - { - Interlocked.Add(ref _readTimingBytesRead, count); - } - - public void StartTimingWrite(long size) - { - Debug.Assert(_http1Connection != null); - - lock (_writeTimingLock) - { - var minResponseDataRate = _http1Connection.MinResponseDataRate; - - if (minResponseDataRate != null) - { - // Add Heartbeat.Interval since this can be called right before the next heartbeat. - var currentTimeUpperBound = _lastTimestamp + Heartbeat.Interval.Ticks; - var ticksToCompleteWriteAtMinRate = TimeSpan.FromSeconds(size / minResponseDataRate.BytesPerSecond).Ticks; - - // If ticksToCompleteWriteAtMinRate is less than the configured grace period, - // allow that write to take up to the grace period to complete. Only add the grace period - // to the current time and not to any accumulated timeout. - var singleWriteTimeoutTimestamp = currentTimeUpperBound + Math.Max( - minResponseDataRate.GracePeriod.Ticks, - ticksToCompleteWriteAtMinRate); - - // Don't penalize a connection for completing previous writes more quickly than required. - // We don't want to kill a connection when flushing the chunk terminator just because the previous - // chunk was large if the previous chunk was flushed quickly. - - // Don't add any grace period to this accumulated timeout because the grace period could - // get accumulated repeatedly making the timeout for a bunch of consecutive small writes - // far too conservative. - var accumulatedWriteTimeoutTimestamp = _writeTimingTimeoutTimestamp + ticksToCompleteWriteAtMinRate; - - _writeTimingTimeoutTimestamp = Math.Max(singleWriteTimeoutTimestamp, accumulatedWriteTimeoutTimestamp); - _writeTimingWrites++; - } - } - } - - public void StopTimingWrite() - { - lock (_writeTimingLock) - { - _writeTimingWrites--; - } - } - - void IConnectionTimeoutFeature.SetTimeout(TimeSpan timeSpan) - { - if (timeSpan < TimeSpan.Zero) - { - throw new ArgumentException(CoreStrings.PositiveFiniteTimeSpanRequired, nameof(timeSpan)); - } - if (_timeoutTimestamp != long.MaxValue) - { - throw new InvalidOperationException(CoreStrings.ConcurrentTimeoutsNotSupported); - } - - SetTimeout(timeSpan.Ticks, TimeoutAction.AbortConnection); - } - - void IConnectionTimeoutFeature.ResetTimeout(TimeSpan timeSpan) - { - if (timeSpan < TimeSpan.Zero) - { - throw new ArgumentException(CoreStrings.PositiveFiniteTimeSpanRequired, nameof(timeSpan)); - } - - ResetTimeout(timeSpan.Ticks, TimeoutAction.AbortConnection); + _timeoutControl.Tick(_systemClock.UtcNow); } private void CloseUninitializedConnection(ConnectionAbortedException abortReason) @@ -643,6 +372,36 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _adaptedTransport.Output.Complete(); } + public void OnTimeout(TimeoutReason reason) + { + // In the cases that don't log directly here, we expect the setter of the timeout to also be the input + // reader, so when the read is canceled or arborted, the reader should write the appropriate log. + switch (reason) + { + case TimeoutReason.KeepAlive: + _http1Connection.StopProcessingNextRequest(); + break; + case TimeoutReason.RequestHeaders: + _http1Connection.SendTimeoutResponse(); + break; + case TimeoutReason.ReadDataRate: + Log.RequestBodyMinimumDataRateNotSatisfied(_context.ConnectionId, _http1Connection.TraceIdentifier, _http1Connection.MinRequestBodyDataRate.BytesPerSecond); + _http1Connection.SendTimeoutResponse(); + break; + case TimeoutReason.WriteDataRate: + Log.ResponseMinimumDataRateNotSatisfied(_http1Connection.ConnectionIdFeature, _http1Connection.TraceIdentifier); + Abort(new ConnectionAbortedException(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)); + break; + case TimeoutReason.RequestBodyDrain: + case TimeoutReason.TimeoutFeature: + Abort(new ConnectionAbortedException(CoreStrings.ConnectionTimedOutByServer)); + break; + default: + Debug.Assert(false, "Invalid TimeoutReason"); + break; + } + } + private enum ProtocolSelectionState { Initializing, diff --git a/src/Kestrel.Core/Internal/HttpConnectionContext.cs b/src/Kestrel.Core/Internal/HttpConnectionContext.cs index c5eceead0a..581c63bfec 100644 --- a/src/Kestrel.Core/Internal/HttpConnectionContext.cs +++ b/src/Kestrel.Core/Internal/HttpConnectionContext.cs @@ -8,6 +8,7 @@ using System.Net; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { @@ -22,6 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public MemoryPool MemoryPool { get; set; } public IPEndPoint LocalEndPoint { get; set; } public IPEndPoint RemoteEndPoint { get; set; } + public ITimeoutControl TimeoutControl { get; set; } public IDuplexPipe Transport { get; set; } } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs index 69175ec0b5..69f0362ce8 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs @@ -5,17 +5,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { public interface ITimeoutControl { - void SetTimeout(long ticks, TimeoutAction timeoutAction); - void ResetTimeout(long ticks, TimeoutAction timeoutAction); + void SetTimeout(long ticks, TimeoutReason timeoutReason); + void ResetTimeout(long ticks, TimeoutReason timeoutReason); void CancelTimeout(); - void StartTimingReads(); + void StartTimingReads(MinDataRate minRate); void PauseTimingReads(); void ResumeTimingReads(); void StopTimingReads(); void BytesRead(long count); - void StartTimingWrite(long size); + void StartTimingWrite(MinDataRate minRate, long size); void StopTimingWrite(); } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/TimeoutAction.cs b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutHandler.cs similarity index 68% rename from src/Kestrel.Core/Internal/Infrastructure/TimeoutAction.cs rename to src/Kestrel.Core/Internal/Infrastructure/ITimeoutHandler.cs index b2aa9df8cf..30eb2b8b29 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/TimeoutAction.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutHandler.cs @@ -3,10 +3,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public enum TimeoutAction + public interface ITimeoutHandler { - StopProcessingNextRequest, - SendTimeoutResponse, - AbortConnection, + void OnTimeout(TimeoutReason reason); } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/ITimeoutReason.cs b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutReason.cs new file mode 100644 index 0000000000..b6900b4501 --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutReason.cs @@ -0,0 +1,15 @@ +// 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. + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public enum TimeoutReason + { + KeepAlive, + RequestHeaders, + ReadDataRate, + WriteDataRate, + RequestBodyDrain, + TimeoutFeature, + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs b/src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs new file mode 100644 index 0000000000..6aa349827c --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs @@ -0,0 +1,247 @@ +// 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 System; +using System.Diagnostics; +using System.Threading; +using Microsoft.AspNetCore.Server.Kestrel.Core.Features; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public class TimeoutControl : ITimeoutControl, IConnectionTimeoutFeature + { + private readonly ITimeoutHandler _timeoutHandler; + + private long _lastTimestamp; + private long _timeoutTimestamp = long.MaxValue; + private TimeoutReason _timeoutReason; + + private readonly object _readTimingLock = new object(); + private MinDataRate _minReadRate; + private bool _readTimingEnabled; + private bool _readTimingPauseRequested; + private long _readTimingElapsedTicks; + private long _readTimingBytesRead; + + private readonly object _writeTimingLock = new object(); + private int _writeTimingWrites; + private long _writeTimingTimeoutTimestamp; + + public TimeoutControl(ITimeoutHandler timeoutHandler) + { + _timeoutHandler = timeoutHandler; + } + + internal IDebugger Debugger { get; set; } = DebuggerWrapper.Singleton; + + public void Initialize(DateTimeOffset now) + { + _lastTimestamp = now.Ticks; + } + + public void Tick(DateTimeOffset now) + { + var timestamp = now.Ticks; + + CheckForTimeout(timestamp); + CheckForReadDataRateTimeout(timestamp); + CheckForWriteDataRateTimeout(timestamp); + + Interlocked.Exchange(ref _lastTimestamp, timestamp); + } + + private void CheckForTimeout(long timestamp) + { + if (!Debugger.IsAttached) + { + if (timestamp > Interlocked.Read(ref _timeoutTimestamp)) + { + CancelTimeout(); + + _timeoutHandler.OnTimeout(_timeoutReason); + } + } + } + + private void CheckForReadDataRateTimeout(long timestamp) + { + // The only time when both a timeout is set and the read data rate could be enforced is + // when draining the request body. Since there's already a (short) timeout set for draining, + // it's safe to not check the data rate at this point. + if (Interlocked.Read(ref _timeoutTimestamp) != long.MaxValue) + { + return; + } + + lock (_readTimingLock) + { + if (!_readTimingEnabled) + { + return; + } + + _readTimingElapsedTicks += timestamp - _lastTimestamp; + + if (_minReadRate.BytesPerSecond > 0 && _readTimingElapsedTicks > _minReadRate.GracePeriod.Ticks) + { + var elapsedSeconds = (double)_readTimingElapsedTicks / TimeSpan.TicksPerSecond; + var rate = Interlocked.Read(ref _readTimingBytesRead) / elapsedSeconds; + + if (rate < _minReadRate.BytesPerSecond && !Debugger.IsAttached) + { + _timeoutHandler.OnTimeout(TimeoutReason.ReadDataRate); + } + } + + // PauseTimingReads() cannot just set _timingReads to false. It needs to go through at least one tick + // before pausing, otherwise _readTimingElapsed might never be updated if PauseTimingReads() is always + // called before the next tick. + if (_readTimingPauseRequested) + { + _readTimingEnabled = false; + _readTimingPauseRequested = false; + } + } + } + + private void CheckForWriteDataRateTimeout(long timestamp) + { + lock (_writeTimingLock) + { + if (_writeTimingWrites > 0 && timestamp > _writeTimingTimeoutTimestamp && !Debugger.IsAttached) + { + _timeoutHandler.OnTimeout(TimeoutReason.WriteDataRate); + } + } + } + + public void SetTimeout(long ticks, TimeoutReason timeoutReason) + { + Debug.Assert(_timeoutTimestamp == long.MaxValue, "Concurrent timeouts are not supported"); + + AssignTimeout(ticks, timeoutReason); + } + + public void ResetTimeout(long ticks, TimeoutReason timeoutReason) + { + AssignTimeout(ticks, timeoutReason); + } + + public void CancelTimeout() + { + Interlocked.Exchange(ref _timeoutTimestamp, long.MaxValue); + } + + private void AssignTimeout(long ticks, TimeoutReason timeoutReason) + { + _timeoutReason = timeoutReason; + + // Add Heartbeat.Interval since this can be called right before the next heartbeat. + Interlocked.Exchange(ref _timeoutTimestamp, _lastTimestamp + ticks + Heartbeat.Interval.Ticks); + } + + public void StartTimingReads(MinDataRate minRate) + { + lock (_readTimingLock) + { + _minReadRate = minRate; + _readTimingElapsedTicks = 0; + _readTimingBytesRead = 0; + _readTimingEnabled = true; + } + } + + public void StopTimingReads() + { + lock (_readTimingLock) + { + _readTimingEnabled = false; + } + } + + public void PauseTimingReads() + { + lock (_readTimingLock) + { + _readTimingPauseRequested = true; + } + } + + public void ResumeTimingReads() + { + lock (_readTimingLock) + { + _readTimingEnabled = true; + + // In case pause and resume were both called between ticks + _readTimingPauseRequested = false; + } + } + + public void BytesRead(long count) + { + Interlocked.Add(ref _readTimingBytesRead, count); + } + + 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 ticksToCompleteWriteAtMinRate = TimeSpan.FromSeconds(size / minRate.BytesPerSecond).Ticks; + + // If ticksToCompleteWriteAtMinRate is less than the configured grace period, + // allow that write to take up to the grace period to complete. Only add the grace period + // to the current time and not to any accumulated timeout. + var singleWriteTimeoutTimestamp = currentTimeUpperBound + Math.Max( + minRate.GracePeriod.Ticks, + ticksToCompleteWriteAtMinRate); + + // Don't penalize a connection for completing previous writes more quickly than required. + // We don't want to kill a connection when flushing the chunk terminator just because the previous + // chunk was large if the previous chunk was flushed quickly. + + // Don't add any grace period to this accumulated timeout because the grace period could + // get accumulated repeatedly making the timeout for a bunch of consecutive small writes + // far too conservative. + var accumulatedWriteTimeoutTimestamp = _writeTimingTimeoutTimestamp + ticksToCompleteWriteAtMinRate; + + _writeTimingTimeoutTimestamp = Math.Max(singleWriteTimeoutTimestamp, accumulatedWriteTimeoutTimestamp); + _writeTimingWrites++; + } + } + + public void StopTimingWrite() + { + lock (_writeTimingLock) + { + _writeTimingWrites--; + } + } + + void IConnectionTimeoutFeature.SetTimeout(TimeSpan timeSpan) + { + if (timeSpan < TimeSpan.Zero) + { + throw new ArgumentException(CoreStrings.PositiveFiniteTimeSpanRequired, nameof(timeSpan)); + } + if (_timeoutTimestamp != long.MaxValue) + { + throw new InvalidOperationException(CoreStrings.ConcurrentTimeoutsNotSupported); + } + + SetTimeout(timeSpan.Ticks, TimeoutReason.TimeoutFeature); + } + + void IConnectionTimeoutFeature.ResetTimeout(TimeSpan timeSpan) + { + if (timeSpan < TimeSpan.Zero) + { + throw new ArgumentException(CoreStrings.PositiveFiniteTimeSpanRequired, nameof(timeSpan)); + } + + ResetTimeout(timeSpan.Ticks, TimeoutReason.TimeoutFeature); + } + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/TimeoutControlExtensions.cs b/src/Kestrel.Core/Internal/Infrastructure/TimeoutControlExtensions.cs new file mode 100644 index 0000000000..931bfb51f4 --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/TimeoutControlExtensions.cs @@ -0,0 +1,24 @@ +// 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. + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public static class TimeoutControlExtensions + { + public static void StartDrainTimeout(this ITimeoutControl timeoutControl, MinDataRate minDataRate, long? maxResponseBufferSize) + { + // If maxResponseBufferSize has no value, there's no backpressure and we can't reasonably timeout draining. + if (minDataRate == null || maxResponseBufferSize == null) + { + return; + } + + // With full backpressure and a connection adapter there could be 2 two pipes buffering. + // We already validate that the buffer size is positive. + // There's no reason to stop timing the write after the connection is closed. + var oneBufferSize = maxResponseBufferSize.Value; + var maxBufferedBytes = oneBufferSize < long.MaxValue / 2 ? oneBufferSize * 2 : long.MaxValue; + timeoutControl.StartTimingWrite(minDataRate, maxBufferedBytes); + } + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/StreamSafePipeFlusher.cs b/src/Kestrel.Core/Internal/Infrastructure/TimingPipeFlusher.cs similarity index 64% rename from src/Kestrel.Core/Internal/Infrastructure/StreamSafePipeFlusher.cs rename to src/Kestrel.Core/Internal/Infrastructure/TimingPipeFlusher.cs index 0caeda11ad..eb9554571a 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/StreamSafePipeFlusher.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/TimingPipeFlusher.cs @@ -12,9 +12,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { /// /// This wraps PipeWriter.FlushAsync() in a way that allows multiple awaiters making it safe to call from publicly - /// exposed Stream implementations. + /// exposed Stream implementations while also tracking response data rate. /// - public class StreamSafePipeFlusher + public class TimingPipeFlusher { private readonly PipeWriter _writer; private readonly ITimeoutControl _timeoutControl; @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure private Task _lastFlushTask = Task.CompletedTask; - public StreamSafePipeFlusher( + public TimingPipeFlusher( PipeWriter writer, ITimeoutControl timeoutControl) { @@ -30,7 +30,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure _timeoutControl = timeoutControl; } - public Task FlushAsync(long count = 0, IHttpOutputProducer outputProducer = null, CancellationToken cancellationToken = default) + public Task FlushAsync() + { + return FlushAsync(outputAborter: null, cancellationToken: default); + } + + public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + { + return FlushAsync(minRate: null, count: 0, outputAborter: outputAborter, cancellationToken: cancellationToken); + } + + public Task FlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { var flushValueTask = _writer.FlushAsync(cancellationToken); @@ -51,13 +61,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure _lastFlushTask = flushValueTask.AsTask(); } - return TimeFlushAsync(count, outputProducer, cancellationToken); + return TimeFlushAsync(minRate, count, outputAborter, cancellationToken); } } - private async Task TimeFlushAsync(long count, IHttpOutputProducer outputProducer, CancellationToken cancellationToken) + private async Task TimeFlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { - _timeoutControl.StartTimingWrite(count); + if (minRate != null) + { + _timeoutControl.StartTimingWrite(minRate, count); + } try { @@ -65,14 +78,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure } catch (OperationCanceledException ex) { - outputProducer.Abort(new ConnectionAbortedException(CoreStrings.ConnectionOrStreamAbortedByCancellationToken, ex)); + outputAborter.Abort(new ConnectionAbortedException(CoreStrings.ConnectionOrStreamAbortedByCancellationToken, ex)); } catch { // A canceled token is the only reason flush should ever throw. } - _timeoutControl.StopTimingWrite(); + if (minRate != null) + { + _timeoutControl.StopTimingWrite(); + } cancellationToken.ThrowIfCancellationRequested(); } diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 51388a5b88..d6a4c620d9 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -2184,6 +2184,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatHPackErrorIntegerTooBig() => GetString("HPackErrorIntegerTooBig"); + /// + /// The client closed the connection. + /// + internal static string ConnectionAbortedByClient + { + get => GetString("ConnectionAbortedByClient"); + } + + /// + /// The client closed the connection. + /// + internal static string FormatConnectionAbortedByClient() + => GetString("ConnectionAbortedByClient"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Kestrel.Transport.Abstractions/Internal/IBytesWrittenFeature.cs b/src/Kestrel.Transport.Abstractions/Internal/IBytesWrittenFeature.cs deleted file mode 100644 index e4bf998f37..0000000000 --- a/src/Kestrel.Transport.Abstractions/Internal/IBytesWrittenFeature.cs +++ /dev/null @@ -1,13 +0,0 @@ -// 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 System; -using System.Collections.Generic; -using System.Text; - -namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal -{ - public interface IBytesWrittenFeature - { - long TotalBytesWritten { get; } - } -} diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs index f88b7189f7..1f4a3c62d2 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs @@ -21,8 +21,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal ITransportSchedulerFeature, IConnectionLifetimeFeature, IConnectionHeartbeatFeature, - IConnectionLifetimeNotificationFeature, - IBytesWrittenFeature + IConnectionLifetimeNotificationFeature { // NOTE: When feature interfaces are added to or removed from this TransportConnection class implementation, // then the list of `features` in the generated code project MUST also be updated. @@ -101,7 +100,5 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { OnHeartbeat(action, state); } - - long IBytesWrittenFeature.TotalBytesWritten => TotalBytesWritten; } } diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs index 2dffb40067..5eba177a89 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs @@ -22,7 +22,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal private static readonly Type IConnectionLifetimeFeatureType = typeof(IConnectionLifetimeFeature); private static readonly Type IConnectionHeartbeatFeatureType = typeof(IConnectionHeartbeatFeature); private static readonly Type IConnectionLifetimeNotificationFeatureType = typeof(IConnectionLifetimeNotificationFeature); - private static readonly Type IBytesWrittenFeatureType = typeof(IBytesWrittenFeature); private object _currentIHttpConnectionFeature; private object _currentIConnectionIdFeature; @@ -34,7 +33,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal private object _currentIConnectionLifetimeFeature; private object _currentIConnectionHeartbeatFeature; private object _currentIConnectionLifetimeNotificationFeature; - private object _currentIBytesWrittenFeature; private int _featureRevision; @@ -52,7 +50,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal _currentIConnectionLifetimeFeature = this; _currentIConnectionHeartbeatFeature = this; _currentIConnectionLifetimeNotificationFeature = this; - _currentIBytesWrittenFeature = this; } @@ -148,10 +145,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { feature = _currentIConnectionLifetimeNotificationFeature; } - else if (key == IBytesWrittenFeatureType) - { - feature = _currentIBytesWrittenFeature; - } else if (MaybeExtra != null) { feature = ExtraFeatureGet(key); @@ -204,10 +197,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { _currentIConnectionLifetimeNotificationFeature = value; } - else if (key == IBytesWrittenFeatureType) - { - _currentIBytesWrittenFeature = value; - } else { ExtraFeatureSet(key, value); @@ -258,10 +247,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { feature = (TFeature)_currentIConnectionLifetimeNotificationFeature; } - else if (typeof(TFeature) == typeof(IBytesWrittenFeature)) - { - feature = (TFeature)_currentIBytesWrittenFeature; - } else if (MaybeExtra != null) { feature = (TFeature)(ExtraFeatureGet(typeof(TFeature))); @@ -313,10 +298,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { _currentIConnectionLifetimeNotificationFeature = feature; } - else if (typeof(TFeature) == typeof(IBytesWrittenFeature)) - { - _currentIBytesWrittenFeature = feature; - } else { ExtraFeatureSet(typeof(TFeature), feature); @@ -365,10 +346,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { yield return new KeyValuePair(IConnectionLifetimeNotificationFeatureType, _currentIConnectionLifetimeNotificationFeature); } - if (_currentIBytesWrittenFeature != null) - { - yield return new KeyValuePair(IBytesWrittenFeatureType, _currentIBytesWrittenFeature); - } if (MaybeExtra != null) { diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs index a620a6e5d3..bc9a8b27a0 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs @@ -38,7 +38,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal public virtual MemoryPool MemoryPool { get; } public virtual PipeScheduler InputWriterScheduler { get; } public virtual PipeScheduler OutputReaderScheduler { get; } - public virtual long TotalBytesWritten { get; } public override IDuplexPipe Transport { get; set; } public IDuplexPipe Application { get; set; } diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs index 663f7bfbd5..47f4b112dd 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs @@ -54,8 +54,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public override PipeScheduler InputWriterScheduler => Thread; public override PipeScheduler OutputReaderScheduler => Thread; - public override long TotalBytesWritten => OutputConsumer?.TotalBytesWritten ?? 0; - public async Task Start() { try @@ -98,7 +96,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal finally { // Now, complete the input so that no more reads can happen - Input.Complete(inputError ?? _abortReason ?? new ConnectionAbortedException()); + Input.Complete(inputError ?? _abortReason ?? new ConnectionAbortedException("The libuv transport's send loop completed gracefully.")); Output.Complete(outputError); // Make sure it isn't possible for a paused read to resume reading after calling uv_close diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs index 6049245537..bd7048c9ac 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvOutputConsumer.cs @@ -17,8 +17,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal private readonly ILibuvTrace _log; private readonly PipeReader _pipe; - private long _totalBytesWritten; - public LibuvOutputConsumer( PipeReader pipe, LibuvThread thread, @@ -33,8 +31,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal _log = log; } - public long TotalBytesWritten => Interlocked.Read(ref _totalBytesWritten); - public async Task WriteOutputAsync() { var pool = _thread.WriteReqPool; @@ -66,10 +62,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal var writeResult = await writeReq.WriteAsync(_socket, buffer); - // This is not interlocked because there could be a concurrent writer. - // Instead it's to prevent read tearing on 32-bit systems. - Interlocked.Add(ref _totalBytesWritten, buffer.Length); - LogWriteInfo(writeResult.Status, writeResult.Error); if (writeResult.Error != null) diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs index 24a7de0283..d9cd3887e5 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs @@ -32,7 +32,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal private readonly object _shutdownLock = new object(); private volatile bool _socketDisposed; private volatile Exception _shutdownReason; - private long _totalBytesWritten; internal SocketConnection(Socket socket, MemoryPool memoryPool, PipeScheduler scheduler, ISocketsTrace trace) { @@ -68,7 +67,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal public override MemoryPool MemoryPool { get; } public override PipeScheduler InputWriterScheduler => _scheduler; public override PipeScheduler OutputReaderScheduler => _scheduler; - public override long TotalBytesWritten => Interlocked.Read(ref _totalBytesWritten); public async Task StartAsync() { @@ -264,10 +262,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal await _sender.SendAsync(buffer); } - // This is not interlocked because there could be a concurrent writer. - // Instead it's to prevent read tearing on 32-bit systems. - Interlocked.Add(ref _totalBytesWritten, buffer.Length); - Output.AdvanceTo(end); if (isCompleted) @@ -294,7 +288,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal // shutdownReason should only be null if the output was completed gracefully, so no one should ever // ever observe the nondescript ConnectionAbortedException except for connection middleware attempting // to half close the connection which is currently unsupported. - _shutdownReason = shutdownReason ?? new ConnectionAbortedException(); + _shutdownReason = shutdownReason ?? new ConnectionAbortedException("The Socket transport's send loop completed gracefully."); _trace.ConnectionWriteFin(ConnectionId); diff --git a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs b/test/Kestrel.Core.Tests/Http1ConnectionTests.cs index 9dd6c5e488..f2c665828a 100644 --- a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs +++ b/test/Kestrel.Core.Tests/Http1ConnectionTests.cs @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests private readonly IDuplexPipe _application; private readonly TestHttp1Connection _http1Connection; private readonly ServiceContext _serviceContext; - private readonly Http1ConnectionContext _http1ConnectionContext; + private readonly HttpConnectionContext _http1ConnectionContext; private readonly MemoryPool _pipelineFactory; private SequencePosition _consumed; private SequencePosition _examined; @@ -52,11 +52,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var connectionFeatures = new FeatureCollection(); connectionFeatures.Set(Mock.Of()); - connectionFeatures.Set(Mock.Of()); _serviceContext = new TestServiceContext(); _timeoutControl = new Mock(); - _http1ConnectionContext = new Http1ConnectionContext + _http1ConnectionContext = new HttpConnectionContext { ServiceContext = _serviceContext, ConnectionContext = Mock.Of(), @@ -425,7 +424,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _transport.Input.AdvanceTo(_consumed, _examined); var expectedRequestHeadersTimeout = _serviceContext.ServerOptions.Limits.RequestHeadersTimeout.Ticks; - _timeoutControl.Verify(cc => cc.ResetTimeout(expectedRequestHeadersTimeout, TimeoutAction.SendTimeoutResponse)); + _timeoutControl.Verify(cc => cc.ResetTimeout(expectedRequestHeadersTimeout, TimeoutReason.RequestHeaders)); } [Fact] @@ -542,7 +541,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var requestProcessingTask = _http1Connection.ProcessRequestsAsync(null); var expectedKeepAliveTimeout = _serviceContext.ServerOptions.Limits.KeepAliveTimeout.Ticks; - _timeoutControl.Verify(cc => cc.SetTimeout(expectedKeepAliveTimeout, TimeoutAction.StopProcessingNextRequest)); + _timeoutControl.Verify(cc => cc.SetTimeout(expectedKeepAliveTimeout, TimeoutReason.KeepAlive)); _http1Connection.StopProcessingNextRequest(); _application.Output.Complete(); diff --git a/test/Kestrel.Core.Tests/HttpConnectionTests.cs b/test/Kestrel.Core.Tests/HttpConnectionTests.cs index 2ade6c44a6..565b7eeae4 100644 --- a/test/Kestrel.Core.Tests/HttpConnectionTests.cs +++ b/test/Kestrel.Core.Tests/HttpConnectionTests.cs @@ -1,592 +1,49 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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 System; -using System.Buffers; -using System.Collections.Generic; using System.IO.Pipelines; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; -using Microsoft.AspNetCore.Connections.Features; -using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; 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 HttpConnectionTests : IDisposable + public class HttpConnectionTests { - private readonly MemoryPool _memoryPool; - private readonly HttpConnectionContext _httpConnectionContext; - private readonly HttpConnection _httpConnection; - - public HttpConnectionTests() + [Fact] + public async Task WriteDataRateTimeoutAbortsConnection() { - _memoryPool = KestrelMemoryPool.Create(); - var options = new PipeOptions(_memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); - var pair = DuplexPipe.CreateConnectionPair(options, options); + var mockConnectionContext = new Mock(); - var connectionFeatures = new FeatureCollection(); - connectionFeatures.Set(Mock.Of()); - connectionFeatures.Set(Mock.Of()); - - _httpConnectionContext = new HttpConnectionContext + var httpConnectionContext = new HttpConnectionContext { - ConnectionId = "0123456789", - ConnectionContext = Mock.Of(), - ConnectionAdapters = new List(), - ConnectionFeatures = connectionFeatures, - MemoryPool = _memoryPool, - Transport = pair.Transport, - ServiceContext = new TestServiceContext - { - SystemClock = new SystemClock() - } + ConnectionContext = mockConnectionContext.Object, + Transport = new DuplexPipe(Mock.Of(), Mock.Of()), + ServiceContext = new TestServiceContext() }; - _httpConnection = new HttpConnection(_httpConnectionContext); - } + var httpConnection = new HttpConnection(httpConnectionContext); - public void Dispose() - { - _memoryPool.Dispose(); - } - - [Fact] - public void DoesNotTimeOutWhenDebuggerIsAttached() - { - var mockDebugger = new Mock(); - mockDebugger.SetupGet(g => g.IsAttached).Returns(true); - _httpConnection.Debugger = mockDebugger.Object; - _httpConnection.Initialize(_httpConnectionContext.Transport); - - var now = DateTimeOffset.Now; - _httpConnection.Tick(now); - _httpConnection.SetTimeout(1, TimeoutAction.SendTimeoutResponse); - _httpConnection.Tick(now.AddTicks(2).Add(Heartbeat.Interval)); - - Assert.False(_httpConnection.RequestTimedOut); - } - - [Fact] - public void DoesNotTimeOutWhenRequestBodyDoesNotSatisfyMinimumDataRateButDebuggerIsAttached() - { - var mockDebugger = new Mock(); - mockDebugger.SetupGet(g => g.IsAttached).Returns(true); - _httpConnection.Debugger = mockDebugger.Object; - var bytesPerSecond = 100; - var mockLogger = new Mock(); - mockLogger.Setup(l => l.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny())).Throws(new InvalidOperationException("Should not log")); - - TickBodyWithMinimumDataRate(mockLogger.Object, bytesPerSecond); - - Assert.False(_httpConnection.RequestTimedOut); - } - - [Fact] - public void TimesOutWhenRequestBodyDoesNotSatisfyMinimumDataRate() - { - var bytesPerSecond = 100; - var mockLogger = new Mock(); - TickBodyWithMinimumDataRate(mockLogger.Object, bytesPerSecond); - - // Timed out - Assert.True(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); - } - - private void TickBodyWithMinimumDataRate(IKestrelTrace logger, int bytesPerSecond) - { - var gracePeriod = TimeSpan.FromSeconds(5); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinDataRate(bytesPerSecond: bytesPerSecond, gracePeriod: gracePeriod); - - _httpConnectionContext.ServiceContext.Log = logger; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - - // Initialize timestamp - var now = DateTimeOffset.UtcNow; - _httpConnection.Tick(now); - - _httpConnection.StartTimingReads(); - - // Tick after grace period w/ low data rate - now += gracePeriod + TimeSpan.FromSeconds(1); - _httpConnection.BytesRead(1); - _httpConnection.Tick(now); - } - - [Fact] - public void RequestBodyMinimumDataRateNotEnforcedDuringGracePeriod() - { - var bytesPerSecond = 100; - var gracePeriod = TimeSpan.FromSeconds(2); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinDataRate(bytesPerSecond: bytesPerSecond, gracePeriod: gracePeriod); - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - - // Initialize timestamp - var now = DateTimeOffset.UtcNow; - _httpConnection.Tick(now); - - _httpConnection.StartTimingReads(); - - // Tick during grace period w/ low data rate - now += TimeSpan.FromSeconds(1); - _httpConnection.BytesRead(10); - _httpConnection.Tick(now); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); - - // Tick after grace period w/ low data rate - now += TimeSpan.FromSeconds(2); - _httpConnection.BytesRead(10); - _httpConnection.Tick(now); - - // Timed out - Assert.True(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); - } - - [Fact] - public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() - { - var bytesPerSecond = 100; - var gracePeriod = TimeSpan.FromSeconds(2); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinDataRate(bytesPerSecond: bytesPerSecond, gracePeriod: gracePeriod); - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - - // Initialize timestamp - var now = DateTimeOffset.UtcNow; - _httpConnection.Tick(now); - - _httpConnection.StartTimingReads(); - - // Set base data rate to 200 bytes/second - now += gracePeriod; - _httpConnection.BytesRead(400); - _httpConnection.Tick(now); - - // Data rate: 200 bytes/second - now += TimeSpan.FromSeconds(1); - _httpConnection.BytesRead(200); - _httpConnection.Tick(now); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); - - // Data rate: 150 bytes/second - now += TimeSpan.FromSeconds(1); - _httpConnection.BytesRead(0); - _httpConnection.Tick(now); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); - - // Data rate: 120 bytes/second - now += TimeSpan.FromSeconds(1); - _httpConnection.BytesRead(0); - _httpConnection.Tick(now); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); - - // Data rate: 100 bytes/second - now += TimeSpan.FromSeconds(1); - _httpConnection.BytesRead(0); - _httpConnection.Tick(now); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); - - // Data rate: ~85 bytes/second - now += TimeSpan.FromSeconds(1); - _httpConnection.BytesRead(0); - _httpConnection.Tick(now); - - // Timed out - Assert.True(_httpConnection.RequestTimedOut); - mockLogger.Verify(logger => - logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); - } - - [Fact] - public void RequestBodyDataRateNotComputedOnPausedTime() - { - var systemClock = new MockSystemClock(); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); - _httpConnectionContext.ServiceContext.SystemClock = systemClock; - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - - // Initialize timestamp - _httpConnection.Tick(systemClock.UtcNow); - - _httpConnection.StartTimingReads(); - - // Tick at 3s, expected counted time is 3s, expected data rate is 200 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(3); - _httpConnection.BytesRead(600); - _httpConnection.Tick(systemClock.UtcNow); - - // Pause at 3.5s - systemClock.UtcNow += TimeSpan.FromSeconds(0.5); - _httpConnection.PauseTimingReads(); - - // 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); - _httpConnection.Tick(systemClock.UtcNow); - - // Tick at 6s, expected counted time is 4s, expected data rate is 150 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(2); - _httpConnection.Tick(systemClock.UtcNow); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify( - logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - - // Resume at 6.5s - systemClock.UtcNow += TimeSpan.FromSeconds(0.5); - _httpConnection.ResumeTimingReads(); - - // Tick at 9s, expected counted time is 6s, expected data rate is 100 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(1.5); - _httpConnection.Tick(systemClock.UtcNow); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify( - logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - - // Tick at 10s, expected counted time is 7s, expected data rate drops below 100 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(1); - _httpConnection.Tick(systemClock.UtcNow); - - // Timed out - Assert.True(_httpConnection.RequestTimedOut); - mockLogger.Verify( - logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), - Times.Once); - } - - [Fact] - public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() - { - var systemClock = new MockSystemClock(); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); - _httpConnectionContext.ServiceContext.SystemClock = systemClock; - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - - // Initialize timestamp - _httpConnection.Tick(systemClock.UtcNow); - - _httpConnection.StartTimingReads(); - - // Tick at 2s, expected counted time is 2s, expected data rate is 100 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(2); - _httpConnection.BytesRead(200); - _httpConnection.Tick(systemClock.UtcNow); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify( - logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - - // Pause at 2.25s - systemClock.UtcNow += TimeSpan.FromSeconds(0.25); - _httpConnection.PauseTimingReads(); - - // Resume at 2.5s - systemClock.UtcNow += TimeSpan.FromSeconds(0.25); - _httpConnection.ResumeTimingReads(); - - // Tick at 3s, expected counted time is 3s, expected data rate is 100 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(0.5); - _httpConnection.BytesRead(100); - _httpConnection.Tick(systemClock.UtcNow); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - mockLogger.Verify( - logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - - // Tick at 4s, expected counted time is 4s, expected data rate drops below 100 bytes/second - systemClock.UtcNow += TimeSpan.FromSeconds(1); - _httpConnection.Tick(systemClock.UtcNow); - - // Timed out - Assert.True(_httpConnection.RequestTimedOut); - mockLogger.Verify( - logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), - Times.Once); - } - - [Fact] - public void ReadTimingNotEnforcedWhenTimeoutIsSet() - { - var systemClock = new MockSystemClock(); - var timeout = TimeSpan.FromSeconds(5); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinRequestBodyDataRate = - new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); - _httpConnectionContext.ServiceContext.SystemClock = systemClock; - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - - var startTime = systemClock.UtcNow; - - // Initialize timestamp - _httpConnection.Tick(startTime); - - _httpConnection.StartTimingReads(); - - _httpConnection.SetTimeout(timeout.Ticks, TimeoutAction.StopProcessingNextRequest); - - // Tick beyond grace period with low data rate - systemClock.UtcNow += TimeSpan.FromSeconds(3); - _httpConnection.BytesRead(1); - _httpConnection.Tick(systemClock.UtcNow); - - // Not timed out - Assert.False(_httpConnection.RequestTimedOut); - - // Tick just past timeout period, adjusted by Heartbeat.Interval - systemClock.UtcNow = startTime + timeout + Heartbeat.Interval + TimeSpan.FromTicks(1); - _httpConnection.Tick(systemClock.UtcNow); - - // Timed out - Assert.True(_httpConnection.RequestTimedOut); - } - - [Fact] - public async Task WriteTimingAbortsConnectionWhenWriteDoesNotCompleteWithMinimumDataRate() - { - var systemClock = new MockSystemClock(); var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = - new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); - _httpConnectionContext.ServiceContext.SystemClock = systemClock; - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - _httpConnection.Http1Connection.RequestAborted.Register(() => + httpConnection.Initialize(httpConnectionContext.Transport); + httpConnection.Http1Connection.Reset(); + httpConnection.Http1Connection.RequestAborted.Register(() => { aborted.SetResult(null); }); - // Initialize timestamp - _httpConnection.Tick(systemClock.UtcNow); + httpConnection.OnTimeout(TimeoutReason.WriteDataRate); - // Should complete within 4 seconds, but the timeout is adjusted by adding Heartbeat.Interval - _httpConnection.StartTimingWrite(400); + mockConnectionContext + .Verify(c => c.Abort(It.Is(ex => ex.Message == CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)), + Times.Once); - // Tick just past 4s plus Heartbeat.Interval - systemClock.UtcNow += TimeSpan.FromSeconds(4) + Heartbeat.Interval + TimeSpan.FromTicks(1); - _httpConnection.Tick(systemClock.UtcNow); - - Assert.True(_httpConnection.RequestTimedOut); await aborted.Task.DefaultTimeout(); } - - [Fact] - public async Task WriteTimingAbortsConnectionWhenSmallWriteDoesNotCompleteWithinGracePeriod() - { - var systemClock = new MockSystemClock(); - var minResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5)); - var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = minResponseDataRate; - _httpConnectionContext.ServiceContext.SystemClock = systemClock; - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - _httpConnection.Http1Connection.RequestAborted.Register(() => - { - aborted.SetResult(null); - }); - - // Initialize timestamp - var startTime = systemClock.UtcNow; - _httpConnection.Tick(startTime); - - // Should complete within 1 second, but the timeout is adjusted by adding Heartbeat.Interval - _httpConnection.StartTimingWrite(100); - - // Tick just past 1s plus Heartbeat.Interval - systemClock.UtcNow += TimeSpan.FromSeconds(1) + Heartbeat.Interval + TimeSpan.FromTicks(1); - _httpConnection.Tick(systemClock.UtcNow); - - // Still within grace period, not timed out - Assert.False(_httpConnection.RequestTimedOut); - - // Tick just past grace period (adjusted by Heartbeat.Interval) - systemClock.UtcNow = startTime + minResponseDataRate.GracePeriod + Heartbeat.Interval + TimeSpan.FromTicks(1); - _httpConnection.Tick(systemClock.UtcNow); - - Assert.True(_httpConnection.RequestTimedOut); - await aborted.Task.DefaultTimeout(); - } - - [Fact] - public async Task WriteTimingTimeoutPushedOnConcurrentWrite() - { - var systemClock = new MockSystemClock(); - var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = - new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); - _httpConnectionContext.ServiceContext.SystemClock = systemClock; - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - _httpConnection.Http1Connection.RequestAborted.Register(() => - { - aborted.SetResult(null); - }); - - // Initialize timestamp - _httpConnection.Tick(systemClock.UtcNow); - - // Should complete within 5 seconds, but the timeout is adjusted by adding Heartbeat.Interval - _httpConnection.StartTimingWrite(500); - - // Start a concurrent write after 3 seconds, which should complete within 3 seconds (adjusted by Heartbeat.Interval) - _httpConnection.StartTimingWrite(300); - - // Tick just past 5s plus Heartbeat.Interval, when the first write should have completed - systemClock.UtcNow += TimeSpan.FromSeconds(5) + Heartbeat.Interval + TimeSpan.FromTicks(1); - _httpConnection.Tick(systemClock.UtcNow); - - // Not timed out because the timeout was pushed by the second write - Assert.False(_httpConnection.RequestTimedOut); - - // Complete the first write, this should have no effect on the timeout - _httpConnection.StopTimingWrite(); - - // Tick just past +3s, when the second write should have completed - systemClock.UtcNow += TimeSpan.FromSeconds(3) + TimeSpan.FromTicks(1); - _httpConnection.Tick(systemClock.UtcNow); - - Assert.True(_httpConnection.RequestTimedOut); - await aborted.Task.DefaultTimeout(); - } - - [Fact] - public async Task WriteTimingAbortsConnectionWhenRepeatedSmallWritesDoNotCompleteWithMinimumDataRate() - { - var systemClock = new MockSystemClock(); - var minResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5)); - var numWrites = 5; - var writeSize = 100; - var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - - _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = minResponseDataRate; - _httpConnectionContext.ServiceContext.SystemClock = systemClock; - - var mockLogger = new Mock(); - _httpConnectionContext.ServiceContext.Log = mockLogger.Object; - - _httpConnection.Initialize(_httpConnectionContext.Transport); - _httpConnection.Http1Connection.Reset(); - _httpConnection.Http1Connection.RequestAborted.Register(() => - { - aborted.SetResult(null); - }); - - // Initialize timestamp - var startTime = systemClock.UtcNow; - _httpConnection.Tick(startTime); - - // 5 consecutive 100 byte writes. - for (var i = 0; i < numWrites - 1; i++) - { - _httpConnection.StartTimingWrite(writeSize); - _httpConnection.StopTimingWrite(); - } - - // Stall the last write. - _httpConnection.StartTimingWrite(writeSize); - - // Move the clock forward Heartbeat.Interval + MinDataRate.GracePeriod + 4 seconds. - // The grace period should only be added for the first write. The subsequent 4 100 byte writes should add 1 second each to the timeout given the 100 byte/s min rate. - systemClock.UtcNow += Heartbeat.Interval + minResponseDataRate.GracePeriod + TimeSpan.FromSeconds((numWrites - 1) * writeSize / minResponseDataRate.BytesPerSecond); - _httpConnection.Tick(systemClock.UtcNow); - - Assert.False(_httpConnection.RequestTimedOut); - - // On more tick forward triggers the timeout. - systemClock.UtcNow += TimeSpan.FromTicks(1); - _httpConnection.Tick(systemClock.UtcNow); - - Assert.True(_httpConnection.RequestTimedOut); - await aborted.Task.TimeoutAfter(TimeSpan.FromSeconds(10)); - } } } diff --git a/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs b/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs index 6ac87ba1a5..5f868bdeea 100644 --- a/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs +++ b/test/Kestrel.Core.Tests/HttpProtocolFeatureCollectionTests.cs @@ -23,9 +23,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests private readonly IDuplexPipe _application; private readonly TestHttp1Connection _http1Connection; private readonly ServiceContext _serviceContext; - private readonly Http1ConnectionContext _http1ConnectionContext; + private readonly HttpConnectionContext _http1ConnectionContext; private readonly MemoryPool _memoryPool; - private Mock _timeoutControl; private readonly IFeatureCollection _collection; @@ -39,13 +38,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _application = pair.Application; _serviceContext = new TestServiceContext(); - _timeoutControl = new Mock(); - _http1ConnectionContext = new Http1ConnectionContext + _http1ConnectionContext = new HttpConnectionContext { ServiceContext = _serviceContext, ConnectionFeatures = new FeatureCollection(), MemoryPool = _memoryPool, - TimeoutControl = _timeoutControl.Object, + TimeoutControl = Mock.Of(), Transport = pair.Transport }; diff --git a/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs b/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs index eddc7701fd..4aad2e3561 100644 --- a/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs +++ b/test/Kestrel.Core.Tests/HttpResponseHeadersTests.cs @@ -2,12 +2,12 @@ // 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.Collections.Generic; using System.Globalization; using System.IO.Pipelines; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; @@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { var options = new PipeOptions(memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); - var http1ConnectionContext = new Http1ConnectionContext + var http1ConnectionContext = new HttpConnectionContext { ServiceContext = new TestServiceContext(), ConnectionFeatures = new FeatureCollection(), diff --git a/test/Kestrel.Core.Tests/MessageBodyTests.cs b/test/Kestrel.Core.Tests/MessageBodyTests.cs index f52d8622e1..620f140b77 100644 --- a/test/Kestrel.Core.Tests/MessageBodyTests.cs +++ b/test/Kestrel.Core.Tests/MessageBodyTests.cs @@ -10,6 +10,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; @@ -422,7 +423,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var options = new PipeOptions(pool: memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); var transport = pair.Transport; - var http1ConnectionContext = new Http1ConnectionContext + var http1ConnectionContext = new HttpConnectionContext { ServiceContext = new TestServiceContext(), ConnectionFeatures = new FeatureCollection(), @@ -747,9 +748,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests .Callback(() => produceContinueCalled = true); input.Http1Connection.HttpResponseControl = mockHttpResponseControl.Object; + var minReadRate = input.Http1Connection.MinRequestBodyDataRate; var mockTimeoutControl = new Mock(); mockTimeoutControl - .Setup(timeoutControl => timeoutControl.StartTimingReads()) + .Setup(timeoutControl => timeoutControl.StartTimingReads(minReadRate)) .Callback(() => startTimingReadsCalledAfterProduceContinue = produceContinueCalled); input.Http1ConnectionContext.TimeoutControl = mockTimeoutControl.Object; @@ -773,6 +775,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { using (var input = new TestInput()) { + var minReadRate = input.Http1Connection.MinRequestBodyDataRate; var mockTimeoutControl = new Mock(); input.Http1ConnectionContext.TimeoutControl = mockTimeoutControl.Object; @@ -786,7 +789,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(0, await body.ReadAsync(new ArraySegment(new byte[1]))); - mockTimeoutControl.Verify(timeoutControl => timeoutControl.StartTimingReads(), Times.Never); + mockTimeoutControl.Verify(timeoutControl => timeoutControl.StartTimingReads(minReadRate), Times.Never); mockTimeoutControl.Verify(timeoutControl => timeoutControl.StopTimingReads(), Times.Never); // Due to the limits set on HttpProtocol.RequestBodyPipe, backpressure should be triggered on every diff --git a/test/Kestrel.Core.Tests/OutputProducerTests.cs b/test/Kestrel.Core.Tests/OutputProducerTests.cs index 22102de0be..5c8fab6d69 100644 --- a/test/Kestrel.Core.Tests/OutputProducerTests.cs +++ b/test/Kestrel.Core.Tests/OutputProducerTests.cs @@ -4,10 +4,9 @@ using System; using System.Buffers; using System.IO.Pipelines; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; -using Microsoft.AspNetCore.Connections.Features; +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.Internal; @@ -96,7 +95,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests connectionContext, serviceContext.Log, Mock.Of(), - Mock.Of()); + Mock.Of()); return socketOutput; } diff --git a/test/Kestrel.Core.Tests/TestHelpers/TestInput.cs b/test/Kestrel.Core.Tests/TestHelpers/TestInput.cs index 78e28dcbc2..81f046b10d 100644 --- a/test/Kestrel.Core.Tests/TestHelpers/TestInput.cs +++ b/test/Kestrel.Core.Tests/TestHelpers/TestInput.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; @@ -31,9 +32,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var connectionFeatures = new FeatureCollection(); connectionFeatures.Set(Mock.Of()); - connectionFeatures.Set(Mock.Of()); - Http1ConnectionContext = new Http1ConnectionContext + Http1ConnectionContext = new HttpConnectionContext { ServiceContext = new TestServiceContext(), ConnectionContext = Mock.Of(), @@ -45,13 +45,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Http1Connection = new Http1Connection(Http1ConnectionContext); Http1Connection.HttpResponseControl = Mock.Of(); + Http1Connection.Reset(); } public IDuplexPipe Transport { get; } public IDuplexPipe Application { get; } - public Http1ConnectionContext Http1ConnectionContext { get; } + public HttpConnectionContext Http1ConnectionContext { get; } public Http1Connection Http1Connection { get; set; } diff --git a/test/Kestrel.Core.Tests/TimeoutControlTests.cs b/test/Kestrel.Core.Tests/TimeoutControlTests.cs new file mode 100644 index 0000000000..6468464af8 --- /dev/null +++ b/test/Kestrel.Core.Tests/TimeoutControlTests.cs @@ -0,0 +1,405 @@ +// 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 System; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Testing; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class TimeoutControlTests + { + private readonly Mock _mockTimeoutHandler; + private readonly TimeoutControl _timeoutControl; + + public TimeoutControlTests() + { + _mockTimeoutHandler = new Mock(); + _timeoutControl = new TimeoutControl(_mockTimeoutHandler.Object); + } + + [Fact] + public void DoesNotTimeOutWhenDebuggerIsAttached() + { + var mockDebugger = new Mock(); + mockDebugger.SetupGet(g => g.IsAttached).Returns(true); + _timeoutControl.Debugger = mockDebugger.Object; + + var now = DateTimeOffset.Now; + _timeoutControl.Tick(now); + _timeoutControl.SetTimeout(1, TimeoutReason.RequestHeaders); + _timeoutControl.Tick(now.AddTicks(2).Add(Heartbeat.Interval)); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + } + + + [Fact] + public void DoesNotTimeOutWhenRequestBodyDoesNotSatisfyMinimumDataRateButDebuggerIsAttached() + { + var mockDebugger = new Mock(); + mockDebugger.SetupGet(g => g.IsAttached).Returns(true); + _timeoutControl.Debugger = mockDebugger.Object; + + TickBodyWithMinimumDataRate(bytesPerSecond: 100); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + } + + [Fact] + public void TimesOutWhenRequestBodyDoesNotSatisfyMinimumDataRate() + { + TickBodyWithMinimumDataRate(bytesPerSecond: 100); + + // Timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Once); + } + + [Fact] + public void RequestBodyMinimumDataRateNotEnforcedDuringGracePeriod() + { + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); + + // Initialize timestamp + var now = DateTimeOffset.UtcNow; + _timeoutControl.Tick(now); + + _timeoutControl.StartTimingReads(minRate); + + // Tick during grace period w/ low data rate + now += TimeSpan.FromSeconds(1); + _timeoutControl.BytesRead(10); + _timeoutControl.Tick(now); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Tick after grace period w/ low data rate + now += TimeSpan.FromSeconds(2); + _timeoutControl.BytesRead(10); + _timeoutControl.Tick(now); + + // Timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + } + + [Fact] + public void RequestBodyDataRateIsAveragedOverTimeSpentReadingRequestBody() + { + var gracePeriod = TimeSpan.FromSeconds(2); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: gracePeriod); + + // Initialize timestamp + var now = DateTimeOffset.UtcNow; + _timeoutControl.Tick(now); + + _timeoutControl.StartTimingReads(minRate); + + // Set base data rate to 200 bytes/second + now += gracePeriod; + _timeoutControl.BytesRead(400); + _timeoutControl.Tick(now); + + // Data rate: 200 bytes/second + now += TimeSpan.FromSeconds(1); + _timeoutControl.BytesRead(200); + _timeoutControl.Tick(now); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Data rate: 150 bytes/second + now += TimeSpan.FromSeconds(1); + _timeoutControl.BytesRead(0); + _timeoutControl.Tick(now); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Data rate: 120 bytes/second + now += TimeSpan.FromSeconds(1); + _timeoutControl.BytesRead(0); + _timeoutControl.Tick(now); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Data rate: 100 bytes/second + now += TimeSpan.FromSeconds(1); + _timeoutControl.BytesRead(0); + _timeoutControl.Tick(now); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Data rate: ~85 bytes/second + now += TimeSpan.FromSeconds(1); + _timeoutControl.BytesRead(0); + _timeoutControl.Tick(now); + + // Timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + } + + + [Fact] + public void RequestBodyDataRateNotComputedOnPausedTime() + { + var systemClock = new MockSystemClock(); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); + + // Initialize timestamp + _timeoutControl.Tick(systemClock.UtcNow); + + _timeoutControl.StartTimingReads(minRate); + + // Tick at 3s, expected counted time is 3s, expected data rate is 200 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(3); + _timeoutControl.BytesRead(600); + _timeoutControl.Tick(systemClock.UtcNow); + + // Pause at 3.5s + systemClock.UtcNow += TimeSpan.FromSeconds(0.5); + _timeoutControl.PauseTimingReads(); + + // 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); + _timeoutControl.Tick(systemClock.UtcNow); + + // Tick at 6s, expected counted time is 4s, expected data rate is 150 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(2); + _timeoutControl.Tick(systemClock.UtcNow); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Resume at 6.5s + systemClock.UtcNow += TimeSpan.FromSeconds(0.5); + _timeoutControl.ResumeTimingReads(); + + // Tick at 9s, expected counted time is 6s, expected data rate is 100 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(1.5); + _timeoutControl.Tick(systemClock.UtcNow); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Tick at 10s, expected counted time is 7s, expected data rate drops below 100 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(1); + _timeoutControl.Tick(systemClock.UtcNow); + + // Timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + } + + [Fact] + public void ReadTimingNotPausedWhenResumeCalledBeforeNextTick() + { + var systemClock = new MockSystemClock(); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); + + // Initialize timestamp + _timeoutControl.Tick(systemClock.UtcNow); + + _timeoutControl.StartTimingReads(minRate); + + // Tick at 2s, expected counted time is 2s, expected data rate is 100 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(2); + _timeoutControl.BytesRead(200); + _timeoutControl.Tick(systemClock.UtcNow); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Pause at 2.25s + systemClock.UtcNow += TimeSpan.FromSeconds(0.25); + _timeoutControl.PauseTimingReads(); + + // Resume at 2.5s + systemClock.UtcNow += TimeSpan.FromSeconds(0.25); + _timeoutControl.ResumeTimingReads(); + + // Tick at 3s, expected counted time is 3s, expected data rate is 100 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(0.5); + _timeoutControl.BytesRead(100); + _timeoutControl.Tick(systemClock.UtcNow); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Tick at 4s, expected counted time is 4s, expected data rate drops below 100 bytes/second + systemClock.UtcNow += TimeSpan.FromSeconds(1); + _timeoutControl.Tick(systemClock.UtcNow); + + // Timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.ReadDataRate), Times.Once); + } + + [Fact] + public void ReadTimingNotEnforcedWhenTimeoutIsSet() + { + var systemClock = new MockSystemClock(); + var timeout = TimeSpan.FromSeconds(5); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); + + var startTime = systemClock.UtcNow; + + // Initialize timestamp + _timeoutControl.Tick(startTime); + + _timeoutControl.StartTimingReads(minRate); + + _timeoutControl.SetTimeout(timeout.Ticks, TimeoutReason.RequestBodyDrain); + + // Tick beyond grace period with low data rate + systemClock.UtcNow += TimeSpan.FromSeconds(3); + _timeoutControl.BytesRead(1); + _timeoutControl.Tick(systemClock.UtcNow); + + // Not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Tick just past timeout period, adjusted by Heartbeat.Interval + systemClock.UtcNow = startTime + timeout + Heartbeat.Interval + TimeSpan.FromTicks(1); + _timeoutControl.Tick(systemClock.UtcNow); + + // Timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.RequestBodyDrain), Times.Once); + } + + [Fact] + public void WriteTimingAbortsConnectionWhenWriteDoesNotCompleteWithMinimumDataRate() + { + var systemClock = new MockSystemClock(); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); + + // Initialize timestamp + _timeoutControl.Tick(systemClock.UtcNow); + + // Should complete within 4 seconds, but the timeout is adjusted by adding Heartbeat.Interval + _timeoutControl.StartTimingWrite(minRate, 400); + + // Tick just past 4s plus Heartbeat.Interval + systemClock.UtcNow += TimeSpan.FromSeconds(4) + Heartbeat.Interval + TimeSpan.FromTicks(1); + _timeoutControl.Tick(systemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.WriteDataRate), Times.Once); + } + + [Fact] + public void WriteTimingAbortsConnectionWhenSmallWriteDoesNotCompleteWithinGracePeriod() + { + var systemClock = new MockSystemClock(); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5)); + + // Initialize timestamp + var startTime = systemClock.UtcNow; + _timeoutControl.Tick(startTime); + + // Should complete within 1 second, but the timeout is adjusted by adding Heartbeat.Interval + _timeoutControl.StartTimingWrite(minRate, 100); + + // Tick just past 1s plus Heartbeat.Interval + systemClock.UtcNow += TimeSpan.FromSeconds(1) + Heartbeat.Interval + TimeSpan.FromTicks(1); + _timeoutControl.Tick(systemClock.UtcNow); + + // Still within grace period, not timed out + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Tick just past grace period (adjusted by Heartbeat.Interval) + systemClock.UtcNow = startTime + minRate.GracePeriod + Heartbeat.Interval + TimeSpan.FromTicks(1); + _timeoutControl.Tick(systemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.WriteDataRate), Times.Once); + } + + + [Fact] + public void WriteTimingTimeoutPushedOnConcurrentWrite() + { + var systemClock = new MockSystemClock(); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); + + // Initialize timestamp + _timeoutControl.Tick(systemClock.UtcNow); + + // Should complete within 5 seconds, but the timeout is adjusted by adding Heartbeat.Interval + _timeoutControl.StartTimingWrite(minRate, 500); + + // Start a concurrent write after 3 seconds, which should complete within 3 seconds (adjusted by Heartbeat.Interval) + _timeoutControl.StartTimingWrite(minRate, 300); + + // Tick just past 5s plus Heartbeat.Interval, when the first write should have completed + systemClock.UtcNow += TimeSpan.FromSeconds(5) + Heartbeat.Interval + TimeSpan.FromTicks(1); + _timeoutControl.Tick(systemClock.UtcNow); + + // Not timed out because the timeout was pushed by the second write + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // Complete the first write, this should have no effect on the timeout + _timeoutControl.StopTimingWrite(); + + // Tick just past +3s, when the second write should have completed + systemClock.UtcNow += TimeSpan.FromSeconds(3) + TimeSpan.FromTicks(1); + _timeoutControl.Tick(systemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.WriteDataRate), Times.Once); + } + + [Fact] + public void WriteTimingAbortsConnectionWhenRepeatedSmallWritesDoNotCompleteWithMinimumDataRate() + { + var systemClock = new MockSystemClock(); + var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5)); + var numWrites = 5; + var writeSize = 100; + + // Initialize timestamp + var startTime = systemClock.UtcNow; + _timeoutControl.Tick(startTime); + + // 5 consecutive 100 byte writes. + for (var i = 0; i < numWrites - 1; i++) + { + _timeoutControl.StartTimingWrite(minRate, writeSize); + _timeoutControl.StopTimingWrite(); + } + + // Stall the last write. + _timeoutControl.StartTimingWrite(minRate, writeSize); + + // Move the clock forward Heartbeat.Interval + MinDataRate.GracePeriod + 4 seconds. + // The grace period should only be added for the first write. The subsequent 4 100 byte writes should add 1 second each to the timeout given the 100 byte/s min rate. + systemClock.UtcNow += Heartbeat.Interval + minRate.GracePeriod + TimeSpan.FromSeconds((numWrites - 1) * writeSize / minRate.BytesPerSecond); + _timeoutControl.Tick(systemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(It.IsAny()), Times.Never); + + // On more tick forward triggers the timeout. + systemClock.UtcNow += TimeSpan.FromTicks(1); + _timeoutControl.Tick(systemClock.UtcNow); + + _mockTimeoutHandler.Verify(h => h.OnTimeout(TimeoutReason.WriteDataRate), Times.Once); + } + + private void TickBodyWithMinimumDataRate(int bytesPerSecond) + { + var gracePeriod = TimeSpan.FromSeconds(5); + + var minRate = new MinDataRate(bytesPerSecond, gracePeriod); + + // Initialize timestamp + var now = DateTimeOffset.UtcNow; + _timeoutControl.Tick(now); + + _timeoutControl.StartTimingReads(minRate); + + // Tick after grace period w/ low data rate + now += gracePeriod + TimeSpan.FromSeconds(1); + _timeoutControl.BytesRead(1); + _timeoutControl.Tick(now); + } + } +} diff --git a/test/Kestrel.InMemory.FunctionalTests/ConnectionAdapterTests.cs b/test/Kestrel.InMemory.FunctionalTests/ConnectionAdapterTests.cs index d5b8f4e761..bcc540d727 100644 --- a/test/Kestrel.InMemory.FunctionalTests/ConnectionAdapterTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/ConnectionAdapterTests.cs @@ -16,7 +16,7 @@ using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { - public class ConnectionAdapterTests : LoggedTest + public class ConnectionAdapterTests : TestApplicationErrorLoggerLoggedTest { [Fact] public async Task CanReadAndWriteWithRewritingConnectionAdapter() @@ -164,6 +164,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests await connection.WaitForConnectionClose(); } } + + Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Message.Contains($"Uncaught exception from the {nameof(IConnectionAdapter.OnConnectionAsync)} method of an {nameof(IConnectionAdapter)}.")); } [Fact] @@ -220,7 +222,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task OnConnectionAsync(ConnectionAdapterContext context) { - await Task.Delay(100); + await Task.Yield(); return new AdaptedConnection(new RewritingStream(context.ConnectionStream)); } } diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index c0ec666f62..063130c963 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -13,9 +13,11 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging; using Microsoft.Net.Http.Headers; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests @@ -2874,6 +2876,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.GOAWAY, streamId: 0, headersStreamId: 1)); } + [Fact] + public async Task GOAWAY_Received_ConnectionClosedWhenResponseNotDrainedAtMinimumDataRate() + { + var mockSystemClock = new MockSystemClock(); + var limits = _connectionContext.ServiceContext.ServerOptions.Limits; + var timeoutControl = _connectionContext.TimeoutControl; + + _timeoutControl.Initialize(mockSystemClock.UtcNow); + + await InitializeConnectionAsync(_noopApplication); + + await SendGoAwayAsync(); + + await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); + + mockSystemClock.UtcNow += + Heartbeat.Interval + + TimeSpan.FromSeconds(limits.MaxResponseBufferSize.Value * 2 / limits.MinResponseDataRate.BytesPerSecond); + _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.WriteDataRate), Times.Once); + } + [Fact] public async Task WINDOW_UPDATE_Received_StreamIdEven_ConnectionError() { @@ -3482,20 +3512,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _pair.Application.Output.Complete(new ConnectionResetException(string.Empty)); - var result = await _pair.Application.Input.ReadAsync(); - Assert.True(result.IsCompleted); + await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); Assert.DoesNotContain(TestApplicationErrorLogger.Messages, m => m.Exception is ConnectionResetException); } [Fact] - public async Task OnInputOrOutputCompletedSendsFinalGOAWAY() + public async Task OnInputOrOutputCompletedCompletesOutput() { await InitializeConnectionAsync(_noopApplication); _connection.OnInputOrOutputCompleted(); await _closedStateReached.Task.DefaultTimeout(); - VerifyGoAway(await ReceiveFrameAsync(), 0, Http2ErrorCode.NO_ERROR); + var result = await _pair.Application.Input.ReadAsync().AsTask().DefaultTimeout(); + Assert.True(result.IsCompleted); + Assert.True(result.Buffer.IsEmpty); } [Fact] @@ -3655,7 +3686,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await StartStreamAsync(3, _browserRequestHeaders, endStream: false); - await WaitForConnectionStopAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + var result = await _pair.Application.Input.ReadAsync().AsTask().DefaultTimeout(); + Assert.True(result.IsCompleted); + Assert.True(result.Buffer.IsEmpty); } [Fact] diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs index d14744906a..49e82eb566 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http; 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.Http2.HPack; @@ -55,6 +56,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected readonly HPackDecoder _hpackDecoder; private readonly byte[] _headerEncodingBuffer = new byte[Http2PeerSettings.MinAllowedMaxFrameSize]; + protected readonly Mock _mockTimeoutHandler = new Mock(); + protected readonly TimeoutControl _timeoutControl; + protected readonly ConcurrentDictionary> _runningStreams = new ConcurrentDictionary>(); protected readonly Dictionary _receivedHeaders = new Dictionary(StringComparer.OrdinalIgnoreCase); protected readonly Dictionary _decodedHeaders = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -77,7 +81,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected readonly RequestDelegate _echoHost; protected readonly RequestDelegate _echoPath; - protected Http2ConnectionContext _connectionContext; + protected HttpConnectionContext _connectionContext; protected Http2Connection _connection; protected Task _connectionTask; @@ -101,6 +105,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions); _hpackDecoder = new HPackDecoder((int)_clientSettings.HeaderTableSize, MaxRequestHeaderFieldSize); + _timeoutControl = new TimeoutControl(_mockTimeoutHandler.Object); _noopApplication = context => Task.CompletedTask; @@ -282,13 +287,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests .Setup(m => m.Http2ConnectionClosed(It.IsAny(), It.IsAny())) .Callback(() => _closedStateReached.SetResult(null)); - _connectionContext = new Http2ConnectionContext + _connectionContext = new HttpConnectionContext { ConnectionContext = Mock.Of(), ConnectionFeatures = new FeatureCollection(), ServiceContext = new TestServiceContext(LoggerFactory, mockKestrelTrace.Object), MemoryPool = _memoryPool, - Transport = _pair.Transport + Transport = _pair.Transport, + TimeoutControl = _timeoutControl }; _connection = new Http2Connection(_connectionContext); diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs index e5fd714179..92d74c860f 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs @@ -100,20 +100,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) { - // When the in-memory connection is aborted, the input PipeWriter is completed behind the scenes - // so eventually connection.Send() throws an InvalidOperationException. - await Assert.ThrowsAsync(async () => + foreach (var ch in "POST / HTTP/1.1\r\nHost:\r\n\r\n") { - foreach (var ch in "POST / HTTP/1.1\r\nHost:\r\n\r\n") - { - await connection.Send(ch.ToString()); + await connection.Send(ch.ToString()); - testContext.MockSystemClock.UtcNow += ShortDelay; - heartbeatManager.OnHeartbeat(testContext.SystemClock.UtcNow); - } - }); + testContext.MockSystemClock.UtcNow += ShortDelay; + heartbeatManager.OnHeartbeat(testContext.SystemClock.UtcNow); + } await ReceiveTimeoutResponse(connection, testContext); + + await connection.WaitForConnectionClose(); } } diff --git a/test/Kestrel.InMemory.FunctionalTests/ResponseDrainingTests.cs b/test/Kestrel.InMemory.FunctionalTests/ResponseDrainingTests.cs new file mode 100644 index 0000000000..432b61c643 --- /dev/null +++ b/test/Kestrel.InMemory.FunctionalTests/ResponseDrainingTests.cs @@ -0,0 +1,80 @@ +// 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 System; +using System.Net; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Core.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class ResponseDrainingTests : TestApplicationErrorLoggerLoggedTest + { + public static TheoryData ConnectionAdapterData => new TheoryData + { + new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)), + new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + { + ConnectionAdapters = { new PassThroughConnectionAdapter() } + } + }; + + [Theory] + [MemberData(nameof(ConnectionAdapterData))] + public async Task ConnectionClosedWhenResponseNotDrainedAtMinimumDataRate(ListenOptions listenOptions) + { + var testContext = new TestServiceContext(LoggerFactory); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); + var minRate = new MinDataRate(16384, TimeSpan.FromSeconds(2)); + + using (var server = new TestServer(context => + { + context.Features.Get().MinDataRate = minRate; + return Task.CompletedTask; + }, testContext, listenOptions)) + { + using (var connection = server.CreateConnection()) + { + var transportConnection = connection.TransportConnection; + + var outputBufferedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + transportConnection.Output.OnWriterCompleted((ex, state) => + { + ((TaskCompletionSource)state).SetResult(null); + }, outputBufferedTcs); + + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "Connection: close", + "", + ""); + + // Wait for the drain timeout to be set. + await outputBufferedTcs.Task.DefaultTimeout(); + + testContext.MockSystemClock.UtcNow += + Heartbeat.Interval + + TimeSpan.FromSeconds(testContext.ServerOptions.Limits.MaxResponseBufferSize.Value * 2 / minRate.BytesPerSecond); + heartbeatManager.OnHeartbeat(testContext.SystemClock.UtcNow); + + Assert.Null(transportConnection.AbortReason); + + testContext.MockSystemClock.UtcNow += TimeSpan.FromTicks(1); + heartbeatManager.OnHeartbeat(testContext.SystemClock.UtcNow); + + Assert.NotNull(transportConnection.AbortReason); + Assert.Equal(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied, transportConnection.AbortReason.Message); + + Assert.Single(TestApplicationErrorLogger.Messages, w => w.EventId.Id == 28 && w.LogLevel == LogLevel.Information); + } + } + } + } +} diff --git a/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs b/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs index 12bddb8d31..4453d46a5e 100644 --- a/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs @@ -107,17 +107,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests serviceContext.ServerOptions.Limits.MaxResponseBufferSize = 5; var cts = new CancellationTokenSource(); var appTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var writeReturnedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var writeStartedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (var server = new TestServer(async context => { try { await context.Response.WriteAsync("hello", cts.Token).DefaultTimeout(); - writeReturnedTcs.TrySetResult(null); var task = context.Response.WriteAsync("world", cts.Token); Assert.False(task.IsCompleted); + + writeStartedTcs.TrySetResult(null); + await task.DefaultTimeout(); } catch (Exception ex) @@ -127,7 +129,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests finally { appTcs.TrySetResult(null); - writeReturnedTcs.TrySetCanceled(); + writeStartedTcs.TrySetCanceled(); } }, serviceContext)) { @@ -146,7 +148,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests "5", "hello"); - await writeReturnedTcs.Task.DefaultTimeout(); + await writeStartedTcs.Task.DefaultTimeout(); cts.Cancel(); diff --git a/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryConnection.cs b/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryConnection.cs index e88948a72e..567f78a1f9 100644 --- a/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryConnection.cs +++ b/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryConnection.cs @@ -9,31 +9,32 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans { public class InMemoryConnection : StreamBackedTestConnection { - private readonly InMemoryTransportConnection _transportConnection; public InMemoryConnection(InMemoryTransportConnection transportConnection) : base(new RawStream(transportConnection.Output, transportConnection.Input)) { - _transportConnection = transportConnection; + TransportConnection = transportConnection; } + public InMemoryTransportConnection TransportConnection { get; } + public override void Reset() { - _transportConnection.Input.Complete(new ConnectionResetException(string.Empty)); - _transportConnection.OnClosed(); + TransportConnection.Input.Complete(new ConnectionResetException(string.Empty)); + TransportConnection.OnClosed(); } public override void ShutdownSend() { - _transportConnection.Input.Complete(); - _transportConnection.OnClosed(); + TransportConnection.Input.Complete(); + TransportConnection.OnClosed(); } public override void Dispose() { - _transportConnection.Input.Complete(); - _transportConnection.Output.Complete(); - _transportConnection.OnClosed(); + TransportConnection.Input.Complete(); + TransportConnection.Output.Complete(); + TransportConnection.OnClosed(); base.Dispose(); } } diff --git a/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs b/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs index bd6297de53..37a83df22b 100644 --- a/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs +++ b/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs @@ -31,9 +31,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans public override PipeScheduler InputWriterScheduler => PipeScheduler.ThreadPool; public override PipeScheduler OutputReaderScheduler => PipeScheduler.ThreadPool; + public ConnectionAbortedException AbortReason { get; private set; } + public override void Abort(ConnectionAbortedException abortReason) { Input.Complete(abortReason); + + AbortReason = abortReason; } public void OnClosed() diff --git a/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs b/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs index 48c2cee506..523a476238 100644 --- a/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs +++ b/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; @@ -735,9 +736,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests var connectionFeatures = new FeatureCollection(); connectionFeatures.Set(Mock.Of()); - connectionFeatures.Set(Mock.Of()); - var http1Connection = new Http1Connection(new Http1ConnectionContext + var http1Connection = new Http1Connection(new HttpConnectionContext { ServiceContext = serviceContext, ConnectionContext = Mock.Of(), diff --git a/test/shared/TestHttp1Connection.cs b/test/shared/TestHttp1Connection.cs index f53a2a52db..092160e11e 100644 --- a/test/shared/TestHttp1Connection.cs +++ b/test/shared/TestHttp1Connection.cs @@ -2,13 +2,14 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; namespace Microsoft.AspNetCore.Testing { public class TestHttp1Connection : Http1Connection { - public TestHttp1Connection(Http1ConnectionContext context) + public TestHttp1Connection(HttpConnectionContext context) : base(context) { } diff --git a/tools/CodeGenerator/TransportConnectionFeatureCollection.cs b/tools/CodeGenerator/TransportConnectionFeatureCollection.cs index 350d853fe8..251af12843 100644 --- a/tools/CodeGenerator/TransportConnectionFeatureCollection.cs +++ b/tools/CodeGenerator/TransportConnectionFeatureCollection.cs @@ -20,8 +20,7 @@ namespace CodeGenerator "ITransportSchedulerFeature", "IConnectionLifetimeFeature", "IConnectionHeartbeatFeature", - "IConnectionLifetimeNotificationFeature", - "IBytesWrittenFeature", + "IConnectionLifetimeNotificationFeature" }; var usings = $@"