diff --git a/src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs b/src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs new file mode 100644 index 0000000000..7828e4dd04 --- /dev/null +++ b/src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs @@ -0,0 +1,14 @@ +// 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.Threading; + +namespace Microsoft.AspNetCore.Connections.Features +{ + public interface IConnectionLifetimeNotificationFeature + { + CancellationToken ConnectionClosedRequested { get; set; } + + void RequestClose(); + } +} diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index 6d72c97ca4..c40ba8d917 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -575,4 +575,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l The frame is too short to contain the fields indicated by the given flags. + + A value between {min} and {max} is required. + \ No newline at end of file diff --git a/src/Kestrel.Core/Http2Limits.cs b/src/Kestrel.Core/Http2Limits.cs index 34a4ef36f1..f4b05c4abb 100644 --- a/src/Kestrel.Core/Http2Limits.cs +++ b/src/Kestrel.Core/Http2Limits.cs @@ -11,6 +11,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core public class Http2Limits { private int _maxStreamsPerConnection = 100; + private int _headerTableSize = MaxAllowedHeaderTableSize; + private int _maxFrameSize = MinAllowedMaxFrameSize; + + // These are limits defined by the RFC https://tools.ietf.org/html/rfc7540#section-4.2 + public const int MaxAllowedHeaderTableSize = 4096; + public const int MinAllowedMaxFrameSize = 16 * 1024; + public const int MaxAllowedMaxFrameSize = 16 * 1024 * 1024 - 1; /// /// Limits the number of concurrent request streams per HTTP/2 connection. Excess streams will be refused. @@ -27,8 +34,49 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core { throw new ArgumentOutOfRangeException(nameof(value), value, CoreStrings.GreaterThanZeroRequired); } + _maxStreamsPerConnection = value; } } + + /// + /// Limits the size of the header compression table, in octets, the HPACK decoder on the server can use. + /// + /// Defaults to 4096 + /// + /// + public int HeaderTableSize + { + get => _headerTableSize; + set + { + if (value <= 0 || value > MaxAllowedHeaderTableSize) + { + throw new ArgumentOutOfRangeException(nameof(value), value, CoreStrings.FormatArgumentOutOfRange(0, MaxAllowedHeaderTableSize)); + } + + _headerTableSize = value; + } + } + + /// + /// Indicates the size of the largest frame payload that is allowed to be received, in octets. The size must be between 2^14 and 2^24-1. + /// + /// Defaults to 2^14 (16,384) + /// + /// + public int MaxFrameSize + { + get => _maxFrameSize; + set + { + if (value < MinAllowedMaxFrameSize || value > MaxAllowedMaxFrameSize) + { + throw new ArgumentOutOfRangeException(nameof(value), value, CoreStrings.FormatArgumentOutOfRange(MinAllowedMaxFrameSize, MaxAllowedMaxFrameSize)); + } + + _maxFrameSize = value; + } + } } } diff --git a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs index 79614ec51b..a3b47a709f 100644 --- a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs +++ b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs @@ -17,6 +17,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { public class ConnectionDispatcher : IConnectionDispatcher { + private static long _lastConnectionId = long.MinValue; + private readonly ServiceContext _serviceContext; private readonly ConnectionDelegate _connectionDelegate; @@ -44,28 +46,51 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal // This *must* be set before returning from OnConnection connection.Application = pair.Application; - return Execute(connection); + return Execute(new KestrelConnection(connection)); } - private async Task Execute(ConnectionContext connectionContext) + private async Task Execute(KestrelConnection connection) { - using (BeginConnectionScope(connectionContext)) + var id = Interlocked.Increment(ref _lastConnectionId); + var connectionContext = connection.TransportConnection; + + try { - try + _serviceContext.ConnectionManager.AddConnection(id, connection); + + Log.ConnectionStart(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStart(connectionContext); + + using (BeginConnectionScope(connectionContext)) { - await _connectionDelegate(connectionContext); - } - catch (Exception ex) - { - Log.LogCritical(0, ex, $"{nameof(ConnectionDispatcher)}.{nameof(Execute)}() {connectionContext.ConnectionId}"); - } - finally - { - // Complete the transport PipeReader and PipeWriter after calling into application code - connectionContext.Transport.Input.Complete(); - connectionContext.Transport.Output.Complete(); + try + { + await _connectionDelegate(connectionContext); + } + catch (Exception ex) + { + Log.LogCritical(0, ex, $"{nameof(ConnectionDispatcher)}.{nameof(Execute)}() {connectionContext.ConnectionId}"); + } + finally + { + // Complete the transport PipeReader and PipeWriter after calling into application code + connectionContext.Transport.Input.Complete(); + connectionContext.Transport.Output.Complete(); + } + + // Wait for the transport to close + await CancellationTokenAsTask(connectionContext.ConnectionClosed); } } + finally + { + Log.ConnectionStop(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStop(connectionContext); + + connection.Complete(); + + _serviceContext.ConnectionManager.RemoveConnection(id); + } } private IDisposable BeginConnectionScope(ConnectionContext connectionContext) @@ -78,6 +103,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal return null; } + private static Task CancellationTokenAsTask(CancellationToken token) + { + if (token.IsCancellationRequested) + { + return Task.CompletedTask; + } + + // Transports already dispatch prior to tripping ConnectionClosed + // since application code can register to this token. + var tcs = new TaskCompletionSource(); + token.Register(state => ((TaskCompletionSource)state).SetResult(null), tcs); + return tcs.Task; + } + // Internal for testing internal static PipeOptions GetInputPipeOptions(ServiceContext serviceContext, MemoryPool memoryPool, PipeScheduler writerScheduler) => new PipeOptions ( diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 3bd7aa7c29..c33674d645 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -69,7 +69,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private readonly Http2PeerSettings _serverSettings = new Http2PeerSettings(); private readonly Http2PeerSettings _clientSettings = new Http2PeerSettings(); - private readonly Http2Frame _incomingFrame = new Http2Frame(); + private readonly Http2Frame _incomingFrame; private Http2Stream _currentHeadersStream; private RequestHeaderParsingState _requestHeaderParsingState; @@ -87,8 +87,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { _context = context; _frameWriter = new Http2FrameWriter(context.Transport.Output, context.ConnectionContext, _outputFlowControl, this, context.ConnectionId, context.ServiceContext.Log); - _hpackDecoder = new HPackDecoder((int)_serverSettings.HeaderTableSize); _serverSettings.MaxConcurrentStreams = (uint)context.ServiceContext.ServerOptions.Limits.Http2.MaxStreamsPerConnection; + _serverSettings.MaxFrameSize = (uint)context.ServiceContext.ServerOptions.Limits.Http2.MaxFrameSize; + _serverSettings.HeaderTableSize = (uint)context.ServiceContext.ServerOptions.Limits.Http2.HeaderTableSize; + _hpackDecoder = new HPackDecoder((int)_serverSettings.HeaderTableSize); + _incomingFrame = new Http2Frame(_serverSettings.MaxFrameSize); } public string ConnectionId => _context.ConnectionId; @@ -601,7 +604,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - private Task ProcessSettingsFrameAsync() + private async Task ProcessSettingsFrameAsync() { if (_currentHeadersStream != null) { @@ -620,7 +623,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorSettingsAckLengthNotZero, Http2ErrorCode.FRAME_SIZE_ERROR); } - return Task.CompletedTask; + return; } if (_incomingFrame.PayloadLength % 6 != 0) @@ -632,10 +635,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { // int.MaxValue is the largest allowed windows size. var previousInitialWindowSize = (int)_clientSettings.InitialWindowSize; + var previousMaxFrameSize = _clientSettings.MaxFrameSize; _clientSettings.Update(_incomingFrame.GetSettings()); - var ackTask = _frameWriter.WriteSettingsAckAsync(); // Ack before we update the windows, they could send data immediately. + // Ack before we update the windows, they could send data immediately. + await _frameWriter.WriteSettingsAckAsync(); + + if (_clientSettings.MaxFrameSize != previousMaxFrameSize) + { + _frameWriter.UpdateMaxFrameSize(_clientSettings.MaxFrameSize); + } // This difference can be negative. var windowSizeDifference = (int)_clientSettings.InitialWindowSize - previousInitialWindowSize; @@ -653,8 +663,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } } - - return ackTask; } catch (Http2SettingsParameterOutOfRangeException ex) { diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.Continuation.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.Continuation.cs index 26f3afe92b..56712f5289 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.Continuation.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.Continuation.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public void PrepareContinuation(Http2ContinuationFrameFlags flags, int streamId) { - PayloadLength = MinAllowedMaxFrameSize - HeaderLength; + PayloadLength = (int)_maxFrameSize; Type = Http2FrameType.CONTINUATION; ContinuationFlags = flags; StreamId = streamId; diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.Data.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.Data.cs index b2df5c4bfd..3bc53971e5 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.Data.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.Data.cs @@ -42,7 +42,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { var padded = padLength != null; - PayloadLength = MinAllowedMaxFrameSize; + PayloadLength = (int)_maxFrameSize; Type = Http2FrameType.DATA; DataFlags = padded ? Http2DataFrameFlags.PADDED : Http2DataFrameFlags.NONE; StreamId = streamId; diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.Headers.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.Headers.cs index e89a2e1675..3028e27200 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.Headers.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.Headers.cs @@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public void PrepareHeaders(Http2HeadersFrameFlags flags, int streamId) { - PayloadLength = MinAllowedMaxFrameSize - HeaderLength; + PayloadLength = (int)_maxFrameSize; Type = Http2FrameType.HEADERS; HeadersFlags = flags; StreamId = streamId; diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.cs index f5f447ae7f..f9d0723bdc 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.cs @@ -18,8 +18,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 */ public partial class Http2Frame { - public const int MinAllowedMaxFrameSize = 16 * 1024; - public const int MaxAllowedMaxFrameSize = 16 * 1024 * 1024 - 1; public const int HeaderLength = 9; private const int LengthOffset = 0; @@ -28,7 +26,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private const int StreamIdOffset = 5; private const int PayloadOffset = 9; - private readonly byte[] _data = new byte[HeaderLength + MinAllowedMaxFrameSize]; + private uint _maxFrameSize; + + private readonly byte[] _data; + + public Http2Frame(uint maxFrameSize) + { + _maxFrameSize = maxFrameSize; + _data = new byte[HeaderLength + _maxFrameSize]; + } public Span Raw => new Span(_data, 0, HeaderLength + PayloadLength); diff --git a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs index 1d101d1591..35def3fc16 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs @@ -13,7 +13,6 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { @@ -22,7 +21,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 // Literal Header Field without Indexing - Indexed Name (Index 8 - :status) private static readonly byte[] _continueBytes = new byte[] { 0x08, 0x03, (byte)'1', (byte)'0', (byte)'0' }; - private readonly Http2Frame _outgoingFrame = new Http2Frame(); + private uint _maxFrameSize = Http2Limits.MinAllowedMaxFrameSize; + private Http2Frame _outgoingFrame; private readonly object _writeLock = new object(); private readonly HPackEncoder _hpackEncoder = new HPackEncoder(); private readonly PipeWriter _outputWriter; @@ -49,6 +49,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _connectionId = connectionId; _log = log; _flusher = new StreamSafePipeFlusher(_outputWriter, timeoutControl); + _outgoingFrame = new Http2Frame(_maxFrameSize); + } + + public void UpdateMaxFrameSize(uint maxFrameSize) + { + lock (_writeLock) + { + _maxFrameSize = maxFrameSize; + _outgoingFrame = new Http2Frame(maxFrameSize); + } } public void Complete() diff --git a/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs b/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs index eeb13bb808..e821daac13 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2PeerSettings.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public const bool DefaultEnablePush = true; public const uint DefaultMaxConcurrentStreams = uint.MaxValue; public const uint DefaultInitialWindowSize = 65535; - public const uint DefaultMaxFrameSize = 16384; + public const uint DefaultMaxFrameSize = Http2Limits.MinAllowedMaxFrameSize; public const uint DefaultMaxHeaderListSize = uint.MaxValue; public const uint MaxWindowSize = int.MaxValue; @@ -38,6 +38,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 switch (setting.Parameter) { case Http2SettingsParameter.SETTINGS_HEADER_TABLE_SIZE: + if (value > Http2Limits.MaxAllowedHeaderTableSize) + { + throw new Http2SettingsParameterOutOfRangeException(Http2SettingsParameter.SETTINGS_HEADER_TABLE_SIZE, + lowerBound: 0, + upperBound: Http2Limits.MaxAllowedHeaderTableSize); + } HeaderTableSize = value; break; case Http2SettingsParameter.SETTINGS_ENABLE_PUSH: @@ -64,11 +70,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 InitialWindowSize = value; break; case Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE: - if (value < Http2Frame.MinAllowedMaxFrameSize || value > Http2Frame.MaxAllowedMaxFrameSize) + if (value < Http2Limits.MinAllowedMaxFrameSize || value > Http2Limits.MaxAllowedMaxFrameSize) { throw new Http2SettingsParameterOutOfRangeException(Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE, - lowerBound: Http2Frame.MinAllowedMaxFrameSize, - upperBound: Http2Frame.MaxAllowedMaxFrameSize); + lowerBound: Http2Limits.MinAllowedMaxFrameSize, + upperBound: Http2Limits.MaxAllowedMaxFrameSize); } MaxFrameSize = value; diff --git a/src/Kestrel.Core/Internal/HttpConnection.cs b/src/Kestrel.Core/Internal/HttpConnection.cs index 28224a6ac0..d860d36ba7 100644 --- a/src/Kestrel.Core/Internal/HttpConnection.cs +++ b/src/Kestrel.Core/Internal/HttpConnection.cs @@ -11,6 +11,7 @@ using System.Net; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; @@ -28,8 +29,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal private static readonly ReadOnlyMemory Http2Id = new[] { (byte)'h', (byte)'2' }; private readonly HttpConnectionContext _context; - private readonly TaskCompletionSource _socketClosedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - private readonly TaskCompletionSource _lifetimeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly ISystemClock _systemClock; private IList _adaptedConnections; private IDuplexPipe _adaptedTransport; @@ -56,6 +56,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public HttpConnection(HttpConnectionContext context) { _context = context; + _systemClock = _context.ServiceContext.SystemClock; } // For testing @@ -100,20 +101,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { try { - // TODO: When we start tracking all connection middleware for shutdown, go back - // to logging connections tart and stop in ConnectionDispatcher so we get these - // logs for all connection middleware. - Log.ConnectionStart(ConnectionId); - KestrelEventSource.Log.ConnectionStart(this); - AdaptedPipeline adaptedPipeline = null; var adaptedPipelineTask = Task.CompletedTask; - // _adaptedTransport must be set prior to adding the connection to the manager in order - // to allow the connection to be aported prior to protocol selection. + // _adaptedTransport must be set prior to wiring up callbacks + // to allow the connection to be aborted prior to protocol selection. _adaptedTransport = _context.Transport; - if (_context.ConnectionAdapters.Count > 0) { adaptedPipeline = new AdaptedPipeline(_adaptedTransport, @@ -124,58 +118,79 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _adaptedTransport = adaptedPipeline; } - _lastTimestamp = _context.ServiceContext.SystemClock.UtcNow.Ticks; + // This feature should never be null in Kestrel + var connectionHeartbeatFeature = _context.ConnectionFeatures.Get(); - _context.ConnectionFeatures.Set(this); + Debug.Assert(connectionHeartbeatFeature != null, nameof(IConnectionHeartbeatFeature) + " is missing!"); - if (adaptedPipeline != null) + connectionHeartbeatFeature?.OnHeartbeat(state => ((HttpConnection)state).Tick(), this); + + var connectionLifetimeNotificationFeature = _context.ConnectionFeatures.Get(); + + Debug.Assert(connectionLifetimeNotificationFeature != null, nameof(IConnectionLifetimeNotificationFeature) + " is missing!"); + + using (connectionLifetimeNotificationFeature?.ConnectionClosedRequested.Register(state => ((HttpConnection)state).StopProcessingNextRequest(), this)) { - // Stream can be null here and run async will close the connection in that case - var stream = await ApplyConnectionAdaptersAsync(); - adaptedPipelineTask = adaptedPipeline.RunAsync(stream); - } + _lastTimestamp = _context.ServiceContext.SystemClock.UtcNow.Ticks; - IRequestProcessor requestProcessor = null; + _context.ConnectionFeatures.Set(this); - lock (_protocolSelectionLock) - { - // Ensure that the connection hasn't already been stopped. - if (_protocolSelectionState == ProtocolSelectionState.Initializing) + if (adaptedPipeline != null) { - switch (SelectProtocol()) - { - case HttpProtocols.Http1: - // _http1Connection must be initialized before adding the connection to the connection manager - requestProcessor = _http1Connection = CreateHttp1Connection(_adaptedTransport); - _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); - _protocolSelectionState = ProtocolSelectionState.Selected; - break; - case HttpProtocols.None: - // An error was already logged in SelectProtocol(), but we should close the connection. - Abort(ex: null); - break; - default: - // SelectProtocol() only returns Http1, Http2 or None. - throw new NotSupportedException($"{nameof(SelectProtocol)} returned something other than Http1, Http2 or None."); - } - - _requestProcessor = requestProcessor; + // Stream can be null here and run async will close the connection in that case + var stream = await ApplyConnectionAdaptersAsync(); + adaptedPipelineTask = adaptedPipeline.RunAsync(stream); } - } - if (requestProcessor != null) - { - await requestProcessor.ProcessRequestsAsync(httpApplication); - } + IRequestProcessor requestProcessor = null; - await adaptedPipelineTask; - await _socketClosedTcs.Task; + lock (_protocolSelectionLock) + { + // Ensure that the connection hasn't already been stopped. + if (_protocolSelectionState == ProtocolSelectionState.Initializing) + { + switch (SelectProtocol()) + { + case HttpProtocols.Http1: + // _http1Connection must be initialized before adding the connection to the connection manager + requestProcessor = _http1Connection = CreateHttp1Connection(_adaptedTransport); + _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); + _protocolSelectionState = ProtocolSelectionState.Selected; + break; + case HttpProtocols.None: + // An error was already logged in SelectProtocol(), but we should close the connection. + Abort(ex: null); + break; + default: + // SelectProtocol() only returns Http1, Http2 or None. + throw new NotSupportedException($"{nameof(SelectProtocol)} returned something other than Http1, Http2 or None."); + } + + _requestProcessor = requestProcessor; + } + } + + _context.Transport.Input.OnWriterCompleted( + (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), + this); + + _context.Transport.Output.OnReaderCompleted( + (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), + this); + + if (requestProcessor != null) + { + await requestProcessor.ProcessRequestsAsync(httpApplication); + } + + await adaptedPipelineTask; + } } catch (Exception ex) { @@ -189,11 +204,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { _context.ServiceContext.ConnectionManager.UpgradedConnectionCount.ReleaseOne(); } - - Log.ConnectionStop(ConnectionId); - KestrelEventSource.Log.ConnectionStop(this); - - _lifetimeTcs.SetResult(null); } } @@ -235,12 +245,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal }); } - public void OnConnectionClosed() - { - _socketClosedTcs.TrySetResult(null); - } - - public Task StopProcessingNextRequestAsync() + private void StopProcessingNextRequest() { lock (_protocolSelectionLock) { @@ -257,11 +262,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal break; } } - - return _lifetimeTcs.Task; } - public void OnInputOrOutputCompleted() + private void OnInputOrOutputCompleted() { lock (_protocolSelectionLock) { @@ -281,7 +284,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } } - public void Abort(ConnectionAbortedException ex) + private void Abort(ConnectionAbortedException ex) { lock (_protocolSelectionLock) { @@ -301,13 +304,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } } - public Task AbortAsync(ConnectionAbortedException ex) - { - Abort(ex); - - return _socketClosedTcs.Task; - } - private async Task ApplyConnectionAdaptersAsync() { var connectionAdapters = _context.ConnectionAdapters; @@ -381,6 +377,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal return http2Enabled && (!hasTls || Http2Id.Span.SequenceEqual(applicationProtocol.Span)) ? HttpProtocols.Http2 : HttpProtocols.Http1; } + private void Tick() + { + Tick(_systemClock.UtcNow); + } + public void Tick(DateTimeOffset now) { if (_protocolSelectionState == ProtocolSelectionState.Aborted) diff --git a/src/Kestrel.Core/Internal/HttpConnectionContext.cs b/src/Kestrel.Core/Internal/HttpConnectionContext.cs index da01a5a7b0..c5eceead0a 100644 --- a/src/Kestrel.Core/Internal/HttpConnectionContext.cs +++ b/src/Kestrel.Core/Internal/HttpConnectionContext.cs @@ -14,7 +14,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public class HttpConnectionContext { public string ConnectionId { get; set; } - public long HttpConnectionId { get; set; } public HttpProtocols Protocols { get; set; } public ConnectionContext ConnectionContext { get; set; } public ServiceContext ServiceContext { get; set; } diff --git a/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs b/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs index 5d05220811..83bca08997 100644 --- a/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs +++ b/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs @@ -16,8 +16,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { public class HttpConnectionMiddleware { - private static long _lastHttpConnectionId = long.MinValue; - private readonly IList _connectionAdapters; private readonly ServiceContext _serviceContext; private readonly IHttpApplication _application; @@ -33,19 +31,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _connectionAdapters = adapters; } - public async Task OnConnectionAsync(ConnectionContext connectionContext) + public Task OnConnectionAsync(ConnectionContext connectionContext) { // We need the transport feature so that we can cancel the output reader that the transport is using // This is a bit of a hack but it preserves the existing semantics var memoryPoolFeature = connectionContext.Features.Get(); - var httpConnectionId = Interlocked.Increment(ref _lastHttpConnectionId); - var httpConnectionContext = new HttpConnectionContext { ConnectionId = connectionContext.ConnectionId, ConnectionContext = connectionContext, - HttpConnectionId = httpConnectionId, Protocols = _protocols, ServiceContext = _serviceContext, ConnectionFeatures = connectionContext.Features, @@ -55,7 +50,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal }; var connectionFeature = connectionContext.Features.Get(); - var lifetimeFeature = connectionContext.Features.Get(); if (connectionFeature != null) { @@ -71,44 +65,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } var connection = new HttpConnection(httpConnectionContext); - _serviceContext.ConnectionManager.AddConnection(httpConnectionId, connection); - try - { - var processingTask = connection.ProcessRequestsAsync(_application); - - connectionContext.Transport.Input.OnWriterCompleted( - (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), - connection); - - connectionContext.Transport.Output.OnReaderCompleted( - (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), - connection); - - await CancellationTokenAsTask(lifetimeFeature.ConnectionClosed); - - connection.OnConnectionClosed(); - - await processingTask; - } - finally - { - _serviceContext.ConnectionManager.RemoveConnection(httpConnectionId); - } - } - - private static Task CancellationTokenAsTask(CancellationToken token) - { - if (token.IsCancellationRequested) - { - return Task.CompletedTask; - } - - // Transports already dispatch prior to tripping ConnectionClosed - // since application code can register to this token. - var tcs = new TaskCompletionSource(); - token.Register(() => tcs.SetResult(null)); - return tcs.Task; + return connection.ProcessRequestsAsync(_application); } } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManager.cs b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManager.cs similarity index 77% rename from src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManager.cs rename to src/Kestrel.Core/Internal/Infrastructure/ConnectionManager.cs index feb3a4cb29..bd94ce5658 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManager.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManager.cs @@ -6,17 +6,17 @@ using System.Collections.Concurrent; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public class HttpConnectionManager + public class ConnectionManager { - private readonly ConcurrentDictionary _connectionReferences = new ConcurrentDictionary(); + private readonly ConcurrentDictionary _connectionReferences = new ConcurrentDictionary(); private readonly IKestrelTrace _trace; - public HttpConnectionManager(IKestrelTrace trace, long? upgradedConnectionLimit) + public ConnectionManager(IKestrelTrace trace, long? upgradedConnectionLimit) : this(trace, GetCounter(upgradedConnectionLimit)) { } - public HttpConnectionManager(IKestrelTrace trace, ResourceCounter upgradedConnections) + public ConnectionManager(IKestrelTrace trace, ResourceCounter upgradedConnections) { UpgradedConnectionCount = upgradedConnections; _trace = trace; @@ -27,9 +27,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure /// public ResourceCounter UpgradedConnectionCount { get; } - public void AddConnection(long id, HttpConnection connection) + public void AddConnection(long id, KestrelConnection connection) { - if (!_connectionReferences.TryAdd(id, new HttpConnectionReference(connection))) + if (!_connectionReferences.TryAdd(id, new ConnectionReference(connection))) { throw new ArgumentException(nameof(id)); } @@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure } } - public void Walk(Action callback) + public void Walk(Action callback) { foreach (var kvp in _connectionReferences) { diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManagerShutdownExtensions.cs b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs similarity index 80% rename from src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManagerShutdownExtensions.cs rename to src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs index 1b601c919b..928db83c92 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManagerShutdownExtensions.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs @@ -8,29 +8,31 @@ using Microsoft.AspNetCore.Connections; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public static class HttpConnectionManagerShutdownExtensions + public static class ConnectionManagerShutdownExtensions { - public static async Task CloseAllConnectionsAsync(this HttpConnectionManager connectionManager, CancellationToken token) + public static async Task CloseAllConnectionsAsync(this ConnectionManager connectionManager, CancellationToken token) { var closeTasks = new List(); connectionManager.Walk(connection => { - closeTasks.Add(connection.StopProcessingNextRequestAsync()); + connection.TransportConnection.RequestClose(); + closeTasks.Add(connection.ExecutionTask); }); var allClosedTask = Task.WhenAll(closeTasks.ToArray()); return await Task.WhenAny(allClosedTask, CancellationTokenAsTask(token)).ConfigureAwait(false) == allClosedTask; } - public static async Task AbortAllConnectionsAsync(this HttpConnectionManager connectionManager) + public static async Task AbortAllConnectionsAsync(this ConnectionManager connectionManager) { var abortTasks = new List(); var canceledException = new ConnectionAbortedException(CoreStrings.ConnectionAbortedDuringServerShutdown); connectionManager.Walk(connection => { - abortTasks.Add(connection.AbortAsync(canceledException)); + connection.TransportConnection.Abort(canceledException); + abortTasks.Add(connection.ExecutionTask); }); var allAbortedTask = Task.WhenAll(abortTasks.ToArray()); diff --git a/src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs b/src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs new file mode 100644 index 0000000000..8525fd724e --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs @@ -0,0 +1,26 @@ +// 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.Connections; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public class ConnectionReference + { + private readonly WeakReference _weakReference; + + public ConnectionReference(KestrelConnection connection) + { + _weakReference = new WeakReference(connection); + ConnectionId = connection.TransportConnection.ConnectionId; + } + + public string ConnectionId { get; } + + public bool TryGetConnection(out KestrelConnection connection) + { + return _weakReference.TryGetTarget(out connection); + } + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpHeartbeatManager.cs b/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs similarity index 57% rename from src/Kestrel.Core/Internal/Infrastructure/HttpHeartbeatManager.cs rename to src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs index fba8e8e1a1..bc54e73385 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpHeartbeatManager.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs @@ -5,27 +5,29 @@ using System; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public class HttpHeartbeatManager : IHeartbeatHandler + public class HeartbeatManager : IHeartbeatHandler, ISystemClock { - private readonly HttpConnectionManager _connectionManager; - private readonly Action _walkCallback; + private readonly ConnectionManager _connectionManager; + private readonly Action _walkCallback; private DateTimeOffset _now; - public HttpHeartbeatManager(HttpConnectionManager connectionManager) + public HeartbeatManager(ConnectionManager connectionManager) { _connectionManager = connectionManager; _walkCallback = WalkCallback; } + public DateTimeOffset UtcNow => _now; + public void OnHeartbeat(DateTimeOffset now) { _now = now; _connectionManager.Walk(_walkCallback); } - private void WalkCallback(HttpConnection connection) + private void WalkCallback(KestrelConnection connection) { - connection.Tick(_now); + connection.TransportConnection.TickHeartbeat(); } } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.cs b/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.cs deleted file mode 100644 index 395b58a9c4..0000000000 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.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; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure -{ - public class HttpConnectionReference - { - private readonly WeakReference _weakReference; - - public HttpConnectionReference(HttpConnection connection) - { - _weakReference = new WeakReference(connection); - ConnectionId = connection.ConnectionId; - } - - public string ConnectionId { get; } - - public bool TryGetConnection(out HttpConnection connection) - { - return _weakReference.TryGetTarget(out connection); - } - } -} diff --git a/src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs b/src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs new file mode 100644 index 0000000000..f6c9d537b7 --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs @@ -0,0 +1,28 @@ +// 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.Diagnostics.Tracing; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public class KestrelConnection + { + private TaskCompletionSource _executionTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + public KestrelConnection(TransportConnection transportConnection) + { + TransportConnection = transportConnection; + ExecutionTask = _executionTcs.Task; + } + + public TransportConnection TransportConnection { get; } + + public Task ExecutionTask { get; } + + internal void Complete() => _executionTcs.TrySetResult(null); + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs b/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs index 7faf486d1f..c14138f0c8 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs @@ -2,8 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Diagnostics.Tracing; +using System.Net; using System.Runtime.CompilerServices; +using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { @@ -25,15 +28,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure // - Avoid renaming methods or parameters marked with EventAttribute. EventSource uses these to form the event object. [NonEvent] - public void ConnectionStart(HttpConnection connection) + public void ConnectionStart(TransportConnection connection) { // avoid allocating strings unless this event source is enabled if (IsEnabled()) { ConnectionStart( connection.ConnectionId, - connection.LocalEndPoint?.ToString(), - connection.RemoteEndPoint?.ToString()); + connection.LocalAddress != null ? new IPEndPoint(connection.LocalAddress, connection.LocalPort).ToString() : null, + connection.RemoteAddress != null ? new IPEndPoint(connection.RemoteAddress, connection.RemotePort).ToString() : null); } } @@ -52,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure } [NonEvent] - public void ConnectionStop(HttpConnection connection) + public void ConnectionStop(TransportConnection connection) { if (IsEnabled()) { diff --git a/src/Kestrel.Core/Internal/ServiceContext.cs b/src/Kestrel.Core/Internal/ServiceContext.cs index 1ca1beb237..270640db2a 100644 --- a/src/Kestrel.Core/Internal/ServiceContext.cs +++ b/src/Kestrel.Core/Internal/ServiceContext.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public DateHeaderValueManager DateHeaderValueManager { get; set; } - public HttpConnectionManager ConnectionManager { get; set; } + public ConnectionManager ConnectionManager { get; set; } public Heartbeat Heartbeat { get; set; } diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index a6cea3cc0f..cc21f738d0 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -67,17 +67,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core var serverOptions = options.Value ?? new KestrelServerOptions(); var logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel"); var trace = new KestrelTrace(logger); - var connectionManager = new HttpConnectionManager( + var connectionManager = new ConnectionManager( trace, serverOptions.Limits.MaxConcurrentUpgradedConnections); - var systemClock = new SystemClock(); - var dateHeaderValueManager = new DateHeaderValueManager(systemClock); - - var httpHeartbeatManager = new HttpHeartbeatManager(connectionManager); + var heartbeatManager = new HeartbeatManager(connectionManager); + var dateHeaderValueManager = new DateHeaderValueManager(heartbeatManager); var heartbeat = new Heartbeat( - new IHeartbeatHandler[] { dateHeaderValueManager, httpHeartbeatManager }, - systemClock, + new IHeartbeatHandler[] { dateHeaderValueManager, heartbeatManager }, + new SystemClock(), DebuggerWrapper.Singleton, trace); @@ -102,7 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core Log = trace, HttpParser = new HttpParser(trace.IsEnabled(LogLevel.Information)), Scheduler = scheduler, - SystemClock = systemClock, + SystemClock = heartbeatManager, DateHeaderValueManager = dateHeaderValueManager, ConnectionManager = connectionManager, Heartbeat = heartbeat, @@ -118,7 +116,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core private IKestrelTrace Trace => ServiceContext.Log; - private HttpConnectionManager ConnectionManager => ServiceContext.ConnectionManager; + private ConnectionManager ConnectionManager => ServiceContext.ConnectionManager; public async Task StartAsync(IHttpApplication application, CancellationToken cancellationToken) { diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 77e6f5c678..06945a5160 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -2142,6 +2142,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatHttp2FrameMissingFields() => GetString("Http2FrameMissingFields"); + /// + /// A value between {min} and {max} is required. + /// + internal static string ArgumentOutOfRange + { + get => GetString("ArgumentOutOfRange"); + } + + /// + /// A value between {min} and {max} is required. + /// + internal static string FormatArgumentOutOfRange(object min, object max) + => string.Format(CultureInfo.CurrentCulture, GetString("ArgumentOutOfRange", "min", "max"), min, max); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs index 63f20ad51f..f88b7189f7 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs @@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal IApplicationTransportFeature, ITransportSchedulerFeature, IConnectionLifetimeFeature, + IConnectionHeartbeatFeature, + IConnectionLifetimeNotificationFeature, IBytesWrittenFeature { // NOTE: When feature interfaces are added to or removed from this TransportConnection class implementation, @@ -85,8 +87,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal set => ConnectionClosed = value; } + CancellationToken IConnectionLifetimeNotificationFeature.ConnectionClosedRequested + { + get => ConnectionClosedRequested; + set => ConnectionClosedRequested = value; + } + void IConnectionLifetimeFeature.Abort() => Abort(abortReason: null); + void IConnectionLifetimeNotificationFeature.RequestClose() => RequestClose(); + + void IConnectionHeartbeatFeature.OnHeartbeat(System.Action action, object state) + { + 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 3a91523b9f..2dffb40067 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs @@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal private static readonly Type IApplicationTransportFeatureType = typeof(IApplicationTransportFeature); private static readonly Type ITransportSchedulerFeatureType = typeof(ITransportSchedulerFeature); 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; @@ -30,6 +32,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal private object _currentIApplicationTransportFeature; private object _currentITransportSchedulerFeature; private object _currentIConnectionLifetimeFeature; + private object _currentIConnectionHeartbeatFeature; + private object _currentIConnectionLifetimeNotificationFeature; private object _currentIBytesWrittenFeature; private int _featureRevision; @@ -46,6 +50,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal _currentIApplicationTransportFeature = this; _currentITransportSchedulerFeature = this; _currentIConnectionLifetimeFeature = this; + _currentIConnectionHeartbeatFeature = this; + _currentIConnectionLifetimeNotificationFeature = this; _currentIBytesWrittenFeature = this; } @@ -134,6 +140,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { feature = _currentIConnectionLifetimeFeature; } + else if (key == IConnectionHeartbeatFeatureType) + { + feature = _currentIConnectionHeartbeatFeature; + } + else if (key == IConnectionLifetimeNotificationFeatureType) + { + feature = _currentIConnectionLifetimeNotificationFeature; + } else if (key == IBytesWrittenFeatureType) { feature = _currentIBytesWrittenFeature; @@ -182,6 +196,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { _currentIConnectionLifetimeFeature = value; } + else if (key == IConnectionHeartbeatFeatureType) + { + _currentIConnectionHeartbeatFeature = value; + } + else if (key == IConnectionLifetimeNotificationFeatureType) + { + _currentIConnectionLifetimeNotificationFeature = value; + } else if (key == IBytesWrittenFeatureType) { _currentIBytesWrittenFeature = value; @@ -228,6 +250,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { feature = (TFeature)_currentIConnectionLifetimeFeature; } + else if (typeof(TFeature) == typeof(IConnectionHeartbeatFeature)) + { + feature = (TFeature)_currentIConnectionHeartbeatFeature; + } + else if (typeof(TFeature) == typeof(IConnectionLifetimeNotificationFeature)) + { + feature = (TFeature)_currentIConnectionLifetimeNotificationFeature; + } else if (typeof(TFeature) == typeof(IBytesWrittenFeature)) { feature = (TFeature)_currentIBytesWrittenFeature; @@ -275,6 +305,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { _currentIConnectionLifetimeFeature = feature; } + else if (typeof(TFeature) == typeof(IConnectionHeartbeatFeature)) + { + _currentIConnectionHeartbeatFeature = feature; + } + else if (typeof(TFeature) == typeof(IConnectionLifetimeNotificationFeature)) + { + _currentIConnectionLifetimeNotificationFeature = feature; + } else if (typeof(TFeature) == typeof(IBytesWrittenFeature)) { _currentIBytesWrittenFeature = feature; @@ -319,6 +357,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { yield return new KeyValuePair(IConnectionLifetimeFeatureType, _currentIConnectionLifetimeFeature); } + if (_currentIConnectionHeartbeatFeature != null) + { + yield return new KeyValuePair(IConnectionHeartbeatFeatureType, _currentIConnectionHeartbeatFeature); + } + if (_currentIConnectionLifetimeNotificationFeature != null) + { + yield return new KeyValuePair(IConnectionLifetimeNotificationFeatureType, _currentIConnectionLifetimeNotificationFeature); + } if (_currentIBytesWrittenFeature != null) { yield return new KeyValuePair(IBytesWrittenFeatureType, _currentIBytesWrittenFeature); diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs index 604e497807..a620a6e5d3 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.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 System; using System.Buffers; using System.Collections.Generic; using System.IO.Pipelines; @@ -14,10 +15,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal public abstract partial class TransportConnection : ConnectionContext { private IDictionary _items; + private List<(Action handler, object state)> _heartbeatHandlers; + private readonly object _heartbeatLock = new object(); + protected readonly CancellationTokenSource _connectionClosingCts = new CancellationTokenSource(); public TransportConnection() { FastReset(); + + ConnectionClosedRequested = _connectionClosingCts.Token; } public IPAddress RemoteAddress { get; set; } @@ -55,6 +61,37 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal public CancellationToken ConnectionClosed { get; set; } + public CancellationToken ConnectionClosedRequested { get; set; } + + public void TickHeartbeat() + { + lock (_heartbeatLock) + { + if (_heartbeatHandlers == null) + { + return; + } + + foreach (var (handler, state) in _heartbeatHandlers) + { + handler(state); + } + } + } + + public void OnHeartbeat(Action action, object state) + { + lock (_heartbeatLock) + { + if (_heartbeatHandlers == null) + { + _heartbeatHandlers = new List<(Action handler, object state)>(); + } + + _heartbeatHandlers.Add((action, state)); + } + } + // DO NOT remove this override to ConnectionContext.Abort. Doing so would cause // any TransportConnection that does not override Abort or calls base.Abort // to stack overflow when IConnectionLifetimeFeature.Abort() is called. @@ -65,5 +102,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { Output.CancelPendingRead(); } + + public void RequestClose() + { + try + { + _connectionClosingCts.Cancel(); + } + catch (ObjectDisposedException) + { + // There's a race where the token could be disposed + // swallow the exception and no-op + } + } } } diff --git a/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs b/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs index 46512ba482..db282bf4b2 100644 --- a/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs @@ -27,5 +27,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal void ConnectionPause(string connectionId); void ConnectionResume(string connectionId); + + void ConnectionAborted(string connectionId); } } diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs index b9f0aa0ab4..24eeaf6d88 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs @@ -120,7 +120,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal { _abortReason = abortReason; Output.CancelPendingRead(); - + // This cancels any pending I/O. Thread.Post(s => s.Dispose(), _socket); } @@ -129,6 +129,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public void Dispose() { _connectionClosedTokenSource.Dispose(); + _connectionClosingCts.Dispose(); } // Called on Libuv thread diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs index 96f3338821..0467c7b776 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs @@ -30,10 +30,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal // ConnectionWriteCallback: Reserved: 12 private static readonly Action _connectionError = - LoggerMessage.Define(LogLevel.Information, 14, @"Connection id ""{ConnectionId}"" communication error."); + LoggerMessage.Define(LogLevel.Information, new EventId(14, nameof(ConnectionError)), @"Connection id ""{ConnectionId}"" communication error."); private static readonly Action _connectionReset = - LoggerMessage.Define(LogLevel.Debug, 19, @"Connection id ""{ConnectionId}"" reset."); + LoggerMessage.Define(LogLevel.Debug, new EventId(19, nameof(ConnectionReset)), @"Connection id ""{ConnectionId}"" reset."); + + private static readonly Action _connectionAborted = + LoggerMessage.Define(LogLevel.Debug, new EventId(20, nameof(ConnectionAborted)), @"Connection id ""{ConnectionId}"" aborted."); private readonly ILogger _logger; @@ -95,6 +98,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal _connectionResume(_logger, connectionId, null); } + public void ConnectionAborted(string connectionId) + { + _connectionAborted(_logger, connectionId, null); + } + public IDisposable BeginScope(TState state) => _logger.BeginScope(state); public bool IsEnabled(LogLevel logLevel) => _logger.IsEnabled(logLevel); diff --git a/src/Kestrel.Transport.Libuv/Properties/AssemblyInfo.cs b/src/Kestrel.Transport.Libuv/Properties/AssemblyInfo.cs new file mode 100644 index 0000000000..62b3dccbef --- /dev/null +++ b/src/Kestrel.Transport.Libuv/Properties/AssemblyInfo.cs @@ -0,0 +1,6 @@ +// 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.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] diff --git a/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs b/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs index 06c2c36f20..d08408e2b0 100644 --- a/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs +++ b/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs @@ -19,5 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal void ConnectionPause(string connectionId); void ConnectionResume(string connectionId); + + void ConnectionAborted(string connectionId); } } diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs index 4cba0b3611..5cd8dfeade 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs @@ -4,7 +4,6 @@ using System; using System.Buffers; using System.Diagnostics; -using System.IO; using System.IO.Pipelines; using System.Net; using System.Net.Sockets; @@ -31,8 +30,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal private readonly CancellationTokenSource _connectionClosedTokenSource = new CancellationTokenSource(); private readonly object _shutdownLock = new object(); - private volatile bool _aborted; - private volatile ConnectionAbortedException _abortReason; + private volatile bool _socketDisposed; + private volatile Exception _shutdownReason; private long _totalBytesWritten; internal SocketConnection(Socket socket, MemoryPool memoryPool, PipeScheduler scheduler, ISocketsTrace trace) @@ -95,17 +94,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal public override void Abort(ConnectionAbortedException abortReason) { - _abortReason = abortReason; - Output.CancelPendingRead(); + _trace.ConnectionAborted(ConnectionId); // Try to gracefully close the socket to match libuv behavior. - Shutdown(); + Shutdown(abortReason); + + // Cancel ProcessSends loop after calling shutdown to ensure the correct _shutdownReason gets set. + Output.CancelPendingRead(); } // Only called after connection middleware is complete which means the ConnectionClosed token has fired. public void Dispose() { _connectionClosedTokenSource.Dispose(); + _connectionClosingCts.Dispose(); } private async Task DoReceive() @@ -118,46 +120,39 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal } catch (SocketException ex) when (IsConnectionResetError(ex.SocketErrorCode)) { - // A connection reset can be reported as SocketError.ConnectionAborted on Windows - if (!_aborted) + // This could be ignored if _shutdownReason is already set. + error = new ConnectionResetException(ex.Message, ex); + + // There's still a small chance that both DoReceive() and DoSend() can log the same connection reset. + // Both logs will have the same ConnectionId. I don't think it's worthwhile to lock just to avoid this. + if (!_socketDisposed) { - error = new ConnectionResetException(ex.Message, ex); _trace.ConnectionReset(ConnectionId); } } - catch (SocketException ex) when (IsConnectionAbortError(ex.SocketErrorCode)) - { - if (!_aborted) - { - // Calling Dispose after ReceiveAsync can cause an "InvalidArgument" error on *nix. - _trace.ConnectionError(ConnectionId, error); - } - } - catch (ObjectDisposedException) - { - if (!_aborted) - { - _trace.ConnectionError(ConnectionId, error); - } - } - catch (IOException ex) + catch (Exception ex) + when ((ex is SocketException socketEx && IsConnectionAbortError(socketEx.SocketErrorCode)) || + ex is ObjectDisposedException) { + // This exception should always be ignored because _shutdownReason should be set. error = ex; - _trace.ConnectionError(ConnectionId, error); + + if (!_socketDisposed) + { + // This is unexpected if the socket hasn't been disposed yet. + _trace.ConnectionError(ConnectionId, error); + } } catch (Exception ex) { - error = new IOException(ex.Message, ex); + // This is unexpected. + error = ex; _trace.ConnectionError(ConnectionId, error); } finally { - if (_aborted) - { - error = error ?? _abortReason ?? new ConnectionAbortedException(); - } - - Input.Complete(error); + // If Shutdown() has already bee called, assume that was the reason ProcessReceives() exited. + Input.Complete(_shutdownReason ?? error); } } @@ -188,17 +183,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal var flushTask = Input.FlushAsync(); - if (!flushTask.IsCompleted) + var paused = !flushTask.IsCompleted; + + if (paused) { _trace.ConnectionPause(ConnectionId); + } - await flushTask; + var result = await flushTask; + if (paused) + { _trace.ConnectionResume(ConnectionId); } - var result = flushTask.GetAwaiter().GetResult(); - if (result.IsCompleted) + if (result.IsCompleted || result.IsCanceled) { // Pipe consumer is shut down, do we stop writing break; @@ -208,7 +207,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal private async Task DoSend() { - Exception error = null; + Exception shutdownReason = null; + Exception unexpectedError = null; try { @@ -216,34 +216,31 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal } catch (SocketException ex) when (IsConnectionResetError(ex.SocketErrorCode)) { - // A connection reset can be reported as SocketError.ConnectionAborted on Windows - error = null; + shutdownReason = new ConnectionResetException(ex.Message, ex);; _trace.ConnectionReset(ConnectionId); } - catch (SocketException ex) when (IsConnectionAbortError(ex.SocketErrorCode)) + catch (Exception ex) + when ((ex is SocketException socketEx && IsConnectionAbortError(socketEx.SocketErrorCode)) || + ex is ObjectDisposedException) { - error = null; - } - catch (ObjectDisposedException) - { - error = null; - } - catch (IOException ex) - { - error = ex; - _trace.ConnectionError(ConnectionId, error); + // This should always be ignored since Shutdown() must have already been called by Abort(). + shutdownReason = ex; } catch (Exception ex) { - error = new IOException(ex.Message, ex); - _trace.ConnectionError(ConnectionId, error); + shutdownReason = ex; + unexpectedError = ex; + _trace.ConnectionError(ConnectionId, unexpectedError); } finally { - Shutdown(); + Shutdown(shutdownReason); // Complete the output after disposing the socket - Output.Complete(error); + Output.Complete(unexpectedError); + + // Cancel any pending flushes so that the input loop is un-paused + Input.CancelPendingFlush(); } } @@ -280,30 +277,38 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal } } - private void Shutdown() + private void Shutdown(Exception shutdownReason) { lock (_shutdownLock) { - if (!_aborted) + if (_socketDisposed) { - // Make sure to close the connection only after the _aborted flag is set. - // Without this, the RequestsCanBeAbortedMidRead test will sometimes fail when - // a BadHttpRequestException is thrown instead of a TaskCanceledException. - _aborted = true; - _trace.ConnectionWriteFin(ConnectionId); - - try - { - // Try to gracefully close the socket even for aborts to match libuv behavior. - _socket.Shutdown(SocketShutdown.Both); - } - catch - { - // Ignore any errors from Socket.Shutdown since we're tearing down the connection anyway. - } - - _socket.Dispose(); + return; } + + // Make sure to close the connection only after the _aborted flag is set. + // Without this, the RequestsCanBeAbortedMidRead test will sometimes fail when + // a BadHttpRequestException is thrown instead of a TaskCanceledException. + _socketDisposed = true; + + // 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(); + + _trace.ConnectionWriteFin(ConnectionId); + + try + { + // Try to gracefully close the socket even for aborts to match libuv behavior. + _socket.Shutdown(SocketShutdown.Both); + } + catch + { + // Ignore any errors from Socket.Shutdown() since we're tearing down the connection anyway. + } + + _socket.Dispose(); } } @@ -321,16 +326,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal private static bool IsConnectionResetError(SocketError errorCode) { + // A connection reset can be reported as SocketError.ConnectionAborted on Windows. + // ProtocolType can be removed once https://github.com/dotnet/corefx/issues/31927 is fixed. return errorCode == SocketError.ConnectionReset || - errorCode == SocketError.ConnectionAborted || - errorCode == SocketError.Shutdown; + errorCode == SocketError.Shutdown || + (errorCode == SocketError.ConnectionAborted && IsWindows) || + (errorCode == SocketError.ProtocolType && IsMacOS); } private static bool IsConnectionAbortError(SocketError errorCode) { + // Calling Dispose after ReceiveAsync can cause an "InvalidArgument" error on *nix. return errorCode == SocketError.OperationAborted || errorCode == SocketError.Interrupted || - errorCode == SocketError.InvalidArgument; + (errorCode == SocketError.InvalidArgument && !IsWindows); } } } diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs b/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs index e8aa5fa286..5b132ccf9c 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs @@ -28,6 +28,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal private static readonly Action _connectionReset = LoggerMessage.Define(LogLevel.Debug, new EventId(19, nameof(ConnectionReset)), @"Connection id ""{ConnectionId}"" reset."); + private static readonly Action _connectionAborted = + LoggerMessage.Define(LogLevel.Debug, new EventId(20, nameof(ConnectionAborted)), @"Connection id ""{ConnectionId}"" aborted."); + private readonly ILogger _logger; public SocketsTrace(ILogger logger) @@ -83,6 +86,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal _connectionResume(_logger, connectionId, null); } + public void ConnectionAborted(string connectionId) + { + _connectionAborted(_logger, connectionId, null); + } + public IDisposable BeginScope(TState state) => _logger.BeginScope(state); public bool IsEnabled(LogLevel logLevel) => _logger.IsEnabled(logLevel); diff --git a/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs b/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs index dbf838f562..a441423b7d 100644 --- a/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs +++ b/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO.Pipelines; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; @@ -23,7 +24,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var tcs = new TaskCompletionSource(); var dispatcher = new ConnectionDispatcher(serviceContext, _ => tcs.Task); - var connection = Mock.Of(); + var connection = new Mock() { CallBase = true }.Object; + connection.ConnectionClosed = new CancellationToken(canceled: true); dispatcher.OnConnection(connection); @@ -51,7 +53,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var serviceContext = new TestServiceContext(); var dispatcher = new ConnectionDispatcher(serviceContext, _ => Task.CompletedTask); - var mockConnection = new Mock(); + var mockConnection = new Mock() { CallBase = true }; + mockConnection.Object.ConnectionClosed = new CancellationToken(canceled: true); var mockPipeReader = new Mock(); var mockPipeWriter = new Mock(); var mockPipe = new Mock(); diff --git a/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs b/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs index 015aa75881..cb201672d0 100644 --- a/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs +++ b/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs @@ -3,8 +3,10 @@ using System; using System.Runtime.CompilerServices; +using Microsoft.AspNetCore.Connections; 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; @@ -18,7 +20,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { var connectionId = "0"; var trace = new Mock(); - var httpConnectionManager = new HttpConnectionManager(trace.Object, ResourceCounter.Unlimited); + var httpConnectionManager = new ConnectionManager(trace.Object, ResourceCounter.Unlimited); // Create HttpConnection in inner scope so it doesn't get rooted by the current frame. UnrootedConnectionsGetRemovedFromHeartbeatInnerScope(connectionId, httpConnectionManager, trace); @@ -36,14 +38,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [MethodImpl(MethodImplOptions.NoInlining)] private void UnrootedConnectionsGetRemovedFromHeartbeatInnerScope( string connectionId, - HttpConnectionManager httpConnectionManager, + ConnectionManager httpConnectionManager, Mock trace) { - var httpConnection = new HttpConnection(new HttpConnectionContext - { - ServiceContext = new TestServiceContext(), - ConnectionId = connectionId - }); + var mock = new Mock(); + mock.Setup(m => m.ConnectionId).Returns(connectionId); + var httpConnection = new KestrelConnection(mock.Object); httpConnectionManager.AddConnection(0, httpConnection); diff --git a/test/Kestrel.Core.Tests/HttpConnectionTests.cs b/test/Kestrel.Core.Tests/HttpConnectionTests.cs index 2c78c922c7..8d0f1e418b 100644 --- a/test/Kestrel.Core.Tests/HttpConnectionTests.cs +++ b/test/Kestrel.Core.Tests/HttpConnectionTests.cs @@ -43,7 +43,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests ConnectionAdapters = new List(), ConnectionFeatures = connectionFeatures, MemoryPool = _memoryPool, - HttpConnectionId = long.MinValue, Transport = pair.Transport, ServiceContext = new TestServiceContext { diff --git a/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs b/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs index d71642f9dd..cf46717f85 100644 --- a/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs @@ -308,6 +308,37 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(TimeSpan.FromSeconds(5), new KestrelServerLimits().MinResponseDataRate.GracePeriod); } + [Fact] + public void Http2MaxFrameSizeDefault() + { + Assert.Equal(1 << 14, new KestrelServerLimits().Http2.MaxFrameSize); + } + + [Theory] + [InlineData(1 << 14 - 1)] + [InlineData(1 << 24)] + [InlineData(-1)] + public void Http2MaxFrameSizeInvalid(int value) + { + var ex = Assert.Throws(() => new KestrelServerLimits().Http2.MaxFrameSize = value); + Assert.Contains("A value between", ex.Message); + } + + [Fact] + public void Http2HeaderTableSizeDefault() + { + Assert.Equal(4096, new KestrelServerLimits().Http2.HeaderTableSize); + } + + [Theory] + [InlineData(4097)] + [InlineData(-1)] + public void Http2HeaderTableSizeInvalid(int value) + { + var ex = Assert.Throws(() => new KestrelServerLimits().Http2.MaxFrameSize = value); + Assert.Contains("A value between", ex.Message); + } + public static TheoryData TimeoutValidData => new TheoryData { TimeSpan.FromTicks(1), diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index c4b6c3ab44..f876a1f01b 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -77,7 +77,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests private static readonly byte[] _worldBytes = Encoding.ASCII.GetBytes("world"); private static readonly byte[] _helloWorldBytes = Encoding.ASCII.GetBytes("hello, world"); private static readonly byte[] _noData = new byte[0]; - private static readonly byte[] _maxData = Encoding.ASCII.GetBytes(new string('a', Http2Frame.MinAllowedMaxFrameSize)); + private static readonly byte[] _maxData = Encoding.ASCII.GetBytes(new string('a', Http2Limits.MinAllowedMaxFrameSize)); [Fact] public async Task Frame_Received_OverMaxSize_FrameError() @@ -85,20 +85,47 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await InitializeConnectionAsync(_echoApplication); await StartStreamAsync(1, _browserRequestHeaders, endStream: false); - // Manually craft a frame where the size is too large. Our own frame class won't allow this. - // See Http2Frame.Length - var length = Http2Frame.MinAllowedMaxFrameSize + 1; // Too big - var frame = new byte[9 + length]; - frame[0] = (byte)((length & 0x00ff0000) >> 16); - frame[1] = (byte)((length & 0x0000ff00) >> 8); - frame[2] = (byte)(length & 0x000000ff); - await SendAsync(frame); + uint length = Http2Limits.MinAllowedMaxFrameSize + 1; + await SendDataAsync(1, new byte[length].AsSpan(), endStream: true); await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: true, expectedLastStreamId: 1, expectedErrorCode: Http2ErrorCode.FRAME_SIZE_ERROR, - expectedErrorMessage: CoreStrings.FormatHttp2ErrorFrameOverLimit(length, Http2Frame.MinAllowedMaxFrameSize)); + expectedErrorMessage: CoreStrings.FormatHttp2ErrorFrameOverLimit(length, Http2Limits.MinAllowedMaxFrameSize)); + } + + [Fact] + public async Task ServerSettings_ChangesRequestMaxFrameSize() + { + var length = Http2Limits.MinAllowedMaxFrameSize + 10; + _connectionContext.ServiceContext.ServerOptions.Limits.Http2.MaxFrameSize = length; + _connection = new Http2Connection(_connectionContext); + + await InitializeConnectionAsync(_echoApplication, expectedSettingsLegnth: 12); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + await SendDataAsync(1, new byte[length].AsSpan(), endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + // The client's settings is still defaulted to Http2PeerSettings.MinAllowedMaxFrameSize so the echo response will come back in two separate frames + await ExpectAsync(Http2FrameType.DATA, + withLength: Http2Limits.MinAllowedMaxFrameSize, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: length - Http2Limits.MinAllowedMaxFrameSize, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } [Fact] @@ -2042,7 +2069,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { await InitializeConnectionAsync(_noopApplication); - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareSettings(Http2SettingsFrameFlags.ACK); await SendAsync(frame.Raw); @@ -2073,6 +2100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [InlineData(Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE, 16 * 1024 - 1, Http2ErrorCode.PROTOCOL_ERROR)] [InlineData(Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE, 16 * 1024 * 1024, Http2ErrorCode.PROTOCOL_ERROR)] [InlineData(Http2SettingsParameter.SETTINGS_MAX_FRAME_SIZE, uint.MaxValue, Http2ErrorCode.PROTOCOL_ERROR)] + [InlineData(Http2SettingsParameter.SETTINGS_HEADER_TABLE_SIZE, 4097, Http2ErrorCode.PROTOCOL_ERROR)] public async Task SETTINGS_Received_InvalidParameterValue_ConnectionError(Http2SettingsParameter parameter, uint value, Http2ErrorCode expectedErrorCode) { await InitializeConnectionAsync(_noopApplication); @@ -2160,6 +2188,60 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests expectedErrorMessage: CoreStrings.Http2ErrorInitialWindowSizeInvalid); } + [Fact] + public async Task SETTINGS_Received_ChangesAllowedResponseMaxFrameSize() + { + // This includes the default response headers such as :status, etc + var defaultResponseHeaderLength = 37; + var headerValueLength = Http2Limits.MinAllowedMaxFrameSize; + // First byte is always 0 + // Second byte is the length of header name which is 1 + // Third byte is the header name which is A/B + // Next three bytes are the 7-bit integer encoding representation of the header length which is 16*1024 + var encodedHeaderLength = 1 + 1 + 1 + 3 + headerValueLength; + // Adding 10 additional bytes for encoding overhead + var payloadLength = defaultResponseHeaderLength + encodedHeaderLength; + + await InitializeConnectionAsync(context => + { + context.Response.Headers["A"] = new string('a', headerValueLength); + context.Response.Headers["B"] = new string('b', headerValueLength); + return context.Response.Body.WriteAsync(new byte[payloadLength], 0, payloadLength); + }); + + // Update client settings + _clientSettings.MaxFrameSize = (uint)payloadLength; + await SendSettingsAsync(); + + // ACK + await ExpectAsync(Http2FrameType.SETTINGS, + withLength: 0, + withFlags: (byte)Http2SettingsFrameFlags.ACK, + withStreamId: 0); + + // Start request + await StartStreamAsync(1, _browserRequestHeaders, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: defaultResponseHeaderLength + encodedHeaderLength, + withFlags: (byte)Http2HeadersFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.CONTINUATION, + withLength: encodedHeaderLength, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: payloadLength, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + [Fact] public async Task PUSH_PROMISE_Received_ConnectionError() { diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs index 4fa61a11df..7449a22f2f 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -141,7 +141,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _echoApplication = async context => { - var buffer = new byte[Http2Frame.MinAllowedMaxFrameSize]; + var buffer = new byte[Http2Limits.MinAllowedMaxFrameSize]; var received = 0; while ((received = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length)) > 0) @@ -152,7 +152,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _echoWaitForAbortApplication = async context => { - var buffer = new byte[Http2Frame.MinAllowedMaxFrameSize]; + var buffer = new byte[Http2Limits.MinAllowedMaxFrameSize]; var received = 0; while ((received = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length)) > 0) @@ -307,7 +307,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiStringNonNullCharacters(); } - protected async Task InitializeConnectionAsync(RequestDelegate application) + protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsLegnth = 6) { _connectionTask = _connection.ProcessRequestsAsync(new DummyApplication(application)); @@ -315,7 +315,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await SendSettingsAsync(); await ExpectAsync(Http2FrameType.SETTINGS, - withLength: 6, + withLength: expectedSettingsLegnth, withFlags: 0, withStreamId: 0); @@ -330,7 +330,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _runningStreams[streamId] = tcs; - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(Http2HeadersFrameFlags.NONE, streamId); var done = _hpackEncoder.BeginEncode(headers, frame.HeadersPayload, out var length); frame.PayloadLength = length; @@ -367,7 +367,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _runningStreams[streamId] = tcs; - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.PADDED, streamId); frame.HeadersPadLength = padLength; @@ -390,7 +390,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _runningStreams[streamId] = tcs; - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.PRIORITY, streamId); frame.HeadersPriorityWeight = priority; frame.HeadersStreamDependency = streamDependency; @@ -412,7 +412,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _runningStreams[streamId] = tcs; - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.PADDED | Http2HeadersFrameFlags.PRIORITY, streamId); frame.HeadersPadLength = padLength; frame.HeadersPriorityWeight = priority; @@ -452,14 +452,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendSettingsAsync() { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareSettings(Http2SettingsFrameFlags.NONE, _clientSettings.GetNonProtocolDefaults()); return SendAsync(frame.Raw); } protected Task SendSettingsAckWithInvalidLengthAsync(int length) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareSettings(Http2SettingsFrameFlags.ACK); frame.PayloadLength = length; return SendAsync(frame.Raw); @@ -467,7 +467,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendSettingsWithInvalidStreamIdAsync(int streamId) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareSettings(Http2SettingsFrameFlags.NONE, _clientSettings.GetNonProtocolDefaults()); frame.StreamId = streamId; return SendAsync(frame.Raw); @@ -475,7 +475,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendSettingsWithInvalidLengthAsync(int length) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareSettings(Http2SettingsFrameFlags.NONE, _clientSettings.GetNonProtocolDefaults()); frame.PayloadLength = length; return SendAsync(frame.Raw); @@ -483,7 +483,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendSettingsWithInvalidParameterValueAsync(Http2SettingsParameter parameter, uint value) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareSettings(Http2SettingsFrameFlags.NONE); frame.PayloadLength = 6; @@ -499,7 +499,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendPushPromiseFrameAsync() { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PayloadLength = 0; frame.Type = Http2FrameType.PUSH_PROMISE; frame.StreamId = 1; @@ -508,7 +508,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected async Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags flags, IEnumerable> headers) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(flags, streamId); var done = _hpackEncoder.BeginEncode(headers, frame.Payload, out var length); @@ -521,7 +521,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendHeadersAsync(int streamId, Http2HeadersFrameFlags flags, byte[] headerBlock) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(flags, streamId); frame.PayloadLength = headerBlock.Length; @@ -534,7 +534,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { Assert.True(padLength >= payloadLength, $"{nameof(padLength)} must be greater than or equal to {nameof(payloadLength)} to create an invalid frame."); - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(Http2HeadersFrameFlags.PADDED, streamId); frame.Payload[0] = padLength; @@ -547,7 +547,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendIncompleteHeadersFrameAsync(int streamId) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareHeaders(Http2HeadersFrameFlags.END_HEADERS, streamId); frame.PayloadLength = 3; @@ -563,7 +563,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected async Task SendContinuationAsync(int streamId, Http2ContinuationFrameFlags flags) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareContinuation(flags, streamId); var done = _hpackEncoder.Encode(frame.Payload, out var length); @@ -576,7 +576,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected async Task SendContinuationAsync(int streamId, Http2ContinuationFrameFlags flags, byte[] payload) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareContinuation(flags, streamId); frame.PayloadLength = payload.Length; @@ -587,7 +587,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendEmptyContinuationFrameAsync(int streamId, Http2ContinuationFrameFlags flags) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareContinuation(flags, streamId); frame.PayloadLength = 0; @@ -597,7 +597,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendIncompleteContinuationFrameAsync(int streamId) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareContinuation(Http2ContinuationFrameFlags.END_HEADERS, streamId); frame.PayloadLength = 3; @@ -613,7 +613,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendDataAsync(int streamId, Span data, bool endStream) { - var frame = new Http2Frame(); + var frame = new Http2Frame((uint)data.Length); frame.PrepareData(streamId); frame.PayloadLength = data.Length; @@ -625,7 +625,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendDataWithPaddingAsync(int streamId, Span data, byte padLength, bool endStream) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareData(streamId, padLength); frame.PayloadLength = data.Length + 1 + padLength; @@ -643,7 +643,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { Assert.True(padLength >= frameLength, $"{nameof(padLength)} must be greater than or equal to {nameof(frameLength)} to create an invalid frame."); - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareData(streamId); frame.DataFlags = Http2DataFrameFlags.PADDED; @@ -657,14 +657,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendPingAsync(Http2PingFrameFlags flags) { - var pingFrame = new Http2Frame(); + var pingFrame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); pingFrame.PreparePing(flags); return SendAsync(pingFrame.Raw); } protected Task SendPingWithInvalidLengthAsync(int length) { - var pingFrame = new Http2Frame(); + var pingFrame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); pingFrame.PreparePing(Http2PingFrameFlags.NONE); pingFrame.PayloadLength = length; return SendAsync(pingFrame.Raw); @@ -674,7 +674,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { Assert.NotEqual(0, streamId); - var pingFrame = new Http2Frame(); + var pingFrame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); pingFrame.PreparePing(Http2PingFrameFlags.NONE); pingFrame.StreamId = streamId; return SendAsync(pingFrame.Raw); @@ -682,14 +682,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendPriorityAsync(int streamId, int streamDependency = 0) { - var priorityFrame = new Http2Frame(); + var priorityFrame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); priorityFrame.PreparePriority(streamId, streamDependency: streamDependency, exclusive: false, weight: 0); return SendAsync(priorityFrame.Raw); } protected Task SendInvalidPriorityFrameAsync(int streamId, int length) { - var priorityFrame = new Http2Frame(); + var priorityFrame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); priorityFrame.PreparePriority(streamId, streamDependency: 0, exclusive: false, weight: 0); priorityFrame.PayloadLength = length; return SendAsync(priorityFrame.Raw); @@ -697,14 +697,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendRstStreamAsync(int streamId) { - var rstStreamFrame = new Http2Frame(); + var rstStreamFrame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); rstStreamFrame.PrepareRstStream(streamId, Http2ErrorCode.CANCEL); return SendAsync(rstStreamFrame.Raw); } protected Task SendInvalidRstStreamFrameAsync(int streamId, int length) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareRstStream(streamId, Http2ErrorCode.CANCEL); frame.PayloadLength = length; return SendAsync(frame.Raw); @@ -712,14 +712,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendGoAwayAsync() { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareGoAway(0, Http2ErrorCode.NO_ERROR); return SendAsync(frame.Raw); } protected Task SendInvalidGoAwayFrameAsync() { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareGoAway(0, Http2ErrorCode.NO_ERROR); frame.StreamId = 1; return SendAsync(frame.Raw); @@ -727,14 +727,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendWindowUpdateAsync(int streamId, int sizeIncrement) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareWindowUpdate(streamId, sizeIncrement); return SendAsync(frame.Raw); } protected Task SendInvalidWindowUpdateAsync(int streamId, int sizeIncrement, int length) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.PrepareWindowUpdate(streamId, sizeIncrement); frame.PayloadLength = length; return SendAsync(frame.Raw); @@ -742,16 +742,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected Task SendUnknownFrameTypeAsync(int streamId, int frameType) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); frame.StreamId = streamId; frame.Type = (Http2FrameType)frameType; frame.PayloadLength = 0; return SendAsync(frame.Raw); } - protected async Task ReceiveFrameAsync() + protected async Task ReceiveFrameAsync(uint maxFrameSize = Http2PeerSettings.DefaultMaxFrameSize) { - var frame = new Http2Frame(); + var frame = new Http2Frame(maxFrameSize); while (true) { @@ -764,7 +764,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { Assert.True(buffer.Length > 0); - if (Http2FrameReader.ReadFrame(buffer, frame, 16_384, out consumed, out examined)) + if (Http2FrameReader.ReadFrame(buffer, frame, maxFrameSize, out consumed, out examined)) { return frame; } @@ -783,7 +783,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected async Task ExpectAsync(Http2FrameType type, int withLength, byte withFlags, int withStreamId) { - var frame = await ReceiveFrameAsync(); + var frame = await ReceiveFrameAsync((uint)withLength); Assert.Equal(type, frame.Type); Assert.Equal(withLength, frame.PayloadLength); diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/TlsTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/TlsTests.cs index fda221726e..93718e2c34 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/TlsTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/TlsTests.cs @@ -92,7 +92,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.Http2 private async Task ReceiveFrameAsync(PipeReader reader) { - var frame = new Http2Frame(); + var frame = new Http2Frame(Http2Limits.MinAllowedMaxFrameSize); while (true) { diff --git a/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs b/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs index 5d3ce03935..9e52de9f12 100644 --- a/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs @@ -312,7 +312,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests LoggerFactory.AddProvider(loggerProvider); var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var handshakeStartedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); TimeSpan handshakeTimeout = default; diff --git a/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs index 2cf8739ca4..716ff9583a 100644 --- a/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionClosedWhenKeepAliveTimeoutExpires() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionKeptAliveBetweenRequests() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionNotTimedOutWhileRequestBeingSent() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -113,7 +113,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task ConnectionNotTimedOutWhileAppIsRunning() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var cts = new CancellationTokenSource(); using (var server = CreateServer(testContext, longRunningCt: cts.Token)) @@ -150,7 +150,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task ConnectionTimesOutWhenOpenedButNoRequestSent() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -167,7 +167,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task KeepAliveTimeoutDoesNotApplyToUpgradedConnections() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var cts = new CancellationTokenSource(); using (var server = CreateServer(testContext, upgradeCt: cts.Token)) diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs index e308b03a99..f8946d707e 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { var gracePeriod = TimeSpan.FromSeconds(5); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); var appRunningEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -136,7 +136,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { var gracePeriod = TimeSpan.FromSeconds(5); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); var appRunningTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var exceptionSwallowedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs index d6a924bf61..e5fd714179 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionAbortedWhenRequestHeadersNotReceivedInTime(string headers) { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task RequestHeadersTimeoutCanceledAfterHeadersReceived() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionAbortedWhenRequestLineNotReceivedInTime(string requestLine) { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task TimeoutNotResetOnEachRequestLineCharacterReceived() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs index 07c4d642e3..34f6b92333 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs @@ -877,7 +877,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests var appEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var delayEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); using (var server = new TestServer(async context => { diff --git a/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs b/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs index 42f5659d5e..7dd73fe650 100644 --- a/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs @@ -254,7 +254,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests const int limit = 10; var upgradeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var serviceContext = new TestServiceContext(LoggerFactory); - serviceContext.ConnectionManager = new HttpConnectionManager(serviceContext.Log, ResourceCounter.Quota(limit)); + serviceContext.ConnectionManager = new ConnectionManager(serviceContext.Log, ResourceCounter.Quota(limit)); using (var server = new TestServer(async context => { diff --git a/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs b/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs index 6014dcc37d..64eceae6a5 100644 --- a/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs @@ -109,6 +109,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 MemoryPoolFactory = memoryPoolFactory.Create }; + TestApplicationErrorLogger.ThrowOnUngracefulShutdown = false; + // Abortive shutdown leaves one request hanging using (var server = new TestServer(async context => { diff --git a/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs b/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs index 2aa92e37d2..466f68ac40 100644 --- a/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs +++ b/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs @@ -396,7 +396,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests // Cause all writes to fail while (completeQueue.TryDequeue(out var triggerNextCompleted)) { - await _libuvThread.PostAsync(cb => cb(-1), triggerNextCompleted); + await _libuvThread.PostAsync(cb => cb(LibuvConstants.ECONNRESET.Value), triggerNextCompleted); } // Second task is now completed diff --git a/test/shared/TestApplicationErrorLogger.cs b/test/shared/TestApplicationErrorLogger.cs index bff77f8efa..8ff01d719e 100644 --- a/test/shared/TestApplicationErrorLogger.cs +++ b/test/shared/TestApplicationErrorLogger.cs @@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Testing public bool ThrowOnCriticalErrors { get; set; } = true; + public bool ThrowOnUngracefulShutdown { get; set; } = true; + public ConcurrentQueue Messages { get; } = new ConcurrentQueue(); public ConcurrentQueue Scopes { get; } = new ConcurrentQueue(); @@ -59,12 +61,22 @@ namespace Microsoft.AspNetCore.Testing } // Fail tests where not all the connections close during server shutdown. - if (eventId.Id == 21 && eventId.Name == nameof(KestrelTrace.NotAllConnectionsAborted)) + if (ThrowOnUngracefulShutdown && + ((eventId.Id == 16 && eventId.Name == nameof(KestrelTrace.NotAllConnectionsClosedGracefully)) || + (eventId.Id == 21 && eventId.Name == nameof(KestrelTrace.NotAllConnectionsAborted)))) { - var log = $"Log {logLevel}[{eventId}]: {formatter(state, exception)} {exception?.Message}"; + var log = $"Log {logLevel}[{eventId}]: {formatter(state, exception)} {exception}"; throw new Exception($"Shutdown failure. {log}"); } + // We don't use nameof here because this is logged by the transports and we don't know which one is + // referenced in this shared source file. + if (eventId.Id == 14 && eventId.Name == "ConnectionError") + { + var log = $"Log {logLevel}[{eventId}]: {formatter(state, exception)} {exception}"; + throw new Exception($"Unexpected connection error. {log}"); + } + Messages.Enqueue(new LogMessage { LogLevel = logLevel, diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index f8b68dd906..356164e725 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Testing SystemClock = new SystemClock(); DateHeaderValueManager = new DateHeaderValueManager(SystemClock); - var heartbeatManager = new HttpHeartbeatManager(ConnectionManager); + var heartbeatManager = new HeartbeatManager(ConnectionManager); Heartbeat = new Heartbeat( new IHeartbeatHandler[] { DateHeaderValueManager, heartbeatManager }, SystemClock, @@ -61,7 +61,7 @@ namespace Microsoft.AspNetCore.Testing MockSystemClock = new MockSystemClock(); SystemClock = MockSystemClock; DateHeaderValueManager = new DateHeaderValueManager(MockSystemClock); - ConnectionManager = new HttpConnectionManager(Log, ResourceCounter.Unlimited); + ConnectionManager = new ConnectionManager(Log, ResourceCounter.Unlimited); HttpParser = new HttpParser(Log.IsEnabled(LogLevel.Information)); ServerOptions = new KestrelServerOptions { diff --git a/tools/CodeGenerator/TransportConnectionFeatureCollection.cs b/tools/CodeGenerator/TransportConnectionFeatureCollection.cs index 9f19f5387f..350d853fe8 100644 --- a/tools/CodeGenerator/TransportConnectionFeatureCollection.cs +++ b/tools/CodeGenerator/TransportConnectionFeatureCollection.cs @@ -19,6 +19,8 @@ namespace CodeGenerator "IApplicationTransportFeature", "ITransportSchedulerFeature", "IConnectionLifetimeFeature", + "IConnectionHeartbeatFeature", + "IConnectionLifetimeNotificationFeature", "IBytesWrittenFeature", };