From d784a5ba13f97a5ec3b4432915e90cd169d2c15f Mon Sep 17 00:00:00 2001 From: Kahbazi Date: Wed, 22 Feb 2023 20:22:06 +0330 Subject: [PATCH 01/11] Resolve conflict --- ...oft.AspNetCore.Http.Microbenchmarks.csproj | 1 + .../RequestTimeoutsMiddlewareBenchmark.cs | 149 +++++++ .../Http/src/Microsoft.AspNetCore.Http.csproj | 2 + src/Http/Http/src/PublicAPI.Unshipped.txt | 37 +- .../src/Timeouts/CancellationTokenLinker.cs | 20 + .../DisableRequestTimeoutAttribute.cs | 15 + .../src/Timeouts/HttpRequestTimeoutFeature.cs | 21 + .../src/Timeouts/ICancellationTokenLinker.cs | 9 + .../Timeouts/IHttpRequestTimeoutFeature.cs | 20 + .../Http/src/Timeouts/LoggerExtensions.cs | 12 + .../src/Timeouts/RequestTimeoutAttribute.cs | 43 ++ .../src/Timeouts/RequestTimeoutOptions.cs | 52 +++ .../Http/src/Timeouts/RequestTimeoutPolicy.cs | 31 ++ ...stTimeoutsIApplicationBuilderExtensions.cs | 26 ++ ...utsIEndpointConventionBuilderExtensions.cs | 64 +++ ...estTimeoutsIServiceCollectionExtensions.cs | 35 ++ .../src/Timeouts/RequestTimeoutsMiddleware.cs | 138 ++++++ .../test/Timeouts/RequestTimeoutOptions.cs | 61 +++ .../RequestTimeoutsMiddlewareTests.cs | 402 ++++++++++++++++++ 19 files changed, 1137 insertions(+), 1 deletion(-) create mode 100644 src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs create mode 100644 src/Http/Http/src/Timeouts/CancellationTokenLinker.cs create mode 100644 src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs create mode 100644 src/Http/Http/src/Timeouts/HttpRequestTimeoutFeature.cs create mode 100644 src/Http/Http/src/Timeouts/ICancellationTokenLinker.cs create mode 100644 src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs create mode 100644 src/Http/Http/src/Timeouts/LoggerExtensions.cs create mode 100644 src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs create mode 100644 src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs create mode 100644 src/Http/Http/src/Timeouts/RequestTimeoutPolicy.cs create mode 100644 src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs create mode 100644 src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs create mode 100644 src/Http/Http/src/Timeouts/RequestTimeoutsIServiceCollectionExtensions.cs create mode 100644 src/Http/Http/src/Timeouts/RequestTimeoutsMiddleware.cs create mode 100644 src/Http/Http/test/Timeouts/RequestTimeoutOptions.cs create mode 100644 src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs diff --git a/src/Http/Http/perf/Microbenchmarks/Microsoft.AspNetCore.Http.Microbenchmarks.csproj b/src/Http/Http/perf/Microbenchmarks/Microsoft.AspNetCore.Http.Microbenchmarks.csproj index f5d277301b12..112bed919ab3 100644 --- a/src/Http/Http/perf/Microbenchmarks/Microsoft.AspNetCore.Http.Microbenchmarks.csproj +++ b/src/Http/Http/perf/Microbenchmarks/Microsoft.AspNetCore.Http.Microbenchmarks.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs b/src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs new file mode 100644 index 000000000000..7b3c71bdc5b4 --- /dev/null +++ b/src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs @@ -0,0 +1,149 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Http.Timeouts; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Http; + +public class RequestTimeoutsMiddlewareBenchmark +{ + RequestTimeoutsMiddleware _middlewareWithNoTimeout; + RequestTimeoutsMiddleware _middleware; + RequestTimeoutsMiddleware _middlewareWithThrow; + + [GlobalSetup] + public void GlobalSetup() + { + _middlewareWithNoTimeout = new RequestTimeoutsMiddleware( + async context => { await Task.Yield(); }, + new CancellationTokenLinker(), + NullLogger.Instance, + Options.Create(new RequestTimeoutOptions())); + + _middleware = new RequestTimeoutsMiddleware( + async context => { await Task.Yield(); }, + new CancellationTokenLinker(), + NullLogger.Instance, + Options.Create(new RequestTimeoutOptions + { + DefaultPolicy = new RequestTimeoutPolicy + { + Timeout = TimeSpan.FromMilliseconds(200) + }, + Policies = + { + ["policy1"] = new RequestTimeoutPolicy { Timeout = TimeSpan.FromMilliseconds(200)} + } + })); + + _middlewareWithThrow = new RequestTimeoutsMiddleware( + async context => + { + await Task.Delay(TimeSpan.FromMicroseconds(2)); + context.RequestAborted.ThrowIfCancellationRequested(); + }, + new CancellationTokenLinker(), + NullLogger.Instance, + Options.Create(new RequestTimeoutOptions + { + DefaultPolicy = new RequestTimeoutPolicy + { + Timeout = TimeSpan.FromMicroseconds(1) + } + })); + } + + [Benchmark] + public async Task NoMetadataNoDefault() + { + var context = CreateHttpContext(new Endpoint(null, null, null)); + await _middlewareWithNoTimeout.Invoke(context); + } + + [Benchmark] + public async Task DefaultTimeout() + { + var context = CreateHttpContext(new Endpoint(null, null, null)); + + await _middleware.Invoke(context); + } + + [Benchmark] + public async Task DefaultTimeoutOverridenByDisable() + { + var context = CreateHttpContext(new Endpoint( + null, + new EndpointMetadataCollection(new DisableRequestTimeoutAttribute()), + null)); + + await _middleware.Invoke(context); + } + + [Benchmark] + public async Task TimeoutMetadata() + { + var context = CreateHttpContext(new Endpoint( + null, + new EndpointMetadataCollection(new RequestTimeoutAttribute(200)), + null)); + + await _middleware.Invoke(context); + } + + [Benchmark] + public async Task NamedPolicyMetadata() + { + var context = CreateHttpContext(new Endpoint( + null, + new EndpointMetadataCollection(new RequestTimeoutAttribute("policy1")), + null)); + + await _middleware.Invoke(context); + } + + [Benchmark] + public async Task TimeoutFires() + { + var context = CreateHttpContext(new Endpoint(null, null, null)); + + await _middlewareWithThrow.Invoke(context); + } + + private HttpContext CreateHttpContext(Endpoint endpoint) + { + var context = new DefaultHttpContext(); + context.SetEndpoint(endpoint); + + var cts = new CancellationTokenSource(); + context.RequestAborted = cts.Token; + + return context; + } + + private sealed class Options : IOptionsMonitor + { + private readonly RequestTimeoutOptions _options; + + private Options(RequestTimeoutOptions options) + { + _options = options; + } + + public static Options Create(RequestTimeoutOptions options) + { + return new Options(options); + } + + public RequestTimeoutOptions CurrentValue => _options; + + public RequestTimeoutOptions Get(string name) => _options; + + public IDisposable OnChange(Action listener) + { + return default; + } + } +} diff --git a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj index 685700a8b76d..62445e7140b6 100644 --- a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj +++ b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj @@ -12,6 +12,7 @@ + @@ -25,6 +26,7 @@ + diff --git a/src/Http/Http/src/PublicAPI.Unshipped.txt b/src/Http/Http/src/PublicAPI.Unshipped.txt index 9a6f8ff2c485..873662a02b6c 100644 --- a/src/Http/Http/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http/src/PublicAPI.Unshipped.txt @@ -1,3 +1,38 @@ #nullable enable +Microsoft.AspNetCore.Builder.RequestTimeoutsIApplicationBuilderExtensions +Microsoft.AspNetCore.Builder.RequestTimeoutsIEndpointConventionBuilderExtensions Microsoft.AspNetCore.Http.BindingAddress.IsNamedPipe.get -> bool -Microsoft.AspNetCore.Http.BindingAddress.NamedPipeName.get -> string! \ No newline at end of file +Microsoft.AspNetCore.Http.BindingAddress.NamedPipeName.get -> string! +Microsoft.AspNetCore.Http.Timeouts.DisableRequestTimeoutAttribute +Microsoft.AspNetCore.Http.Timeouts.DisableRequestTimeoutAttribute.DisableRequestTimeoutAttribute() -> void +Microsoft.AspNetCore.Http.Timeouts.IHttpRequestTimeoutFeature +Microsoft.AspNetCore.Http.Timeouts.IHttpRequestTimeoutFeature.DisableTimeout() -> void +Microsoft.AspNetCore.Http.Timeouts.IHttpRequestTimeoutFeature.RequestTimeoutToken.get -> System.Threading.CancellationToken +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutAttribute +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutAttribute.PolicyName.get -> string? +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutAttribute.RequestTimeoutAttribute(int milliseconds) -> void +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutAttribute.RequestTimeoutAttribute(string! policyName) -> void +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutAttribute.Timeout.get -> System.TimeSpan? +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions.AddPolicy(string! policyName, Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy! policy) -> Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions! +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions.AddPolicy(string! policyName, System.TimeSpan timeout) -> Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions! +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions.DefaultPolicy.get -> Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy? +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions.DefaultPolicy.set -> void +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions.Policies.get -> System.Collections.Generic.IDictionary! +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutOptions.RequestTimeoutOptions() -> void +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy.RequestTimeoutPolicy() -> void +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy.Timeout.get -> System.TimeSpan? +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy.Timeout.init -> void +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy.TimeoutStatusCode.get -> int? +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy.TimeoutStatusCode.init -> void +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy.WriteTimeoutResponse.get -> Microsoft.AspNetCore.Http.RequestDelegate? +Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy.WriteTimeoutResponse.init -> void +Microsoft.Extensions.DependencyInjection.RequestTimeoutsIServiceCollectionExtensions +static Microsoft.AspNetCore.Builder.RequestTimeoutsIApplicationBuilderExtensions.UseRequestTimeouts(this Microsoft.AspNetCore.Builder.IApplicationBuilder! builder) -> Microsoft.AspNetCore.Builder.IApplicationBuilder! +static Microsoft.AspNetCore.Builder.RequestTimeoutsIEndpointConventionBuilderExtensions.DisableRequestTimeout(this Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! builder) -> Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.RequestTimeoutsIEndpointConventionBuilderExtensions.WithRequestTimeout(this Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! builder, Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy! policy) -> Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.RequestTimeoutsIEndpointConventionBuilderExtensions.WithRequestTimeout(this Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! builder, string! policyName) -> Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.RequestTimeoutsIEndpointConventionBuilderExtensions.WithRequestTimeout(this Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! builder, System.TimeSpan timeout) -> Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! +static Microsoft.Extensions.DependencyInjection.RequestTimeoutsIServiceCollectionExtensions.AddRequestTimeouts(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! +static Microsoft.Extensions.DependencyInjection.RequestTimeoutsIServiceCollectionExtensions.AddRequestTimeouts(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/Http/Http/src/Timeouts/CancellationTokenLinker.cs b/src/Http/Http/src/Timeouts/CancellationTokenLinker.cs new file mode 100644 index 000000000000..95f64b2192de --- /dev/null +++ b/src/Http/Http/src/Timeouts/CancellationTokenLinker.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Internal; + +namespace Microsoft.AspNetCore.Http.Timeouts; + +internal sealed class CancellationTokenLinker : ICancellationTokenLinker +{ + private readonly CancellationTokenSourcePool _ctsPool = new(); + + public (CancellationTokenSource linkedCts, CancellationTokenSource timeoutCts) GetLinkedCancellationTokenSource(HttpContext httpContext, CancellationToken originalToken, TimeSpan timeSpan) + { + var timeoutCts = _ctsPool.Rent(); + timeoutCts.CancelAfter(timeSpan); + httpContext.Response.RegisterForDispose(timeoutCts); + var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(originalToken, timeoutCts.Token); + return (linkedCts, timeoutCts); + } +} diff --git a/src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs b/src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs new file mode 100644 index 000000000000..34194b046dd0 --- /dev/null +++ b/src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http.Timeouts; + +/// +/// Metadata that disables request timeouts limiting on an endpoint. +/// +/// +/// Completely disables the request timeouts middleware from applying to this endpoint. +/// +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Class)] +public sealed class DisableRequestTimeoutAttribute : Attribute +{ +} diff --git a/src/Http/Http/src/Timeouts/HttpRequestTimeoutFeature.cs b/src/Http/Http/src/Timeouts/HttpRequestTimeoutFeature.cs new file mode 100644 index 000000000000..bc71d12d5f3f --- /dev/null +++ b/src/Http/Http/src/Timeouts/HttpRequestTimeoutFeature.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http.Timeouts; + +internal sealed class HttpRequestTimeoutFeature : IHttpRequestTimeoutFeature +{ + private readonly CancellationTokenSource _timeoutCancellationTokenSource; + + public HttpRequestTimeoutFeature(CancellationTokenSource timeoutCancellationTokenSource) + { + _timeoutCancellationTokenSource = timeoutCancellationTokenSource; + } + + public CancellationToken RequestTimeoutToken => _timeoutCancellationTokenSource.Token; + + public void DisableTimeout() + { + _timeoutCancellationTokenSource.CancelAfter(Timeout.Infinite); + } +} diff --git a/src/Http/Http/src/Timeouts/ICancellationTokenLinker.cs b/src/Http/Http/src/Timeouts/ICancellationTokenLinker.cs new file mode 100644 index 000000000000..a0da31190f25 --- /dev/null +++ b/src/Http/Http/src/Timeouts/ICancellationTokenLinker.cs @@ -0,0 +1,9 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http.Timeouts; + +internal interface ICancellationTokenLinker +{ + (CancellationTokenSource linkedCts, CancellationTokenSource timeoutCts) GetLinkedCancellationTokenSource(HttpContext httpContext, CancellationToken originalToken, TimeSpan timeSpan); +} diff --git a/src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs b/src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs new file mode 100644 index 000000000000..6ae362189bf2 --- /dev/null +++ b/src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http.Timeouts; + +/// +/// +/// +public interface IHttpRequestTimeoutFeature +{ + /// + /// + /// + CancellationToken RequestTimeoutToken { get; } + + /// + /// + /// + void DisableTimeout(); +} diff --git a/src/Http/Http/src/Timeouts/LoggerExtensions.cs b/src/Http/Http/src/Timeouts/LoggerExtensions.cs new file mode 100644 index 000000000000..13fd14335504 --- /dev/null +++ b/src/Http/Http/src/Timeouts/LoggerExtensions.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 Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Http.Timeouts; + +internal static partial class LoggerExtensions +{ + [LoggerMessage(1, LogLevel.Warning, "Timeout exception handled.")] + public static partial void TimeoutExceptionHandled(this ILogger logger, Exception exception); +} diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs b/src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs new file mode 100644 index 000000000000..88d85002b52e --- /dev/null +++ b/src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http.Timeouts; + +/// +/// Metadata that provides endpoint-specific request timeouts. +/// +/// +/// The default policy will be ignored with this attribute applied. +/// +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Class)] +public sealed class RequestTimeoutAttribute : Attribute +{ + /// + /// The timeout to apply for this endpoint. + /// + public TimeSpan? Timeout { get; } + + /// + /// The name of the policy which needs to be applied. + /// This value is case insensitve. + /// + public string? PolicyName { get; } + + /// + /// Creates a new instance of using the specified timeout. + /// + /// The amount of timeout for this specific endpoint. + public RequestTimeoutAttribute(int milliseconds) + { + Timeout = TimeSpan.FromMilliseconds(milliseconds); + } + + /// + /// Creates a new instance of using the specified policy. + /// + /// The case-insensitve name of the policy which needs to be applied. + public RequestTimeoutAttribute(string policyName) + { + PolicyName = policyName; + } +} diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs new file mode 100644 index 000000000000..73fe285986cb --- /dev/null +++ b/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http.Timeouts; + +/// +/// Specifies options for the request timeouts middleware. +/// +public sealed class RequestTimeoutOptions +{ + /// + /// Applied to any request without a policy set via endpoint metadata. No value by default. + /// + public RequestTimeoutPolicy? DefaultPolicy { get; set; } + + /// + /// Dictionary of policies that would be applied per endpoint. + /// Policy names are case insensitive. + /// + public IDictionary Policies { get; } = new Dictionary(StringComparer.OrdinalIgnoreCase); + + /// + /// Adds a new policy. + /// + /// The case-insensitive name of the policy. + /// The timeout to apply for this policy. + public RequestTimeoutOptions AddPolicy(string policyName, TimeSpan timeout) + { + return AddPolicy(policyName, new RequestTimeoutPolicy + { + Timeout = timeout + }); + } + + /// + /// Adds a new policy. + /// + /// The case-insensitive name of the policy. + /// The policy to be added. + public RequestTimeoutOptions AddPolicy(string policyName, RequestTimeoutPolicy policy) + { + _ = policy ?? throw new ArgumentNullException(nameof(policy)); + + if (string.IsNullOrEmpty(policyName)) + { + throw new ArgumentNullException(nameof(policyName)); + } + + Policies[policyName] = policy; + return this; + } +} diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutPolicy.cs b/src/Http/Http/src/Timeouts/RequestTimeoutPolicy.cs new file mode 100644 index 000000000000..3c333e54a4f1 --- /dev/null +++ b/src/Http/Http/src/Timeouts/RequestTimeoutPolicy.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http.Timeouts; + +/// +/// Defines the policy for request timeouts middleware. +/// +public sealed class RequestTimeoutPolicy +{ + /// + /// The timeout to apply. + /// + public TimeSpan? Timeout { get; init; } + + /// + /// Status code to be set in response when a timeout results in an being caught by the middleware. + /// The status code cannot be applied if the response has already started. + /// 504 will be used if none is specified. + /// + public int? TimeoutStatusCode { get; init; } + + /// + /// A callback for creating a timeout response. + /// This is called if a timeout results in an being caught by the middleware. + /// The status code will be set first. + /// The status code and callback cannot be applied if the response has already started. + /// The default behavior is an empty response with only the status code. + /// + public RequestDelegate? WriteTimeoutResponse { get; init; } +} diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs new file mode 100644 index 000000000000..3b6892355791 --- /dev/null +++ b/src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs @@ -0,0 +1,26 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http.Timeouts; + +namespace Microsoft.AspNetCore.Builder; + +/// +/// Extension methods for the request timeouts middleware. +/// +public static class RequestTimeoutsIApplicationBuilderExtensions +{ + /// + /// Enables request timeouts for the application. + /// No timeouts are configured by default, + /// they must be configured in , + /// the on endpoints, or + /// using the WithRequestTimeout routing extensions. + /// + /// + /// + public static IApplicationBuilder UseRequestTimeouts(this IApplicationBuilder builder) + { + return builder.UseMiddleware(); + } +} diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs new file mode 100644 index 000000000000..da2a116727c1 --- /dev/null +++ b/src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs @@ -0,0 +1,64 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http.Timeouts; + +namespace Microsoft.AspNetCore.Builder; + +/// +/// Request timeouts extension methods for . +/// +public static class RequestTimeoutsIEndpointConventionBuilderExtensions +{ + private static readonly DisableRequestTimeoutAttribute _disableRequestTimeoutAttribute = new DisableRequestTimeoutAttribute(); + + /// + /// Specifies a timeout for the endpoint(s). + /// + /// The endpoint convention builder. + /// The timeout to apply for the endpoint(s). + /// The original convention builder parameter. + public static IEndpointConventionBuilder WithRequestTimeout(this IEndpointConventionBuilder builder, TimeSpan timeout) + { + return builder.WithRequestTimeout(new RequestTimeoutPolicy + { + Timeout = timeout + }); + } + + /// + /// Specifies a timeout policy for to the endpoint(s). + /// + /// The endpoint convention builder. + /// The case-insensitve name of the policy to apply for the endpoint(s). + /// The original convention builder parameter. + public static IEndpointConventionBuilder WithRequestTimeout(this IEndpointConventionBuilder builder, string policyName) + { + builder.Add(b => b.Metadata.Add(new RequestTimeoutAttribute(policyName))); + return builder; + } + + /// + /// Specifies a timeout policy for to the endpoint(s). + /// + /// The endpoint convention builder. + /// The request timeout policy. + /// The original convention builder parameter. + public static IEndpointConventionBuilder WithRequestTimeout(this IEndpointConventionBuilder builder, RequestTimeoutPolicy policy) + { + builder.Add(b => b.Metadata.Add(policy)); + return builder; + } + + /// + /// Disables request timeout on the endpoint(s). + /// + /// The endpoint convention builder. + /// The original convention builder parameter. + /// Will skip both the default timeout, and any endpoint-specific timeout that apply to the endpoint(s). + public static IEndpointConventionBuilder DisableRequestTimeout(this IEndpointConventionBuilder builder) + { + builder.Add(b => b.Metadata.Add(_disableRequestTimeoutAttribute)); + return builder; + } +} diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutsIServiceCollectionExtensions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutsIServiceCollectionExtensions.cs new file mode 100644 index 000000000000..33c94d1dc4a4 --- /dev/null +++ b/src/Http/Http/src/Timeouts/RequestTimeoutsIServiceCollectionExtensions.cs @@ -0,0 +1,35 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http.Timeouts; +using Microsoft.Extensions.DependencyInjection.Extensions; + +namespace Microsoft.Extensions.DependencyInjection; + +/// +/// Extension methods for the request timeouts middleware. +/// +public static class RequestTimeoutsIServiceCollectionExtensions +{ + /// + /// Add request timeout services. + /// + /// The for adding services. + /// + public static IServiceCollection AddRequestTimeouts(this IServiceCollection services) + { + services.TryAddSingleton(); + return services; + } + + /// + /// Add request timeout services and configure the related options. + /// + /// The for adding services. + /// A delegate to configure the . + /// + public static IServiceCollection AddRequestTimeouts(this IServiceCollection services, Action configure) + { + return services.AddRequestTimeouts().Configure(configure); + } +} diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutsMiddleware.cs b/src/Http/Http/src/Timeouts/RequestTimeoutsMiddleware.cs new file mode 100644 index 000000000000..d32014b9e82a --- /dev/null +++ b/src/Http/Http/src/Timeouts/RequestTimeoutsMiddleware.cs @@ -0,0 +1,138 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Http.Timeouts; + +internal sealed class RequestTimeoutsMiddleware +{ + private readonly RequestDelegate _next; + private readonly ICancellationTokenLinker _cancellationTokenProvider; + private readonly ILogger _logger; + private readonly IOptionsMonitor _options; + + public RequestTimeoutsMiddleware( + RequestDelegate next, + ICancellationTokenLinker cancellationTokenProvider, + ILogger logger, + IOptionsMonitor options) + { + _next = next; + _cancellationTokenProvider = cancellationTokenProvider; + _logger = logger; + _options = options; + } + + public Task Invoke(HttpContext context) + { + if (Debugger.IsAttached) + { + return _next(context); + } + + var endpoint = context.GetEndpoint(); + var timeoutMetadata = endpoint?.Metadata.GetMetadata(); + var policyMetadata = endpoint?.Metadata.GetMetadata(); + + var options = _options.CurrentValue; + + if (timeoutMetadata is null && policyMetadata?.Timeout is null && options.DefaultPolicy?.Timeout is null) + { + return _next(context); + } + + var disableMetadata = endpoint?.Metadata.GetMetadata(); + if (disableMetadata is not null) + { + return _next(context); + } + + RequestTimeoutPolicy? selectedPolicy = null; + TimeSpan? timeSpan = null; + if (policyMetadata is not null) + { + selectedPolicy = policyMetadata; + } + else if (timeoutMetadata is not null) + { + if (timeoutMetadata.Timeout is not null) + { + timeSpan = timeoutMetadata.Timeout.Value; + } + else + { + if (options.Policies.TryGetValue(timeoutMetadata.PolicyName!, out var policy)) + { + selectedPolicy = policy; + } + else + { + throw new InvalidOperationException($"The requested timeout policy '{timeoutMetadata.PolicyName}' is not available."); + } + } + } + else + { + selectedPolicy = options.DefaultPolicy; + } + + timeSpan ??= selectedPolicy?.Timeout; + + if (timeSpan is null || timeSpan == Timeout.InfiniteTimeSpan) + { + return _next(context); + } + + if (context.RequestAborted.IsCancellationRequested) + { + return _next(context); + } + + return SetTimeoutAsync(); + + async Task SetTimeoutAsync() + { + var originalToken = context.RequestAborted; + var (linkedCts, timeoutCts) = _cancellationTokenProvider.GetLinkedCancellationTokenSource(context, originalToken, timeSpan.Value); + + try + { + var feature = new HttpRequestTimeoutFeature(timeoutCts); + context.Features.Set(feature); + + context.RequestAborted = linkedCts.Token; + await _next(context); + } + catch (OperationCanceledException operationCanceledException) + { + if (context.Response.HasStarted || + !linkedCts.Token.IsCancellationRequested || + originalToken.IsCancellationRequested) + { + // We can't produce a response, or it wasn't our timeout that caused this. + throw; + } + + _logger.TimeoutExceptionHandled(operationCanceledException); + + context.Response.Clear(); + + context.Response.StatusCode = selectedPolicy?.TimeoutStatusCode ?? StatusCodes.Status504GatewayTimeout; + + if (selectedPolicy?.WriteTimeoutResponse is not null) + { + await selectedPolicy.WriteTimeoutResponse(context); + } + } + finally + { + linkedCts.Dispose(); + context.RequestAborted = originalToken; + context.Features.Set(null); + } + } + } +} diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutOptions.cs b/src/Http/Http/test/Timeouts/RequestTimeoutOptions.cs new file mode 100644 index 000000000000..58e943c687b2 --- /dev/null +++ b/src/Http/Http/test/Timeouts/RequestTimeoutOptions.cs @@ -0,0 +1,61 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http.Timeouts; + +namespace Microsoft.AspNetCore.Http.Tests.Timeouts; +public class RequestTimeoutOptionsTests +{ + [Fact] + public void AddPolicyTimeSpanWorks() + { + var options = new RequestTimeoutOptions(); + options.AddPolicy("policy1", TimeSpan.FromSeconds(47)); + + var policy = options.Policies["policy1"]; + Assert.Equal(TimeSpan.FromSeconds(47), policy.Timeout); + } + + [Fact] + public void AddPolicyWorks() + { + var options = new RequestTimeoutOptions(); + var addedPolicy = new RequestTimeoutPolicy + { + Timeout = TimeSpan.FromSeconds(47) + }; + + options.AddPolicy("policy1", addedPolicy); + + var policy = options.Policies["policy1"]; + Assert.Equal(TimeSpan.FromSeconds(47), policy.Timeout); + } + + [Fact] + public void AddPolicyOverrideExistingPolicy() + { + var options = new RequestTimeoutOptions(); + options.AddPolicy("policy1", TimeSpan.FromSeconds(1)); + + options.AddPolicy("policy1", TimeSpan.FromSeconds(47)); + Assert.Equal(TimeSpan.FromSeconds(47), options.Policies["policy1"].Timeout); + } + + [Fact] + public void AddNullPolicyThrows() + { + var options = new RequestTimeoutOptions(); + Assert.Throws(() => options.AddPolicy("", TimeSpan.FromSeconds(47))); + Assert.Throws(() => options.AddPolicy(null, TimeSpan.FromSeconds(47))); + + Assert.Throws(() => options.AddPolicy("", new RequestTimeoutPolicy())); + Assert.Throws(() => options.AddPolicy(null, new RequestTimeoutPolicy())); + + Assert.Throws(() => options.AddPolicy("policy1", null)); + } +} diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs new file mode 100644 index 000000000000..9cb39a2be03e --- /dev/null +++ b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs @@ -0,0 +1,402 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http.Timeouts; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Http.Tests.Timeouts; + +public class RequestTimeoutsMiddlewareTests +{ + [Fact] + public async Task DefaultTimeoutWhenNoEndpoint() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 10, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task DefaultTimeoutWhenNoMetadata() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 10, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + + var endpoint = CreateEndpoint(); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task TimeOutFromMetadataPolicy() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 47); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + + var endpoint = CreateEndpoint(new RequestTimeoutPolicy { Timeout = TimeSpan.FromSeconds(47) }); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task TimeOutFromMetadataAttributeWithPolicy() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 2); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy2")); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task TimeOutFromMetadataAttributeWithTimeSpan() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 3); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + var endpoint = CreateEndpoint(new RequestTimeoutAttribute(3000)); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task SkipWhenNoDefaultDefaultTimeout() + { + var context = new DefaultHttpContext(); + + var middlewre = CreateMiddleware(originalCancellationToken: context.RequestAborted, linkerCalled: false); + + var originalToken = context.RequestAborted; + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: false); + } + + [Fact] + public async Task TimeOutsAttributeWithPolicyWinsOverDefault() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 1, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + + var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy1")); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task TimeOutsAttributeWithTimeSpanWinsOverDefault() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 3, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + + var endpoint = CreateEndpoint(new RequestTimeoutAttribute(3000)); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task TimeOutsPolicyWinsOverDefault() + { + var middlewre = CreateMiddleware(expectedTimeSpan: 47, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + + var endpoint = CreateEndpoint(new RequestTimeoutPolicy { Timeout = TimeSpan.FromSeconds(47) }, new RequestTimeoutAttribute("policy1")); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: true); + } + + [Fact] + public async Task DisableTimeoutAttributeSkipTheMiddleware() + { + var context = new DefaultHttpContext(); + var originalToken = context.RequestAborted; + + var middlewre = CreateMiddleware(defaultTimeout: 10, originalCancellationToken: originalToken, linkerCalled: false); + + var endpoint = CreateEndpoint(new DisableRequestTimeoutAttribute(), + new RequestTimeoutPolicy { Timeout = TimeSpan.FromSeconds(47) }, + new RequestTimeoutAttribute("policy1")); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(originalToken, context.RequestAborted); + CheckTimeoutFeature(context, shouldExist: false); + } + + [Fact] + public async Task ThrowExceptionWhenPolicyNotFound() + { + var middlewre = CreateMiddleware(); + + var context = new DefaultHttpContext(); + + var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy47")); + context.SetEndpoint(endpoint); + + await Assert.ThrowsAsync(() => middlewre.Invoke(context)); + } + + [Fact] + public async Task HandleTimeoutExceptionDefaultPolicy() + { + var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + context.Response.Headers.Add("ToBeCleared", "Later"); + var originalToken = context.RequestAborted; + + await middlewre.Invoke(context); + + Assert.Equal(StatusCodes.Status418ImATeapot, context.Response.StatusCode); + Assert.Empty(context.Response.Headers); + Assert.Equal(originalToken, context.RequestAborted); + } + + [Fact] + public async Task HandleTimeoutExceptionFromDefaultPolicy() + { + var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + context.Response.Headers.Add("ToBeCleared", "Later"); + var originalToken = context.RequestAborted; + + await middlewre.Invoke(context); + + Assert.Equal(StatusCodes.Status418ImATeapot, context.Response.StatusCode); + Assert.Empty(context.Response.Headers); + Assert.Equal("default", context.Items["SetFrom"]); + Assert.Equal(originalToken, context.RequestAborted); + } + + [Fact] + public async Task HandleTimeoutExceptionFromEndpointPolicy() + { + var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 1, defaultTimeout: 10); + + var context = new DefaultHttpContext(); + context.Response.Headers.Add("ToBeCleared", "Later"); + var originalToken = context.RequestAborted; + + var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy1")); + context.SetEndpoint(endpoint); + + await middlewre.Invoke(context); + + Assert.Equal(111, context.Response.StatusCode); + Assert.Empty(context.Response.Headers); + Assert.Equal("policy1", context.Items["SetFrom"]); + Assert.Equal(originalToken, context.RequestAborted); + } + + [Fact] + public async Task SkipHandleTimeoutException() + { + var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10, cancelledCts: false); + + var context = new DefaultHttpContext(); + context.Response.Headers.Add("NotGonnaBeCleared", "Not Today!"); + var originalToken = context.RequestAborted; + + await Assert.ThrowsAsync(() => middlewre.Invoke(context)); + + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + Assert.NotEmpty(context.Response.Headers); + Assert.False(context.Items.ContainsKey("SetFrom")); + Assert.Equal(originalToken, context.RequestAborted); + } + + private static void CheckTimeoutFeature(HttpContext httpContext, bool shouldExist) + { + var timeoutFeature = httpContext.Features.Get(); + Assert.Equal(shouldExist, timeoutFeature is not null); + } + + private static RequestTimeoutsMiddleware CreateMiddlewareWithCancel( + double? expectedTimeSpan = null, + double? defaultTimeout = null, + bool cancelledCts = true, + CancellationToken originalCancellationToken = default, + bool linkerCalled = true) + { + return CreateMiddleware(context => + { + + throw new OperationCanceledException(context.RequestAborted); + }, + expectedTimeSpan, + defaultTimeout, + cancelledCts, + originalCancellationToken, + linkerCalled); + } + + private static RequestTimeoutsMiddleware CreateMiddleware( + RequestDelegate requestDelegate = null, + double? expectedTimeSpan = null, + double? defaultTimeout = null, + bool cancelledCts = false, + CancellationToken originalCancellationToken = default, + bool linkerCalled = true) + { + var ctsLinker = new MockCancellationTokenSourceProvider(expectedTimeSpan.HasValue ? TimeSpan.FromSeconds(expectedTimeSpan.Value) : null, cancelledCts); + var options = new RequestTimeoutOptions + { + DefaultPolicy = defaultTimeout.HasValue ? new RequestTimeoutPolicy + { + Timeout = TimeSpan.FromSeconds(defaultTimeout.Value), + TimeoutStatusCode = StatusCodes.Status418ImATeapot, + WriteTimeoutResponse = context => + { + context.Items["SetFrom"] = "default"; + return Task.CompletedTask; + } + } : null, + }; + options.Policies.Add("policy1", new RequestTimeoutPolicy + { + Timeout = TimeSpan.FromSeconds(1), + TimeoutStatusCode = 111, + WriteTimeoutResponse = context => + { + context.Items["SetFrom"] = "policy1"; + return Task.CompletedTask; + } + }); + options.Policies.Add("policy2", new RequestTimeoutPolicy + { + Timeout = TimeSpan.FromSeconds(2), + TimeoutStatusCode = 222, + WriteTimeoutResponse = context => + { + context.Items["SetFrom"] = "policy2"; + return Task.CompletedTask; + } + }); + + var optionsMonitor = new MiddlewareOptions(options); + + return new RequestTimeoutsMiddleware(requestDelegate ?? next, ctsLinker, NullLogger.Instance, optionsMonitor); + + Task next(HttpContext context) + { + Assert.Equal(linkerCalled, ctsLinker.Called); + if (ctsLinker.Called) + { + Assert.Equal(ctsLinker.ReplacedToken, context.RequestAborted); + } + else + { + Assert.Equal(originalCancellationToken, context.RequestAborted); + } + return Task.CompletedTask; + } + } + + private static Endpoint CreateEndpoint(params object[] metadata) + { + return new Endpoint(null, new EndpointMetadataCollection(metadata), "endpoint"); + } + + private class MockCancellationTokenSourceProvider : ICancellationTokenLinker + { + private readonly TimeSpan? _expectedTimeSpan; + private readonly bool _cancelledCts; + + public CancellationToken ReplacedToken { get; private set; } + public CancellationTokenSource LinkedCts { get; private set; } + + public bool Called { get; private set; } + + public MockCancellationTokenSourceProvider(TimeSpan? expectedTimeSpan, bool cancelledCts) + { + _expectedTimeSpan = expectedTimeSpan; + _cancelledCts = cancelledCts; + } + + public (CancellationTokenSource linkedCts, CancellationTokenSource timeoutCts) GetLinkedCancellationTokenSource(HttpContext httpContext, CancellationToken originalToken, TimeSpan timeSpan) + { + Assert.Equal(_expectedTimeSpan, timeSpan); + + Called = true; + + var cts = new CancellationTokenSource(); + if (_cancelledCts) + { + cts.Cancel(); + } + + ReplacedToken = cts.Token; + return (cts, new CancellationTokenSource()); + } + } + + private class MiddlewareOptions : IOptionsMonitor + { + private readonly RequestTimeoutOptions _options; + + public MiddlewareOptions(RequestTimeoutOptions options) + { + _options = options; + } + public RequestTimeoutOptions CurrentValue => _options; + + public RequestTimeoutOptions Get(string name) => _options; + + public IDisposable OnChange(Action listener) + { + return default; + } + } +} From 17dffdd31a5c5064c82bf9db6327d73fc37155bf Mon Sep 17 00:00:00 2001 From: Kahbazi Date: Fri, 10 Mar 2023 01:14:07 +0330 Subject: [PATCH 02/11] Fix tests --- .../RequestTimeoutsMiddlewareTests.cs | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs index 9cb39a2be03e..e1795f4436ea 100644 --- a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs +++ b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Net.Http; using Microsoft.AspNetCore.Http.Timeouts; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; @@ -20,7 +21,6 @@ public async Task DefaultTimeoutWhenNoEndpoint() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -37,7 +37,6 @@ public async Task DefaultTimeoutWhenNoMetadata() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -54,7 +53,6 @@ public async Task TimeOutFromMetadataPolicy() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -70,7 +68,6 @@ public async Task TimeOutFromMetadataAttributeWithPolicy() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -86,7 +83,6 @@ public async Task TimeOutFromMetadataAttributeWithTimeSpan() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -94,14 +90,16 @@ public async Task SkipWhenNoDefaultDefaultTimeout() { var context = new DefaultHttpContext(); - var middlewre = CreateMiddleware(originalCancellationToken: context.RequestAborted, linkerCalled: false); + var middlewre = CreateMiddleware( + originalCancellationToken: context.RequestAborted, + linkerCalled: false, + timeoutFeatureExists: false); var originalToken = context.RequestAborted; await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: false); } [Fact] @@ -118,7 +116,6 @@ public async Task TimeOutsAttributeWithPolicyWinsOverDefault() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -135,7 +132,6 @@ public async Task TimeOutsAttributeWithTimeSpanWinsOverDefault() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -152,7 +148,6 @@ public async Task TimeOutsPolicyWinsOverDefault() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: true); } [Fact] @@ -161,7 +156,10 @@ public async Task DisableTimeoutAttributeSkipTheMiddleware() var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; - var middlewre = CreateMiddleware(defaultTimeout: 10, originalCancellationToken: originalToken, linkerCalled: false); + var middlewre = CreateMiddleware(defaultTimeout: 10, + originalCancellationToken: originalToken, + linkerCalled: false, + timeoutFeatureExists: false); var endpoint = CreateEndpoint(new DisableRequestTimeoutAttribute(), new RequestTimeoutPolicy { Timeout = TimeSpan.FromSeconds(47) }, @@ -171,7 +169,6 @@ public async Task DisableTimeoutAttributeSkipTheMiddleware() await middlewre.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); - CheckTimeoutFeature(context, shouldExist: false); } [Fact] @@ -257,12 +254,6 @@ public async Task SkipHandleTimeoutException() Assert.Equal(originalToken, context.RequestAborted); } - private static void CheckTimeoutFeature(HttpContext httpContext, bool shouldExist) - { - var timeoutFeature = httpContext.Features.Get(); - Assert.Equal(shouldExist, timeoutFeature is not null); - } - private static RequestTimeoutsMiddleware CreateMiddlewareWithCancel( double? expectedTimeSpan = null, double? defaultTimeout = null, @@ -288,7 +279,8 @@ private static RequestTimeoutsMiddleware CreateMiddleware( double? defaultTimeout = null, bool cancelledCts = false, CancellationToken originalCancellationToken = default, - bool linkerCalled = true) + bool linkerCalled = true, + bool timeoutFeatureExists = true) { var ctsLinker = new MockCancellationTokenSourceProvider(expectedTimeSpan.HasValue ? TimeSpan.FromSeconds(expectedTimeSpan.Value) : null, cancelledCts); var options = new RequestTimeoutOptions @@ -331,6 +323,9 @@ private static RequestTimeoutsMiddleware CreateMiddleware( Task next(HttpContext context) { + var timeoutFeature = context.Features.Get(); + Assert.Equal(timeoutFeatureExists, timeoutFeature is not null); + Assert.Equal(linkerCalled, ctsLinker.Called); if (ctsLinker.Called) { From 4250e31c9a65d2e0b79cc367fb20fec705555b17 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 15 Mar 2023 19:08:20 -0700 Subject: [PATCH 03/11] Fix typos/casing. --- .../RequestTimeoutsMiddlewareTests.cs | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs index e1795f4436ea..104ef9b1b762 100644 --- a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs +++ b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs @@ -13,12 +13,12 @@ public class RequestTimeoutsMiddlewareTests [Fact] public async Task DefaultTimeoutWhenNoEndpoint() { - var middlewre = CreateMiddleware(expectedTimeSpan: 10, defaultTimeout: 10); + var middleware = CreateMiddleware(expectedTimeSpan: 10, defaultTimeout: 10); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } @@ -26,7 +26,7 @@ public async Task DefaultTimeoutWhenNoEndpoint() [Fact] public async Task DefaultTimeoutWhenNoMetadata() { - var middlewre = CreateMiddleware(expectedTimeSpan: 10, defaultTimeout: 10); + var middleware = CreateMiddleware(expectedTimeSpan: 10, defaultTimeout: 10); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; @@ -34,15 +34,15 @@ public async Task DefaultTimeoutWhenNoMetadata() var endpoint = CreateEndpoint(); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } [Fact] - public async Task TimeOutFromMetadataPolicy() + public async Task TimeoutFromMetadataPolicy() { - var middlewre = CreateMiddleware(expectedTimeSpan: 47); + var middleware = CreateMiddleware(expectedTimeSpan: 47); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; @@ -50,37 +50,37 @@ public async Task TimeOutFromMetadataPolicy() var endpoint = CreateEndpoint(new RequestTimeoutPolicy { Timeout = TimeSpan.FromSeconds(47) }); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } [Fact] - public async Task TimeOutFromMetadataAttributeWithPolicy() + public async Task TimeoutFromMetadataAttributeWithPolicy() { - var middlewre = CreateMiddleware(expectedTimeSpan: 2); + var middleware = CreateMiddleware(expectedTimeSpan: 2); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy2")); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } [Fact] - public async Task TimeOutFromMetadataAttributeWithTimeSpan() + public async Task TimeoutFromMetadataAttributeWithTimeSpan() { - var middlewre = CreateMiddleware(expectedTimeSpan: 3); + var middleware = CreateMiddleware(expectedTimeSpan: 3); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; var endpoint = CreateEndpoint(new RequestTimeoutAttribute(3000)); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } @@ -90,22 +90,22 @@ public async Task SkipWhenNoDefaultDefaultTimeout() { var context = new DefaultHttpContext(); - var middlewre = CreateMiddleware( + var middleware = CreateMiddleware( originalCancellationToken: context.RequestAborted, linkerCalled: false, timeoutFeatureExists: false); var originalToken = context.RequestAborted; - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } [Fact] - public async Task TimeOutsAttributeWithPolicyWinsOverDefault() + public async Task TimeoutsAttributeWithPolicyWinsOverDefault() { - var middlewre = CreateMiddleware(expectedTimeSpan: 1, defaultTimeout: 10); + var middleware = CreateMiddleware(expectedTimeSpan: 1, defaultTimeout: 10); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; @@ -113,15 +113,15 @@ public async Task TimeOutsAttributeWithPolicyWinsOverDefault() var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy1")); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } [Fact] - public async Task TimeOutsAttributeWithTimeSpanWinsOverDefault() + public async Task TimeoutsAttributeWithTimeSpanWinsOverDefault() { - var middlewre = CreateMiddleware(expectedTimeSpan: 3, defaultTimeout: 10); + var middleware = CreateMiddleware(expectedTimeSpan: 3, defaultTimeout: 10); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; @@ -129,15 +129,15 @@ public async Task TimeOutsAttributeWithTimeSpanWinsOverDefault() var endpoint = CreateEndpoint(new RequestTimeoutAttribute(3000)); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } [Fact] - public async Task TimeOutsPolicyWinsOverDefault() + public async Task TimeoutsPolicyWinsOverDefault() { - var middlewre = CreateMiddleware(expectedTimeSpan: 47, defaultTimeout: 10); + var middleware = CreateMiddleware(expectedTimeSpan: 47, defaultTimeout: 10); var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; @@ -145,7 +145,7 @@ public async Task TimeOutsPolicyWinsOverDefault() var endpoint = CreateEndpoint(new RequestTimeoutPolicy { Timeout = TimeSpan.FromSeconds(47) }, new RequestTimeoutAttribute("policy1")); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } @@ -156,7 +156,7 @@ public async Task DisableTimeoutAttributeSkipTheMiddleware() var context = new DefaultHttpContext(); var originalToken = context.RequestAborted; - var middlewre = CreateMiddleware(defaultTimeout: 10, + var middleware = CreateMiddleware(defaultTimeout: 10, originalCancellationToken: originalToken, linkerCalled: false, timeoutFeatureExists: false); @@ -166,7 +166,7 @@ public async Task DisableTimeoutAttributeSkipTheMiddleware() new RequestTimeoutAttribute("policy1")); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(originalToken, context.RequestAborted); } @@ -174,26 +174,26 @@ public async Task DisableTimeoutAttributeSkipTheMiddleware() [Fact] public async Task ThrowExceptionWhenPolicyNotFound() { - var middlewre = CreateMiddleware(); + var middleware = CreateMiddleware(); var context = new DefaultHttpContext(); var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy47")); context.SetEndpoint(endpoint); - await Assert.ThrowsAsync(() => middlewre.Invoke(context)); + await Assert.ThrowsAsync(() => middleware.Invoke(context)); } [Fact] public async Task HandleTimeoutExceptionDefaultPolicy() { - var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10); + var middleware = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10); var context = new DefaultHttpContext(); context.Response.Headers.Add("ToBeCleared", "Later"); var originalToken = context.RequestAborted; - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(StatusCodes.Status418ImATeapot, context.Response.StatusCode); Assert.Empty(context.Response.Headers); @@ -203,13 +203,13 @@ public async Task HandleTimeoutExceptionDefaultPolicy() [Fact] public async Task HandleTimeoutExceptionFromDefaultPolicy() { - var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10); + var middleware = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10); var context = new DefaultHttpContext(); context.Response.Headers.Add("ToBeCleared", "Later"); var originalToken = context.RequestAborted; - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(StatusCodes.Status418ImATeapot, context.Response.StatusCode); Assert.Empty(context.Response.Headers); @@ -220,7 +220,7 @@ public async Task HandleTimeoutExceptionFromDefaultPolicy() [Fact] public async Task HandleTimeoutExceptionFromEndpointPolicy() { - var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 1, defaultTimeout: 10); + var middleware = CreateMiddlewareWithCancel(expectedTimeSpan: 1, defaultTimeout: 10); var context = new DefaultHttpContext(); context.Response.Headers.Add("ToBeCleared", "Later"); @@ -229,7 +229,7 @@ public async Task HandleTimeoutExceptionFromEndpointPolicy() var endpoint = CreateEndpoint(new RequestTimeoutAttribute("policy1")); context.SetEndpoint(endpoint); - await middlewre.Invoke(context); + await middleware.Invoke(context); Assert.Equal(111, context.Response.StatusCode); Assert.Empty(context.Response.Headers); @@ -240,13 +240,13 @@ public async Task HandleTimeoutExceptionFromEndpointPolicy() [Fact] public async Task SkipHandleTimeoutException() { - var middlewre = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10, cancelledCts: false); + var middleware = CreateMiddlewareWithCancel(expectedTimeSpan: 10, defaultTimeout: 10, cancelledCts: false); var context = new DefaultHttpContext(); context.Response.Headers.Add("NotGonnaBeCleared", "Not Today!"); var originalToken = context.RequestAborted; - await Assert.ThrowsAsync(() => middlewre.Invoke(context)); + await Assert.ThrowsAsync(() => middleware.Invoke(context)); Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); Assert.NotEmpty(context.Response.Headers); From 0e71e8e37663afa4481f0a0d6c0f41b0945a1ef4 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 15 Mar 2023 19:10:19 -0700 Subject: [PATCH 04/11] Doc comments. --- src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs b/src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs index 6ae362189bf2..34c18975d85a 100644 --- a/src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs +++ b/src/Http/Http/src/Timeouts/IHttpRequestTimeoutFeature.cs @@ -4,17 +4,18 @@ namespace Microsoft.AspNetCore.Http.Timeouts; /// -/// +/// Used to control timeouts on the current request. /// public interface IHttpRequestTimeoutFeature { /// - /// + /// A that will trigger when the request times out. /// CancellationToken RequestTimeoutToken { get; } /// - /// + /// Disables the request timeout if it hasn't already expired. This does not + /// trigger the . /// void DisableTimeout(); } From e7839c621b4c22924aad4f329b425a00a7619364 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 15 Mar 2023 19:12:41 -0700 Subject: [PATCH 05/11] Rename test file. --- .../{RequestTimeoutOptions.cs => RequestTimeoutOptionsTests.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Http/Http/test/Timeouts/{RequestTimeoutOptions.cs => RequestTimeoutOptionsTests.cs} (100%) diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutOptions.cs b/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs similarity index 100% rename from src/Http/Http/test/Timeouts/RequestTimeoutOptions.cs rename to src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs From 61eb2ee013eee7c6b44a3812b756a7df3a0e2f5d Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 15 Mar 2023 19:18:24 -0700 Subject: [PATCH 06/11] Misc cleanup. --- src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs | 8 ++------ src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs | 5 ----- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs index 73fe285986cb..7a6049fe3abc 100644 --- a/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs +++ b/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs @@ -39,12 +39,8 @@ public RequestTimeoutOptions AddPolicy(string policyName, TimeSpan timeout) /// The policy to be added. public RequestTimeoutOptions AddPolicy(string policyName, RequestTimeoutPolicy policy) { - _ = policy ?? throw new ArgumentNullException(nameof(policy)); - - if (string.IsNullOrEmpty(policyName)) - { - throw new ArgumentNullException(nameof(policyName)); - } + ArgumentNullException.ThrowIfNull(policy); + ArgumentException.ThrowIfNullOrEmpty(policyName); Policies[policyName] = policy; return this; diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs index 58e943c687b2..b2d62c362d3d 100644 --- a/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs +++ b/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs @@ -1,11 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; using Microsoft.AspNetCore.Http.Timeouts; namespace Microsoft.AspNetCore.Http.Tests.Timeouts; From 4a291599ec0300e1b3901869175f1956189bfc4c Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 15 Mar 2023 19:40:59 -0700 Subject: [PATCH 07/11] Add to HttpProtocolFeatureCollection. --- src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs | 2 +- .../tools/CodeGenerator/HttpProtocolFeatureCollection.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs index 104ef9b1b762..2300eae5ff7a 100644 --- a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs +++ b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs @@ -86,7 +86,7 @@ public async Task TimeoutFromMetadataAttributeWithTimeSpan() } [Fact] - public async Task SkipWhenNoDefaultDefaultTimeout() + public async Task SkipWhenNoDefaultTimeout() { var context = new DefaultHttpContext(); diff --git a/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs b/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs index e165fcfcbd5a..312756c4af0d 100644 --- a/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs +++ b/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs @@ -40,7 +40,8 @@ public static string GenerateFile() "IHttpUpgradeFeature", "IHttpWebSocketFeature", "IHttpWebTransportFeature", - "IBadRequestExceptionFeature" + "IBadRequestExceptionFeature", + "IHttpRequestTimeoutFeature" }; var maybeFeatures = new[] { From 2515bf1ea23e4836df058e1e6d12838c8a16f95c Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 15 Mar 2023 20:06:41 -0700 Subject: [PATCH 08/11] More edits. --- .../RequestTimeoutsMiddlewareBenchmark.cs | 2 +- .../src/Timeouts/DisableRequestTimeoutAttribute.cs | 2 +- src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs | 6 +++--- src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs | 6 +++--- .../RequestTimeoutsIApplicationBuilderExtensions.cs | 11 +++++------ ...estTimeoutsIEndpointConventionBuilderExtensions.cs | 2 +- .../test/Timeouts/RequestTimeoutsMiddlewareTests.cs | 1 - 7 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs b/src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs index 7b3c71bdc5b4..a914912d720c 100644 --- a/src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs +++ b/src/Http/Http/perf/Microbenchmarks/RequestTimeoutsMiddlewareBenchmark.cs @@ -72,7 +72,7 @@ public async Task DefaultTimeout() } [Benchmark] - public async Task DefaultTimeoutOverridenByDisable() + public async Task DefaultTimeoutOverriddenByDisable() { var context = CreateHttpContext(new Endpoint( null, diff --git a/src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs b/src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs index 34194b046dd0..e757b939f8e0 100644 --- a/src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs +++ b/src/Http/Http/src/Timeouts/DisableRequestTimeoutAttribute.cs @@ -4,7 +4,7 @@ namespace Microsoft.AspNetCore.Http.Timeouts; /// -/// Metadata that disables request timeouts limiting on an endpoint. +/// Metadata that disables request timeouts on an endpoint. /// /// /// Completely disables the request timeouts middleware from applying to this endpoint. diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs b/src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs index 88d85002b52e..828a060c61e4 100644 --- a/src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs +++ b/src/Http/Http/src/Timeouts/RequestTimeoutAttribute.cs @@ -19,14 +19,14 @@ public sealed class RequestTimeoutAttribute : Attribute /// /// The name of the policy which needs to be applied. - /// This value is case insensitve. + /// This value is case-insensitive. /// public string? PolicyName { get; } /// /// Creates a new instance of using the specified timeout. /// - /// The amount of timeout for this specific endpoint. + /// The duration, in milliseconds, of the timeout for this endpoint. public RequestTimeoutAttribute(int milliseconds) { Timeout = TimeSpan.FromMilliseconds(milliseconds); @@ -35,7 +35,7 @@ public RequestTimeoutAttribute(int milliseconds) /// /// Creates a new instance of using the specified policy. /// - /// The case-insensitve name of the policy which needs to be applied. + /// The name of the policy which needs to be applied (case-insensitive). public RequestTimeoutAttribute(string policyName) { PolicyName = policyName; diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs index 7a6049fe3abc..b4f7dd1806cc 100644 --- a/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs +++ b/src/Http/Http/src/Timeouts/RequestTimeoutOptions.cs @@ -15,14 +15,14 @@ public sealed class RequestTimeoutOptions /// /// Dictionary of policies that would be applied per endpoint. - /// Policy names are case insensitive. + /// Policy names are case-insensitive. /// public IDictionary Policies { get; } = new Dictionary(StringComparer.OrdinalIgnoreCase); /// /// Adds a new policy. /// - /// The case-insensitive name of the policy. + /// The name of the policy (case-insensitive). /// The timeout to apply for this policy. public RequestTimeoutOptions AddPolicy(string policyName, TimeSpan timeout) { @@ -35,7 +35,7 @@ public RequestTimeoutOptions AddPolicy(string policyName, TimeSpan timeout) /// /// Adds a new policy. /// - /// The case-insensitive name of the policy. + /// The name of the policy (case-insensitive). /// The policy to be added. public RequestTimeoutOptions AddPolicy(string policyName, RequestTimeoutPolicy policy) { diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs index 3b6892355791..365abb87d875 100644 --- a/src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs +++ b/src/Http/Http/src/Timeouts/RequestTimeoutsIApplicationBuilderExtensions.cs @@ -12,13 +12,12 @@ public static class RequestTimeoutsIApplicationBuilderExtensions { /// /// Enables request timeouts for the application. - /// No timeouts are configured by default, - /// they must be configured in , - /// the on endpoints, or - /// using the WithRequestTimeout routing extensions. + /// + /// No timeouts are configured by default. They must be configured in , + /// the on endpoints, or using the WithRequestTimeout routing extensions. + /// /// - /// - /// + /// The . public static IApplicationBuilder UseRequestTimeouts(this IApplicationBuilder builder) { return builder.UseMiddleware(); diff --git a/src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs b/src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs index da2a116727c1..6c28fb6fb1bc 100644 --- a/src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs +++ b/src/Http/Http/src/Timeouts/RequestTimeoutsIEndpointConventionBuilderExtensions.cs @@ -30,7 +30,7 @@ public static IEndpointConventionBuilder WithRequestTimeout(this IEndpointConven /// Specifies a timeout policy for to the endpoint(s). /// /// The endpoint convention builder. - /// The case-insensitve name of the policy to apply for the endpoint(s). + /// The name (case-insensitive) of the policy to apply for the endpoint(s). /// The original convention builder parameter. public static IEndpointConventionBuilder WithRequestTimeout(this IEndpointConventionBuilder builder, string policyName) { diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs index 2300eae5ff7a..56da4c2878e7 100644 --- a/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs +++ b/src/Http/Http/test/Timeouts/RequestTimeoutsMiddlewareTests.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Net.Http; using Microsoft.AspNetCore.Http.Timeouts; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; From 2e5a6a17fc06fa6d09b503ed93dd19ed42e62cfa Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 15 Mar 2023 20:10:59 -0700 Subject: [PATCH 09/11] Re-run generator for feature collection. --- .../Internal/Http/HttpProtocol.Generated.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs index 6575f31f7d5d..b24005453828 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs @@ -67,6 +67,7 @@ internal partial class HttpProtocol : IFeatureCollection, internal protected IHttpResponseTrailersFeature? _currentIHttpResponseTrailersFeature; internal protected ITlsConnectionFeature? _currentITlsConnectionFeature; internal protected IHttpWebSocketFeature? _currentIHttpWebSocketFeature; + internal protected IHttpRequestTimeoutFeature? _currentIHttpRequestTimeoutFeature; internal protected IHttp2StreamIdFeature? _currentIHttp2StreamIdFeature; internal protected IHttpMinRequestBodyDataRateFeature? _currentIHttpMinRequestBodyDataRateFeature; internal protected IHttpMinResponseDataRateFeature? _currentIHttpMinResponseDataRateFeature; @@ -108,6 +109,7 @@ private void FastReset() _currentIHttpResponseTrailersFeature = null; _currentITlsConnectionFeature = null; _currentIHttpWebSocketFeature = null; + _currentIHttpRequestTimeoutFeature = null; _currentIHttp2StreamIdFeature = null; _currentIHttpMinRequestBodyDataRateFeature = null; _currentIHttpMinResponseDataRateFeature = null; @@ -280,6 +282,10 @@ private void ExtraFeatureSet(Type key, object? value) { feature = _currentIBadRequestExceptionFeature; } + else if (key == typeof(IHttpRequestTimeoutFeature)) + { + feature = _currentIHttpRequestTimeoutFeature; + } else if (key == typeof(IHttp2StreamIdFeature)) { feature = _currentIHttp2StreamIdFeature; @@ -424,6 +430,10 @@ private void ExtraFeatureSet(Type key, object? value) { _currentIBadRequestExceptionFeature = (IBadRequestExceptionFeature?)value; } + else if (key == typeof(IHttpRequestTimeoutFeature)) + { + _currentIHttpRequestTimeoutFeature = (IHttpRequestTimeoutFeature?)value; + } else if (key == typeof(IHttp2StreamIdFeature)) { _currentIHttp2StreamIdFeature = (IHttp2StreamIdFeature?)value; @@ -570,6 +580,10 @@ private void ExtraFeatureSet(Type key, object? value) { feature = Unsafe.As(ref _currentIBadRequestExceptionFeature); } + else if (typeof(TFeature) == typeof(IHttpRequestTimeoutFeature)) + { + feature = Unsafe.As(ref _currentIHttpRequestTimeoutFeature); + } else if (typeof(TFeature) == typeof(IHttp2StreamIdFeature)) { feature = Unsafe.As(ref _currentIHttp2StreamIdFeature); @@ -722,6 +736,10 @@ private void ExtraFeatureSet(Type key, object? value) { _currentIBadRequestExceptionFeature = Unsafe.As(ref feature); } + else if (typeof(TFeature) == typeof(IHttpRequestTimeoutFeature)) + { + _currentIHttpRequestTimeoutFeature = Unsafe.As(ref feature); + } else if (typeof(TFeature) == typeof(IHttp2StreamIdFeature)) { _currentIHttp2StreamIdFeature = Unsafe.As(ref feature); @@ -862,6 +880,10 @@ private IEnumerable> FastEnumerable() { yield return new KeyValuePair(typeof(IBadRequestExceptionFeature), _currentIBadRequestExceptionFeature); } + if (_currentIHttpRequestTimeoutFeature != null) + { + yield return new KeyValuePair(typeof(IHttpRequestTimeoutFeature), _currentIHttpRequestTimeoutFeature); + } if (_currentIHttp2StreamIdFeature != null) { yield return new KeyValuePair(typeof(IHttp2StreamIdFeature), _currentIHttp2StreamIdFeature); From 8f28de5f05b456cb12f4ea46d0732467ca7e0aa6 Mon Sep 17 00:00:00 2001 From: Chris R Date: Thu, 16 Mar 2023 09:17:22 -0700 Subject: [PATCH 10/11] Fix namespaces --- src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs | 1 + .../Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs | 1 + .../Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs | 1 + 3 files changed, 3 insertions(+) diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs index b2d62c362d3d..3a7bdd20be38 100644 --- a/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs +++ b/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs @@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Http.Timeouts; namespace Microsoft.AspNetCore.Http.Tests.Timeouts; + public class RequestTimeoutOptionsTests { [Fact] diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs index b24005453828..05b40bb6e497 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Features.Authentication; +using Microsoft.AspNetCore.Http.Timeouts; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; #pragma warning disable CA2252 // WebTransport is a preview feature diff --git a/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs b/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs index 312756c4af0d..87d80b16a898 100644 --- a/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs +++ b/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs @@ -89,6 +89,7 @@ public static string GenerateFile() using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Features.Authentication; +using Microsoft.AspNetCore.Http.Timeouts; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; #pragma warning disable CA2252 // WebTransport is a preview feature"; From fb7422b3be826e0fe28246409379530587164ae1 Mon Sep 17 00:00:00 2001 From: Chris R Date: Thu, 16 Mar 2023 11:33:14 -0700 Subject: [PATCH 11/11] Fix test --- src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs b/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs index 3a7bdd20be38..d0ce35b3d4dc 100644 --- a/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs +++ b/src/Http/Http/test/Timeouts/RequestTimeoutOptionsTests.cs @@ -46,10 +46,10 @@ public void AddPolicyOverrideExistingPolicy() public void AddNullPolicyThrows() { var options = new RequestTimeoutOptions(); - Assert.Throws(() => options.AddPolicy("", TimeSpan.FromSeconds(47))); + Assert.Throws(() => options.AddPolicy("", TimeSpan.FromSeconds(47))); Assert.Throws(() => options.AddPolicy(null, TimeSpan.FromSeconds(47))); - Assert.Throws(() => options.AddPolicy("", new RequestTimeoutPolicy())); + Assert.Throws(() => options.AddPolicy("", new RequestTimeoutPolicy())); Assert.Throws(() => options.AddPolicy(null, new RequestTimeoutPolicy())); Assert.Throws(() => options.AddPolicy("policy1", null));