diff --git a/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs b/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs index 50215618de22..05f6a5e9fa55 100644 --- a/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs +++ b/src/Servers/IIS/IIS/test/Common.LongTests/ShutdownTests.cs @@ -538,8 +538,15 @@ public async Task ClosesConnectionOnServerAbortOutOfProcess() var response = await deploymentResult.HttpClient.GetAsync("/Abort").TimeoutAfter(TimeoutExtensions.DefaultTimeoutValue); Assert.Equal(HttpStatusCode.BadGateway, response.StatusCode); + +#if NEWSHIM_FUNCTIONALS + // In-proc SocketConnection isn't used and there's no abort // 0x80072f78 ERROR_HTTP_INVALID_SERVER_RESPONSE The server returned an invalid or unrecognized response Assert.Contains("0x80072f78", await response.Content.ReadAsStringAsync()); +#else + // 0x80072efe ERROR_INTERNET_CONNECTION_ABORTED The connection with the server was terminated abnormally + Assert.Contains("0x80072efe", await response.Content.ReadAsStringAsync()); +#endif } catch (HttpRequestException) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 9de5568317a8..8d462478853f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -95,7 +95,14 @@ protected override void OnRequestProcessingEnded() _http1Output.Dispose(); } - public void OnInputOrOutputCompleted() + void IRequestProcessor.OnInputOrOutputCompleted() + { + // Closed gracefully. + _http1Output.Abort(ServerOptions.FinOnError ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!); + CancelRequestAbortedToken(); + } + + void IHttpOutputAborter.OnInputOrOutputCompleted() { _http1Output.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient)); CancelRequestAbortedToken(); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs index f0d34685e004..dbdf2a3f5fc2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs @@ -226,7 +226,7 @@ protected void ThrowUnexpectedEndOfRequestContent() // so we call OnInputOrOutputCompleted() now to prevent a race in our tests where a 400 // response is written after observing the unexpected end of request content instead of just // closing the connection without a response as expected. - _context.OnInputOrOutputCompleted(); + ((IHttpOutputAborter)_context).OnInputOrOutputCompleted(); KestrelBadHttpRequestException.Throw(RequestRejectionReason.UnexpectedEndOfRequestContent); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index eee6ad18ebda..43acb5ad0e8c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -161,7 +161,8 @@ public Http2Connection(HttpConnectionContext context) public void OnInputOrOutputCompleted() { TryClose(); - _frameWriter.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient)); + var useException = _context.ServiceContext.ServerOptions.FinOnError || _clientActiveStreamCount != 0; + _frameWriter.Abort(useException ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!); } public void Abort(ConnectionAbortedException ex) diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index c9e8db2f7a93..175c208efc9e 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -27,10 +27,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core; public class KestrelServerOptions { internal const string DisableHttp1LineFeedTerminatorsSwitchKey = "Microsoft.AspNetCore.Server.Kestrel.DisableHttp1LineFeedTerminators"; + private const string FinOnErrorSwitch = "Microsoft.AspNetCore.Server.Kestrel.FinOnError"; + private static readonly bool _finOnError; + + static KestrelServerOptions() + { + AppContext.TryGetSwitch(FinOnErrorSwitch, out _finOnError); + } // internal to fast-path header decoding when RequestHeaderEncodingSelector is unchanged. internal static readonly Func DefaultHeaderEncodingSelector = _ => null; + // Opt-out flag for back compat. Remove in 9.0 (or make public). + internal bool FinOnError { get; set; } = _finOnError; + private Func _requestHeaderEncodingSelector = DefaultHeaderEncodingSelector; private Func _responseHeaderEncodingSelector = DefaultHeaderEncodingSelector; diff --git a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs index d77422629dfe..f60b8ca9a206 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs @@ -29,6 +29,7 @@ internal sealed partial class SocketConnection : TransportConnection private readonly TaskCompletionSource _waitForConnectionClosedTcs = new TaskCompletionSource(); private bool _connectionClosed; private readonly bool _waitForData; + private readonly bool _finOnError; internal SocketConnection(Socket socket, MemoryPool memoryPool, @@ -37,7 +38,8 @@ internal SocketConnection(Socket socket, SocketSenderPool socketSenderPool, PipeOptions inputOptions, PipeOptions outputOptions, - bool waitForData = true) + bool waitForData = true, + bool finOnError = false) { Debug.Assert(socket != null); Debug.Assert(memoryPool != null); @@ -48,6 +50,7 @@ internal SocketConnection(Socket socket, _logger = logger; _waitForData = waitForData; _socketSenderPool = socketSenderPool; + _finOnError = finOnError; LocalEndPoint = _socket.LocalEndPoint; RemoteEndPoint = _socket.RemoteEndPoint; @@ -376,11 +379,21 @@ private void Shutdown(Exception? shutdownReason) // ever observe this ConnectionAbortedException except for connection middleware attempting // to half close the connection which is currently unsupported. The message is always logged though. _shutdownReason = shutdownReason ?? new ConnectionAbortedException("The Socket transport's send loop completed gracefully."); + + // NB: not _shutdownReason since we don't want to do this on graceful completion + if (!_finOnError && shutdownReason is not null) + { + SocketsLog.ConnectionWriteRst(_logger, this, shutdownReason.Message); + + // This forces an abortive close with linger time 0 (and implies Dispose) + _socket.Close(timeout: 0); + return; + } + SocketsLog.ConnectionWriteFin(_logger, this, _shutdownReason.Message); try { - // Try to gracefully close the socket even for aborts to match libuv behavior. _socket.Shutdown(SocketShutdown.Both); } catch diff --git a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketsLog.cs b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketsLog.cs index 7d5c791fc98f..e0cdbf77e5c1 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketsLog.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketsLog.cs @@ -31,6 +31,17 @@ public static void ConnectionWriteFin(ILogger logger, SocketConnection connectio } } + [LoggerMessage(8, LogLevel.Debug, @"Connection id ""{ConnectionId}"" sending RST because: ""{Reason}""", EventName = "ConnectionWriteRst", SkipEnabledCheck = true)] + private static partial void ConnectionWriteRstCore(ILogger logger, string connectionId, string reason); + + public static void ConnectionWriteRst(ILogger logger, SocketConnection connection, string reason) + { + if (logger.IsEnabled(LogLevel.Debug)) + { + ConnectionWriteRstCore(logger, connection.ConnectionId, reason); + } + } + // Reserved: Event ID 11, EventName = ConnectionWrite // Reserved: Event ID 12, EventName = ConnectionWriteCallback diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionContextFactory.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionContextFactory.cs index fe1621a2c800..db98e36842e1 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionContextFactory.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionContextFactory.cs @@ -96,7 +96,8 @@ public ConnectionContext Create(Socket socket) setting.SocketSenderPool, setting.InputOptions, setting.OutputOptions, - waitForData: _options.WaitForDataBeforeAllocatingBuffer); + waitForData: _options.WaitForDataBeforeAllocatingBuffer, + finOnError: _options.FinOnError); connection.Start(); return connection; diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionFactoryOptions.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionFactoryOptions.cs index 69d524adf41e..35ce2bd8fd75 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionFactoryOptions.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionFactoryOptions.cs @@ -23,8 +23,12 @@ internal SocketConnectionFactoryOptions(SocketTransportOptions transportOptions) MaxWriteBufferSize = transportOptions.MaxWriteBufferSize; UnsafePreferInlineScheduling = transportOptions.UnsafePreferInlineScheduling; MemoryPoolFactory = transportOptions.MemoryPoolFactory; + FinOnError = transportOptions.FinOnError; } + // Opt-out flag for back compat. Remove in 9.0 (or make public). + internal bool FinOnError { get; set; } + /// /// The number of I/O queues used to process requests. Set to 0 to directly schedule I/O to the ThreadPool. /// diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs index e89e8ca51d07..a307465447ad 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs @@ -13,6 +13,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets; /// public class SocketTransportOptions { + private const string FinOnErrorSwitch = "Microsoft.AspNetCore.Server.Kestrel.FinOnError"; + private static readonly bool _finOnError; + + static SocketTransportOptions() + { + AppContext.TryGetSwitch(FinOnErrorSwitch, out _finOnError); + } + + // Opt-out flag for back compat. Remove in 9.0 (or make public). + internal bool FinOnError { get; set; } = _finOnError; + /// /// The number of I/O queues used to process requests. Set to 0 to directly schedule I/O to the ThreadPool. /// diff --git a/src/Servers/Kestrel/shared/test/StreamBackedTestConnection.cs b/src/Servers/Kestrel/shared/test/StreamBackedTestConnection.cs index 9edcd7e5c253..787e7ff4bdc7 100644 --- a/src/Servers/Kestrel/shared/test/StreamBackedTestConnection.cs +++ b/src/Servers/Kestrel/shared/test/StreamBackedTestConnection.cs @@ -138,7 +138,7 @@ public async Task ReceiveEnd(params string[] lines) public async Task WaitForConnectionClose() { var buffer = new byte[128]; - var bytesTransferred = await _stream.ReadAsync(buffer, 0, 128).TimeoutAfter(Timeout); + var bytesTransferred = await _stream.ReadAsync(buffer, 0, 128).ContinueWith(t => t.IsFaulted ? 0 : t.Result).TimeoutAfter(Timeout); if (bytesTransferred > 0) { diff --git a/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs b/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs index 27c6eac5b731..3f370af3fec9 100644 --- a/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs +++ b/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs @@ -48,7 +48,7 @@ public TestServer(RequestDelegate app, TestServiceContext context, ListenOptions { } - public TestServer(RequestDelegate app, TestServiceContext context, Action configureListenOptions) + public TestServer(RequestDelegate app, TestServiceContext context, Action configureListenOptions, Action configureServices = null) : this(app, context, options => { var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) @@ -57,7 +57,10 @@ public TestServer(RequestDelegate app, TestServiceContext context, Action { }) + }, s => + { + configureServices?.Invoke(s); + }) { } diff --git a/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs b/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs index 839feecb47f7..c0b354254419 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs @@ -44,6 +44,60 @@ public ShutdownTests() }; } + [ConditionalFact] + public async Task ConnectionClosedWithoutActiveRequestsOrGoAwayFIN() + { + var connectionClosed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var readFin = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var writeFin = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + TestSink.MessageLogged += context => + { + + if (context.EventId.Name == "Http2ConnectionClosed") + { + connectionClosed.SetResult(); + } + else if (context.EventId.Name == "ConnectionReadFin") + { + readFin.SetResult(); + } + else if (context.EventId.Name == "ConnectionWriteFin") + { + writeFin.SetResult(); + } + }; + + var testContext = new TestServiceContext(LoggerFactory); + + testContext.InitializeHeartbeat(); + + await using (var server = new TestServer(context => + { + return context.Response.WriteAsync("hello world " + context.Request.Protocol); + }, + testContext, + kestrelOptions => + { + kestrelOptions.Listen(IPAddress.Loopback, 0, listenOptions => + { + listenOptions.Protocols = HttpProtocols.Http2; + listenOptions.UseHttps(_x509Certificate2); + }); + })) + { + var response = await Client.GetStringAsync($"https://localhost:{server.Port}/"); + Assert.Equal("hello world HTTP/2", response); + Client.Dispose(); // Close the socket, no GoAway is sent. + + await readFin.Task.DefaultTimeout(); + await writeFin.Task.DefaultTimeout(); + await connectionClosed.Task.DefaultTimeout(); + + await server.StopAsync(); + } + } + [CollectDump] [ConditionalFact] public async Task GracefulShutdownWaitsForRequestsToFinish() diff --git a/src/Servers/Kestrel/test/FunctionalTests/RequestTests.cs b/src/Servers/Kestrel/test/FunctionalTests/RequestTests.cs index 93e1223b23ea..97a8b472130d 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/RequestTests.cs @@ -21,7 +21,9 @@ using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.FunctionalTests; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets; using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Moq; @@ -38,6 +40,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests; public class RequestTests : LoggedTest { private const int _connectionStartedEventId = 1; + private const int _connectionReadFinEventId = 6; private const int _connectionResetEventId = 19; private static readonly int _semaphoreWaitTimeout = Debugger.IsAttached ? 10000 : 2500; @@ -232,6 +235,58 @@ await connection2.Receive($"HTTP/1.1 200 OK", } } + [Fact] + public async Task ConnectionClosedPriorToRequestIsLoggedAsDebug() + { + var connectionStarted = new SemaphoreSlim(0); + var connectionReadFin = new SemaphoreSlim(0); + var loggedHigherThanDebug = false; + + TestSink.MessageLogged += context => + { + if (context.LoggerName != "Microsoft.AspNetCore.Server.Kestrel" && + context.LoggerName != "Microsoft.AspNetCore.Server.Kestrel.Connections" && + context.LoggerName != "Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets") + { + return; + } + + if (context.EventId.Id == _connectionStartedEventId) + { + connectionStarted.Release(); + } + else if (context.EventId.Id == _connectionReadFinEventId) + { + connectionReadFin.Release(); + } + + if (context.LogLevel > LogLevel.Debug) + { + loggedHigherThanDebug = true; + } + }; + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + // Wait until connection is established + Assert.True(await connectionStarted.WaitAsync(TestConstants.DefaultTimeout)); + + connection.ShutdownSend(); + + // If the reset is correctly logged as Debug, the wait below should complete shortly. + // This check MUST come before disposing the server, otherwise there's a race where the RST + // is still in flight when the connection is aborted, leading to the reset never being received + // and therefore not logged. + Assert.True(await connectionReadFin.WaitAsync(TestConstants.DefaultTimeout)); + await connection.ReceiveEnd(); + } + } + + Assert.False(loggedHigherThanDebug); + } + [Fact] public async Task ConnectionResetPriorToRequestIsLoggedAsDebug() { @@ -283,6 +338,65 @@ public async Task ConnectionResetPriorToRequestIsLoggedAsDebug() Assert.False(loggedHigherThanDebug); } + [Fact] + public async Task ConnectionClosedBetweenRequestsIsLoggedAsDebug() + { + var connectionReadFin = new SemaphoreSlim(0); + var loggedHigherThanDebug = false; + + TestSink.MessageLogged += context => + { + if (context.LoggerName != "Microsoft.AspNetCore.Server.Kestrel" && + context.LoggerName != "Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets") + { + return; + } + + if (context.LogLevel > LogLevel.Debug) + { + loggedHigherThanDebug = true; + } + + if (context.EventId.Id == _connectionReadFinEventId) + { + connectionReadFin.Release(); + } + }; + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + + // Make sure the response is fully received, so a write failure (e.g. EPIPE) doesn't cause + // a more critical log message. + await connection.Receive( + "HTTP/1.1 200 OK", + "Content-Length: 0", + $"Date: {server.Context.DateHeaderValue}", + "", + ""); + + connection.ShutdownSend(); + + // If the reset is correctly logged as Debug, the wait below should complete shortly. + // This check MUST come before disposing the server, otherwise there's a race where the RST + // is still in flight when the connection is aborted, leading to the reset never being received + // and therefore not logged. + Assert.True(await connectionReadFin.WaitAsync(TestConstants.DefaultTimeout)); + + await connection.ReceiveEnd(); + } + } + + Assert.False(loggedHigherThanDebug); + } + [Fact] public async Task ConnectionResetBetweenRequestsIsLoggedAsDebug() { @@ -341,10 +455,13 @@ await connection.Receive( Assert.False(loggedHigherThanDebug); } - [Fact] - public async Task ConnectionResetMidRequestIsLoggedAsDebug() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ConnectionClosedOrResetMidRequestIsLoggedAsDebug(bool close) { var requestStarted = new SemaphoreSlim(0); + var connectionReadFin = new SemaphoreSlim(0); var connectionReset = new SemaphoreSlim(0); var connectionClosing = new SemaphoreSlim(0); var loggedHigherThanDebug = false; @@ -362,6 +479,11 @@ public async Task ConnectionResetMidRequestIsLoggedAsDebug() loggedHigherThanDebug = true; } + if (context.EventId.Id == _connectionReadFinEventId) + { + connectionReadFin.Release(); + } + if (context.EventId.Id == _connectionResetEventId) { connectionReset.Release(); @@ -382,15 +504,23 @@ public async Task ConnectionResetMidRequestIsLoggedAsDebug() // Wait until connection is established Assert.True(await requestStarted.WaitAsync(TestConstants.DefaultTimeout), "request should have started"); - connection.Reset(); - } + if (close) + { + connection.ShutdownSend(); + Assert.True(await connectionReadFin.WaitAsync(TestConstants.DefaultTimeout), "Connection close event should have been logged"); + } + else + { + connection.Reset(); - // If the reset is correctly logged as Debug, the wait below should complete shortly. - // This check MUST come before disposing the server, otherwise there's a race where the RST - // is still in flight when the connection is aborted, leading to the reset never being received - // and therefore not logged. - Assert.True(await connectionReset.WaitAsync(TestConstants.DefaultTimeout), "Connection reset event should have been logged"); - connectionClosing.Release(); + // If the reset is correctly logged as Debug, the wait below should complete shortly. + // This check MUST come before disposing the server, otherwise there's a race where the RST + // is still in flight when the connection is aborted, leading to the reset never being received + // and therefore not logged. + Assert.True(await connectionReset.WaitAsync(TestConstants.DefaultTimeout), "Connection reset event should have been logged"); + } + connectionClosing.Release(); + } } Assert.False(loggedHigherThanDebug, "Logged event should not have been higher than debug."); @@ -534,14 +664,23 @@ await connection1.Send( Assert.Equal(beforeAbort.Value, afterAbort.Value); } - [Fact] - public async Task AbortingTheConnectionSendsFIN() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task AbortingTheConnection(bool fin) { var builder = TransportSelector.GetHostBuilder() .ConfigureWebHost(webHostBuilder => { webHostBuilder - .UseKestrel() + .UseSockets(options => + { + options.FinOnError = fin; + }) + .UseKestrel(o => + { + o.FinOnError = fin; + }) .UseUrls("http://127.0.0.1:0") .Configure(app => app.Run(context => { @@ -559,8 +698,15 @@ public async Task AbortingTheConnectionSendsFIN() { socket.Connect(new IPEndPoint(IPAddress.Loopback, host.GetPort())); socket.Send(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\nHost:\r\n\r\n")); - int result = socket.Receive(new byte[32]); - Assert.Equal(0, result); + if (fin) + { + int result = socket.Receive(new byte[32]); + Assert.Equal(0, result); + } + else + { + Assert.Throws(() => socket.Receive(new byte[32])); + } } await host.StopAsync(); @@ -770,16 +916,21 @@ await connection.Send("POST / HTTP/1.1", } [Theory] - [MemberData(nameof(ConnectionMiddlewareDataName))] - public async Task ServerCanAbortConnectionAfterUnobservedClose(string listenOptionsName) + [InlineData("Loopback", true)] + [InlineData("Loopback", false)] + [InlineData("PassThrough", true)] + [InlineData("PassThrough", false)] + public async Task ServerCanAbortConnectionAfterUnobservedClose(string listenOptionsName, bool fin) { const int connectionPausedEventId = 4; const int connectionFinSentEventId = 7; + const int connectionRstSentEventId = 8; const int maxRequestBufferSize = 4096; var readCallbackUnwired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var clientClosedConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var serverClosedConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var serverFinConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var serverRstConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var appFuncCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); TestSink.MessageLogged += context => @@ -795,7 +946,11 @@ public async Task ServerCanAbortConnectionAfterUnobservedClose(string listenOpti } else if (context.EventId == connectionFinSentEventId) { - serverClosedConnection.SetResult(); + serverFinConnection.SetResult(); + } + else if (context.EventId == connectionRstSentEventId) + { + serverRstConnection.SetResult(); } }; @@ -803,6 +958,7 @@ public async Task ServerCanAbortConnectionAfterUnobservedClose(string listenOpti { ServerOptions = { + FinOnError = fin, Limits = { MaxRequestBufferSize = maxRequestBufferSize, @@ -820,10 +976,30 @@ public async Task ServerCanAbortConnectionAfterUnobservedClose(string listenOpti context.Abort(); - await serverClosedConnection.Task; + if (fin) + { + await serverFinConnection.Task.DefaultTimeout(); + } + else + { + await serverRstConnection.Task.DefaultTimeout(); + } appFuncCompleted.SetResult(); - }, testContext, ConnectionMiddlewareData[listenOptionsName]())) + }, testContext, listen => + { + if (listenOptionsName == "PassThrough") + { + listen.UsePassThrough(); + } + }, + services => + { + services.Configure(options => + { + options.FinOnError = fin; + }); + })) { using (var connection = server.CreateConnection()) { @@ -949,21 +1125,21 @@ await context.Response.WriteAsync(JsonConvert.SerializeObject(new private static async Task AssertStreamContains(Stream stream, string expectedSubstring) { var expectedBytes = Encoding.ASCII.GetBytes(expectedSubstring); - var exptectedLength = expectedBytes.Length; - var responseBuffer = new byte[exptectedLength]; + var expectedLength = expectedBytes.Length; + var responseBuffer = new byte[expectedLength]; var matchedChars = 0; - while (matchedChars < exptectedLength) + while (matchedChars < expectedLength) { - var count = await stream.ReadAsync(responseBuffer, 0, exptectedLength - matchedChars).DefaultTimeout(); + var count = await stream.ReadAsync(responseBuffer, 0, expectedLength - matchedChars).DefaultTimeout(); if (count == 0) { Assert.True(false, "Stream completed without expected substring."); } - for (var i = 0; i < count && matchedChars < exptectedLength; i++) + for (var i = 0; i < count && matchedChars < expectedLength; i++) { if (responseBuffer[i] == expectedBytes[matchedChars]) { diff --git a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs index 50c67b81a968..7f301c7cbe52 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs @@ -22,7 +22,9 @@ using Microsoft.AspNetCore.Server.Kestrel.FunctionalTests; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets; using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; @@ -452,8 +454,10 @@ await connection.Send( Assert.Empty(coreLogs.Where(w => w.LogLevel > LogLevel.Information)); } - [Fact] - public async Task ConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate(bool fin) { var logger = LoggerFactory.CreateLogger($"{ typeof(ResponseTests).FullName}.{ nameof(ConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate)}"); const int chunkSize = 1024; @@ -463,18 +467,27 @@ public async Task ConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate() var responseRateTimeoutMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var connectionStopMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectionWriteFinMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectionWriteRstMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var requestAborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var appFuncCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); TestSink.MessageLogged += context => { - if (context.EventId.Name == "ResponseMinimumDataRateNotSatisfied") - { - responseRateTimeoutMessageLogged.SetResult(); - } - if (context.EventId.Name == "ConnectionStop") + switch (context.EventId.Name) { - connectionStopMessageLogged.SetResult(); + case "ResponseMinimumDataRateNotSatisfied": + responseRateTimeoutMessageLogged.SetResult(); + break; + case "ConnectionStop": + connectionStopMessageLogged.SetResult(); + break; + case "ConnectionWriteFin": + connectionWriteFinMessageLogged.SetResult(); + break; + case "ConnectionWriteRst": + connectionWriteRstMessageLogged.SetResult(); + break; } }; @@ -482,6 +495,7 @@ public async Task ConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate() { ServerOptions = { + FinOnError = fin, Limits = { MinResponseDataRate = new MinDataRate(bytesPerSecond: 1024 * 1024, gracePeriod: TimeSpan.FromSeconds(2)) @@ -527,7 +541,14 @@ async Task App(HttpContext context) } } - await using (var server = new TestServer(App, testContext)) + await using (var server = new TestServer(App, testContext, configureListenOptions: _ => { }, + services => + { + services.Configure(o => + { + o.FinOnError = fin; + }); + })) { using (var connection = server.CreateConnection()) { @@ -547,6 +568,14 @@ await connection.Send( await requestAborted.Task.DefaultTimeout(TimeSpan.FromSeconds(30)); await responseRateTimeoutMessageLogged.Task.DefaultTimeout(); await connectionStopMessageLogged.Task.DefaultTimeout(); + if (fin) + { + await connectionWriteFinMessageLogged.Task.DefaultTimeout(); + } + else + { + await connectionWriteRstMessageLogged.Task.DefaultTimeout(); + } await appFuncCompleted.Task.DefaultTimeout(); await AssertStreamAborted(connection.Stream, chunkSize * chunks); @@ -556,8 +585,10 @@ await connection.Send( } } - [Fact] - public async Task HttpsConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task HttpsConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate(bool fin) { const int chunkSize = 1024; const int chunks = 256 * 1024; @@ -567,18 +598,27 @@ public async Task HttpsConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate var responseRateTimeoutMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var connectionStopMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectionWriteFinMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectionWriteRstMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var appFuncCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); TestSink.MessageLogged += context => { - if (context.EventId.Name == "ResponseMinimumDataRateNotSatisfied") - { - responseRateTimeoutMessageLogged.SetResult(); - } - if (context.EventId.Name == "ConnectionStop") + switch (context.EventId.Name) { - connectionStopMessageLogged.SetResult(); + case "ResponseMinimumDataRateNotSatisfied": + responseRateTimeoutMessageLogged.SetResult(); + break; + case "ConnectionStop": + connectionStopMessageLogged.SetResult(); + break; + case "ConnectionWriteFin": + connectionWriteFinMessageLogged.SetResult(); + break; + case "ConnectionWriteRst": + connectionWriteRstMessageLogged.SetResult(); + break; } }; @@ -586,6 +626,7 @@ public async Task HttpsConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate { ServerOptions = { + FinOnError = fin, Limits = { MinResponseDataRate = new MinDataRate(bytesPerSecond: 1024 * 1024, gracePeriod: TimeSpan.FromSeconds(2)) @@ -625,7 +666,14 @@ void ConfigureListenOptions(ListenOptions listenOptions) { await aborted.Task.DefaultTimeout(); } - }, testContext, ConfigureListenOptions)) + }, testContext, ConfigureListenOptions, + services => + { + services.Configure(o => + { + o.FinOnError = fin; + }); + })) { using (var connection = server.CreateConnection()) { @@ -640,6 +688,14 @@ void ConfigureListenOptions(ListenOptions listenOptions) await aborted.Task.DefaultTimeout(TimeSpan.FromSeconds(30)); await responseRateTimeoutMessageLogged.Task.DefaultTimeout(); await connectionStopMessageLogged.Task.DefaultTimeout(); + if (fin) + { + await connectionWriteFinMessageLogged.Task.DefaultTimeout(); + } + else + { + await connectionWriteRstMessageLogged.Task.DefaultTimeout(); + } await appFuncCompleted.Task.DefaultTimeout(); await AssertStreamAborted(connection.Stream, chunkSize * chunks); @@ -648,8 +704,10 @@ void ConfigureListenOptions(ListenOptions listenOptions) } } - [Fact] - public async Task ConnectionClosedWhenBothRequestAndResponseExperienceBackPressure() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ConnectionClosedWhenBothRequestAndResponseExperienceBackPressure(bool fin) { const int bufferSize = 65536; const int bufferCount = 100; @@ -658,18 +716,27 @@ public async Task ConnectionClosedWhenBothRequestAndResponseExperienceBackPressu var responseRateTimeoutMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var connectionStopMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectionWriteFinMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectionWriteRstMessageLogged = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var requestAborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var copyToAsyncCts = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); TestSink.MessageLogged += context => { - if (context.EventId.Name == "ResponseMinimumDataRateNotSatisfied") - { - responseRateTimeoutMessageLogged.SetResult(); - } - if (context.EventId.Name == "ConnectionStop") + switch (context.EventId.Name) { - connectionStopMessageLogged.SetResult(); + case "ResponseMinimumDataRateNotSatisfied": + responseRateTimeoutMessageLogged.SetResult(); + break; + case "ConnectionStop": + connectionStopMessageLogged.SetResult(); + break; + case "ConnectionWriteFin": + connectionWriteFinMessageLogged.SetResult(); + break; + case "ConnectionWriteRst": + connectionWriteRstMessageLogged.SetResult(); + break; } }; @@ -677,6 +744,7 @@ public async Task ConnectionClosedWhenBothRequestAndResponseExperienceBackPressu { ServerOptions = { + FinOnError = fin, Limits = { MinResponseDataRate = new MinDataRate(bytesPerSecond: 1024 * 1024, gracePeriod: TimeSpan.FromSeconds(2)), @@ -687,8 +755,6 @@ public async Task ConnectionClosedWhenBothRequestAndResponseExperienceBackPressu testContext.InitializeHeartbeat(); - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)); - async Task App(HttpContext context) { context.RequestAborted.Register(() => @@ -713,7 +779,14 @@ async Task App(HttpContext context) copyToAsyncCts.SetException(new Exception("This shouldn't be reached.")); } - await using (var server = new TestServer(App, testContext, listenOptions)) + await using (var server = new TestServer(App, testContext, configureListenOptions: _ => { }, + services => + { + services.Configure(o => + { + o.FinOnError = fin; + }); + })) { using (var connection = server.CreateConnection()) { @@ -738,6 +811,14 @@ await connection.Send( await requestAborted.Task.DefaultTimeout(TimeSpan.FromSeconds(30)); await responseRateTimeoutMessageLogged.Task.DefaultTimeout(); await connectionStopMessageLogged.Task.DefaultTimeout(); + if (fin) + { + await connectionWriteFinMessageLogged.Task.DefaultTimeout(); + } + else + { + await connectionWriteRstMessageLogged.Task.DefaultTimeout(); + } // Expect OperationCanceledException instead of IOException because the server initiated the abort due to a response rate timeout. await Assert.ThrowsAnyAsync(() => copyToAsyncCts.Task).DefaultTimeout();