From 891ba8d42bf95b91c8ae50fedc3285eded6425ed Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Mon, 9 Aug 2021 14:51:42 -0700 Subject: [PATCH 1/2] Remove QuicTransportOptions.IdleTimeout --- .../Internal/KestrelServerLimitsFeature.cs | 12 ++++++++++ .../src/Internal/QuicConnectionListener.cs | 4 ++-- .../src/PublicAPI.Unshipped.txt | 2 -- .../src/QuicTransportFactory.cs | 9 +++++++- .../src/QuicTransportOptions.cs | 5 ----- .../Transport.Quic/test/QuicTestHelpers.cs | 6 ++++- .../test/QuicTransportFactoryTests.cs | 22 +++++++++++++++++++ 7 files changed, 49 insertions(+), 11 deletions(-) create mode 100644 src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs new file mode 100644 index 000000000000..bea44ae8aac5 --- /dev/null +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Internal +{ + internal class KestrelServerLimitsFeature + { + public TimeSpan KeepAliveTimeout { get; set; } + } +} diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs index 975aa8bf3946..0d6d8ea61175 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs @@ -24,7 +24,7 @@ internal class QuicConnectionListener : IMultiplexedConnectionListener, IAsyncDi private readonly QuicTransportContext _context; private readonly QuicListener _listener; - public QuicConnectionListener(QuicTransportOptions options, IQuicTrace log, EndPoint endpoint, SslServerAuthenticationOptions sslServerAuthenticationOptions) + public QuicConnectionListener(QuicTransportOptions options, IQuicTrace log, EndPoint endpoint, SslServerAuthenticationOptions sslServerAuthenticationOptions, KestrelServerLimitsFeature kestrelServerLimitsFeature) { if (!QuicImplementationProviders.Default.IsSupported) { @@ -37,9 +37,9 @@ public QuicConnectionListener(QuicTransportOptions options, IQuicTrace log, EndP quicListenerOptions.ServerAuthenticationOptions = sslServerAuthenticationOptions; quicListenerOptions.ListenEndPoint = endpoint as IPEndPoint; - quicListenerOptions.IdleTimeout = options.IdleTimeout; quicListenerOptions.MaxBidirectionalStreams = options.MaxBidirectionalStreamCount; quicListenerOptions.MaxUnidirectionalStreams = options.MaxUnidirectionalStreamCount; + quicListenerOptions.IdleTimeout = kestrelServerLimitsFeature.KeepAliveTimeout; _listener = new QuicListener(quicListenerOptions); diff --git a/src/Servers/Kestrel/Transport.Quic/src/PublicAPI.Unshipped.txt b/src/Servers/Kestrel/Transport.Quic/src/PublicAPI.Unshipped.txt index 4ab5cd10661e..6e2af063e8e7 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/PublicAPI.Unshipped.txt +++ b/src/Servers/Kestrel/Transport.Quic/src/PublicAPI.Unshipped.txt @@ -26,8 +26,6 @@ *REMOVED*~static Microsoft.AspNetCore.Hosting.WebHostBuilderMsQuicExtensions.UseQuic(this Microsoft.AspNetCore.Hosting.IWebHostBuilder hostBuilder) -> Microsoft.AspNetCore.Hosting.IWebHostBuilder Microsoft.AspNetCore.Hosting.WebHostBuilderQuicExtensions Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.QuicTransportOptions -Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.QuicTransportOptions.IdleTimeout.get -> System.TimeSpan -Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.QuicTransportOptions.IdleTimeout.set -> void Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.QuicTransportOptions.MaxBidirectionalStreamCount.get -> ushort Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.QuicTransportOptions.MaxBidirectionalStreamCount.set -> void Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.QuicTransportOptions.MaxReadBufferSize.get -> long? diff --git a/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs b/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs index db03c9a308b6..15a7bbaca4ee 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs @@ -60,7 +60,14 @@ public ValueTask BindAsync(EndPoint endpoint, IF throw new InvalidOperationException(message); } - var transport = new QuicConnectionListener(_options, _log, endpoint, sslServerAuthenticationOptions); + var kestrelServerLimitsFeature = features?.Get(); + + if (kestrelServerLimitsFeature == null) + { + throw new InvalidOperationException("Couldn't find server limits configuration for QUIC transport."); + } + + var transport = new QuicConnectionListener(_options, _log, endpoint, sslServerAuthenticationOptions, kestrelServerLimitsFeature); return new ValueTask(transport); } } diff --git a/src/Servers/Kestrel/Transport.Quic/src/QuicTransportOptions.cs b/src/Servers/Kestrel/Transport.Quic/src/QuicTransportOptions.cs index 8283fed106a2..7accc13bc108 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/QuicTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/QuicTransportOptions.cs @@ -23,11 +23,6 @@ public class QuicTransportOptions /// public ushort MaxUnidirectionalStreamCount { get; set; } = 10; - /// - /// Sets the idle timeout for connections and streams. - /// - public TimeSpan IdleTimeout { get; set; } = TimeSpan.FromSeconds(130); // Matches KestrelServerLimits.KeepAliveTimeout. - /// /// The maximum read size. /// diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs index fc5edcfa7004..f56659e092a0 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Internal; using Microsoft.AspNetCore.Testing; @@ -31,7 +32,6 @@ internal static class QuicTestHelpers public static QuicTransportFactory CreateTransportFactory(ILoggerFactory loggerFactory = null, ISystemClock systemClock = null) { var quicTransportOptions = new QuicTransportOptions(); - quicTransportOptions.IdleTimeout = TimeSpan.FromMinutes(1); quicTransportOptions.MaxBidirectionalStreamCount = 200; quicTransportOptions.MaxUnidirectionalStreamCount = 200; if (systemClock != null) @@ -63,8 +63,12 @@ public static FeatureCollection CreateBindAsyncFeatures(bool clientCertificateRe sslServerAuthenticationOptions.RemoteCertificateValidationCallback = RemoteCertificateValidationCallback; sslServerAuthenticationOptions.ClientCertificateRequired = clientCertificateRequired; + var kestrelServerLimitsFeature = new KestrelServerLimitsFeature(); + kestrelServerLimitsFeature.KeepAliveTimeout = new KestrelServerLimits().KeepAliveTimeout; + var features = new FeatureCollection(); features.Set(sslServerAuthenticationOptions); + features.Set(kestrelServerLimitsFeature); return features; } diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs index 730be811833b..897070e54b44 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs @@ -51,5 +51,27 @@ public async Task BindAsync_NoServerCertificate_Error() // Assert Assert.Equal("SslServerAuthenticationOptions.ServerCertificate must be configured with a value.", ex.Message); } + + [ConditionalFact] + [MsQuicSupported] + public async Task BindAsync_NoKestrelServerLimits_Error() + { + // Arrange + var quicTransportOptions = new QuicTransportOptions(); + var quicTransportFactory = new QuicTransportFactory(NullLoggerFactory.Instance, Options.Create(quicTransportOptions)); + var features = new FeatureCollection(); + + var cert = TestResources.GetTestCertificate(); + + var sslServerAuthenticationOptions = new SslServerAuthenticationOptions(); + sslServerAuthenticationOptions.ServerCertificate = cert; + features.Set(sslServerAuthenticationOptions); + + // Act + var ex = await Assert.ThrowsAsync(() => quicTransportFactory.BindAsync(new IPEndPoint(0, 0), features: features, cancellationToken: CancellationToken.None).AsTask()).DefaultTimeout(); + + // Assert + Assert.Equal("Couldn't find server limits configuration for QUIC transport.", ex.Message); + } } } From dafa63cab23aaa9fc6372fb80e24dd46bcbaf1d1 Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Wed, 11 Aug 2021 14:20:26 -0700 Subject: [PATCH 2/2] Remove Feature --- .../Internal/KestrelServerLimitsFeature.cs | 12 ---------- .../src/Internal/QuicConnectionListener.cs | 6 +++-- .../src/QuicTransportFactory.cs | 9 +------- .../Transport.Quic/test/QuicTestHelpers.cs | 5 ----- .../test/QuicTransportFactoryTests.cs | 22 ------------------- 5 files changed, 5 insertions(+), 49 deletions(-) delete mode 100644 src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs deleted file mode 100644 index bea44ae8aac5..000000000000 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/KestrelServerLimitsFeature.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; - -namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Internal -{ - internal class KestrelServerLimitsFeature - { - public TimeSpan KeepAliveTimeout { get; set; } - } -} diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs index 0d6d8ea61175..d09c75753f3c 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs @@ -24,7 +24,7 @@ internal class QuicConnectionListener : IMultiplexedConnectionListener, IAsyncDi private readonly QuicTransportContext _context; private readonly QuicListener _listener; - public QuicConnectionListener(QuicTransportOptions options, IQuicTrace log, EndPoint endpoint, SslServerAuthenticationOptions sslServerAuthenticationOptions, KestrelServerLimitsFeature kestrelServerLimitsFeature) + public QuicConnectionListener(QuicTransportOptions options, IQuicTrace log, EndPoint endpoint, SslServerAuthenticationOptions sslServerAuthenticationOptions) { if (!QuicImplementationProviders.Default.IsSupported) { @@ -39,7 +39,9 @@ public QuicConnectionListener(QuicTransportOptions options, IQuicTrace log, EndP quicListenerOptions.ListenEndPoint = endpoint as IPEndPoint; quicListenerOptions.MaxBidirectionalStreams = options.MaxBidirectionalStreamCount; quicListenerOptions.MaxUnidirectionalStreams = options.MaxUnidirectionalStreamCount; - quicListenerOptions.IdleTimeout = kestrelServerLimitsFeature.KeepAliveTimeout; + // Set Quic idle timeout to a conservative 10 minutes - we handle idle itemouts gracefully in the Kestrel layer + // based off of KestrelServerLimits.KeepAliveTimeout + quicListenerOptions.IdleTimeout = TimeSpan.FromMinutes(10); _listener = new QuicListener(quicListenerOptions); diff --git a/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs b/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs index 15a7bbaca4ee..db03c9a308b6 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs @@ -60,14 +60,7 @@ public ValueTask BindAsync(EndPoint endpoint, IF throw new InvalidOperationException(message); } - var kestrelServerLimitsFeature = features?.Get(); - - if (kestrelServerLimitsFeature == null) - { - throw new InvalidOperationException("Couldn't find server limits configuration for QUIC transport."); - } - - var transport = new QuicConnectionListener(_options, _log, endpoint, sslServerAuthenticationOptions, kestrelServerLimitsFeature); + var transport = new QuicConnectionListener(_options, _log, endpoint, sslServerAuthenticationOptions); return new ValueTask(transport); } } diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs index f56659e092a0..51e413d945af 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs @@ -13,7 +13,6 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Internal; using Microsoft.AspNetCore.Testing; @@ -63,12 +62,8 @@ public static FeatureCollection CreateBindAsyncFeatures(bool clientCertificateRe sslServerAuthenticationOptions.RemoteCertificateValidationCallback = RemoteCertificateValidationCallback; sslServerAuthenticationOptions.ClientCertificateRequired = clientCertificateRequired; - var kestrelServerLimitsFeature = new KestrelServerLimitsFeature(); - kestrelServerLimitsFeature.KeepAliveTimeout = new KestrelServerLimits().KeepAliveTimeout; - var features = new FeatureCollection(); features.Set(sslServerAuthenticationOptions); - features.Set(kestrelServerLimitsFeature); return features; } diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs index 897070e54b44..730be811833b 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicTransportFactoryTests.cs @@ -51,27 +51,5 @@ public async Task BindAsync_NoServerCertificate_Error() // Assert Assert.Equal("SslServerAuthenticationOptions.ServerCertificate must be configured with a value.", ex.Message); } - - [ConditionalFact] - [MsQuicSupported] - public async Task BindAsync_NoKestrelServerLimits_Error() - { - // Arrange - var quicTransportOptions = new QuicTransportOptions(); - var quicTransportFactory = new QuicTransportFactory(NullLoggerFactory.Instance, Options.Create(quicTransportOptions)); - var features = new FeatureCollection(); - - var cert = TestResources.GetTestCertificate(); - - var sslServerAuthenticationOptions = new SslServerAuthenticationOptions(); - sslServerAuthenticationOptions.ServerCertificate = cert; - features.Set(sslServerAuthenticationOptions); - - // Act - var ex = await Assert.ThrowsAsync(() => quicTransportFactory.BindAsync(new IPEndPoint(0, 0), features: features, cancellationToken: CancellationToken.None).AsTask()).DefaultTimeout(); - - // Assert - Assert.Equal("Couldn't find server limits configuration for QUIC transport.", ex.Message); - } } }