From 1e465e9643eee56f90087d08413a6814e5b6f43c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 30 Aug 2017 17:25:16 -0700 Subject: [PATCH] Improve timeout logic --- .../Mocks/MockTimeoutControl.cs | 2 - src/Kestrel.Core/CoreStrings.resx | 6 + .../Features/IConnectionTimeoutFeature.cs | 31 +++ src/Kestrel.Core/Internal/FrameConnection.cs | 80 +++--- src/Kestrel.Core/Internal/Http/Frame.cs | 37 +-- src/Kestrel.Core/Internal/Http/FrameOfT.cs | 237 +++++++++--------- src/Kestrel.Core/Internal/Http/MessageBody.cs | 2 +- .../Internal/Http2/Http2Connection.cs | 2 - ...rameConnectionManagerShutdownExtensions.cs | 2 +- .../Infrastructure/ITimeoutControl.cs | 2 - .../Internal/Infrastructure/TimeoutAction.cs | 5 +- .../Properties/CoreStrings.Designer.cs | 28 +++ .../HttpsConnectionAdapterOptions.cs | 20 ++ src/Kestrel.Https/HttpsStrings.resx | 6 + .../Internal/HttpsConnectionAdapter.cs | 14 ++ .../Properties/HttpsStrings.Designer.cs | 28 +++ .../FrameConnectionTests.cs | 48 ++-- test/Kestrel.Core.Tests/FrameTests.cs | 6 +- .../KestrelServerLimitsTests.cs | 5 - test/Kestrel.Core.Tests/MessageBodyTests.cs | 18 +- .../HttpsConnectionAdapterOptionsTest.cs | 55 ++++ test/Kestrel.FunctionalTests/HttpsTests.cs | 39 +++ 22 files changed, 452 insertions(+), 221 deletions(-) create mode 100644 src/Kestrel.Core/Features/IConnectionTimeoutFeature.cs create mode 100644 test/Kestrel.FunctionalTests/HttpsConnectionAdapterOptionsTest.cs diff --git a/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs b/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs index 24f76b8203..879c86bab6 100644 --- a/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs +++ b/benchmarks/Kestrel.Performance/Mocks/MockTimeoutControl.cs @@ -7,8 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance.Mocks { public class MockTimeoutControl : ITimeoutControl { - public bool TimedOut { get; } - public void CancelTimeout() { } diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index 63f8986ab5..eb0cff90a5 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -345,4 +345,10 @@ Value must be a positive number. To disable a minimum data rate, use null where a MinDataRate instance is expected. + + Concurrent timeouts are not supported. + + + Timespan must be positive and finite. + \ No newline at end of file diff --git a/src/Kestrel.Core/Features/IConnectionTimeoutFeature.cs b/src/Kestrel.Core/Features/IConnectionTimeoutFeature.cs new file mode 100644 index 0000000000..e7634c3d88 --- /dev/null +++ b/src/Kestrel.Core/Features/IConnectionTimeoutFeature.cs @@ -0,0 +1,31 @@ +// 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.Features +{ + /// + /// Feature for efficiently handling connection timeouts. + /// + public interface IConnectionTimeoutFeature + { + /// + /// Close the connection after the specified positive finite + /// unless the timeout is canceled or reset. This will fail if there is an ongoing timeout. + /// + void SetTimeout(TimeSpan timeSpan); + + /// + /// Close the connection after the specified positive finite + /// unless the timeout is canceled or reset. This will cancel any ongoing timeouts. + /// + void ResetTimeout(TimeSpan timeSpan); + + /// + /// Prevent the connection from closing after a timeout specified by + /// or . + /// + void CancelTimeout(); + } +} diff --git a/src/Kestrel.Core/Internal/FrameConnection.cs b/src/Kestrel.Core/Internal/FrameConnection.cs index 9232c515da..85c58e7dd7 100644 --- a/src/Kestrel.Core/Internal/FrameConnection.cs +++ b/src/Kestrel.Core/Internal/FrameConnection.cs @@ -10,7 +10,6 @@ using System.Net; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting.Server; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; @@ -20,7 +19,7 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { - public class FrameConnection : ITimeoutControl + public class FrameConnection : ITimeoutControl, IConnectionTimeoutFeature { private const int Http2ConnectionNotStarted = 0; private const int Http2ConnectionStarted = 1; @@ -58,7 +57,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal internal Frame Frame => _frame; internal IDebugger Debugger { get; set; } = DebuggerWrapper.Singleton; - public bool TimedOut { get; private set; } + // For testing + internal bool RequestTimedOut { get; private set; } public string ConnectionId => _context.ConnectionId; public IPEndPoint LocalEndPoint => _context.LocalEndPoint; @@ -106,8 +106,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { adaptedPipeline = new AdaptedPipeline(transport, application, - PipeFactory.Create(AdaptedInputPipeOptions), - PipeFactory.Create(AdaptedOutputPipeOptions)); + PipeFactory.Create(AdaptedInputPipeOptions), + PipeFactory.Create(AdaptedOutputPipeOptions)); transport = adaptedPipeline; } @@ -134,6 +134,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _context.ServiceContext.ConnectionManager.AddConnection(_context.FrameConnectionId, this); _lastTimestamp = _context.ServiceContext.SystemClock.UtcNow.Ticks; + _frame.ConnectionFeatures.Set(this); + if (adaptedPipeline != null) { // Stream can be null here and run async will close the connection in that case @@ -195,7 +197,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _socketClosedTcs.TrySetResult(null); } - public Task StopAsync() + public Task StopProcessingNextRequestAsync() { Debug.Assert(_frame != null, $"{nameof(_frame)} is null"); @@ -205,7 +207,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } else { - _frame.Stop(); + _frame.StopProcessingNextRequest(); } return _lifetimeTask; @@ -233,19 +235,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal return _lifetimeTask; } - public void SetTimeoutResponse() + public void SendTimeoutResponse() { Debug.Assert(_frame != null, $"{nameof(_frame)} is null"); - _frame.SetBadRequestState(RequestRejectionReason.RequestTimeout); + RequestTimedOut = true; + _frame.SendTimeoutResponse(); } - public void Timeout() + public void StopProcessingNextRequest() { Debug.Assert(_frame != null, $"{nameof(_frame)} is null"); - TimedOut = true; - _frame.Stop(); + _frame.StopProcessingNextRequest(); } private async Task ApplyConnectionAdaptersAsync() @@ -303,11 +305,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal private void CheckForTimeout(long timestamp) { - if (TimedOut) - { - return; - } - // TODO: Use PlatformApis.VolatileRead equivalent again if (timestamp > Interlocked.Read(ref _timeoutTimestamp)) { @@ -315,12 +312,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { CancelTimeout(); - if (_timeoutAction == TimeoutAction.SendTimeoutResponse) + switch (_timeoutAction) { - SetTimeoutResponse(); + case TimeoutAction.StopProcessingNextRequest: + StopProcessingNextRequest(); + break; + case TimeoutAction.SendTimeoutResponse: + SendTimeoutResponse(); + break; + case TimeoutAction.AbortConnection: + Abort(new TimeoutException()); + break; } - - Timeout(); } } } @@ -330,7 +333,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal // The only time when both a timeout is set and the read data rate could be enforced is // when draining the request body. Since there's already a (short) timeout set for draining, // it's safe to not check the data rate at this point. - if (TimedOut || Interlocked.Read(ref _timeoutTimestamp) != long.MaxValue) + if (Interlocked.Read(ref _timeoutTimestamp) != long.MaxValue) { return; } @@ -352,7 +355,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal if (rate < minRequestBodyDataRate.BytesPerSecond && !Debugger.IsAttached) { Log.RequestBodyMininumDataRateNotSatisfied(_context.ConnectionId, _frame.TraceIdentifier, minRequestBodyDataRate.BytesPerSecond); - Timeout(); + SendTimeoutResponse(); } } @@ -370,16 +373,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal private void CheckForWriteDataRateTimeout(long timestamp) { - if (TimedOut) - { - return; - } - lock (_writeTimingLock) { if (_writeTimingWrites > 0 && timestamp > _writeTimingTimeoutTimestamp && !Debugger.IsAttached) { - TimedOut = true; + RequestTimedOut = true; Log.ResponseMininumDataRateNotSatisfied(_frame.ConnectionIdFeature, _frame.TraceIdentifier); Abort(new TimeoutException()); } @@ -484,5 +482,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _writeTimingWrites--; } } + + void IConnectionTimeoutFeature.SetTimeout(TimeSpan timeSpan) + { + if (timeSpan < TimeSpan.Zero) + { + throw new ArgumentException(CoreStrings.PositiveFiniteTimeSpanRequired, nameof(timeSpan)); + } + if (_timeoutTimestamp != long.MaxValue) + { + throw new InvalidOperationException(CoreStrings.ConcurrentTimeoutsNotSupported); + } + + SetTimeout(timeSpan.Ticks, TimeoutAction.AbortConnection); + } + + void IConnectionTimeoutFeature.ResetTimeout(TimeSpan timeSpan) + { + if (timeSpan < TimeSpan.Zero) + { + throw new ArgumentException(CoreStrings.PositiveFiniteTimeSpanRequired, nameof(timeSpan)); + } + + ResetTimeout(timeSpan.Ticks, TimeoutAction.AbortConnection); + } } } diff --git a/src/Kestrel.Core/Internal/Http/Frame.cs b/src/Kestrel.Core/Internal/Http/Frame.cs index 6195d79d5e..64c24e2813 100644 --- a/src/Kestrel.Core/Internal/Http/Frame.cs +++ b/src/Kestrel.Core/Internal/Http/Frame.cs @@ -15,7 +15,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Protocols; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; @@ -52,13 +51,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected Stack, object>> _onStarting; protected Stack, object>> _onCompleted; - protected volatile bool _requestProcessingStopping; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx - protected int _requestAborted; + protected volatile int _requestAborted; + private volatile bool _requestTimedOut; private CancellationTokenSource _abortedCts; private CancellationToken? _manuallySetRequestAbortToken; protected RequestProcessingStatus _requestProcessingStatus; - protected bool _keepAlive; + protected volatile bool _keepAlive = true; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx protected bool _upgradeAvailable; private volatile bool _wasUpgraded; private bool _canHaveBody; @@ -111,6 +110,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http public IPipeReader Input => _frameContext.Transport.Input; public OutputProducer Output { get; } public ITimeoutControl TimeoutControl => _frameContext.TimeoutControl; + public bool RequestTimedOut => _requestTimedOut; protected IKestrelTrace Log => ServiceContext.Log; private DateHeaderValueManager DateHeaderValueManager => ServiceContext.DateHeaderValueManager; @@ -260,7 +260,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var cts = _abortedCts; return cts != null ? cts.Token : - (Volatile.Read(ref _requestAborted) == 1) ? new CancellationToken(true) : + (_requestAborted == 1) ? new CancellationToken(true) : RequestAbortedSource.Token; } set @@ -285,7 +285,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var cts = LazyInitializer.EnsureInitialized(ref _abortedCts, () => new CancellationTokenSource()) ?? new CancellationTokenSource(); - if (Volatile.Read(ref _requestAborted) == 1) + if (_requestAborted == 1) { cts.Cancel(); } @@ -329,9 +329,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _onCompleted = null; _requestProcessingStatus = RequestProcessingStatus.RequestPending; - _keepAlive = false; _autoChunk = false; _applicationException = null; + _requestTimedOut = false; + _requestRejectedException = null; ResetFeatureCollection(); @@ -389,9 +390,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http /// Called on all active connections when the server wants to initiate a shutdown /// and after a keep-alive timeout. /// - public void Stop() + public void StopProcessingNextRequest() { - _requestProcessingStopping = true; + _keepAlive = false; + Input.CancelPendingRead(); + } + + public void SendTimeoutResponse() + { + _requestTimedOut = true; Input.CancelPendingRead(); } @@ -415,7 +422,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { if (Interlocked.Exchange(ref _requestAborted, 1) == 0) { - _requestProcessingStopping = true; + _keepAlive = false; _frameStreams?.Abort(error); @@ -789,7 +796,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected Task TryProduceInvalidRequestResponse() { - if (_requestRejectedException != null) + // If _requestAborted is set, the connection has already been closed. + if (_requestRejectedException != null && _requestAborted == 0) { return ProduceEnd(); } @@ -804,7 +812,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http if (HasResponseStarted) { // We can no longer change the response, so we simply close the connection. - _requestProcessingStopping = true; + _keepAlive = false; return Task.CompletedTask; } @@ -883,9 +891,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var hasTransferEncoding = responseHeaders.HasTransferEncoding; var transferCoding = FrameHeaders.GetFinalTransferCoding(responseHeaders.HeaderTransferEncoding); - if (_keepAlive && hasConnection) + if (_keepAlive && hasConnection && (connectionOptions & ConnectionOptions.KeepAlive) != ConnectionOptions.KeepAlive) { - _keepAlive = (connectionOptions & ConnectionOptions.KeepAlive) == ConnectionOptions.KeepAlive; + _keepAlive = false; } // https://tools.ietf.org/html/rfc7230#section-3.3.1 @@ -1171,7 +1179,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } _keepAlive = false; - _requestProcessingStopping = true; _requestRejectedException = ex; } diff --git a/src/Kestrel.Core/Internal/Http/FrameOfT.cs b/src/Kestrel.Core/Internal/Http/FrameOfT.cs index 79ac93384a..b60b88049a 100644 --- a/src/Kestrel.Core/Internal/Http/FrameOfT.cs +++ b/src/Kestrel.Core/Internal/Http/FrameOfT.cs @@ -3,7 +3,6 @@ using System; using System.IO; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Protocols; @@ -32,15 +31,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { try { - while (!_requestProcessingStopping) + while (_keepAlive) { - TimeoutControl.SetTimeout(_keepAliveTicks, TimeoutAction.CloseConnection); - Reset(); + TimeoutControl.SetTimeout(_keepAliveTicks, TimeoutAction.StopProcessingNextRequest); - while (!_requestProcessingStopping) + while (_requestProcessingStatus != RequestProcessingStatus.AppStarted) { var result = await Input.ReadAsync(); + var examined = result.Buffer.End; var consumed = result.Buffer.End; @@ -62,11 +61,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http Input.Advance(consumed, examined); } - if (_requestProcessingStatus == RequestProcessingStatus.AppStarted) - { - break; - } - if (result.IsCompleted) { switch (_requestProcessingStatus) @@ -81,132 +75,140 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http RequestRejectionReason.MalformedRequestInvalidHeaders); } } + else if (!_keepAlive && _requestProcessingStatus == RequestProcessingStatus.RequestPending) + { + // Stop the request processing loop if the server is shutting down or there was a keep-alive timeout + // and there is no ongoing request. + return; + } + else if (RequestTimedOut) + { + // In this case, there is an ongoing request but the start line/header parsing has timed out, so send + // a 408 response. + throw BadHttpRequestException.GetException(RequestRejectionReason.RequestTimeout); + } } - if (!_requestProcessingStopping) + EnsureHostHeaderExists(); + + var messageBody = MessageBody.For(_httpVersion, FrameRequestHeaders, this); + + if (!messageBody.RequestKeepAlive) { - EnsureHostHeaderExists(); + _keepAlive = false; + } - var messageBody = MessageBody.For(_httpVersion, FrameRequestHeaders, this); - _keepAlive = messageBody.RequestKeepAlive; - _upgradeAvailable = messageBody.RequestUpgrade; + _upgradeAvailable = messageBody.RequestUpgrade; - InitializeStreams(messageBody); + InitializeStreams(messageBody); - var context = _application.CreateContext(this); + var context = _application.CreateContext(this); + try + { try { - try + KestrelEventSource.Log.RequestStart(this); + + await _application.ProcessRequestAsync(context); + + if (_requestAborted == 0) { - KestrelEventSource.Log.RequestStart(this); - - await _application.ProcessRequestAsync(context); - - if (Volatile.Read(ref _requestAborted) == 0) - { - VerifyResponseContentLength(); - } - } - catch (Exception ex) - { - ReportApplicationError(ex); - - if (ex is BadHttpRequestException) - { - throw; - } - } - finally - { - KestrelEventSource.Log.RequestStop(this); - - // Trigger OnStarting if it hasn't been called yet and the app hasn't - // already failed. If an OnStarting callback throws we can go through - // our normal error handling in ProduceEnd. - // https://github.com/aspnet/KestrelHttpServer/issues/43 - if (!HasResponseStarted && _applicationException == null && _onStarting != null) - { - await FireOnStarting(); - } - - PauseStreams(); - - if (_onCompleted != null) - { - await FireOnCompleted(); - } - } - - // If _requestAbort is set, the connection has already been closed. - if (Volatile.Read(ref _requestAborted) == 0) - { - if (HasResponseStarted) - { - // If the response has already started, call ProduceEnd() before - // consuming the rest of the request body to prevent - // delaying clients waiting for the chunk terminator: - // - // https://github.com/dotnet/corefx/issues/17330#issuecomment-288248663 - // - // ProduceEnd() must be called before _application.DisposeContext(), to ensure - // HttpContext.Response.StatusCode is correctly set when - // IHttpContextFactory.Dispose(HttpContext) is called. - await ProduceEnd(); - } - - // ForZeroContentLength does not complete the reader nor the writer - if (!messageBody.IsEmpty && _keepAlive) - { - // Finish reading the request body in case the app did not. - TimeoutControl.SetTimeout(Constants.RequestBodyDrainTimeout.Ticks, TimeoutAction.SendTimeoutResponse); - await messageBody.ConsumeAsync(); - TimeoutControl.CancelTimeout(); - } - - if (!HasResponseStarted) - { - await ProduceEnd(); - } - } - else if (!HasResponseStarted) - { - // If the request was aborted and no response was sent, there's no - // meaningful status code to log. - StatusCode = 0; + VerifyResponseContentLength(); } } - catch (BadHttpRequestException ex) + catch (Exception ex) { - // Handle BadHttpRequestException thrown during app execution or remaining message body consumption. - // This has to be caught here so StatusCode is set properly before disposing the HttpContext - // (DisposeContext logs StatusCode). - SetBadRequestState(ex); + ReportApplicationError(ex); + + if (ex is BadHttpRequestException) + { + throw; + } } finally { - _application.DisposeContext(context, _applicationException); + KestrelEventSource.Log.RequestStop(this); - // StopStreams should be called before the end of the "if (!_requestProcessingStopping)" block - // to ensure InitializeStreams has been called. - StopStreams(); - - if (HasStartedConsumingRequestBody) + // Trigger OnStarting if it hasn't been called yet and the app hasn't + // already failed. If an OnStarting callback throws we can go through + // our normal error handling in ProduceEnd. + // https://github.com/aspnet/KestrelHttpServer/issues/43 + if (!HasResponseStarted && _applicationException == null && _onStarting != null) { - RequestBodyPipe.Reader.Complete(); + await FireOnStarting(); + } - // Wait for MessageBody.PumpAsync() to call RequestBodyPipe.Writer.Complete(). - await messageBody.StopAsync(); + PauseStreams(); - // At this point both the request body pipe reader and writer should be completed. - RequestBodyPipe.Reset(); + if (_onCompleted != null) + { + await FireOnCompleted(); } } - } - if (!_keepAlive) + // If _requestAbort is set, the connection has already been closed. + if (_requestAborted == 0) + { + if (HasResponseStarted) + { + // If the response has already started, call ProduceEnd() before + // consuming the rest of the request body to prevent + // delaying clients waiting for the chunk terminator: + // + // https://github.com/dotnet/corefx/issues/17330#issuecomment-288248663 + // + // ProduceEnd() must be called before _application.DisposeContext(), to ensure + // HttpContext.Response.StatusCode is correctly set when + // IHttpContextFactory.Dispose(HttpContext) is called. + await ProduceEnd(); + } + + // ForZeroContentLength does not complete the reader nor the writer + if (!messageBody.IsEmpty && _keepAlive) + { + // Finish reading the request body in case the app did not. + TimeoutControl.SetTimeout(Constants.RequestBodyDrainTimeout.Ticks, TimeoutAction.SendTimeoutResponse); + await messageBody.ConsumeAsync(); + TimeoutControl.CancelTimeout(); + } + + if (!HasResponseStarted) + { + await ProduceEnd(); + } + } + else if (!HasResponseStarted) + { + // If the request was aborted and no response was sent, there's no + // meaningful status code to log. + StatusCode = 0; + } + } + catch (BadHttpRequestException ex) { - // End the connection for non keep alive as data incoming may have been thrown off - return; + // Handle BadHttpRequestException thrown during app execution or remaining message body consumption. + // This has to be caught here so StatusCode is set properly before disposing the HttpContext + // (DisposeContext logs StatusCode). + SetBadRequestState(ex); + } + finally + { + _application.DisposeContext(context, _applicationException); + + // StopStreams should be called before the end of the "if (!_requestProcessingStopping)" block + // to ensure InitializeStreams has been called. + StopStreams(); + + if (HasStartedConsumingRequestBody) + { + RequestBodyPipe.Reader.Complete(); + + // Wait for MessageBody.PumpAsync() to call RequestBodyPipe.Writer.Complete(). + await messageBody.StopAsync(); + + // At this point both the request body pipe reader and writer should be completed. + RequestBodyPipe.Reset(); + } } } } @@ -237,13 +239,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http try { Input.Complete(); - - // If _requestAborted is set, the connection has already been closed. - if (Volatile.Read(ref _requestAborted) == 0) - { - await TryProduceInvalidRequestResponse(); - Output.Dispose(); - } + await TryProduceInvalidRequestResponse(); + Output.Dispose(); } catch (Exception ex) { diff --git a/src/Kestrel.Core/Internal/Http/MessageBody.cs b/src/Kestrel.Core/Internal/Http/MessageBody.cs index 86e9abf729..22c378b3ce 100644 --- a/src/Kestrel.Core/Internal/Http/MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/MessageBody.cs @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { var result = await awaitable; - if (_context.TimeoutControl.TimedOut) + if (_context.RequestTimedOut) { _context.ThrowRequestRejected(RequestRejectionReason.RequestTimeout); } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 5419790ff2..57e5a812af 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -50,8 +50,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public IKestrelTrace Log => _context.ServiceContext.Log; - bool ITimeoutControl.TimedOut => throw new NotImplementedException(); - public void Abort(Exception ex) { _stopping = true; diff --git a/src/Kestrel.Core/Internal/Infrastructure/FrameConnectionManagerShutdownExtensions.cs b/src/Kestrel.Core/Internal/Infrastructure/FrameConnectionManagerShutdownExtensions.cs index 1a0df064db..a0319b0db9 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/FrameConnectionManagerShutdownExtensions.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/FrameConnectionManagerShutdownExtensions.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure connectionManager.Walk(connection => { - closeTasks.Add(connection.StopAsync()); + closeTasks.Add(connection.StopProcessingNextRequestAsync()); }); var allClosedTask = Task.WhenAll(closeTasks.ToArray()); diff --git a/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs index 2e2c34ff55..69175ec0b5 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs @@ -5,8 +5,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { public interface ITimeoutControl { - bool TimedOut { get; } - void SetTimeout(long ticks, TimeoutAction timeoutAction); void ResetTimeout(long ticks, TimeoutAction timeoutAction); void CancelTimeout(); diff --git a/src/Kestrel.Core/Internal/Infrastructure/TimeoutAction.cs b/src/Kestrel.Core/Internal/Infrastructure/TimeoutAction.cs index 96af620550..b2aa9df8cf 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/TimeoutAction.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/TimeoutAction.cs @@ -5,7 +5,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { public enum TimeoutAction { - CloseConnection, - SendTimeoutResponse + StopProcessingNextRequest, + SendTimeoutResponse, + AbortConnection, } } diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 4598adc6fa..0d7341ba08 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -1074,6 +1074,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatPositiveNumberOrNullMinDataRateRequired() => GetString("PositiveNumberOrNullMinDataRateRequired"); + /// + /// Concurrent timeouts are not supported. + /// + internal static string ConcurrentTimeoutsNotSupported + { + get => GetString("ConcurrentTimeoutsNotSupported"); + } + + /// + /// Concurrent timeouts are not supported. + /// + internal static string FormatConcurrentTimeoutsNotSupported() + => GetString("ConcurrentTimeoutsNotSupported"); + + /// + /// Timespan must be positive and finite. + /// + internal static string PositiveFiniteTimeSpanRequired + { + get => GetString("PositiveFiniteTimeSpanRequired"); + } + + /// + /// Timespan must be positive and finite. + /// + internal static string FormatPositiveFiniteTimeSpanRequired() + => GetString("PositiveFiniteTimeSpanRequired"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Kestrel.Https/HttpsConnectionAdapterOptions.cs b/src/Kestrel.Https/HttpsConnectionAdapterOptions.cs index f09ea10bc1..39b9f9615a 100644 --- a/src/Kestrel.Https/HttpsConnectionAdapterOptions.cs +++ b/src/Kestrel.Https/HttpsConnectionAdapterOptions.cs @@ -5,6 +5,7 @@ using System; using System.Net.Security; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; +using System.Threading; namespace Microsoft.AspNetCore.Server.Kestrel.Https { @@ -13,6 +14,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https /// public class HttpsConnectionAdapterOptions { + private TimeSpan _handshakeTimeout; + /// /// Initializes a new instance of . /// @@ -20,6 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https { ClientCertificateMode = ClientCertificateMode.NoCertificate; SslProtocols = SslProtocols.Tls12 | SslProtocols.Tls11; + HandshakeTimeout = TimeSpan.FromSeconds(10); } /// @@ -51,5 +55,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https /// Specifies whether the certificate revocation list is checked during authentication. /// public bool CheckCertificateRevocation { get; set; } + + /// + /// Specifies the maximum amount of time allowed for the TLS/SSL handshake. This must be positive and finite. + /// + public TimeSpan HandshakeTimeout + { + get => _handshakeTimeout; + set + { + if (value <= TimeSpan.Zero && value != Timeout.InfiniteTimeSpan) + { + throw new ArgumentOutOfRangeException(nameof(value), HttpsStrings.PositiveTimeSpanRequired); + } + _handshakeTimeout = value != Timeout.InfiniteTimeSpan ? value : TimeSpan.MaxValue; + } + } } } diff --git a/src/Kestrel.Https/HttpsStrings.resx b/src/Kestrel.Https/HttpsStrings.resx index b7ab3141db..689b5c4779 100644 --- a/src/Kestrel.Https/HttpsStrings.resx +++ b/src/Kestrel.Https/HttpsStrings.resx @@ -120,9 +120,15 @@ Failed to authenticate HTTPS connection. + + Authentication of the HTTPS connection timed out. + Certificate {thumbprint} cannot be used as an SSL server certificate. It has an Extended Key Usage extension but the usages do not include Server Authentication (OID 1.3.6.1.5.5.7.3.1). + + Value must be a positive TimeSpan. + The server certificate parameter is required. diff --git a/src/Kestrel.Https/Internal/HttpsConnectionAdapter.cs b/src/Kestrel.Https/Internal/HttpsConnectionAdapter.cs index 68e7268ab0..e8dfc3ec68 100644 --- a/src/Kestrel.Https/Internal/HttpsConnectionAdapter.cs +++ b/src/Kestrel.Https/Internal/HttpsConnectionAdapter.cs @@ -9,6 +9,7 @@ using System.Security.Cryptography.X509Certificates; using System.Threading.Tasks; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal @@ -108,17 +109,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal certificateRequired = true; } + var timeoutFeature = context.Features.Get(); + timeoutFeature.SetTimeout(_options.HandshakeTimeout); + try { await sslStream.AuthenticateAsServerAsync(_serverCertificate, certificateRequired, _options.SslProtocols, _options.CheckCertificateRevocation); } + catch (OperationCanceledException) + { + _logger?.LogInformation(2, HttpsStrings.AuthenticationTimedOut); + sslStream.Dispose(); + return _closedAdaptedConnection; + } catch (IOException ex) { _logger?.LogInformation(1, ex, HttpsStrings.AuthenticationFailed); sslStream.Dispose(); return _closedAdaptedConnection; } + finally + { + timeoutFeature.CancelTimeout(); + } // Always set the feature even though the cert might be null context.Features.Set(new TlsConnectionFeature diff --git a/src/Kestrel.Https/Properties/HttpsStrings.Designer.cs b/src/Kestrel.Https/Properties/HttpsStrings.Designer.cs index 03676ba6a7..417309ee92 100644 --- a/src/Kestrel.Https/Properties/HttpsStrings.Designer.cs +++ b/src/Kestrel.Https/Properties/HttpsStrings.Designer.cs @@ -24,6 +24,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https internal static string FormatAuthenticationFailed() => GetString("AuthenticationFailed"); + /// + /// Authentication of the HTTPS connection timed out. + /// + internal static string AuthenticationTimedOut + { + get => GetString("AuthenticationTimedOut"); + } + + /// + /// Authentication of the HTTPS connection timed out. + /// + internal static string FormatAuthenticationTimedOut() + => GetString("AuthenticationTimedOut"); + /// /// Certificate {thumbprint} cannot be used as an SSL server certificate. It has an Extended Key Usage extension but the usages do not include Server Authentication (OID 1.3.6.1.5.5.7.3.1). /// @@ -38,6 +52,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https internal static string FormatInvalidServerCertificateEku(object thumbprint) => string.Format(CultureInfo.CurrentCulture, GetString("InvalidServerCertificateEku", "thumbprint"), thumbprint); + /// + /// Value must be a positive TimeSpan. + /// + internal static string PositiveTimeSpanRequired + { + get => GetString("PositiveTimeSpanRequired"); + } + + /// + /// Value must be a positive TimeSpan. + /// + internal static string FormatPositiveTimeSpanRequired() + => GetString("PositiveTimeSpanRequired"); + /// /// The server certificate parameter is required. /// diff --git a/test/Kestrel.Core.Tests/FrameConnectionTests.cs b/test/Kestrel.Core.Tests/FrameConnectionTests.cs index 847910d677..7ddb0856b7 100644 --- a/test/Kestrel.Core.Tests/FrameConnectionTests.cs +++ b/test/Kestrel.Core.Tests/FrameConnectionTests.cs @@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.SetTimeout(1, TimeoutAction.SendTimeoutResponse); _frameConnection.Tick(now.AddTicks(2).Add(Heartbeat.Interval)); - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); } [Fact] @@ -77,7 +77,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests TickBodyWithMinimumDataRate(mockLogger.Object, bytesPerSecond); - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); } [Fact] @@ -88,7 +88,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests TickBodyWithMinimumDataRate(mockLogger.Object, bytesPerSecond); // Timed out - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); } @@ -144,7 +144,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(now); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); @@ -154,7 +154,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(now); // Timed out - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); } @@ -191,7 +191,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(now); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); @@ -201,7 +201,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(now); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); @@ -211,7 +211,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(now); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); @@ -221,7 +221,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(now); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); @@ -231,7 +231,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(now); // Timed out - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); mockLogger.Verify(logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); } @@ -274,7 +274,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify( logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); @@ -288,7 +288,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify( logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); @@ -298,7 +298,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Timed out - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); mockLogger.Verify( logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); @@ -330,7 +330,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify( logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); @@ -349,7 +349,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); mockLogger.Verify( logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); @@ -359,7 +359,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Timed out - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); mockLogger.Verify( logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); @@ -388,7 +388,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.StartTimingReads(); - _frameConnection.SetTimeout(timeout.Ticks, TimeoutAction.CloseConnection); + _frameConnection.SetTimeout(timeout.Ticks, TimeoutAction.StopProcessingNextRequest); // Tick beyond grace period with low data rate systemClock.UtcNow += TimeSpan.FromSeconds(3); @@ -396,14 +396,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); // Tick just past timeout period, adjusted by Heartbeat.Interval systemClock.UtcNow = startTime + timeout + Heartbeat.Interval + TimeSpan.FromTicks(1); _frameConnection.Tick(systemClock.UtcNow); // Timed out - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); } [Fact] @@ -436,7 +436,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests systemClock.UtcNow += TimeSpan.FromSeconds(4) + Heartbeat.Interval + TimeSpan.FromTicks(1); _frameConnection.Tick(systemClock.UtcNow); - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); Assert.True(aborted.Wait(TimeSpan.FromSeconds(10))); } @@ -472,13 +472,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Still within grace period, not timed out - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); // Tick just past grace period (adjusted by Heartbeat.Interval) systemClock.UtcNow = startTime + minResponseDataRate.GracePeriod + Heartbeat.Interval + TimeSpan.FromTicks(1); _frameConnection.Tick(systemClock.UtcNow); - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); Assert.True(aborted.Wait(TimeSpan.FromSeconds(10))); } @@ -516,7 +516,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _frameConnection.Tick(systemClock.UtcNow); // Not timed out because the timeout was pushed by the second write - Assert.False(_frameConnection.TimedOut); + Assert.False(_frameConnection.RequestTimedOut); // Complete the first write, this should have no effect on the timeout _frameConnection.StopTimingWrite(); @@ -525,7 +525,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests systemClock.UtcNow += TimeSpan.FromSeconds(3) + TimeSpan.FromTicks(1); _frameConnection.Tick(systemClock.UtcNow); - Assert.True(_frameConnection.TimedOut); + Assert.True(_frameConnection.RequestTimedOut); Assert.True(aborted.Wait(TimeSpan.FromSeconds(10))); } } diff --git a/test/Kestrel.Core.Tests/FrameTests.cs b/test/Kestrel.Core.Tests/FrameTests.cs index eb7317c498..ed4db8e6bb 100644 --- a/test/Kestrel.Core.Tests/FrameTests.cs +++ b/test/Kestrel.Core.Tests/FrameTests.cs @@ -511,9 +511,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var requestProcessingTask = _frame.ProcessRequestsAsync(); var expectedKeepAliveTimeout = _serviceContext.ServerOptions.Limits.KeepAliveTimeout.Ticks; - _timeoutControl.Verify(cc => cc.SetTimeout(expectedKeepAliveTimeout, TimeoutAction.CloseConnection)); + _timeoutControl.Verify(cc => cc.SetTimeout(expectedKeepAliveTimeout, TimeoutAction.StopProcessingNextRequest)); - _frame.Stop(); + _frame.StopProcessingNextRequest(); _application.Output.Complete(); requestProcessingTask.Wait(); @@ -598,7 +598,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var data = Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\nHost:\r\n\r\n"); await _application.Output.WriteAsync(data); - _frame.Stop(); + _frame.StopProcessingNextRequest(); Assert.IsNotType>(requestProcessingTask); await requestProcessingTask.TimeoutAfter(TimeSpan.FromSeconds(10)); diff --git a/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs b/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs index 66de5d41e5..d71642f9dd 100644 --- a/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs @@ -314,11 +314,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests TimeSpan.MaxValue, }; - public static TheoryData TimeoutInfiniteData => new TheoryData - { - Timeout.InfiniteTimeSpan, - }; - public static TheoryData TimeoutInvalidData => new TheoryData { TimeSpan.MinValue, diff --git a/test/Kestrel.Core.Tests/MessageBodyTests.cs b/test/Kestrel.Core.Tests/MessageBodyTests.cs index 0d5a11ee27..58d7c5b480 100644 --- a/test/Kestrel.Core.Tests/MessageBodyTests.cs +++ b/test/Kestrel.Core.Tests/MessageBodyTests.cs @@ -566,11 +566,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(1, await body.ReadAsync(new ArraySegment(new byte[1]))); // Time out on the next read - mockTimeoutControl - .Setup(timeoutControl => timeoutControl.TimedOut) - .Returns(true); - - input.Cancel(); + input.Frame.SendTimeoutResponse(); var exception = await Assert.ThrowsAsync(() => body.ReadAsync(new ArraySegment(new byte[1]))); Assert.Equal(StatusCodes.Status408RequestTimeout, exception.StatusCode); @@ -595,11 +591,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(1, await body.ReadAsync(new ArraySegment(new byte[1]))); // Time out on the next read - mockTimeoutControl - .Setup(timeoutControl => timeoutControl.TimedOut) - .Returns(true); - - input.Cancel(); + input.Frame.SendTimeoutResponse(); var exception = await Assert.ThrowsAsync(() => body.ConsumeAsync()); Assert.Equal(StatusCodes.Status408RequestTimeout, exception.StatusCode); @@ -624,11 +616,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(1, await body.ReadAsync(new ArraySegment(new byte[1]))); // Time out on the next read - mockTimeoutControl - .Setup(timeoutControl => timeoutControl.TimedOut) - .Returns(true); - - input.Cancel(); + input.Frame.SendTimeoutResponse(); using (var ms = new MemoryStream()) { diff --git a/test/Kestrel.FunctionalTests/HttpsConnectionAdapterOptionsTest.cs b/test/Kestrel.FunctionalTests/HttpsConnectionAdapterOptionsTest.cs new file mode 100644 index 0000000000..b626416092 --- /dev/null +++ b/test/Kestrel.FunctionalTests/HttpsConnectionAdapterOptionsTest.cs @@ -0,0 +1,55 @@ +// 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.Threading; +using Microsoft.AspNetCore.Server.Kestrel.Https; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests +{ + public class HttpsConnectionAdapterOptionsTests + { + [Fact] + public void HandshakeTimeoutDefault() + { + Assert.Equal(TimeSpan.FromSeconds(10), new HttpsConnectionAdapterOptions().HandshakeTimeout); + } + + [Theory] + [MemberData(nameof(TimeoutValidData))] + public void HandshakeTimeoutValid(TimeSpan value) + { + Assert.Equal(value, new HttpsConnectionAdapterOptions { HandshakeTimeout = value }.HandshakeTimeout); + } + + [Fact] + public void HandshakeTimeoutCanBeSetToInfinite() + { + Assert.Equal(TimeSpan.MaxValue, new HttpsConnectionAdapterOptions { HandshakeTimeout = Timeout.InfiniteTimeSpan }.HandshakeTimeout); + } + + [Theory] + [MemberData(nameof(TimeoutInvalidData))] + public void HandshakeTimeoutInvalid(TimeSpan value) + { + var exception = Assert.Throws(() => new HttpsConnectionAdapterOptions { HandshakeTimeout = value }); + + Assert.Equal("value", exception.ParamName); + Assert.StartsWith(HttpsStrings.PositiveTimeSpanRequired, exception.Message); + } + + public static TheoryData TimeoutValidData => new TheoryData + { + TimeSpan.FromTicks(1), + TimeSpan.MaxValue, + }; + + public static TheoryData TimeoutInvalidData => new TheoryData + { + TimeSpan.MinValue, + TimeSpan.FromTicks(-1), + TimeSpan.Zero + }; + } +} diff --git a/test/Kestrel.FunctionalTests/HttpsTests.cs b/test/Kestrel.FunctionalTests/HttpsTests.cs index 9a7f147b2b..604df5943c 100644 --- a/test/Kestrel.FunctionalTests/HttpsTests.cs +++ b/test/Kestrel.FunctionalTests/HttpsTests.cs @@ -3,15 +3,18 @@ using System; using System.Collections.Generic; +using System.IO; using System.Net; using System.Net.Security; using System.Net.Sockets; using System.Security.Authentication; +using System.Security.Cryptography.X509Certificates; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; @@ -258,6 +261,42 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } + [Fact] + public async Task HandshakeTimesOutAndIsLoggedAsInformation() + { + var loggerProvider = new HandshakeErrorLoggerProvider(); + var hostBuilder = new WebHostBuilder() + .UseKestrel(options => + { + options.Listen(new IPEndPoint(IPAddress.Loopback, 0), listenOptions => + { + listenOptions.UseHttps(new HttpsConnectionAdapterOptions + { + ServerCertificate = new X509Certificate2(TestResources.TestCertificatePath, "testPassword"), + HandshakeTimeout = TimeSpan.FromSeconds(1) + }); + }); + }) + .ConfigureLogging(builder => builder.AddProvider(loggerProvider)) + .Configure(app => app.Run(httpContext => Task.CompletedTask)); + + using (var host = hostBuilder.Build()) + { + host.Start(); + + using (var socket = await HttpClientSlim.GetSocket(new Uri($"https://127.0.0.1:{host.GetPort()}/"))) + using (var stream = new NetworkStream(socket, ownsSocket: false)) + { + // No data should be sent and the connection should be closed in well under 30 seconds. + Assert.Equal(0, await stream.ReadAsync(new byte[1], 0, 1).TimeoutAfter(TimeSpan.FromSeconds(30))); + } + } + + await loggerProvider.FilterLogger.LogTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(10)); + Assert.Equal(2, loggerProvider.FilterLogger.LastEventId); + Assert.Equal(LogLevel.Information, loggerProvider.FilterLogger.LastLogLevel); + } + private class HandshakeErrorLoggerProvider : ILoggerProvider { public HttpsConnectionFilterLogger FilterLogger { get; } = new HttpsConnectionFilterLogger();