From c343628926800a9a9668500b15fc5b2f13805241 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Fri, 26 May 2017 12:26:05 -0700 Subject: [PATCH] Implement max connection limits - Added new options to allow configuring the maximum number of concurrent connections and upgraded connections. - `KestrelServerLimits.MaxConcurrentConnections` defaults unlimited. - `KestrelServerLimits.MaxConcurrentUpgradedConnections` defaults to unlimited. - Calls to IHttpUpgradeFeature.UpgradeAsync() will throw when the MaxConcurrentUpgradedConnections limit has been reached. - Kestrel will close new connections without response when MaxConcurrentConnections is reached. --- .vscode/launch.json | 8 +- .vscode/tasks.json | 17 ++ .../CoreStrings.resx | 23 +- .../Internal/ConnectionHandler.cs | 7 + .../Internal/FrameConnection.cs | 9 + .../Internal/Http/Frame.FeatureCollection.cs | 16 +- .../Internal/Http/Frame.cs | 18 +- .../Internal/Http/FrameOfT.cs | 4 +- .../Internal/Http/FrameRequestStream.cs | 2 +- .../Infrastructure/FrameConnectionManager.cs | 24 ++- .../Internal/Infrastructure/IKestrelTrace.cs | 2 + .../Infrastructure/ITimeoutControl.cs | 2 +- .../Infrastructure/KestrelEventSource.cs | 10 + .../Internal/Infrastructure/KestrelTrace.cs | 10 +- .../Infrastructure/ResourceCounter.cs | 77 +++++++ .../Internal/RejectionConnection.cs | 46 ++++ .../KestrelServer.cs | 5 +- .../KestrelServerLimits.cs | 104 +++++---- .../Properties/CoreStrings.Designer.cs | 78 +++++-- .../FrameConnectionManagerTests.cs | 2 +- .../KestrelServerLimitsTests.cs | 62 +++++- .../ResourceCounterTests.cs | 56 +++++ .../ConnectionLimitTests.cs | 198 ++++++++++++++++++ .../FrameConnectionManagerTests.cs | 5 +- .../MaxRequestBufferSizeTests.cs | 8 +- .../RequestTests.cs | 5 +- .../UpgradeTests.cs | 123 +++++++++-- .../Mocks/MockTimeoutControl.cs | 6 +- .../Mocks/MockTrace.cs | 1 + test/shared/DisposableStack.cs | 20 ++ test/shared/EventRaisingResourceCounter.cs | 34 +++ test/shared/TestConnection.cs | 26 +++ test/shared/TestServiceContext.cs | 2 +- 33 files changed, 888 insertions(+), 122 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ResourceCounter.cs create mode 100644 src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/RejectionConnection.cs create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/ResourceCounterTests.cs create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ConnectionLimitTests.cs create mode 100644 test/shared/DisposableStack.cs create mode 100644 test/shared/EventRaisingResourceCounter.cs diff --git a/.vscode/launch.json b/.vscode/launch.json index f97435a509..2db39e4359 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -7,6 +7,12 @@ "request": "attach", "processId": "${command:pickProcess}" }, + { + "name": "Attach: .NET Framework", + "type": "clr", + "request": "attach", + "processId": "${command:pickProcess}" + }, { "name": "Debug: SampleApp", "type": "coreclr", @@ -62,7 +68,7 @@ "type": "coreclr", "request": "launch", "preLaunchTask": "Compile: CodeGenerator", - "program": "${workspaceRoot}/tools/CodeGenerator/bin/Debug/netcoreapp1.1/CodeGenerator.dll", + "program": "${workspaceRoot}/tools/CodeGenerator/bin/Debug/netcoreapp2.0/CodeGenerator.dll", "args": [], "cwd": "${workspaceRoot}", "console": "internalConsole", diff --git a/.vscode/tasks.json b/.vscode/tasks.json index db382e6056..af0e4b2245 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -76,6 +76,23 @@ "cwd": "${workspaceRoot}/tools/CodeGenerator/" } }, + { + "taskName": "Run: resx generation", + "suppressTaskName": true, + "command": "build.cmd", + "args": [ + "/t:resx" + ], + "options": { + "cwd": "${workspaceRoot}" + }, + "osx": { + "command": "./build.sh" + }, + "linux": { + "command": "./build.sh" + } + }, { "taskName": "Run: Benchmarks", "args": [ diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx b/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx index c024ae82a9..f2f873fe3b 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/CoreStrings.resx @@ -228,14 +228,17 @@ Invalid Content-Length: "{value}". Value must be a positive integral number. - - Value must be null or a non-negative integer. + + Value must be null or a non-negative number. - - Value must be a positive integer. + + Value must be a non-negative number. - - Value must be null or a positive integer. + + Value must be a positive number. + + + Value must be null or a positive number. Unix socket path must be absolute. @@ -309,4 +312,10 @@ Cannot upgrade a non-upgradable request. Check IHttpUpgradeFeature.IsUpgradableRequest to determine if a request can be upgraded. - \ No newline at end of file + + Request cannot be upgraded because the server has already opened the maximum number of upgraded connections. + + + IHttpUpgradeFeature.UpgradeAsync was already called and can only be called once per connection. + + diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionHandler.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionHandler.cs index 96f510ea45..bee5dec4e1 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionHandler.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/ConnectionHandler.cs @@ -33,6 +33,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal var connectionId = CorrelationIdGenerator.GetNextId(); var frameConnectionId = Interlocked.Increment(ref _lastFrameConnectionId); + if (!_serviceContext.ConnectionManager.NormalConnectionCount.TryLockOne()) + { + var goAway = new RejectionConnection(inputPipe, outputPipe, connectionId, _serviceContext); + goAway.Reject(); + return goAway; + } + var connection = new FrameConnection(new FrameConnectionContext { ConnectionId = connectionId, diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/FrameConnection.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/FrameConnection.cs index 9b4da51ec1..dad0f21cd2 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/FrameConnection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/FrameConnection.cs @@ -126,6 +126,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _context.ServiceContext.ConnectionManager.RemoveConnection(_context.FrameConnectionId); DisposeAdaptedConnections(); + if (_frame.WasUpgraded) + { + _context.ServiceContext.ConnectionManager.UpgradedConnectionCount.ReleaseOne(); + } + else + { + _context.ServiceContext.ConnectionManager.NormalConnectionCount.ReleaseOne(); + } + Log.ConnectionStop(ConnectionId); KestrelEventSource.Log.ConnectionStop(this); } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.FeatureCollection.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.FeatureCollection.cs index 429c517899..6ec4f45348 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.FeatureCollection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.FeatureCollection.cs @@ -160,7 +160,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http bool IHttpResponseFeature.HasStarted => HasResponseStarted; - bool IHttpUpgradeFeature.IsUpgradableRequest => _upgrade; + bool IHttpUpgradeFeature.IsUpgradableRequest => _upgradeAvailable; bool IFeatureCollection.IsReadOnly => false; @@ -235,6 +235,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http throw new InvalidOperationException(CoreStrings.CannotUpgradeNonUpgradableRequest); } + if (_wasUpgraded) + { + throw new InvalidOperationException(CoreStrings.UpgradeCannotBeCalledMultipleTimes); + } + + if (!ServiceContext.ConnectionManager.UpgradedConnectionCount.TryLockOne()) + { + throw new InvalidOperationException(CoreStrings.UpgradedConnectionLimitReached); + } + + _wasUpgraded = true; + + ServiceContext.ConnectionManager.NormalConnectionCount.ReleaseOne(); + StatusCode = StatusCodes.Status101SwitchingProtocols; ReasonPhrase = "Switching Protocols"; ResponseHeaders["Connection"] = "Upgrade"; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.cs index 0387c0ce8e..374b9e6518 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/Frame.cs @@ -41,7 +41,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private static readonly byte[] _bytesTransferEncodingChunked = Encoding.ASCII.GetBytes("\r\nTransfer-Encoding: chunked"); private static readonly byte[] _bytesHttpVersion11 = Encoding.ASCII.GetBytes("HTTP/1.1 "); private static readonly byte[] _bytesEndHeaders = Encoding.ASCII.GetBytes("\r\n\r\n"); - private static readonly byte[] _bytesServer = Encoding.ASCII.GetBytes("\r\nServer: Kestrel"); + private static readonly byte[] _bytesServer = Encoding.ASCII.GetBytes("\r\nServer: " + Constants.ServerName); private const string EmptyPath = "/"; private const string Asterisk = "*"; @@ -61,7 +61,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http protected RequestProcessingStatus _requestProcessingStatus; protected bool _keepAlive; - protected bool _upgrade; + protected bool _upgradeAvailable; + private volatile bool _wasUpgraded; private bool _canHaveBody; private bool _autoChunk; protected Exception _applicationException; @@ -138,6 +139,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } + public bool WasUpgraded => _wasUpgraded; public IPAddress RemoteIpAddress { get; set; } public int RemotePort { get; set; } public IPAddress LocalIpAddress { get; set; } @@ -208,10 +210,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private int _statusCode; public int StatusCode { - get - { - return _statusCode; - } + get => _statusCode; set { if (HasResponseStarted) @@ -227,10 +226,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http public string ReasonPhrase { - get - { - return _reasonPhrase; - } + get => _reasonPhrase; + set { if (HasResponseStarted) @@ -1038,6 +1035,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues(); responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes); + responseHeaders.ContentLength = 0; if (ServerOptions.AddServerHeader) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameOfT.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameOfT.cs index 4fdda6d9f1..769cd8e9db 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameOfT.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameOfT.cs @@ -89,7 +89,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var messageBody = MessageBody.For(_httpVersion, FrameRequestHeaders, this); _keepAlive = messageBody.RequestKeepAlive; - _upgrade = messageBody.RequestUpgrade; + _upgradeAvailable = messageBody.RequestUpgrade; InitializeStreams(messageBody); @@ -165,7 +165,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // An upgraded request has no defined request body length. // Cancel any pending read so the read loop ends. - if (_upgrade) + if (_upgradeAvailable) { Input.CancelPendingRead(); } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameRequestStream.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameRequestStream.cs index 772668cf2c..be5f9fd66f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameRequestStream.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/FrameRequestStream.cs @@ -130,7 +130,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } if (bufferSize <= 0) { - throw new ArgumentException(CoreStrings.PositiveIntRequired, nameof(bufferSize)); + throw new ArgumentException(CoreStrings.PositiveNumberRequired, nameof(bufferSize)); } var task = ValidateState(cancellationToken); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/FrameConnectionManager.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/FrameConnectionManager.cs index 3fd4987568..ed4c2d44ae 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/FrameConnectionManager.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/FrameConnectionManager.cs @@ -11,11 +11,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure private readonly ConcurrentDictionary _connectionReferences = new ConcurrentDictionary(); private readonly IKestrelTrace _trace; - public FrameConnectionManager(IKestrelTrace trace) + public FrameConnectionManager(IKestrelTrace trace, long? normalConnectionLimit, long? upgradedConnectionLimit) + : this(trace, GetCounter(normalConnectionLimit), GetCounter(upgradedConnectionLimit)) { + } + + public FrameConnectionManager(IKestrelTrace trace, ResourceCounter normalConnections, ResourceCounter upgradedConnections) + { + NormalConnectionCount = normalConnections; + UpgradedConnectionCount = upgradedConnections; _trace = trace; } + /// + /// TCP connections processed by Kestrel. + /// + public ResourceCounter NormalConnectionCount { get; } + + /// + /// Connections that have been switched to a different protocol. + /// + public ResourceCounter UpgradedConnectionCount { get; } + public void AddConnection(long id, FrameConnection connection) { if (!_connectionReferences.TryAdd(id, new FrameConnectionReference(connection))) @@ -52,5 +69,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure // If both conditions are false, the connection was removed during the heartbeat. } } + + private static ResourceCounter GetCounter(long? number) + => number.HasValue + ? ResourceCounter.Quota(number.Value) + : ResourceCounter.Unlimited; } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs index 1d88029d3d..91012fdbf5 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs @@ -16,6 +16,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure void ConnectionResume(string connectionId); + void ConnectionRejected(string connectionId); + void ConnectionKeepAlive(string connectionId); void ConnectionDisconnect(string connectionId); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs index d71b423873..e031495f1d 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ITimeoutControl.cs @@ -9,4 +9,4 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure void ResetTimeout(long ticks, TimeoutAction timeoutAction); void CancelTimeout(); } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs index eb04b7f486..04ead7ab1e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs @@ -68,6 +68,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure WriteEvent(2, connectionId); } + [MethodImpl(MethodImplOptions.NoInlining)] + [Event(5, Level = EventLevel.Verbose)] + public void ConnectionRejected(string connectionId) + { + if (IsEnabled()) + { + WriteEvent(5, connectionId); + } + } + [NonEvent] public void RequestStart(Frame frame) { diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs index c7ef78a67c..e04ca4e0b8 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs @@ -54,6 +54,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal private static readonly Action _applicationNeverCompleted = LoggerMessage.Define(LogLevel.Critical, 23, @"Connection id ""{ConnectionId}"" application never completed"); + private static readonly Action _connectionRejected = + LoggerMessage.Define(LogLevel.Warning, 24, @"Connection id ""{ConnectionId}"" rejected because the maximum number of concurrent connections has been reached."); + protected readonly ILogger _logger; public KestrelTrace(ILogger logger) @@ -86,6 +89,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _connectionKeepAlive(_logger, connectionId, null); } + public void ConnectionRejected(string connectionId) + { + _connectionRejected(_logger, connectionId, null); + } + public virtual void ConnectionDisconnect(string connectionId) { _connectionDisconnect(_logger, connectionId, null); @@ -138,4 +146,4 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public virtual IDisposable BeginScope(TState state) => _logger.BeginScope(state); } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ResourceCounter.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ResourceCounter.cs new file mode 100644 index 0000000000..68c96e25ac --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/ResourceCounter.cs @@ -0,0 +1,77 @@ +// 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.Diagnostics; +using System.Threading; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public abstract class ResourceCounter + { + public abstract bool TryLockOne(); + public abstract void ReleaseOne(); + + public static ResourceCounter Unlimited { get; } = new UnlimitedCounter(); + public static ResourceCounter Quota(long amount) => new FiniteCounter(amount); + + private class UnlimitedCounter : ResourceCounter + { + public override bool TryLockOne() => true; + public override void ReleaseOne() + { + } + } + + internal class FiniteCounter : ResourceCounter + { + private readonly long _max; + private long _count; + + public FiniteCounter(long max) + { + if (max < 0) + { + throw new ArgumentOutOfRangeException(CoreStrings.NonNegativeNumberRequired); + } + + _max = max; + } + + public override bool TryLockOne() + { + var count = _count; + + // Exit if count == MaxValue as incrementing would overflow. + + while (count < _max && count != long.MaxValue) + { + var prev = Interlocked.CompareExchange(ref _count, count + 1, count); + if (prev == count) + { + return true; + } + + // Another thread changed the count before us. Try again with the new counter value. + count = prev; + } + + return false; + } + + public override void ReleaseOne() + { + Interlocked.Decrement(ref _count); + + Debug.Assert(_count >= 0, "Resource count is negative. More resources were released than were locked."); + } + + // for testing + internal long Count + { + get => _count; + set => _count = value; + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/RejectionConnection.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/RejectionConnection.cs new file mode 100644 index 0000000000..0cd9799db7 --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/RejectionConnection.cs @@ -0,0 +1,46 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.Internal.System.IO.Pipelines; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal +{ + public class RejectionConnection : IConnectionContext + { + private readonly IKestrelTrace _log; + private readonly IPipe _input; + private readonly IPipe _output; + + public RejectionConnection(IPipe input, IPipe output, string connectionId, ServiceContext serviceContext) + { + ConnectionId = connectionId; + _log = serviceContext.Log; + _input = input; + _output = output; + } + + public string ConnectionId { get; } + public IPipeWriter Input => _input.Writer; + public IPipeReader Output => _output.Reader; + + public void Reject() + { + KestrelEventSource.Log.ConnectionRejected(ConnectionId); + _log.ConnectionRejected(ConnectionId); + _input.Reader.Complete(); + _output.Writer.Complete(); + } + + // TODO: Remove these (https://github.com/aspnet/KestrelHttpServer/issues/1772) + void IConnectionContext.OnConnectionClosed(Exception ex) + { + } + + void IConnectionContext.Abort(Exception ex) + { + } + } +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs index 2eb204c0eb..8781882b06 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs @@ -67,7 +67,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core var serverOptions = options.Value ?? new KestrelServerOptions(); var logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel"); var trace = new KestrelTrace(logger); - var connectionManager = new FrameConnectionManager(trace); + var connectionManager = new FrameConnectionManager( + trace, + serverOptions.Limits.MaxConcurrentConnections, + serverOptions.Limits.MaxConcurrentUpgradedConnections); var systemClock = new SystemClock(); var dateHeaderValueManager = new DateHeaderValueManager(systemClock); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServerLimits.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServerLimits.cs index 9ed2293b6b..dc86c064e6 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServerLimits.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServerLimits.cs @@ -28,6 +28,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core private TimeSpan _requestHeadersTimeout = TimeSpan.FromSeconds(30); + // default to unlimited + private long? _maxConcurrentConnections = null; + private long? _maxConcurrentUpgradedConnections = null; + /// /// Gets or sets the maximum size of the response buffer before write /// calls begin to block or return tasks that don't complete until the @@ -41,15 +45,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// public long? MaxResponseBufferSize { - get - { - return _maxResponseBufferSize; - } + get => _maxResponseBufferSize; set { if (value.HasValue && value.Value < 0) { - throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.NonNegativeNullableIntRequired); + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.NonNegativeNumberOrNullRequired); } _maxResponseBufferSize = value; } @@ -64,15 +65,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// public long? MaxRequestBufferSize { - get - { - return _maxRequestBufferSize; - } + get => _maxRequestBufferSize; set { if (value.HasValue && value.Value <= 0) { - throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveNullableIntRequired); + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveNumberOrNullRequired); } _maxRequestBufferSize = value; } @@ -86,15 +84,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// public int MaxRequestLineSize { - get - { - return _maxRequestLineSize; - } + get => _maxRequestLineSize; set { if (value <= 0) { - throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveIntRequired); + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveNumberRequired); } _maxRequestLineSize = value; } @@ -108,15 +103,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// public int MaxRequestHeadersTotalSize { - get - { - return _maxRequestHeadersTotalSize; - } + get => _maxRequestHeadersTotalSize; set { if (value <= 0) { - throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveIntRequired); + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveNumberRequired); } _maxRequestHeadersTotalSize = value; } @@ -130,15 +122,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// public int MaxRequestHeaderCount { - get - { - return _maxRequestHeaderCount; - } + get => _maxRequestHeaderCount; set { if (value <= 0) { - throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveIntRequired); + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveNumberRequired); } _maxRequestHeaderCount = value; } @@ -152,14 +141,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// public TimeSpan KeepAliveTimeout { - get - { - return _keepAliveTimeout; - } - set - { - _keepAliveTimeout = value; - } + get => _keepAliveTimeout; + set => _keepAliveTimeout = value; } /// @@ -170,13 +153,58 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// public TimeSpan RequestHeadersTimeout { - get - { - return _requestHeadersTimeout; - } + get => _requestHeadersTimeout; + set => _requestHeadersTimeout = value; + } + + /// + /// Gets or sets the maximum number of open HTTP/S connections. When set to null, the number of connections is unlimited. + /// + /// + /// + /// Defaults to null. + /// + /// + /// When a connection is upgraded to another protocol, such as WebSockets, its connection is counted against the + /// limit instead of . + /// + /// + public long? MaxConcurrentConnections + { + get => _maxConcurrentConnections; set { - _requestHeadersTimeout = value; + if (value.HasValue && value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.PositiveNumberOrNullRequired); + } + _maxConcurrentConnections = value; + } + } + + /// + /// Gets or sets the maximum number of open, upgraded connections. When set to null, the number of upgraded connections is unlimited. + /// An upgraded connection is one that has been switched from HTTP to another protocol, such as WebSockets. + /// + /// + /// + /// Defaults to null. + /// + /// + /// When a connection is upgraded to another protocol, such as WebSockets, its connection is counted against the + /// limit instead of . + /// + /// + public long? MaxConcurrentUpgradedConnections + { + get => _maxConcurrentUpgradedConnections; + set + { + if (value.HasValue && value < 0) + { + throw new ArgumentOutOfRangeException(nameof(value), CoreStrings.NonNegativeNumberOrNullRequired); + } + _maxConcurrentUpgradedConnections = value; } } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs index 2971494a5b..5303d1e5b8 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -529,46 +529,60 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core => string.Format(CultureInfo.CurrentCulture, GetString("InvalidContentLength_InvalidNumber", "value"), value); /// - /// Value must be null or a non-negative integer. + /// Value must be null or a non-negative number. /// - internal static string NonNegativeNullableIntRequired + internal static string NonNegativeNumberOrNullRequired { - get => GetString("NonNegativeNullableIntRequired"); + get => GetString("NonNegativeNumberOrNullRequired"); } /// - /// Value must be null or a non-negative integer. + /// Value must be null or a non-negative number. /// - internal static string FormatNonNegativeNullableIntRequired() - => GetString("NonNegativeNullableIntRequired"); + internal static string FormatNonNegativeNumberOrNullRequired() + => GetString("NonNegativeNumberOrNullRequired"); /// - /// Value must be a positive integer. + /// Value must be a non-negative number. /// - internal static string PositiveIntRequired + internal static string NonNegativeNumberRequired { - get => GetString("PositiveIntRequired"); + get => GetString("NonNegativeNumberRequired"); } /// - /// Value must be a positive integer. + /// Value must be a non-negative number. /// - internal static string FormatPositiveIntRequired() - => GetString("PositiveIntRequired"); + internal static string FormatNonNegativeNumberRequired() + => GetString("NonNegativeNumberRequired"); /// - /// Value must be null or a positive integer. + /// Value must be a positive number. /// - internal static string PositiveNullableIntRequired + internal static string PositiveNumberRequired { - get => GetString("PositiveNullableIntRequired"); + get => GetString("PositiveNumberRequired"); } /// - /// Value must be null or a positive integer. + /// Value must be a positive number. /// - internal static string FormatPositiveNullableIntRequired() - => GetString("PositiveNullableIntRequired"); + internal static string FormatPositiveNumberRequired() + => GetString("PositiveNumberRequired"); + + /// + /// Value must be null or a positive number. + /// + internal static string PositiveNumberOrNullRequired + { + get => GetString("PositiveNumberOrNullRequired"); + } + + /// + /// Value must be null or a positive number. + /// + internal static string FormatPositiveNumberOrNullRequired() + => GetString("PositiveNumberOrNullRequired"); /// /// Unix socket path must be absolute. @@ -906,6 +920,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatCannotUpgradeNonUpgradableRequest() => GetString("CannotUpgradeNonUpgradableRequest"); + /// + /// Request cannot be upgraded because the server has already opened the maximum number of upgraded connections. + /// + internal static string UpgradedConnectionLimitReached + { + get => GetString("UpgradedConnectionLimitReached"); + } + + /// + /// Request cannot be upgraded because the server has already opened the maximum number of upgraded connections. + /// + internal static string FormatUpgradedConnectionLimitReached() + => GetString("UpgradedConnectionLimitReached"); + + /// + /// IHttpUpgradeFeature.UpgradeAsync was already called and can only be called once per connection. + /// + internal static string UpgradeCannotBeCalledMultipleTimes + { + get => GetString("UpgradeCannotBeCalledMultipleTimes"); + } + + /// + /// IHttpUpgradeFeature.UpgradeAsync was already called and can only be called once per connection. + /// + internal static string FormatUpgradeCannotBeCalledMultipleTimes() + => GetString("UpgradeCannotBeCalledMultipleTimes"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionManagerTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionManagerTests.cs index 7aaafa4022..a6e6bb4879 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionManagerTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameConnectionManagerTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { var connectionId = "0"; var trace = new Mock(); - var frameConnectionManager = new FrameConnectionManager(trace.Object); + var frameConnectionManager = new FrameConnectionManager(trace.Object, ResourceCounter.Unlimited, ResourceCounter.Unlimited); // Create FrameConnection in inner scope so it doesn't get rooted by the current frame. UnrootedConnectionsGetRemovedFromHeartbeatInnerScope(connectionId, frameConnectionManager, trace); diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/KestrelServerLimitsTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/KestrelServerLimitsTests.cs index a4bf75c59b..25db797284 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/KestrelServerLimitsTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/KestrelServerLimitsTests.cs @@ -105,10 +105,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [InlineData(0)] public void MaxRequestHeadersTotalSizeInvalid(int value) { - Assert.Throws(() => - { - (new KestrelServerLimits()).MaxRequestHeadersTotalSize = value; - }); + var ex = Assert.Throws(() => new KestrelServerLimits().MaxRequestHeadersTotalSize = value); + Assert.StartsWith(CoreStrings.PositiveNumberRequired, ex.Message); } [Theory] @@ -187,5 +185,61 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests o.RequestHeadersTimeout = TimeSpan.FromSeconds(seconds); Assert.Equal(seconds, o.RequestHeadersTimeout.TotalSeconds); } + + [Fact] + public void MaxConnectionsDefault() + { + Assert.Null(new KestrelServerLimits().MaxConcurrentConnections); + Assert.Null(new KestrelServerLimits().MaxConcurrentUpgradedConnections); + } + + [Theory] + [InlineData(null)] + [InlineData(1u)] + [InlineData(long.MaxValue)] + public void MaxConnectionsValid(long? value) + { + var limits = new KestrelServerLimits + { + MaxConcurrentConnections = value + }; + + Assert.Equal(value, limits.MaxConcurrentConnections); + } + + [Theory] + [InlineData(long.MinValue)] + [InlineData(-1)] + [InlineData(0)] + public void MaxConnectionsInvalid(long value) + { + var ex = Assert.Throws(() => new KestrelServerLimits().MaxConcurrentConnections = value); + Assert.StartsWith(CoreStrings.PositiveNumberOrNullRequired, ex.Message); + } + + [Theory] + [InlineData(null)] + [InlineData(0)] + [InlineData(1)] + [InlineData(long.MaxValue)] + public void MaxUpgradedConnectionsValid(long? value) + { + var limits = new KestrelServerLimits + { + MaxConcurrentUpgradedConnections = value + }; + + Assert.Equal(value, limits.MaxConcurrentUpgradedConnections); + } + + + [Theory] + [InlineData(long.MinValue)] + [InlineData(-1)] + public void MaxUpgradedConnectionsInvalid(long value) + { + var ex = Assert.Throws(() => new KestrelServerLimits().MaxConcurrentUpgradedConnections = value); + Assert.StartsWith(CoreStrings.NonNegativeNumberOrNullRequired, ex.Message); + } } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/ResourceCounterTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/ResourceCounterTests.cs new file mode 100644 index 0000000000..118b195248 --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/ResourceCounterTests.cs @@ -0,0 +1,56 @@ +// 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.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class ResourceCounterTests + { + [Theory] + [InlineData(-1)] + [InlineData(long.MinValue)] + public void QuotaInvalid(long max) + { + Assert.Throws(() => ResourceCounter.Quota(max)); + } + + [Fact] + public void QuotaAcceptsUpToButNotMoreThanMax() + { + var counter = ResourceCounter.Quota(1); + Assert.True(counter.TryLockOne()); + Assert.False(counter.TryLockOne()); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(10)] + [InlineData(100)] + public void QuotaValid(long max) + { + var counter = ResourceCounter.Quota(max); + Parallel.For(0, max, i => + { + Assert.True(counter.TryLockOne()); + }); + + Parallel.For(0, 10, i => + { + Assert.False(counter.TryLockOne()); + }); + } + + [Fact] + public void QuotaDoesNotWrapAround() + { + var counter = new ResourceCounter.FiniteCounter(long.MaxValue); + counter.Count = long.MaxValue; + Assert.False(counter.TryLockOne()); + } + } +} diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ConnectionLimitTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ConnectionLimitTests.cs new file mode 100644 index 0000000000..64ecf16f56 --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ConnectionLimitTests.cs @@ -0,0 +1,198 @@ +// 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.IO; +using System.Net.Sockets; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.Tests; +using Microsoft.AspNetCore.Testing; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests +{ + public class ConnectionLimitTests + { + [Fact] + public async Task ResetsCountWhenConnectionClosed() + { + var requestTcs = new TaskCompletionSource(); + var releasedTcs = new TaskCompletionSource(); + var lockedTcs = new TaskCompletionSource(); + var (serviceContext, counter) = SetupMaxConnections(max: 1); + counter.OnLock += (s, e) => lockedTcs.TrySetResult(e); + counter.OnRelease += (s, e) => releasedTcs.TrySetResult(null); + + using (var server = new TestServer(async context => + { + await context.Response.WriteAsync("Hello"); + await requestTcs.Task; + }, serviceContext)) + using (var connection = server.CreateConnection()) + { + await connection.SendEmptyGetAsKeepAlive(); ; + await connection.Receive("HTTP/1.1 200 OK"); + Assert.True(await lockedTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(10))); + requestTcs.TrySetResult(null); + } + + await releasedTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(10)); + } + + [Fact] + public async Task UpgradedConnectionsCountsAgainstDifferentLimit() + { + var (serviceContext, _) = SetupMaxConnections(max: 1); + using (var server = new TestServer(async context => + { + var feature = context.Features.Get(); + if (feature.IsUpgradableRequest) + { + var stream = await feature.UpgradeAsync(); + // keep it running until aborted + while (!context.RequestAborted.IsCancellationRequested) + { + await Task.Delay(100); + } + } + }, serviceContext)) + using (var disposables = new DisposableStack()) + { + var upgraded = server.CreateConnection(); + disposables.Push(upgraded); + + await upgraded.SendEmptyGetWithUpgrade(); + await upgraded.Receive("HTTP/1.1 101"); + // once upgraded, normal connection limit is decreased to allow room for more "normal" connections + + var connection = server.CreateConnection(); + disposables.Push(connection); + + await connection.SendEmptyGetAsKeepAlive(); + await connection.Receive("HTTP/1.1 200 OK"); + + using (var rejected = server.CreateConnection()) + { + try + { + // this may throw IOException, depending on how fast Kestrel closes the socket + await rejected.SendEmptyGetAsKeepAlive(); + } catch { } + + // connection should close without sending any data + await rejected.WaitForConnectionClose().TimeoutAfter(TimeSpan.FromSeconds(15)); + } + } + } + + [Fact] + public async Task RejectsConnectionsWhenLimitReached() + { + const int max = 10; + var (serviceContext, _) = SetupMaxConnections(max); + var requestTcs = new TaskCompletionSource(); + + using (var server = new TestServer(async context => + { + await context.Response.WriteAsync("Hello"); + await requestTcs.Task; + }, serviceContext)) + using (var disposables = new DisposableStack()) + { + for (var i = 0; i < max; i++) + { + var connection = server.CreateConnection(); + disposables.Push(connection); + + await connection.SendEmptyGetAsKeepAlive(); + await connection.Receive("HTTP/1.1 200 OK"); + } + + // limit has been reached + for (var i = 0; i < 10; i++) + { + using (var connection = server.CreateConnection()) + { + try + { + // this may throw IOException, depending on how fast Kestrel closes the socket + await connection.SendEmptyGetAsKeepAlive(); + } catch { } + + // connection should close without sending any data + await connection.WaitForConnectionClose().TimeoutAfter(TimeSpan.FromSeconds(15)); + } + } + + requestTcs.TrySetResult(null); + } + } + + [Fact] + public async Task ConnectionCountingReturnsToZero() + { + const int count = 500; + var opened = 0; + var closed = 0; + var openedTcs = new TaskCompletionSource(); + var closedTcs = new TaskCompletionSource(); + + var (serviceContext, counter) = SetupMaxConnections(uint.MaxValue); + + counter.OnLock += (o, e) => + { + if (e && Interlocked.Increment(ref opened) >= count) + { + openedTcs.TrySetResult(null); + } + }; + + counter.OnRelease += (o, e) => + { + if (Interlocked.Increment(ref closed) >= count) + { + closedTcs.TrySetResult(null); + } + }; + + using (var server = new TestServer(_ => Task.CompletedTask, serviceContext)) + { + // open a bunch of connections in parallel + Parallel.For(0, count, async i => + { + try + { + using (var connection = server.CreateConnection()) + { + await connection.SendEmptyGetAsKeepAlive(); + await connection.Receive("HTTP/1.1 200"); + } + } + catch (Exception ex) + { + closedTcs.TrySetException(ex); + } + }); + + // wait until resource counter has called lock for each connection + await openedTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(60)); + // wait until resource counter has released all normal connections + await closedTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(60)); + Assert.Equal(count, opened); + Assert.Equal(count, closed); + } + } + + private (TestServiceContext serviceContext, EventRaisingResourceCounter counter) SetupMaxConnections(long max) + { + var counter = new EventRaisingResourceCounter(ResourceCounter.Quota(max)); + var serviceContext = new TestServiceContext(); + serviceContext.ConnectionManager = new FrameConnectionManager(serviceContext.Log, counter, ResourceCounter.Unlimited); + return (serviceContext, counter); + } + } +} diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/FrameConnectionManagerTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/FrameConnectionManagerTests.cs index ab292378b0..ac71db8730 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/FrameConnectionManagerTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/FrameConnectionManagerTests.cs @@ -46,10 +46,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { using (var connection = server.CreateConnection()) { - await connection.Send("GET / HTTP/1.1", - "Host:", - "", - ""); + await connection.SendEmptyGet(); Assert.True(await appStartedWh.WaitAsync(TimeSpan.FromSeconds(10))); diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs index 698e3bd150..e05d50fc9d 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests Tuple.Create((long?)_dataLength - 1, false), // Buffer is exactly the same size as data. Exposed race condition where - // IConnectionControl.Resume() was called after socket was disconnected. + // the connection was resumed after socket was disconnected. Tuple.Create((long?)_dataLength, false), // Largest possible buffer, should never trigger backpressure. @@ -244,9 +244,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } - private static IWebHost StartWebHost(long? maxRequestBufferSize, - byte[] expectedBody, - bool useConnectionAdapter, + private static IWebHost StartWebHost(long? maxRequestBufferSize, + byte[] expectedBody, + bool useConnectionAdapter, TaskCompletionSource startReadingRequestBody, TaskCompletionSource clientFinishedSendingRequestBody) { diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs index b8f65c99b9..a5fbc348aa 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs @@ -662,10 +662,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var buffer = new char[identifierLength]; for (var i = 0; i < iterations; i++) { - await connection.Send("GET / HTTP/1.1", - "Host:", - "", - ""); + await connection.SendEmptyGet(); await connection.Receive($"HTTP/1.1 200 OK", $"Date: {server.Context.DateHeaderValue}", diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/UpgradeTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/UpgradeTests.cs index 3696219f1c..d1d981bed6 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/UpgradeTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/UpgradeTests.cs @@ -6,14 +6,24 @@ using System.IO; using System.Threading.Tasks; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.Tests; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Internal; using Xunit; +using Xunit.Abstractions; namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { public class UpgradeTests { + private readonly ITestOutputHelper _output; + + public UpgradeTests(ITestOutputHelper output) + { + _output = output; + } + [Fact] public async Task ResponseThrowsAfterUpgrade() { @@ -36,11 +46,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { using (var connection = server.CreateConnection()) { - await connection.Send("GET / HTTP/1.1", - "Host:", - "Connection: Upgrade", - "", - ""); + await connection.SendEmptyGetWithUpgrade(); await connection.Receive("HTTP/1.1 101 Switching Protocols", "Connection: Upgrade", $"Date: {server.Context.DateHeaderValue}", @@ -90,11 +96,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { using (var connection = server.CreateConnection()) { - await connection.Send("GET / HTTP/1.1", - "Host:", - "Connection: Upgrade", - "", - ""); + await connection.SendEmptyGetWithUpgrade(); await connection.Receive("HTTP/1.1 101 Switching Protocols", "Connection: Upgrade", @@ -110,6 +112,45 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } + [Fact] + public async Task UpgradeCannotBeCalledMultipleTimes() + { + var upgradeTcs = new TaskCompletionSource(); + using (var server = new TestServer(async context => + { + var feature = context.Features.Get(); + await feature.UpgradeAsync(); + + try + { + await feature.UpgradeAsync(); + } + catch (Exception e) + { + upgradeTcs.TrySetException(e); + throw; + } + + while (!context.RequestAborted.IsCancellationRequested) + { + await Task.Delay(100); + } + })) + using (var connection = server.CreateConnection()) + { + await connection.SendEmptyGetWithUpgrade(); + await connection.Receive("HTTP/1.1 101 Switching Protocols", + "Connection: Upgrade", + $"Date: {server.Context.DateHeaderValue}", + "", + ""); + await connection.WaitForConnectionClose().TimeoutAfter(TimeSpan.FromSeconds(15)); + } + + var ex = await Assert.ThrowsAsync(async () => await upgradeTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(15))); + Assert.Equal(CoreStrings.UpgradeCannotBeCalledMultipleTimes, ex.Message); + } + [Fact] public async Task RejectsRequestWithContentLengthAndUpgrade() { @@ -151,11 +192,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var connection = server.CreateConnection()) { - await connection.Send("GET / HTTP/1.1", - "Host:", - "Connection: Upgrade", - "", - ""); + await connection.SendEmptyGetWithUpgrade(); await connection.Receive("HTTP/1.1 200 OK"); } } @@ -207,10 +244,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { using (var connection = server.CreateConnection()) { - await connection.Send("GET / HTTP/1.1", - "Host:", - "", - ""); + await connection.SendEmptyGet(); await connection.Receive("HTTP/1.1 200 OK"); } } @@ -218,5 +252,56 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var ex = await Assert.ThrowsAsync(async () => await upgradeTcs.Task).TimeoutAfter(TimeSpan.FromSeconds(15)); Assert.Equal(CoreStrings.CannotUpgradeNonUpgradableRequest, ex.Message); } + + [Fact] + public async Task RejectsUpgradeWhenLimitReached() + { + const int limit = 10; + var upgradeTcs = new TaskCompletionSource(); + var serviceContext = new TestServiceContext(); + serviceContext.ConnectionManager = new FrameConnectionManager(serviceContext.Log, ResourceCounter.Unlimited, ResourceCounter.Quota(limit)); + + using (var server = new TestServer(async context => + { + var feature = context.Features.Get(); + if (feature.IsUpgradableRequest) + { + try + { + var stream = await feature.UpgradeAsync(); + while (!context.RequestAborted.IsCancellationRequested) + { + await Task.Delay(100); + } + } + catch (InvalidOperationException ex) + { + upgradeTcs.TrySetException(ex); + } + } + }, serviceContext)) + { + using (var disposables = new DisposableStack()) + { + for (var i = 0; i < limit; i++) + { + var connection = server.CreateConnection(); + disposables.Push(connection); + + await connection.SendEmptyGetWithUpgradeAndKeepAlive(); + await connection.Receive("HTTP/1.1 101"); + } + + using (var connection = server.CreateConnection()) + { + await connection.SendEmptyGetWithUpgradeAndKeepAlive(); + await connection.Receive("HTTP/1.1 200"); + } + } + } + + var exception = await Assert.ThrowsAsync(async () => await upgradeTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(60))); + Assert.Equal(CoreStrings.UpgradedConnectionLimitReached, exception.Message); + } } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTimeoutControl.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTimeoutControl.cs index 3acf5e626c..1f721f07d1 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTimeoutControl.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTimeoutControl.cs @@ -1,6 +1,6 @@ -using System; -using System.Collections.Generic; -using System.Text; +// 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.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Performance.Mocks diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTrace.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTrace.cs index 1ab3c8df70..30fb4f3e2f 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTrace.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTrace.cs @@ -22,6 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public void ConnectionReadFin(string connectionId) { } public void ConnectionReset(string connectionId) { } public void ConnectionResume(string connectionId) { } + public void ConnectionRejected(string connectionId) { } public void ConnectionStart(string connectionId) { } public void ConnectionStop(string connectionId) { } public void ConnectionWrite(string connectionId, int count) { } diff --git a/test/shared/DisposableStack.cs b/test/shared/DisposableStack.cs new file mode 100644 index 0000000000..325fc6f8c8 --- /dev/null +++ b/test/shared/DisposableStack.cs @@ -0,0 +1,20 @@ +// 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.Collections.Generic; + +namespace Microsoft.AspNetCore.Server.Kestrel.Tests +{ + public class DisposableStack : Stack, IDisposable + where T : IDisposable + { + public void Dispose() + { + while (Count > 0) + { + Pop()?.Dispose(); + } + } + } +} diff --git a/test/shared/EventRaisingResourceCounter.cs b/test/shared/EventRaisingResourceCounter.cs new file mode 100644 index 0000000000..8f7d5efea3 --- /dev/null +++ b/test/shared/EventRaisingResourceCounter.cs @@ -0,0 +1,34 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; + +namespace Microsoft.AspNetCore.Server.Kestrel.Tests +{ + public class EventRaisingResourceCounter : ResourceCounter + { + private readonly ResourceCounter _wrapped; + + public EventRaisingResourceCounter(ResourceCounter wrapped) + { + _wrapped = wrapped; + } + + public event EventHandler OnRelease; + public event EventHandler OnLock; + + public override void ReleaseOne() + { + _wrapped.ReleaseOne(); + OnRelease?.Invoke(this, EventArgs.Empty); + } + + public override bool TryLockOne() + { + var retVal = _wrapped.TryLockOne(); + OnLock?.Invoke(this, retVal); + return retVal; + } + } +} diff --git a/test/shared/TestConnection.cs b/test/shared/TestConnection.cs index bc21df517e..f4ff0a2b62 100644 --- a/test/shared/TestConnection.cs +++ b/test/shared/TestConnection.cs @@ -61,6 +61,32 @@ namespace Microsoft.AspNetCore.Testing } } + public Task SendEmptyGet() + { + return Send("GET / HTTP/1.1", + "Host:", + "", + ""); + } + + public Task SendEmptyGetWithUpgradeAndKeepAlive() + => SendEmptyGetWithConnection("Upgrade, keep-alive"); + + public Task SendEmptyGetWithUpgrade() + => SendEmptyGetWithConnection("Upgrade"); + + public Task SendEmptyGetAsKeepAlive() + => SendEmptyGetWithConnection("keep-alive"); + + private Task SendEmptyGetWithConnection(string connection) + { + return Send("GET / HTTP/1.1", + "Host:", + "Connection: " + connection, + "", + ""); + } + public async Task SendAll(params string[] lines) { var text = string.Join("\r\n", lines); diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index b06362b3fe..9d59a66d44 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Testing ThreadPool = new LoggingThreadPool(Log); SystemClock = new MockSystemClock(); DateHeaderValueManager = new DateHeaderValueManager(SystemClock); - ConnectionManager = new FrameConnectionManager(Log); + ConnectionManager = new FrameConnectionManager(Log, ResourceCounter.Unlimited, ResourceCounter.Unlimited); DateHeaderValue = DateHeaderValueManager.GetDateHeaderValues().String; HttpParserFactory = frameAdapter => new HttpParser(frameAdapter.Frame.ServiceContext.Log.IsEnabled(LogLevel.Information)); ServerOptions = new KestrelServerOptions