From 7af0302d27b5f914791e97af438bb5d07f6f5967 Mon Sep 17 00:00:00 2001 From: Chris R Date: Wed, 8 Nov 2023 16:19:07 -0800 Subject: [PATCH 1/6] Timeout integration --- .../ConfigurationConfigProvider.cs | 4 ++ .../Configuration/ConfigValidator.cs | 47 ++++++++++++++++++- src/ReverseProxy/Configuration/RouteConfig.cs | 17 +++++++ .../Configuration/TimeoutPolicyConstants.cs | 10 ++++ .../Routing/ProxyEndpointFactory.cs | 26 ++++++++++ .../ReverseProxy.Code.csproj | 2 +- testassets/ReverseProxy.Code/Startup.cs | 21 ++++++++- .../TestServer/Controllers/HttpController.cs | 12 +++++ 8 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs diff --git a/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs b/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs index be174be44..4e29effd5 100644 --- a/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs +++ b/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs @@ -149,6 +149,10 @@ private static RouteConfig CreateRoute(IConfigurationSection section) AuthorizationPolicy = section[nameof(RouteConfig.AuthorizationPolicy)], #if NET7_0_OR_GREATER RateLimiterPolicy = section[nameof(RouteConfig.RateLimiterPolicy)], +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = section[nameof(RouteConfig.TimeoutPolicy)], + Timeout = section.ReadTimeSpan(nameof(RouteConfig.Timeout)), #endif CorsPolicy = section[nameof(RouteConfig.CorsPolicy)], Metadata = section.GetSection(nameof(RouteConfig.Metadata)).ReadStringDictionary(), diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index 51d9c4da0..50d4a92c5 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -11,8 +11,14 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.Logging; +#if NET8_0_OR_GREATER +using Microsoft.Extensions.Options; +#endif using Yarp.ReverseProxy.Health; using Yarp.ReverseProxy.LoadBalancing; using Yarp.ReverseProxy.SessionAffinity; @@ -32,6 +38,9 @@ internal sealed class ConfigValidator : IConfigValidator private readonly IAuthorizationPolicyProvider _authorizationPolicyProvider; private readonly IYarpRateLimiterPolicyProvider _rateLimiterPolicyProvider; private readonly ICorsPolicyProvider _corsPolicyProvider; +#if NET8_0_OR_GREATER + private readonly IOptionsMonitor _timeoutOptions; +#endif private readonly FrozenDictionary _loadBalancingPolicies; private readonly FrozenDictionary _affinityFailurePolicies; private readonly FrozenDictionary _availableDestinationsPolicies; @@ -39,11 +48,13 @@ internal sealed class ConfigValidator : IConfigValidator private readonly FrozenDictionary _passiveHealthCheckPolicies; private readonly ILogger _logger; - public ConfigValidator(ITransformBuilder transformBuilder, IAuthorizationPolicyProvider authorizationPolicyProvider, IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, ICorsPolicyProvider corsPolicyProvider, +#if NET8_0_OR_GREATER + IOptionsMonitor timeoutOptions, +#endif IEnumerable loadBalancingPolicies, IEnumerable affinityFailurePolicies, IEnumerable availableDestinationsPolicies, @@ -55,6 +66,9 @@ public ConfigValidator(ITransformBuilder transformBuilder, _authorizationPolicyProvider = authorizationPolicyProvider ?? throw new ArgumentNullException(nameof(authorizationPolicyProvider)); _rateLimiterPolicyProvider = rateLimiterPolicyProvider ?? throw new ArgumentNullException(nameof(rateLimiterPolicyProvider)); _corsPolicyProvider = corsPolicyProvider ?? throw new ArgumentNullException(nameof(corsPolicyProvider)); +#if NET8_0_OR_GREATER + _timeoutOptions = timeoutOptions ?? throw new ArgumentNullException(nameof(timeoutOptions)); +#endif _loadBalancingPolicies = loadBalancingPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(loadBalancingPolicies)); _affinityFailurePolicies = affinityFailurePolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(affinityFailurePolicies)); _availableDestinationsPolicies = availableDestinationsPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); @@ -294,7 +308,38 @@ private async ValueTask ValidateAuthorizationPolicyAsync(IList errors errors.Add(new ArgumentException($"Unable to retrieve the authorization policy '{authorizationPolicyName}' for route '{routeId}'.", ex)); } } +#if NET8_0_OR_GREATER + private void ValidateTimeoutPolicy(IList errors, string? timeoutPolicyName, TimeSpan? timeout, string routeId) + { + if (!string.IsNullOrEmpty(timeoutPolicyName)) + { + var policies = _timeoutOptions.CurrentValue.Policies; + if (string.Equals(TimeoutPolicyConstants.Default, timeoutPolicyName, StringComparison.OrdinalIgnoreCase) + || string.Equals(TimeoutPolicyConstants.Disable, timeoutPolicyName, StringComparison.OrdinalIgnoreCase)) + { + if (policies.TryGetValue(timeoutPolicyName, out var _)) + { + errors.Add(new ArgumentException($"The application has registered a timeout policy named '{timeoutPolicyName}' that conflicts with the reserved timeout policy name used on this route. The registered policy name needs to be changed for this route to function.")); + } + } + else if (!policies.TryGetValue(timeoutPolicyName, out var _)) + { + errors.Add(new ArgumentException($"Timeout policy '{timeoutPolicyName}' not found for route '{routeId}'.")); + } + + if (timeout.HasValue) + { + errors.Add(new ArgumentException($"Route '{routeId}' has both a Timeout '{timeout}' and TimeoutPolicy '{timeoutPolicyName}'.")); + } + } + + if (timeout.HasValue && timeout.Value.TotalMicroseconds <= 0) + { + errors.Add(new ArgumentException($"The Timeout value '{timeout.Value}' is invalid for route '{routeId}'. The Timeout must be greater than zero milliseconds.")); + } + } +#endif private async ValueTask ValidateRateLimiterPolicyAsync(IList errors, string? rateLimiterPolicyName, string routeId) { if (string.IsNullOrEmpty(rateLimiterPolicyName)) diff --git a/src/ReverseProxy/Configuration/RouteConfig.cs b/src/ReverseProxy/Configuration/RouteConfig.cs index 0ca059266..24aa0fa94 100644 --- a/src/ReverseProxy/Configuration/RouteConfig.cs +++ b/src/ReverseProxy/Configuration/RouteConfig.cs @@ -51,6 +51,23 @@ public sealed record RouteConfig /// Set to "Default" or leave empty to use the global rate limits, if any. /// public string? RateLimiterPolicy { get; init; } +#endif +#if NET8_0_OR_GREATER + /// + /// The name of the TimeoutPolicy to apply to this route. + /// Setting both Timeout and TimeoutPolicy is an error. + /// If not set then only the system default will apply. + /// Set to "Disable" to disable timeouts for this route. + /// Set to "Default" or leave empty to use the system defaults, if any. + /// + public string? TimeoutPolicy { get; init; } + + /// + /// The Timeout to apply to this route. This overrides any system defaults. + /// Setting both Timeout and TimeoutPolicy is an error. + /// Timeout granularity is limited to milliseconds. + /// + public TimeSpan? Timeout { get; init; } #endif /// /// The name of the CorsPolicy to apply to this route. diff --git a/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs b/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs new file mode 100644 index 000000000..09af45ffc --- /dev/null +++ b/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Yarp.ReverseProxy.Configuration; + +internal static class TimeoutPolicyConstants +{ + internal const string Default = "Default"; + internal const string Disable = "Disable"; +} diff --git a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs index 2ab8aeae8..c7408fba2 100644 --- a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs +++ b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs @@ -9,6 +9,9 @@ using Microsoft.AspNetCore.Cors; using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif #if NET7_0_OR_GREATER using Microsoft.AspNetCore.RateLimiting; #endif @@ -18,6 +21,7 @@ using CorsConstants = Yarp.ReverseProxy.Configuration.CorsConstants; using AuthorizationConstants = Yarp.ReverseProxy.Configuration.AuthorizationConstants; using RateLimitingConstants = Yarp.ReverseProxy.Configuration.RateLimitingConstants; +using TimeoutPolicyConstants = Yarp.ReverseProxy.Configuration.TimeoutPolicyConstants; namespace Yarp.ReverseProxy.Routing; @@ -26,6 +30,9 @@ internal sealed class ProxyEndpointFactory private static readonly IAuthorizeData _defaultAuthorization = new AuthorizeAttribute(); #if NET7_0_OR_GREATER private static readonly DisableRateLimitingAttribute _disableRateLimit = new(); +#endif +#if NET8_0_OR_GREATER + private static readonly DisableRequestTimeoutAttribute _disableRequestTimeout = new(); #endif private static readonly IEnableCorsAttribute _defaultCors = new EnableCorsAttribute(); private static readonly IDisableCorsAttribute _disableCors = new DisableCorsAttribute(); @@ -132,6 +139,25 @@ public Endpoint CreateEndpoint(RouteModel route, IReadOnlyList - net6.0;net7.0 + net6.0;net7.0;net8.0 Exe Yarp.ReverseProxy.Sample diff --git a/testassets/ReverseProxy.Code/Startup.cs b/testassets/ReverseProxy.Code/Startup.cs index ccd8a5f52..e5b9eb1df 100644 --- a/testassets/ReverseProxy.Code/Startup.cs +++ b/testassets/ReverseProxy.Code/Startup.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Net; using System.Net.Http.Headers; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Builder; @@ -35,7 +36,10 @@ public void ConfigureServices(IServiceCollection services) Match = new RouteMatch { Path = "{**catch-all}" - } + }, +#if NET8_0_OR_GREATER + Timeout = TimeSpan.FromSeconds(5), +#endif } }; var clusters = new[] @@ -72,7 +76,7 @@ public void ConfigureServices(IServiceCollection services) // For each route+cluster pair decide if we want to add transforms, and if so, which? // This logic is re-run each time a route is rebuilt. - transformBuilderContext.AddPathPrefix("/prefix"); + // transformBuilderContext.AddPathPrefix("/prefix"); // Only do this for routes that require auth. if (string.Equals("token", transformBuilderContext.Route.AuthorizationPolicy)) @@ -104,6 +108,16 @@ public void ConfigureServices(IServiceCollection services) services.AddSingleton, ForwarderMetricsConsumer>(); services.AddTelemetryConsumer(); services.AddTelemetryListeners(); +#if NET8_0_OR_GREATER + services.AddRequestTimeouts(o => + { + o.DefaultPolicy = new Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy() + { + Timeout = TimeSpan.FromSeconds(1), + TimeoutStatusCode = StatusCodes.Status418ImATeapot, + }; + }); +#endif } /// @@ -113,6 +127,9 @@ public void Configure(IApplicationBuilder app, IProxyStateLookup lookup) { app.UseRouting(); app.UseAuthorization(); +#if NET8_0_OR_GREATER + app.UseRequestTimeouts(); +#endif app.UseEndpoints(endpoints => { endpoints.MapControllers(); diff --git a/testassets/TestServer/Controllers/HttpController.cs b/testassets/TestServer/Controllers/HttpController.cs index fa9f9f30b..e265f3649 100644 --- a/testassets/TestServer/Controllers/HttpController.cs +++ b/testassets/TestServer/Controllers/HttpController.cs @@ -37,6 +37,18 @@ public IActionResult SkipBody() return StatusCode(StatusCodes.Status409Conflict); } + /// + /// Returns a 409 response without consuming the request body. + /// This is used to exercise Expect:100-continue behavior. + /// + [HttpGet] + [Route("/api/slow")] + public async Task Slow() + { + await Task.Delay(TimeSpan.FromSeconds(3)); + return StatusCode(StatusCodes.Status200OK); + } + /// /// Returns a 200 response dumping all info from the incoming request. /// From 8726e554441970016260c96e3c90ed6f3aedf1c5 Mon Sep 17 00:00:00 2001 From: Chris R Date: Thu, 9 Nov 2023 17:00:28 -0800 Subject: [PATCH 2/6] Tests --- .../Converters/YarpIngressOptions.cs | 3 + .../Converters/YarpParser.cs | 4 + .../Configuration/ConfigValidator.cs | 3 + src/ReverseProxy/Configuration/RouteConfig.cs | 8 + .../Routing/ProxyEndpointFactory.cs | 2 +- .../ConfigurationConfigProviderTests.cs | 6 + .../Configuration/ConfigValidatorTests.cs | 139 ++++++++++++++++++ .../Configuration/RouteConfigTests.cs | 22 +++ .../Routing/ProxyEndpointFactoryTests.cs | 124 ++++++++++++++++ 9 files changed, 310 insertions(+), 1 deletion(-) diff --git a/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs b/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs index 6d2957698..6f7069f71 100644 --- a/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs +++ b/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; using Yarp.ReverseProxy.Configuration; @@ -18,6 +19,8 @@ internal sealed class YarpIngressOptions public HttpClientConfig HttpClientConfig { get; set; } public string LoadBalancingPolicy { get; set; } public string CorsPolicy { get; set; } + public string TimeoutPolicy { get; set; } + public TimeSpan? Timeout { get; set; } public HealthCheckConfig HealthCheck { get; set; } public Dictionary RouteMetadata { get; set; } public List RouteHeaders { get; set; } diff --git a/src/Kubernetes.Controller/Converters/YarpParser.cs b/src/Kubernetes.Controller/Converters/YarpParser.cs index bf940c310..3da4617ec 100644 --- a/src/Kubernetes.Controller/Converters/YarpParser.cs +++ b/src/Kubernetes.Controller/Converters/YarpParser.cs @@ -142,6 +142,10 @@ private static RouteConfig CreateRoute(YarpIngressContext ingressContext, V1HTTP AuthorizationPolicy = ingressContext.Options.AuthorizationPolicy, #if NET7_0_OR_GREATER RateLimiterPolicy = ingressContext.Options.RateLimiterPolicy, +#endif +#if NET8_0_OR_GREATER + Timeout = ingressContext.Options.Timeout, + TimeoutPolicy = ingressContext.Options.TimeoutPolicy, #endif CorsPolicy = ingressContext.Options.CorsPolicy, Metadata = ingressContext.Options.RouteMetadata, diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index 50d4a92c5..d06a795f0 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -92,6 +92,9 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) await ValidateAuthorizationPolicyAsync(errors, route.AuthorizationPolicy, route.RouteId); #if NET7_0_OR_GREATER await ValidateRateLimiterPolicyAsync(errors, route.RateLimiterPolicy, route.RouteId); +#endif +#if NET8_0_OR_GREATER + ValidateTimeoutPolicy(errors, route.TimeoutPolicy, route.Timeout, route.RouteId); #endif await ValidateCorsPolicyAsync(errors, route.CorsPolicy, route.RouteId); diff --git a/src/ReverseProxy/Configuration/RouteConfig.cs b/src/ReverseProxy/Configuration/RouteConfig.cs index 24aa0fa94..817203b86 100644 --- a/src/ReverseProxy/Configuration/RouteConfig.cs +++ b/src/ReverseProxy/Configuration/RouteConfig.cs @@ -106,6 +106,10 @@ public bool Equals(RouteConfig? other) && string.Equals(AuthorizationPolicy, other.AuthorizationPolicy, StringComparison.OrdinalIgnoreCase) #if NET7_0_OR_GREATER && string.Equals(RateLimiterPolicy, other.RateLimiterPolicy, StringComparison.OrdinalIgnoreCase) +#endif +#if NET8_0_OR_GREATER + && string.Equals(TimeoutPolicy, other.TimeoutPolicy, StringComparison.OrdinalIgnoreCase) + && Timeout == other.Timeout #endif && string.Equals(CorsPolicy, other.CorsPolicy, StringComparison.OrdinalIgnoreCase) && Match == other.Match @@ -123,6 +127,10 @@ public override int GetHashCode() hash.Add(AuthorizationPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); #if NET7_0_OR_GREATER hash.Add(RateLimiterPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); +#endif +#if NET8_0_OR_GREATER + hash.Add(Timeout?.GetHashCode()); + hash.Add(TimeoutPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); #endif hash.Add(CorsPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); hash.Add(Match); diff --git a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs index c7408fba2..716e8cbd1 100644 --- a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs +++ b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs @@ -138,7 +138,6 @@ public Endpoint CreateEndpoint(RouteModel route, IReadOnlyList err.Message.Equals($"The application has registered an authorization policy named '{authorizationPolicy}' that conflicts with the reserved authorization policy name used on this route. The registered policy name needs to be changed for this route to function.")); } +#if NET8_0_OR_GREATER + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("defaulT")] + [InlineData("disAble")] + public async Task Accepts_ReservedTimeoutPolicy(string policy) + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = policy, + Match = new RouteMatch + { + Hosts = new[] { "localhost" }, + }, + ClusterId = "cluster1", + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.Empty(result); + } + + [Fact] + public async Task Accepts_CustomTimeoutPolicy() + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "custom", + Match = new RouteMatch + { + Hosts = new[] { "localhost" }, + }, + ClusterId = "cluster1", + }; + + var services = CreateServices(services => + { + services.AddRequestTimeouts(options => + { + options.AddPolicy("custom", TimeSpan.FromSeconds(1)); + }); + }); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.Empty(result); + } + + [Fact] + public async Task Accepts_CustomTimeout() + { + var route = new RouteConfig + { + RouteId = "route1", + Timeout = TimeSpan.FromSeconds(1), + Match = new RouteMatch + { + Hosts = new[] { "localhost" }, + }, + ClusterId = "cluster1", + }; + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.Empty(result); + } + + [Fact] + public async Task Rejects_UnknownTimeoutPolicy() + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "unknown", + ClusterId = "cluster1", + Match = new RouteMatch(), + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.NotEmpty(result); + Assert.Contains(result, err => err.Message.Equals("Timeout policy 'unknown' not found for route 'route1'.")); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public async Task Rejects_InvalidTimeouts(int timeout) + { + var route = new RouteConfig + { + RouteId = "route1", + Timeout = TimeSpan.FromMilliseconds(timeout), + ClusterId = "cluster1", + Match = new RouteMatch(), + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.NotEmpty(result); + Assert.Contains(result, err => err.Message.Equals($"The Timeout value '{TimeSpan.FromMilliseconds(timeout)}' is invalid for route 'route1'. The Timeout must be greater than zero milliseconds.")); + } + + [Fact] + public async Task Rejects_TimeoutWithTimeoutPolicy() + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "unknown", + Timeout = TimeSpan.FromSeconds(1), + ClusterId = "cluster1", + Match = new RouteMatch(), + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.NotEmpty(result); + Assert.Contains(result, err => err.Message.Equals("Timeout policy 'unknown' not found for route 'route1'.")); + } +#endif [Theory] [InlineData(null)] [InlineData("")] diff --git a/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs b/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs index 073e2b843..9b071b017 100644 --- a/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs +++ b/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs @@ -1,8 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; using System.Text.Json; +using System.Threading; using Xunit; namespace Yarp.ReverseProxy.Configuration.Tests; @@ -17,6 +19,10 @@ public void Equals_Positive() AuthorizationPolicy = "a", #if NET7_0_OR_GREATER RateLimiterPolicy = "rl", +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = "t", + Timeout = TimeSpan.FromSeconds(1), #endif ClusterId = "c", CorsPolicy = "co", @@ -48,6 +54,10 @@ public void Equals_Positive() AuthorizationPolicy = "A", #if NET7_0_OR_GREATER RateLimiterPolicy = "RL", +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = "T", + Timeout = TimeSpan.FromSeconds(1), #endif ClusterId = "C", CorsPolicy = "Co", @@ -90,6 +100,10 @@ public void Equals_Negative() AuthorizationPolicy = "a", #if NET7_0_OR_GREATER RateLimiterPolicy = "rl", +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = "t", + Timeout = TimeSpan.FromSeconds(1), #endif ClusterId = "c", CorsPolicy = "co", @@ -126,6 +140,10 @@ public void Equals_Negative() #if NET7_0_OR_GREATER var i = a with { RateLimiterPolicy = "i" }; #endif +#if NET8_0_OR_GREATER + var j = a with { TimeoutPolicy = "j" }; + var k = a with { Timeout = TimeSpan.FromSeconds(107) }; +#endif Assert.False(a.Equals(b)); Assert.False(a.Equals(c)); @@ -136,6 +154,10 @@ public void Equals_Negative() Assert.False(a.Equals(h)); #if NET7_0_OR_GREATER Assert.False(a.Equals(i)); +#endif +#if NET8_0_OR_GREATER + Assert.False(a.Equals(j)); + Assert.False(a.Equals(k)); #endif } diff --git a/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs b/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs index c1c3a88ab..64f4a9ac9 100644 --- a/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs +++ b/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs @@ -5,6 +5,9 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Cors; using Microsoft.AspNetCore.Cors.Infrastructure; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif #if NET7_0_OR_GREATER using Microsoft.AspNetCore.RateLimiting; #endif @@ -418,6 +421,127 @@ public void AddEndpoint_NoRateLimiter_Works() Assert.Null(routeEndpoint.Metadata.GetMetadata()); } #endif +#if NET8_0_OR_GREATER + [Fact] + public void AddEndpoint_DefaultTimeoutPolicy_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "defaulT", + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } + + [Fact] + public void AddEndpoint_CustomTimeoutPolicy_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "custom", + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + var attribute = routeEndpoint.Metadata.GetMetadata(); + Assert.NotNull(attribute); + Assert.Equal("custom", attribute.PolicyName); + Assert.Null(attribute.Timeout); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } + + [Fact] + public void AddEndpoint_CustomTimeout_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + Timeout = TimeSpan.FromSeconds(5), + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + var attribute = routeEndpoint.Metadata.GetMetadata(); + Assert.NotNull(attribute); + Assert.Null(attribute.PolicyName); + Assert.Equal(TimeSpan.FromSeconds(5), attribute.Timeout); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } + + [Fact] + public void AddEndpoint_DisableTimeoutPolicy_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "disAble", + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + Assert.NotNull(routeEndpoint.Metadata.GetMetadata()); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } + + [Fact] + public void AddEndpoint_NoTimeoutPolicy_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } +#endif [Fact] public void AddEndpoint_DefaultCors_Works() From ed0959276e3be88fef9553246fae90037657a631 Mon Sep 17 00:00:00 2001 From: Chris R Date: Fri, 10 Nov 2023 11:23:59 -0800 Subject: [PATCH 3/6] Detect missing middleware --- .../ProxyPipelineInitializerMiddleware.cs | 28 ++++++- src/ReverseProxy/Utilities/EventIds.cs | 1 + ...ProxyPipelineInitializerMiddlewareTests.cs | 76 ++++++++++++++++++- 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs b/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs index 8006bce0f..2a087c9f4 100644 --- a/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs +++ b/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs @@ -5,7 +5,13 @@ using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.Extensions.Logging; +#if NET8_0_OR_GREATER +using Yarp.ReverseProxy.Configuration; +#endif using Yarp.ReverseProxy.Utilities; namespace Yarp.ReverseProxy.Model; @@ -41,7 +47,17 @@ public Task Invoke(HttpContext context) context.Response.StatusCode = StatusCodes.Status503ServiceUnavailable; return Task.CompletedTask; } - +#if NET8_0_OR_GREATER + // There's no way to detect the presence of the timeout middleware before this, only the options. + if (endpoint.Metadata.GetMetadata() != null + && context.Features.Get() == null) + { + Log.TimeoutNotApplied(_logger, route.Config.RouteId); + // Out of an abundance of caution, refuse the request rather than allowing it to proceed without the configured timeout. + throw new InvalidOperationException($"The timeout was not applied for route '{route.Config.RouteId}', ensure `IApplicationBuilder.UseRequestTimeouts()`" + + " is called between `IApplicationBuilder.UseRouting()` and `IApplicationBuilder.UseEndpoints()`."); + } +#endif var destinationsState = cluster.DestinationsState; context.Features.Set(new ReverseProxyFeature { @@ -80,9 +96,19 @@ private static class Log EventIds.NoClusterFound, "Route '{routeId}' has no cluster information."); + private static readonly Action _timeoutNotApplied = LoggerMessage.Define( + LogLevel.Error, + EventIds.TimeoutNotApplied, + "The timeout was not applied for route '{routeId}', ensure `IApplicationBuilder.UseRequestTimeouts()` is called between `IApplicationBuilder.UseRouting()` and `IApplicationBuilder.UseEndpoints()`."); + public static void NoClusterFound(ILogger logger, string routeId) { _noClusterFound(logger, routeId, null); } + + public static void TimeoutNotApplied(ILogger logger, string routeId) + { + _timeoutNotApplied(logger, routeId, null); + } } } diff --git a/src/ReverseProxy/Utilities/EventIds.cs b/src/ReverseProxy/Utilities/EventIds.cs index ac9e2c20c..8db78fd46 100644 --- a/src/ReverseProxy/Utilities/EventIds.cs +++ b/src/ReverseProxy/Utilities/EventIds.cs @@ -67,4 +67,5 @@ internal static class EventIds public static readonly EventId RetryingWebSocketDowngradeNoConnect = new EventId(61, "RetryingWebSocketDowngradeNoConnect"); public static readonly EventId RetryingWebSocketDowngradeNoHttp2 = new EventId(62, "RetryingWebSocketDowngradeNoHttp2"); public static readonly EventId InvalidSecWebSocketKeyHeader = new EventId(63, "InvalidSecWebSocketKeyHeader"); + public static readonly EventId TimeoutNotApplied = new(64, nameof(TimeoutNotApplied)); } diff --git a/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs b/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs index d1b9d02e4..ad3668ab4 100644 --- a/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs +++ b/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs @@ -7,6 +7,9 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; using Moq; @@ -119,14 +122,85 @@ public async Task Invoke_NoHealthyEndpoints_CallsNext() Assert.Equal(StatusCodes.Status418ImATeapot, httpContext.Response.StatusCode); } +#if NET8_0_OR_GREATER + [Fact] + public async Task Invoke_MissingTimeoutMiddleware_RefuseRequest() + { + var httpClient = new HttpMessageInvoker(new Mock().Object); + var cluster1 = new ClusterState(clusterId: "cluster1") + { + Model = new ClusterModel(new ClusterConfig(), httpClient) + }; + + var aspNetCoreEndpoints = new List(); + var routeConfig = new RouteModel( + config: new RouteConfig(), + cluster: cluster1, + transformer: HttpTransformer.Default); + var aspNetCoreEndpoint = CreateAspNetCoreEndpoint(routeConfig, + builder => + { + builder.Metadata.Add(new RequestTimeoutAttribute(1)); + }); + aspNetCoreEndpoints.Add(aspNetCoreEndpoint); + var httpContext = new DefaultHttpContext(); + httpContext.SetEndpoint(aspNetCoreEndpoint); + + var sut = Create(); + + await sut.Invoke(httpContext); + + Assert.Equal(StatusCodes.Status503ServiceUnavailable, httpContext.Response.StatusCode); + } + + [Fact] + public async Task Invoke_MissingTimeoutMiddleware_DefaultPolicyAllowed() + { + var httpClient = new HttpMessageInvoker(new Mock().Object); + var cluster1 = new ClusterState(clusterId: "cluster1"); + cluster1.Model = new ClusterModel(new ClusterConfig(), httpClient); + var destination1 = cluster1.Destinations.GetOrAdd( + "destination1", + id => new DestinationState(id) { Model = new DestinationModel(new DestinationConfig { Address = "https://localhost:123/a/b/" }) }); + cluster1.DestinationsState = new ClusterDestinationsState(new[] { destination1 }, new[] { destination1 }); + + var aspNetCoreEndpoints = new List(); + var routeConfig = new RouteModel( + config: new RouteConfig(), + cluster1, + HttpTransformer.Default); + var aspNetCoreEndpoint = CreateAspNetCoreEndpoint(routeConfig, + builder => + { + builder.Metadata.Add(new RequestTimeoutAttribute(TimeoutPolicyConstants.Default)); + }); + aspNetCoreEndpoints.Add(aspNetCoreEndpoint); + var httpContext = new DefaultHttpContext(); + httpContext.SetEndpoint(aspNetCoreEndpoint); + + var sut = Create(); + + await sut.Invoke(httpContext); + + var proxyFeature = httpContext.GetReverseProxyFeature(); + Assert.NotNull(proxyFeature); + Assert.NotNull(proxyFeature.AvailableDestinations); + Assert.Single(proxyFeature.AvailableDestinations); + Assert.Same(destination1, proxyFeature.AvailableDestinations[0]); + Assert.Same(cluster1.Model, proxyFeature.Cluster); + + Assert.Equal(StatusCodes.Status418ImATeapot, httpContext.Response.StatusCode); + } +#endif - private static Endpoint CreateAspNetCoreEndpoint(RouteModel routeConfig) + private static Endpoint CreateAspNetCoreEndpoint(RouteModel routeConfig, Action configure = null) { var endpointBuilder = new RouteEndpointBuilder( requestDelegate: httpContext => Task.CompletedTask, routePattern: RoutePatternFactory.Parse("/"), order: 0); endpointBuilder.Metadata.Add(routeConfig); + configure?.Invoke(endpointBuilder); return endpointBuilder.Build(); } } From 1c1409b3bdf098271d8c2f00c63efb68a4607336 Mon Sep 17 00:00:00 2001 From: Chris R Date: Fri, 10 Nov 2023 15:29:59 -0800 Subject: [PATCH 4/6] Docs --- docs/docfx/articles/timeouts.md | 86 +++++++++++++++++++++ src/ReverseProxy/Forwarder/HttpForwarder.cs | 7 ++ 2 files changed, 93 insertions(+) create mode 100644 docs/docfx/articles/timeouts.md diff --git a/docs/docfx/articles/timeouts.md b/docs/docfx/articles/timeouts.md new file mode 100644 index 000000000..a5027e9bb --- /dev/null +++ b/docs/docfx/articles/timeouts.md @@ -0,0 +1,86 @@ +# Request Timeouts + +## Introduction + +.NET 8 introduced the [Request Timeouts Middleware](https://learn.microsoft.com/aspnet/core/performance/timeouts) to enable configuring request timeouts globally as well as per endpoint. This functionality is also available in YARP 2.1 when running on .NET 8. + +## Defaults +Requests do not have any timeouts by default, other than the [Activity Timeout](http-client-config.md#HttpRequest) used to clean up idle requests. A default policy specified in [RequestTimeoutOptions](https://learn.microsoft.com/dotnet/api/microsoft.aspnetcore.http.timeouts.requesttimeoutoptions) will apply to proxied requests as well. + +## Configuration +Timeouts and Timeout Policies can be specified per route via [RouteConfig](xref:Yarp.ReverseProxy.Configuration.RouteConfig) and can be bound from the `Routes` sections of the config file. As with other route properties, this can be modified and reloaded without restarting the proxy. Policy names are case insensitive. + +Timeouts are specified in a TimeSpan HH:MM:SS format. Specifying both Timeout and TimeoutPolicy on the same route is invalid and will cause the configuration to be rejected. + +Example: +```JSON +{ + "ReverseProxy": { + "Routes": { + "route1" : { + "ClusterId": "cluster1", + "TimeoutPolicy": "customPolicy", + "Match": { + "Hosts": [ "localhost" ] + }, + } + "route2" : { + "ClusterId": "cluster1", + "Timeout": "00:01:00", + "Match": { + "Hosts": [ "localhost2" ] + }, + } + }, + "Clusters": { + "cluster1": { + "Destinations": { + "cluster1/destination1": { + "Address": "https://localhost:10001/" + } + } + } + } + } +} +``` + +Timeout policies can be configured in Startup.ConfigureServices as follows: +``` +public void ConfigureServices(IServiceCollection services) +{ + services.AddRequestTimeouts(options => + { + options.AddPolicy("customPolicy", TimeSpan.FromSeconds(20)); + }); +} +``` + +In Startup.Configure add the timeout middleware between Routing and Endpoints. + +``` +public void Configure(IApplicationBuilder app) +{ + app.UseRouting(); + + app.UseRequestTimeouts(); + + app.UseEndpoints(endpoints => + { + endpoints.MapReverseProxy(); + }); +} +``` + + +### DefaultPolicy + +Specifying the value `default` in a route's `TimeoutPolicy` parameter means that route will use the policy defined in [RequestTimeoutOptions.DefaultPolicy](https://learn.microsoft.com/dotnet/api/microsoft.aspnetcore.http.timeouts.requesttimeoutoptions.defaultpolicy#microsoft-aspnetcore-http-timeouts-requesttimeoutoptions-defaultpolicy). + +### Disable timeouts + +Specifying the value `disable` in a route's `TimeoutPolicy` parameter means the request timeout middleware will not apply timeouts to this route. + +### WebSockets + +Request timeouts are disabled after the initial WebSocket handshake. diff --git a/src/ReverseProxy/Forwarder/HttpForwarder.cs b/src/ReverseProxy/Forwarder/HttpForwarder.cs index f905e2a4e..81c3ef626 100644 --- a/src/ReverseProxy/Forwarder/HttpForwarder.cs +++ b/src/ReverseProxy/Forwarder/HttpForwarder.cs @@ -12,6 +12,9 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; @@ -719,6 +722,10 @@ private async ValueTask HandleUpgradedResponse(HttpContext conte Debug.Assert(upgradeFeature != null); upgradeResult = await upgradeFeature.UpgradeAsync(); } +#if NET8_0_OR_GREATER + // Disable request timeout, if there is one, after the upgrade has been accepted + context.Features.Get()?.DisableTimeout(); +#endif } catch (Exception ex) { From 0927f21a39a7f2b9a87433f5a832488c3af29c6f Mon Sep 17 00:00:00 2001 From: Chris R Date: Fri, 10 Nov 2023 15:34:21 -0800 Subject: [PATCH 5/6] Fixup tests --- ...ProxyPipelineInitializerMiddlewareTests.cs | 43 +------------------ 1 file changed, 1 insertion(+), 42 deletions(-) diff --git a/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs b/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs index ad3668ab4..fdf917ab0 100644 --- a/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs +++ b/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs @@ -148,48 +148,7 @@ public async Task Invoke_MissingTimeoutMiddleware_RefuseRequest() var sut = Create(); - await sut.Invoke(httpContext); - - Assert.Equal(StatusCodes.Status503ServiceUnavailable, httpContext.Response.StatusCode); - } - - [Fact] - public async Task Invoke_MissingTimeoutMiddleware_DefaultPolicyAllowed() - { - var httpClient = new HttpMessageInvoker(new Mock().Object); - var cluster1 = new ClusterState(clusterId: "cluster1"); - cluster1.Model = new ClusterModel(new ClusterConfig(), httpClient); - var destination1 = cluster1.Destinations.GetOrAdd( - "destination1", - id => new DestinationState(id) { Model = new DestinationModel(new DestinationConfig { Address = "https://localhost:123/a/b/" }) }); - cluster1.DestinationsState = new ClusterDestinationsState(new[] { destination1 }, new[] { destination1 }); - - var aspNetCoreEndpoints = new List(); - var routeConfig = new RouteModel( - config: new RouteConfig(), - cluster1, - HttpTransformer.Default); - var aspNetCoreEndpoint = CreateAspNetCoreEndpoint(routeConfig, - builder => - { - builder.Metadata.Add(new RequestTimeoutAttribute(TimeoutPolicyConstants.Default)); - }); - aspNetCoreEndpoints.Add(aspNetCoreEndpoint); - var httpContext = new DefaultHttpContext(); - httpContext.SetEndpoint(aspNetCoreEndpoint); - - var sut = Create(); - - await sut.Invoke(httpContext); - - var proxyFeature = httpContext.GetReverseProxyFeature(); - Assert.NotNull(proxyFeature); - Assert.NotNull(proxyFeature.AvailableDestinations); - Assert.Single(proxyFeature.AvailableDestinations); - Assert.Same(destination1, proxyFeature.AvailableDestinations[0]); - Assert.Same(cluster1.Model, proxyFeature.Cluster); - - Assert.Equal(StatusCodes.Status418ImATeapot, httpContext.Response.StatusCode); + await Assert.ThrowsAsync(() => sut.Invoke(httpContext)); } #endif From 62c95a62bfd57f2ec4a975acb20991983d3fe447 Mon Sep 17 00:00:00 2001 From: Chris R Date: Mon, 13 Nov 2023 12:41:03 -0800 Subject: [PATCH 6/6] Docs cleanup, remove default, feedback. --- docs/docfx/articles/timeouts.md | 37 +++++++------------ docs/docfx/articles/toc.yml | 2 + docs/docfx/articles/websockets.md | 4 ++ .../ReverseProxy.Minimal.Sample/Program.cs | 11 +++++- .../Configuration/ConfigValidator.cs | 5 +-- .../Configuration/TimeoutPolicyConstants.cs | 1 - .../ProxyPipelineInitializerMiddleware.cs | 4 +- .../Routing/ProxyEndpointFactory.cs | 6 +-- .../Configuration/ConfigValidatorTests.cs | 1 - .../Routing/ProxyEndpointFactoryTests.cs | 23 ------------ 10 files changed, 34 insertions(+), 60 deletions(-) diff --git a/docs/docfx/articles/timeouts.md b/docs/docfx/articles/timeouts.md index a5027e9bb..df7b504c8 100644 --- a/docs/docfx/articles/timeouts.md +++ b/docs/docfx/articles/timeouts.md @@ -45,37 +45,26 @@ Example: } ``` -Timeout policies can be configured in Startup.ConfigureServices as follows: -``` -public void ConfigureServices(IServiceCollection services) -{ - services.AddRequestTimeouts(options => - { - options.AddPolicy("customPolicy", TimeSpan.FromSeconds(20)); - }); -} -``` +Timeout policies and the default policy can be configured in the service collection and the middleware can be added as follows: +```csharp +var builder = WebApplication.CreateBuilder(args); -In Startup.Configure add the timeout middleware between Routing and Endpoints. +builder.Services.AddReverseProxy() + .LoadFromConfig(builder.Configuration.GetSection("ReverseProxy")); -``` -public void Configure(IApplicationBuilder app) +builder.Services.AddRequestTimeouts(options => { - app.UseRouting(); + options.AddPolicy("customPolicy", TimeSpan.FromSeconds(20)); +}); - app.UseRequestTimeouts(); +var app = builder.Build(); - app.UseEndpoints(endpoints => - { - endpoints.MapReverseProxy(); - }); -} -``` +app.UseRequestTimeouts(); +app.MapReverseProxy(); -### DefaultPolicy - -Specifying the value `default` in a route's `TimeoutPolicy` parameter means that route will use the policy defined in [RequestTimeoutOptions.DefaultPolicy](https://learn.microsoft.com/dotnet/api/microsoft.aspnetcore.http.timeouts.requesttimeoutoptions.defaultpolicy#microsoft-aspnetcore-http-timeouts-requesttimeoutoptions-defaultpolicy). +app.Run(); +``` ### Disable timeouts diff --git a/docs/docfx/articles/toc.yml b/docs/docfx/articles/toc.yml index e340d8442..b9d69d5c9 100644 --- a/docs/docfx/articles/toc.yml +++ b/docs/docfx/articles/toc.yml @@ -42,6 +42,8 @@ href: grpc.md - name: WebSockets and SPDY href: websockets.md +- name: Timeouts + href: timeouts.md - name: Service Fabric Integration href: service-fabric-int.md - name: Http.sys Delegation diff --git a/docs/docfx/articles/websockets.md b/docs/docfx/articles/websockets.md index c1fe082b2..fb893c424 100644 --- a/docs/docfx/articles/websockets.md +++ b/docs/docfx/articles/websockets.md @@ -21,3 +21,7 @@ The incoming and outgoing protocol versions do not need to match. The incoming W WebSockets require different HTTP headers for HTTP/2 so YARP will add and remove these headers as needed when adapting between the different versions. After the initial handshake WebSockets function the same way over both HTTP versions. + +## Timeout + +[Http Request Timeouts](https://learn.microsoft.com/aspnet/core/performance/timeouts) (.NET 8+) can apply timeouts to all requests by default or by policy. These timeouts will be disabled after a WebSocket handshake. They will still apply to gRPC requests. For additional configuration see [Timeouts](timeouts.md). diff --git a/samples/ReverseProxy.Minimal.Sample/Program.cs b/samples/ReverseProxy.Minimal.Sample/Program.cs index 0e6beaca8..62a48e722 100644 --- a/samples/ReverseProxy.Minimal.Sample/Program.cs +++ b/samples/ReverseProxy.Minimal.Sample/Program.cs @@ -2,9 +2,16 @@ builder.Services.AddReverseProxy() .LoadFromConfig(builder.Configuration.GetSection("ReverseProxy")); - +#if NET8_0_OR_GREATER +builder.Services.AddRequestTimeouts(options => +{ + options.AddPolicy("customPolicy", TimeSpan.FromSeconds(20)); +}); +#endif var app = builder.Build(); - +#if NET8_0_OR_GREATER +app.UseRequestTimeouts(); +#endif app.MapReverseProxy(); app.Run(); diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index d06a795f0..3b61c373a 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -318,8 +318,7 @@ private void ValidateTimeoutPolicy(IList errors, string? timeoutPolic { var policies = _timeoutOptions.CurrentValue.Policies; - if (string.Equals(TimeoutPolicyConstants.Default, timeoutPolicyName, StringComparison.OrdinalIgnoreCase) - || string.Equals(TimeoutPolicyConstants.Disable, timeoutPolicyName, StringComparison.OrdinalIgnoreCase)) + if (string.Equals(TimeoutPolicyConstants.Disable, timeoutPolicyName, StringComparison.OrdinalIgnoreCase)) { if (policies.TryGetValue(timeoutPolicyName, out var _)) { @@ -337,7 +336,7 @@ private void ValidateTimeoutPolicy(IList errors, string? timeoutPolic } } - if (timeout.HasValue && timeout.Value.TotalMicroseconds <= 0) + if (timeout.HasValue && timeout.Value.TotalMilliseconds <= 0) { errors.Add(new ArgumentException($"The Timeout value '{timeout.Value}' is invalid for route '{routeId}'. The Timeout must be greater than zero milliseconds.")); } diff --git a/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs b/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs index 09af45ffc..71f0fd38d 100644 --- a/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs +++ b/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs @@ -5,6 +5,5 @@ namespace Yarp.ReverseProxy.Configuration; internal static class TimeoutPolicyConstants { - internal const string Default = "Default"; internal const string Disable = "Disable"; } diff --git a/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs b/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs index 2a087c9f4..c11050b6f 100644 --- a/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs +++ b/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs @@ -50,7 +50,9 @@ public Task Invoke(HttpContext context) #if NET8_0_OR_GREATER // There's no way to detect the presence of the timeout middleware before this, only the options. if (endpoint.Metadata.GetMetadata() != null - && context.Features.Get() == null) + && context.Features.Get() == null + // The feature is skipped if the request is already canceled. We'll handle canceled requests later for consistency. + && !context.RequestAborted.IsCancellationRequested) { Log.TimeoutNotApplied(_logger, route.Config.RouteId); // Out of an abundance of caution, refuse the request rather than allowing it to proceed without the configured timeout. diff --git a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs index 716e8cbd1..1be5aae6e 100644 --- a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs +++ b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs @@ -139,11 +139,7 @@ public Endpoint CreateEndpoint(RouteModel route, IReadOnlyList(); - factory.SetProxyPipeline(context => Task.CompletedTask); - - var route = new RouteConfig - { - RouteId = "route1", - TimeoutPolicy = "defaulT", - Order = 12, - Match = new RouteMatch(), - }; - var cluster = new ClusterState("cluster1"); - var routeState = new RouteState("route1"); - - var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); - - Assert.Null(routeEndpoint.Metadata.GetMetadata()); - Assert.Null(routeEndpoint.Metadata.GetMetadata()); - } - [Fact] public void AddEndpoint_CustomTimeoutPolicy_Works() {