diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index 635d4e3e49..b17228eec4 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -587,4 +587,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l The client closed the connection. + + A frame of type {frameType} was received after stream {streamId} was reset or aborted. + \ No newline at end of file diff --git a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs index f0c90a4388..780ede5a35 100644 --- a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs +++ b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs @@ -82,12 +82,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } finally { + Log.ConnectionStop(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStop(connectionContext); + connection.Complete(); _serviceContext.ConnectionManager.RemoveConnection(id); - - Log.ConnectionStop(connectionContext.ConnectionId); - KestrelEventSource.Log.ConnectionStop(connectionContext); } } diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index 07b3be55de..ebcd74a0b2 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -39,12 +39,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private Stack, object>> _onStarting; private Stack, object>> _onCompleted; - private volatile int _ioCompleted; + private object _abortLock = new object(); + private volatile bool _requestAborted; + private bool _preventRequestAbortedCancellation; private CancellationTokenSource _abortedCts; private CancellationToken? _manuallySetRequestAbortToken; protected RequestProcessingStatus _requestProcessingStatus; - protected volatile bool _keepAlive; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx + + // Keep-alive is default for HTTP/1.1 and HTTP/2; parsing and errors will change its value + // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx + protected volatile bool _keepAlive = true; protected bool _upgradeAvailable; private bool _canHaveBody; private bool _autoChunk; @@ -235,16 +240,26 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { return _manuallySetRequestAbortToken.Value; } - // Otherwise, get the abort CTS. If we have one, which would mean that someone previously - // asked for the RequestAborted token, simply return its token. If we don't, - // check to see whether we've already aborted, in which case just return an - // already canceled token. Finally, force a source into existence if we still - // don't have one, and return its token. - var cts = _abortedCts; - return - cts != null ? cts.Token : - (_ioCompleted == 1) ? new CancellationToken(true) : - RequestAbortedSource.Token; + + lock (_abortLock) + { + if (_preventRequestAbortedCancellation) + { + return new CancellationToken(false); + } + + if (_requestAborted) + { + return new CancellationToken(true); + } + + if (_abortedCts == null) + { + _abortedCts = new CancellationTokenSource(); + } + + return _abortedCts.Token; + } } set { @@ -254,28 +269,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } - private CancellationTokenSource RequestAbortedSource - { - get - { - // Get the abort token, lazily-initializing it if necessary. - // Make sure it's canceled if an abort request already came in. - - // EnsureInitialized can return null since _abortedCts is reset to null - // after it's already been initialized to a non-null value. - // If EnsureInitialized does return null, this property was accessed between - // requests so it's safe to return an ephemeral CancellationTokenSource. - var cts = LazyInitializer.EnsureInitialized(ref _abortedCts, () => new CancellationTokenSource()) - ?? new CancellationTokenSource(); - - if (_ioCompleted == 1) - { - cts.Cancel(); - } - return cts; - } - } - public bool HasResponseStarted => _requestProcessingStatus == RequestProcessingStatus.ResponseStarted; protected HttpRequestHeaders HttpRequestHeaders { get; } = new HttpRequestHeaders(); @@ -354,9 +347,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http Scheme = _scheme; _manuallySetRequestAbortToken = null; - _abortedCts = null; + _preventRequestAbortedCancellation = false; + + // Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS. + lock (_abortLock) + { + if (!_requestAborted) + { + _abortedCts?.Dispose(); + _abortedCts = null; + } + } - // Allow two bytes for \r\n after headers _requestHeadersParsed = 0; _responseBytesWritten = 0; @@ -401,7 +403,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { try { - RequestAbortedSource.Cancel(); + _abortedCts.Cancel(); + _abortedCts.Dispose(); _abortedCts = null; } catch (Exception ex) @@ -412,15 +415,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected void AbortRequest() { - if (Interlocked.Exchange(ref _ioCompleted, 1) != 0) + lock (_abortLock) { - return; + if (_requestAborted) + { + return; + } + + _requestAborted = true; } - _keepAlive = false; - - // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. - ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); + if (_abortedCts != null) + { + // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. + ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); + } } protected void PoisonRequestBodyStream(Exception abortReason) @@ -428,6 +437,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _streams?.Abort(abortReason); } + // Prevents the RequestAborted token from firing for the duration of the request. + private void PreventRequestAbortedCancellation() + { + lock (_abortLock) + { + if (_requestAborted) + { + return; + } + + _preventRequestAbortedCancellation = true; + _abortedCts?.Dispose(); + _abortedCts = null; + } + } + public void OnHeader(Span name, Span value) { _requestHeadersParsed++; @@ -487,9 +512,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private async Task ProcessRequests(IHttpApplication application) { - // Keep-alive is default for HTTP/1.1 and HTTP/2; parsing and errors will change its value - _keepAlive = true; - while (_keepAlive) { BeginRequestProcessing(); @@ -529,7 +551,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // Run the application code for this request await application.ProcessRequestAsync(httpContext); - if (_ioCompleted == 0) + if (!_requestAborted) { VerifyResponseContentLength(); } @@ -565,7 +587,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // 4XX responses are written by TryProduceInvalidRequestResponse during connection tear down. if (_requestRejectedException == null) { - if (_ioCompleted == 0) + if (!_requestAborted) { // Call ProduceEnd() before consuming the rest of the request body to prevent // delaying clients waiting for the chunk terminator: @@ -597,7 +619,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http application.DisposeContext(httpContext, _applicationException); // Even for non-keep-alive requests, try to consume the entire body to avoid RSTs. - if (_ioCompleted == 0 && _requestRejectedException == null && !messageBody.IsEmpty) + if (!_requestAborted && _requestRejectedException == null && !messageBody.IsEmpty) { await messageBody.ConsumeAsync(); } @@ -877,7 +899,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http responseHeaders.ContentLength.HasValue && _responseBytesWritten == responseHeaders.ContentLength.Value) { - _abortedCts = null; + PreventRequestAbortedCancellation(); } } @@ -995,8 +1017,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected Task TryProduceInvalidRequestResponse() { - // If _ioCompleted is set, the connection has already been closed. - if (_requestRejectedException != null && _ioCompleted == 0) + // If _requestAborted is set, the connection has already been closed. + if (_requestRejectedException != null && !_requestAborted) { return ProduceEnd(); } @@ -1073,7 +1095,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private async Task WriteSuffixAwaited() { // For the same reason we call CheckLastWrite() in Content-Length responses. - _abortedCts = null; + PreventRequestAbortedCancellation(); await Output.WriteStreamSuffixAsync(); diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index c8d465f9df..87c6c4ed3b 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -224,6 +224,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 catch (Http2StreamErrorException ex) { Log.Http2StreamError(ConnectionId, ex); + // The client doesn't know this error is coming, allow draining additional frames for now. AbortStream(_incomingFrame.StreamId, new IOException(ex.Message, ex)); await _frameWriter.WriteRstStreamAsync(ex.StreamId, ex.ErrorCode); } @@ -448,6 +449,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream)) { + if (stream.DoNotDrainRequest) + { + // Hard abort, do not allow any more frames on this stream. + throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED); + } if (stream.EndStreamReceived) { // http://httpwg.org/specs/rfc7540.html#rfc.section.5.1 @@ -501,6 +507,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream)) { + if (stream.DoNotDrainRequest) + { + // Hard abort, do not allow any more frames on this stream. + throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED); + } + // http://httpwg.org/specs/rfc7540.html#rfc.section.5.1 // // ...an endpoint that receives any frames after receiving a frame with the @@ -609,7 +621,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } ThrowIfIncomingFrameSentToIdleStream(); - AbortStream(_incomingFrame.StreamId, new IOException(CoreStrings.Http2StreamResetByClient)); + + if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream)) + { + // Second reset + if (stream.DoNotDrainRequest) + { + // Hard abort, do not allow any more frames on this stream. + throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED); + } + + // No additional inbound header or data frames are allowed for this stream after receiving a reset. + stream.DisallowAdditionalRequestFrames(); + stream.Abort(new IOException(CoreStrings.Http2StreamResetByClient)); + } return Task.CompletedTask; } @@ -771,6 +796,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } else if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream)) { + if (stream.DoNotDrainRequest) + { + // Hard abort, do not allow any more frames on this stream. + throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED); + } + if (!stream.TryUpdateOutputWindow(_incomingFrame.WindowUpdateSizeIncrement)) { throw new Http2StreamErrorException(_incomingFrame.StreamId, CoreStrings.Http2ErrorWindowUpdateSizeInvalid, Http2ErrorCode.FLOW_CONTROL_ERROR); diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index 2f5566084d..be7c9b9d09 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -53,6 +53,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public bool RequestBodyStarted { get; private set; } public bool EndStreamReceived => (_completionState & StreamCompletionFlags.EndStreamReceived) == StreamCompletionFlags.EndStreamReceived; private bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted; + internal bool DoNotDrainRequest => (_completionState & StreamCompletionFlags.DoNotDrainRequest) == StreamCompletionFlags.DoNotDrainRequest; public override bool IsUpgradableRequest => false; @@ -381,6 +382,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return _context.FrameWriter.TryUpdateStreamWindow(_outputFlowControl, bytes); } + public void DisallowAdditionalRequestFrames() + { + ApplyCompletionFlag(StreamCompletionFlags.DoNotDrainRequest); + } + public void Abort(IOException abortReason) { var states = ApplyCompletionFlag(StreamCompletionFlags.Aborted); @@ -415,6 +421,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private void ResetAndAbort(ConnectionAbortedException abortReason, Http2ErrorCode error) { + // Future incoming frames will drain for a default grace period to avoid destabilizing the connection. var states = ApplyCompletionFlag(StreamCompletionFlags.Aborted); if (states.OldState == states.NewState) @@ -507,6 +514,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 RequestProcessingEnded = 1, EndStreamReceived = 2, Aborted = 4, + DoNotDrainRequest = 8, } } } diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index d6a4c620d9..7690976935 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -2198,6 +2198,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatConnectionAbortedByClient() => GetString("ConnectionAbortedByClient"); + /// + /// A frame of type {frameType} was received after stream {streamId} was reset or aborted. + /// + internal static string Http2ErrorStreamAborted + { + get => GetString("Http2ErrorStreamAborted"); + } + + /// + /// A frame of type {frameType} was received after stream {streamId} was reset or aborted. + /// + internal static string FormatHttp2ErrorStreamAborted(object frameType, object streamId) + => string.Format(CultureInfo.CurrentCulture, GetString("Http2ErrorStreamAborted", "frameType", "streamId"), frameType, streamId); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs b/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs index db282bf4b2..4906efadf0 100644 --- a/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs @@ -14,8 +14,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal void ConnectionWriteFin(string connectionId); - void ConnectionWroteFin(string connectionId, int status); - void ConnectionWrite(string connectionId, int count); void ConnectionWriteCallback(string connectionId, int status); @@ -28,6 +26,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal void ConnectionResume(string connectionId); - void ConnectionAborted(string connectionId); + void ConnectionAborted(string connectionId, string message); } } diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs index 47f4b112dd..4d01959cda 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs @@ -119,7 +119,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public override void Abort(ConnectionAbortedException abortReason) { + Log.ConnectionAborted(ConnectionId, abortReason?.Message); + _abortReason = abortReason; + + // Cancel WriteOutputAsync loop after setting _abortReason. Output.CancelPendingRead(); // This cancels any pending I/O. diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConstants.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConstants.cs index d1657b63b8..0e07f1a69c 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConstants.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConstants.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool IsConnectionReset(int errno) { - return errno == ECONNRESET || errno == EPIPE || errno == ENOTCONN | errno == EINVAL; + return errno == ECONNRESET || errno == EPIPE || errno == ENOTCONN || errno == EINVAL; } private static int? GetECONNRESET() diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs index 0467c7b776..fa70bd3853 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs @@ -22,9 +22,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal private static readonly Action _connectionWriteFin = LoggerMessage.Define(LogLevel.Debug, new EventId(7, nameof(ConnectionWriteFin)), @"Connection id ""{ConnectionId}"" sending FIN."); - private static readonly Action _connectionWroteFin = - LoggerMessage.Define(LogLevel.Debug, new EventId(8, nameof(ConnectionWroteFin)), @"Connection id ""{ConnectionId}"" sent FIN with status ""{Status}""."); - // ConnectionWrite: Reserved: 11 // ConnectionWriteCallback: Reserved: 12 @@ -35,8 +32,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.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 static readonly Action _connectionAborted = + LoggerMessage.Define(LogLevel.Debug, new EventId(20, nameof(ConnectionAborted)), @"Connection id ""{ConnectionId}"" closing because: ""{Message}"""); private readonly ILogger _logger; @@ -61,11 +58,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal _connectionWriteFin(_logger, connectionId, null); } - public void ConnectionWroteFin(string connectionId, int status) - { - _connectionWroteFin(_logger, connectionId, status, null); - } - public void ConnectionWrite(string connectionId, int count) { // Don't log for now since this could be *too* verbose. @@ -98,9 +90,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal _connectionResume(_logger, connectionId, null); } - public void ConnectionAborted(string connectionId) + public void ConnectionAborted(string connectionId, string message) { - _connectionAborted(_logger, connectionId, null); + _connectionAborted(_logger, connectionId, message, null); } public IDisposable BeginScope(TState state) => _logger.BeginScope(state); diff --git a/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs b/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs index d08408e2b0..585bfcc816 100644 --- a/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs +++ b/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs @@ -20,6 +20,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal void ConnectionResume(string connectionId); - void ConnectionAborted(string connectionId); + void ConnectionAborted(string connectionId, string message); } } diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs index d9cd3887e5..f78cd69584 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs @@ -92,7 +92,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal public override void Abort(ConnectionAbortedException abortReason) { - _trace.ConnectionAborted(ConnectionId); + _trace.ConnectionAborted(ConnectionId, abortReason?.Message); // Try to gracefully close the socket to match libuv behavior. Shutdown(abortReason); diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs b/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs index 5b132ccf9c..53d902312b 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs @@ -22,14 +22,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal private static readonly Action _connectionWriteFin = LoggerMessage.Define(LogLevel.Debug, new EventId(7, nameof(ConnectionWriteFin)), @"Connection id ""{ConnectionId}"" sending FIN."); + // ConnectionWrite: Reserved: 11 + + // ConnectionWriteCallback: Reserved: 12 + private static readonly Action _connectionError = LoggerMessage.Define(LogLevel.Information, new EventId(14, nameof(ConnectionError)), @"Connection id ""{ConnectionId}"" communication error."); 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 static readonly Action _connectionAborted = + LoggerMessage.Define(LogLevel.Debug, new EventId(20, nameof(ConnectionAborted)), @"Connection id ""{ConnectionId}"" closing because: ""{Message}"""); private readonly ILogger _logger; @@ -86,9 +90,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal _connectionResume(_logger, connectionId, null); } - public void ConnectionAborted(string connectionId) + public void ConnectionAborted(string connectionId, string message) { - _connectionAborted(_logger, connectionId, null); + _connectionAborted(_logger, connectionId, message, null); } public IDisposable BeginScope(TState state) => _logger.BeginScope(state); diff --git a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs b/test/Kestrel.Core.Tests/Http1ConnectionTests.cs index f2c665828a..39d29e5e59 100644 --- a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs +++ b/test/Kestrel.Core.Tests/Http1ConnectionTests.cs @@ -640,17 +640,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { _http1Connection.ResponseHeaders["Content-Length"] = "12"; - // Need to compare WaitHandle ref since CancellationToken is struct - var original = _http1Connection.RequestAborted.WaitHandle; + var original = _http1Connection.RequestAborted; foreach (var ch in "hello, worl") { await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)ch })); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); } await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)'d' })); - Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle); + Assert.NotEqual(original, _http1Connection.RequestAborted); + + _http1Connection.Abort(new ConnectionAbortedException()); + + Assert.False(original.IsCancellationRequested); + Assert.False(_http1Connection.RequestAborted.IsCancellationRequested); } [Fact] @@ -658,17 +662,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { _http1Connection.ResponseHeaders["Content-Length"] = "12"; - // Need to compare WaitHandle ref since CancellationToken is struct - var original = _http1Connection.RequestAborted.WaitHandle; + var original = _http1Connection.RequestAborted; foreach (var ch in "hello, worl") { await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)ch }), default(CancellationToken)); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); } await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)'d' }), default(CancellationToken)); - Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle); + Assert.NotEqual(original, _http1Connection.RequestAborted); + + _http1Connection.Abort(new ConnectionAbortedException()); + + Assert.False(original.IsCancellationRequested); + Assert.False(_http1Connection.RequestAborted.IsCancellationRequested); } [Fact] @@ -676,36 +684,44 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { _http1Connection.ResponseHeaders["Content-Length"] = "12"; - // Need to compare WaitHandle ref since CancellationToken is struct - var original = _http1Connection.RequestAborted.WaitHandle; + var original = _http1Connection.RequestAborted; // Only first write can be WriteAsyncAwaited var startingTask = _http1Connection.InitializeResponseAwaited(Task.CompletedTask, 1); await _http1Connection.WriteAsyncAwaited(startingTask, new ArraySegment(new[] { (byte)'h' }), default(CancellationToken)); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); foreach (var ch in "ello, worl") { await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)ch }), default(CancellationToken)); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); } await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)'d' }), default(CancellationToken)); - Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle); + Assert.NotEqual(original, _http1Connection.RequestAborted); + + _http1Connection.Abort(new ConnectionAbortedException()); + + Assert.False(original.IsCancellationRequested); + Assert.False(_http1Connection.RequestAborted.IsCancellationRequested); } [Fact] public async Task RequestAbortedTokenIsResetBeforeLastWriteWithChunkedEncoding() { - // Need to compare WaitHandle ref since CancellationToken is struct - var original = _http1Connection.RequestAborted.WaitHandle; + var original = _http1Connection.RequestAborted; _http1Connection.HttpVersion = "HTTP/1.1"; await _http1Connection.WriteAsync(new ArraySegment(Encoding.ASCII.GetBytes("hello, world")), default(CancellationToken)); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); await _http1Connection.ProduceEndAsync(); - Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle); + Assert.NotEqual(original, _http1Connection.RequestAborted); + + _http1Connection.Abort(new ConnectionAbortedException()); + + Assert.False(original.IsCancellationRequested); + Assert.False(_http1Connection.RequestAborted.IsCancellationRequested); } [Fact] diff --git a/test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs b/test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs index 20c911ac33..39434509a6 100644 --- a/test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs @@ -201,7 +201,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private TestServer CreateServerWithMaxConnections(RequestDelegate app, ResourceCounter concurrentConnectionCounter) { - var serviceContext = new TestServiceContext(LoggerFactory); + var serviceContext = new TestServiceContext(LoggerFactory) + { + ExpectedConnectionMiddlewareCount = 1 + }; + var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)); listenOptions.Use(next => { diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 600def2cd3..bbe3471951 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -2195,6 +2195,101 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.RST_STREAM, streamId: 1, headersStreamId: 1)); } + // Compare to h2spec http2/5.1/8 + [Fact] + public async Task RST_STREAM_IncompleteRequest_AdditionalDataFrames_ConnectionAborted() + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(context => tcs.Task); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[1], endStream: false); + await SendDataAsync(1, new byte[2], endStream: false); + await SendRstStreamAsync(1); + await SendDataAsync(1, new byte[10], endStream: false); + tcs.TrySetResult(0); + + await WaitForConnectionErrorAsync(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1, + Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.DATA, 1)); + } + + [Fact] + public async Task RST_STREAM_IncompleteRequest_AdditionalTrailerFrames_ConnectionAborted() + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(context => tcs.Task); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[1], endStream: false); + await SendDataAsync(1, new byte[2], endStream: false); + await SendRstStreamAsync(1); + await SendHeadersAsync(1, Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM, _requestTrailers); + tcs.TrySetResult(0); + + await WaitForConnectionErrorAsync(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1, + Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.HEADERS, 1)); + } + + [Fact] + public async Task RST_STREAM_IncompleteRequest_AdditionalResetFrame_ConnectionAborted() + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(context => tcs.Task); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[1], endStream: false); + await SendRstStreamAsync(1); + await SendRstStreamAsync(1); + tcs.TrySetResult(0); + + await WaitForConnectionErrorAsync(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1, + Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.RST_STREAM, 1)); + } + + [Fact] + public async Task RST_STREAM_IncompleteRequest_AdditionalWindowUpdateFrame_ConnectionAborted() + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(context => tcs.Task); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[1], endStream: false); + await SendRstStreamAsync(1); + await SendWindowUpdateAsync(1, 1024); + tcs.TrySetResult(0); + + await WaitForConnectionErrorAsync(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1, + Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.WINDOW_UPDATE, 1)); + } + [Fact] public async Task SETTINGS_KestrelDefaults_Sent() { diff --git a/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs b/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs index 37a83df22b..50db85725d 100644 --- a/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs +++ b/test/Kestrel.InMemory.FunctionalTests/TestTransport/InMemoryTransportConnection.cs @@ -8,6 +8,7 @@ using System.Net; using System.Threading; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport { @@ -15,11 +16,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans { private readonly CancellationTokenSource _connectionClosedTokenSource = new CancellationTokenSource(); + private readonly ILogger _logger; private bool _isClosed; - public InMemoryTransportConnection(MemoryPool memoryPool) + public InMemoryTransportConnection(MemoryPool memoryPool, ILogger logger) { MemoryPool = memoryPool; + _logger = logger; LocalAddress = IPAddress.Loopback; RemoteAddress = IPAddress.Loopback; @@ -28,6 +31,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans } public override MemoryPool MemoryPool { get; } + public override PipeScheduler InputWriterScheduler => PipeScheduler.ThreadPool; public override PipeScheduler OutputReaderScheduler => PipeScheduler.ThreadPool; @@ -35,6 +39,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans public override void Abort(ConnectionAbortedException abortReason) { + _logger.LogDebug(@"Connection id ""{ConnectionId}"" closing because: ""{Message}""", ConnectionId, abortReason?.Message); + Input.Complete(abortReason); AbortReason = abortReason; diff --git a/test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs b/test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs index eab6513a53..17be223833 100644 --- a/test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs +++ b/test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; +using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport { @@ -66,7 +67,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans _transportFactory = new InMemoryTransportFactory(); HttpClientSlim = new InMemoryHttpClientSlim(this); - var hostBuilder = new WebHostBuilder() .ConfigureServices(services => { @@ -79,6 +79,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans { context.ServerOptions.ApplicationServices = sp; configureKestrel(context.ServerOptions); + + // Prevent ListenOptions reuse. This is easily done accidentally when trying to debug a test by running it + // in a loop, but will cause problems because only the app func from the first loop will ever be invoked. + Assert.All(context.ServerOptions.ListenOptions, lo => + Assert.Equal(context.ExpectedConnectionMiddlewareCount, lo._middleware.Count)); + return new KestrelServer(_transportFactory, context); }); }); @@ -96,7 +102,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans public InMemoryConnection CreateConnection() { - var transportConnection = new InMemoryTransportConnection(_memoryPool); + var transportConnection = new InMemoryTransportConnection(_memoryPool, Context.Log); _ = HandleConnection(transportConnection); return new InMemoryConnection(transportConnection); } diff --git a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs index 3bdfbd9929..4d9822cae5 100644 --- a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 get { var dataset = new TheoryData(); - var toSkip = new[] { "http2/5.1/8" }; + var toSkip = new string[] { /*"http2/5.1/8"*/ }; foreach (var testcase in H2SpecCommands.EnumerateTestCases()) { diff --git a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs b/test/Kestrel.Transport.FunctionalTests/RequestTests.cs index e3f0b9ebb5..347ea3661f 100644 --- a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/RequestTests.cs @@ -679,6 +679,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests readTcs.SetException(ex); throw; } + finally + { + await registrationTcs.Task.DefaultTimeout(); + } readTcs.SetException(new Exception("This shouldn't be reached.")); } @@ -711,7 +715,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } - await Assert.ThrowsAsync(async () => await readTcs.Task); + await Assert.ThrowsAsync(async () => await readTcs.Task).DefaultTimeout(); // The cancellation token for only the last request should be triggered. var abortedRequestId = await registrationTcs.Task.DefaultTimeout(); diff --git a/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs b/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs index aaa39ba476..a36c0dc89f 100644 --- a/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs @@ -211,6 +211,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests writeTcs.SetException(ex); throw; } + finally + { + await requestAbortedWh.Task.DefaultTimeout(); + } writeTcs.SetException(new Exception("This shouldn't be reached.")); }, new TestServiceContext(LoggerFactory), listenOptions)) @@ -243,7 +247,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests const int connectionPausedEventId = 4; const int maxRequestBufferSize = 4096; - var requestAborted = false; + var requestAborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var readCallbackUnwired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var clientClosedConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var writeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -291,10 +295,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var server = new TestServer(async context => { - context.RequestAborted.Register(() => - { - requestAborted = true; - }); + context.RequestAborted.Register(() => requestAborted.SetResult(null)); await clientClosedConnection.Task; @@ -311,6 +312,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests writeTcs.SetException(ex); throw; } + finally + { + await requestAborted.Task.DefaultTimeout(); + } writeTcs.SetException(new Exception("This shouldn't be reached.")); }, testContext, listenOptions)) @@ -336,7 +341,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); - Assert.True(requestAborted); + Assert.True(requestAborted.Task.IsCompleted); } [Theory] @@ -345,22 +350,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { const int connectionResetEventId = 19; const int connectionFinEventId = 6; - //const int connectionStopEventId = 2; + const int connectionStopEventId = 2; const int responseBodySegmentSize = 65536; const int responseBodySegmentCount = 100; + var requestAborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var appCompletedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var requestAborted = false; var scratchBuffer = new byte[responseBodySegmentSize]; using (var server = new TestServer(async context => { - context.RequestAborted.Register(() => - { - requestAborted = true; - }); + context.RequestAborted.Register(() => requestAborted.SetResult(null)); for (var i = 0; i < responseBodySegmentCount; i++) { @@ -368,6 +370,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await Task.Delay(10); } + await requestAborted.Task.DefaultTimeout(); appCompletedTcs.SetResult(null); }, new TestServiceContext(LoggerFactory), listenOptions)) { @@ -386,9 +389,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests connection.Reset(); } - await appCompletedTcs.Task.DefaultTimeout(); + await requestAborted.Task.DefaultTimeout(); - // After the app is done with the write loop, the connection reset should be logged. + // After the RequestAborted token is tripped, the connection reset should be logged. // On Linux and macOS, the connection close is still sometimes observed as a FIN despite the LingerState. var presShutdownTransportLogs = TestSink.Writes.Where( w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv" || @@ -398,14 +401,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests (!TestPlatformHelper.IsWindows && w.EventId == connectionFinEventId)); Assert.NotEmpty(connectionResetLogs); + + // On macOS, the default 5 shutdown timeout is insufficient for the write loop to complete, so give it extra time. + await appCompletedTcs.Task.DefaultTimeout(); } - // TODO: Figure out what the following assertion is flaky. The server shouldn't shutdown before all - // the connections are closed, yet sometimes the connection stop log isn't observed here. - //var coreLogs = TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel"); - //Assert.Single(coreLogs.Where(w => w.EventId == connectionStopEventId)); - - Assert.True(requestAborted, "RequestAborted token didn't fire."); + var coreLogs = TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel"); + Assert.Single(coreLogs.Where(w => w.EventId == connectionStopEventId)); var transportLogs = TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel" || w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv" || @@ -512,6 +514,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests appFuncCompleted.SetResult(null); throw; } + finally + { + await requestAborted.Task.DefaultTimeout(); + } } using (var server = new TestServer(App, testContext, listenOptions)) @@ -607,6 +613,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests appFuncCompleted.SetResult(null); throw; } + finally + { + await aborted.Task.DefaultTimeout(); + } }, testContext, listenOptions)) { using (var connection = server.CreateConnection()) @@ -682,6 +692,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests copyToAsyncCts.SetException(ex); throw; } + finally + { + await requestAborted.Task.DefaultTimeout(); + } copyToAsyncCts.SetException(new Exception("This shouldn't be reached.")); } @@ -777,12 +791,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var targetBytesPerSecond = chunkSize / 4; await AssertStreamCompleted(connection.Stream, minTotalOutputSize, targetBytesPerSecond); await appFuncCompleted.Task.DefaultTimeout(); - - mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); - Assert.False(requestAborted); } } + + mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + Assert.False(requestAborted); } [Fact] @@ -852,12 +866,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests // Make sure consuming a single set of response headers exceeds the 2 second timeout. var targetBytesPerSecond = responseSize / 4; await AssertStreamCompleted(connection.Stream, minTotalOutputSize, targetBytesPerSecond); - - mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); - Assert.False(requestAborted); } } + + mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + Assert.False(requestAborted); } private async Task AssertStreamAborted(Stream stream, int totalBytes) diff --git a/test/Kestrel.Transport.Libuv.Tests/LibuvConstantsTests.cs b/test/Kestrel.Transport.Libuv.Tests/LibuvConstantsTests.cs new file mode 100644 index 0000000000..6ff5920fdd --- /dev/null +++ b/test/Kestrel.Transport.Libuv.Tests/LibuvConstantsTests.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests +{ + public class LibuvConstantsTests + { + [Fact] + public void IsConnectionResetReturnsTrueForExpectedLibuvErrorConstants() + { + // All the below constants are defined on all supported platforms (Windows, Linux, macOS) + Assert.True(LibuvConstants.IsConnectionReset(LibuvConstants.ECONNRESET.Value)); + Assert.True(LibuvConstants.IsConnectionReset(LibuvConstants.EPIPE.Value)); + Assert.True(LibuvConstants.IsConnectionReset(LibuvConstants.ENOTCONN.Value)); + Assert.True(LibuvConstants.IsConnectionReset(LibuvConstants.EINVAL.Value)); + + // All libuv error constants are negative on all platforms. + Assert.False(LibuvConstants.IsConnectionReset(0)); + } + } +} diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index 5944eece8b..b0532b874d 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -77,6 +77,8 @@ namespace Microsoft.AspNetCore.Testing public Func> MemoryPoolFactory { get; set; } = KestrelMemoryPool.Create; + public int ExpectedConnectionMiddlewareCount { get; set; } + public string DateHeaderValue => DateHeaderValueManager.GetDateHeaderValues().String; } } diff --git a/test/shared/TransportTestHelpers/TestServer.cs b/test/shared/TransportTestHelpers/TestServer.cs index 9d3bb0cf06..276eeaf768 100644 --- a/test/shared/TransportTestHelpers/TestServer.cs +++ b/test/shared/TransportTestHelpers/TestServer.cs @@ -17,6 +17,7 @@ using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { @@ -48,6 +49,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests : this(app, context, options => options.ListenOptions.Add(listenOptions), configureServices) { } + public TestServer(RequestDelegate app, TestServiceContext context, Action configureKestrel) : this(app, context, configureKestrel, _ => { }) { @@ -77,6 +79,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { c.Configure(context.ServerOptions); } + + // Prevent ListenOptions reuse. This is easily done accidentally when trying to debug a test by running it + // in a loop, but will cause problems because only the app func from the first loop will ever be invoked. + Assert.All(context.ServerOptions.ListenOptions, lo => + Assert.Equal(context.ExpectedConnectionMiddlewareCount, lo._middleware.Count)); + return new KestrelServer(sp.GetRequiredService(), context); }); configureServices(services);