From 68a08635244df0941b5edd4cc07b2fd0a3cbddcd Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 22 Aug 2018 20:35:28 -0700 Subject: [PATCH 1/5] General connection management (#2834) This change makes the handling of graceful shutdown work for more than just http scenarios. This should allow us to move TLS further out and should also allow us to start moving things to connection middleware instead of connection adapters. Summary of the things changed/added: - Added IConnectionLifetimeNotificationFeature that represents an attempt to gracefully close the connection that isn't being aborted. This feels pretty awful but we may have to do it. - Moved connection management to the ConnectionDispatcher and out of the HttpConnectionMiddleware - Removed Http from the names of the ConnectionManager and Heartbeat --- .../IConnectionLifetimeNotificationFeature.cs | 14 ++ .../Internal/ConnectionDispatcher.cs | 69 ++++++-- src/Kestrel.Core/Internal/HttpConnection.cs | 153 +++++++++--------- .../Internal/HttpConnectionContext.cs | 1 - .../Internal/HttpConnectionMiddleware.cs | 46 +----- ...nectionManager.cs => ConnectionManager.cs} | 14 +- ...=> ConnectionManagerShutdownExtensions.cs} | 12 +- .../Infrastructure/ConnectionReference.cs | 26 +++ ...eartbeatManager.cs => HeartbeatManager.cs} | 14 +- .../Infrastructure/HttpConnectionReference.cs | 25 --- .../Infrastructure/KestrelConnection.cs | 28 ++++ .../Infrastructure/KestrelEventSource.cs | 11 +- src/Kestrel.Core/Internal/ServiceContext.cs | 2 +- src/Kestrel.Core/KestrelServer.cs | 16 +- .../TransportConnection.FeatureCollection.cs | 15 ++ .../Internal/TransportConnection.Generated.cs | 46 ++++++ .../Internal/TransportConnection.cs | 50 ++++++ .../Internal/ILibuvTrace.cs | 2 + .../Internal/LibuvConnection.cs | 3 +- .../Internal/LibuvTrace.cs | 8 + .../Internal/ISocketsTrace.cs | 2 + .../Internal/SocketConnection.cs | 18 ++- .../Internal/SocketsTrace.cs | 8 + .../ConnectionDispatcherTests.cs | 7 +- .../HttpConnectionManagerTests.cs | 14 +- .../Kestrel.Core.Tests/HttpConnectionTests.cs | 1 - .../HttpsTests.cs | 2 +- .../KeepAliveTimeoutTests.cs | 12 +- .../RequestBodyTimeoutTests.cs | 4 +- .../RequestHeadersTimeoutTests.cs | 8 +- .../RequestTests.cs | 2 +- .../UpgradeTests.cs | 2 +- .../Http2/ShutdownTests.cs | 2 + test/shared/TestApplicationErrorLogger.cs | 6 +- test/shared/TestServiceContext.cs | 4 +- .../TransportConnectionFeatureCollection.cs | 2 + 36 files changed, 423 insertions(+), 226 deletions(-) create mode 100644 src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs rename src/Kestrel.Core/Internal/Infrastructure/{HttpConnectionManager.cs => ConnectionManager.cs} (77%) rename src/Kestrel.Core/Internal/Infrastructure/{HttpConnectionManagerShutdownExtensions.cs => ConnectionManagerShutdownExtensions.cs} (80%) create mode 100644 src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs rename src/Kestrel.Core/Internal/Infrastructure/{HttpHeartbeatManager.cs => HeartbeatManager.cs} (57%) delete mode 100644 src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.cs create mode 100644 src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs 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/Internal/ConnectionDispatcher.cs b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs index 79614ec51b..20829ebb14 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 + { + connection.Complete(); + + _serviceContext.ConnectionManager.RemoveConnection(id); + + Log.ConnectionStop(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStop(connectionContext); + } } 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/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.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..14e42c6002 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs @@ -35,6 +35,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal private static readonly Action _connectionReset = LoggerMessage.Define(LogLevel.Debug, 19, @"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 LibuvTrace(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.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..20c301a97f 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs @@ -95,6 +95,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal public override void Abort(ConnectionAbortedException abortReason) { + _trace.ConnectionAborted(ConnectionId); + _abortReason = abortReason; Output.CancelPendingRead(); @@ -106,6 +108,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal public void Dispose() { _connectionClosedTokenSource.Dispose(); + _connectionClosingCts.Dispose(); } private async Task DoReceive() @@ -188,17 +191,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; @@ -244,6 +251,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal // Complete the output after disposing the socket Output.Complete(error); + + // Cancel any pending flushes so that the input loop is un-paused + Input.CancelPendingFlush(); } } 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.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/shared/TestApplicationErrorLogger.cs b/test/shared/TestApplicationErrorLogger.cs index bff77f8efa..3f0f75f77b 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,7 +61,9 @@ 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}"; throw new Exception($"Shutdown failure. {log}"); 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", }; From e5a110123995aad778d0cd64037781df9ef5e3a1 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 23 Aug 2018 16:59:02 -0700 Subject: [PATCH 2/5] Improve exception handling in SocketConnection (#2844) - Avoid race where a connection reset observed by both DoSend() and DoReceive() resulted in a ConnectionAbortedException being thrown from the input Pipe instead of a ConnectionResetException. --- .../Internal/LibuvTrace.cs | 4 +- .../Properties/AssemblyInfo.cs | 6 + .../Internal/SocketConnection.cs | 139 +++++++++--------- .../LibuvOutputConsumerTests.cs | 2 +- test/shared/TestApplicationErrorLogger.cs | 10 +- 5 files changed, 86 insertions(+), 75 deletions(-) create mode 100644 src/Kestrel.Transport.Libuv/Properties/AssemblyInfo.cs diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs index 14e42c6002..0467c7b776 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs @@ -30,10 +30,10 @@ 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."); 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/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs index 20c301a97f..fcb27fa305 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) @@ -97,11 +96,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal { _trace.ConnectionAborted(ConnectionId); - _abortReason = abortReason; - Output.CancelPendingRead(); - // 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. @@ -121,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); } } @@ -215,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 { @@ -223,34 +216,28 @@ 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(); @@ -290,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(); } } @@ -331,6 +326,7 @@ 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. return errorCode == SocketError.ConnectionReset || errorCode == SocketError.ConnectionAborted || errorCode == SocketError.Shutdown; @@ -338,6 +334,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal 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; 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 3f0f75f77b..8ff01d719e 100644 --- a/test/shared/TestApplicationErrorLogger.cs +++ b/test/shared/TestApplicationErrorLogger.cs @@ -65,10 +65,18 @@ namespace Microsoft.AspNetCore.Testing ((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, From fc3c2eef5ed118beb16cb5d249db6b87520b2814 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 23 Aug 2018 19:16:44 -0700 Subject: [PATCH 3/5] Handle SocketError.ProtocolType as a connection reset on macOS (#2845) * Handle SocketError.ProtocolType as a connection reset on macOS * Make IsConnectionResetError and IsConnectionAbortError stricter --- .../Internal/SocketConnection.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs index fcb27fa305..5cd8dfeade 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs @@ -327,9 +327,11 @@ 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) @@ -337,7 +339,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal // 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); } } } From d318d7b94dd4e48c5718ca35487ab7e3e8b73951 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 23 Aug 2018 22:29:56 -0700 Subject: [PATCH 4/5] Make tests less flaky (#2848) - Re-order logs with completing the task --- src/Kestrel.Core/Internal/ConnectionDispatcher.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs index 20829ebb14..a3b47a709f 100644 --- a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs +++ b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs @@ -84,12 +84,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } finally { + Log.ConnectionStop(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStop(connectionContext); + connection.Complete(); _serviceContext.ConnectionManager.RemoveConnection(id); - - Log.ConnectionStop(connectionContext.ConnectionId); - KestrelEventSource.Log.ConnectionStop(connectionContext); } } From 64127e6c766b221cf147383c16079d3b7aad2ded Mon Sep 17 00:00:00 2001 From: John Luo Date: Mon, 20 Aug 2018 21:13:37 -0700 Subject: [PATCH 5/5] Implement MaxFrameSize and HeaderTableSize for HTTP/2 --- src/Kestrel.Core/CoreStrings.resx | 3 + src/Kestrel.Core/Http2Limits.cs | 48 ++++++++ .../Internal/Http2/Http2Connection.cs | 22 ++-- .../Internal/Http2/Http2Frame.Continuation.cs | 2 +- .../Internal/Http2/Http2Frame.Data.cs | 2 +- .../Internal/Http2/Http2Frame.Headers.cs | 2 +- src/Kestrel.Core/Internal/Http2/Http2Frame.cs | 12 +- .../Internal/Http2/Http2FrameWriter.cs | 14 ++- .../Internal/Http2/Http2PeerSettings.cs | 14 ++- .../Properties/CoreStrings.Designer.cs | 14 +++ .../KestrelServerLimitsTests.cs | 31 ++++++ .../Http2/Http2ConnectionTests.cs | 104 ++++++++++++++++-- .../Http2/Http2TestBase.cs | 82 +++++++------- .../Http2/TlsTests.cs | 2 +- 14 files changed, 280 insertions(+), 72 deletions(-) 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/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/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/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) {