diff --git a/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs b/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs index de48294c7b..aa249aba90 100644 --- a/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs +++ b/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs @@ -569,14 +569,16 @@ namespace Microsoft.AspNetCore.Routing.Matching bool Microsoft.AspNetCore.Routing.Matching.IEndpointSelectorPolicy.AppliesToEndpoints(System.Collections.Generic.IReadOnlyList endpoints) { throw null; } bool Microsoft.AspNetCore.Routing.Matching.INodeBuilderPolicy.AppliesToEndpoints(System.Collections.Generic.IReadOnlyList endpoints) { throw null; } } - public sealed partial class HttpMethodMatcherPolicy : Microsoft.AspNetCore.Routing.MatcherPolicy, Microsoft.AspNetCore.Routing.Matching.IEndpointComparerPolicy, Microsoft.AspNetCore.Routing.Matching.INodeBuilderPolicy + public sealed partial class HttpMethodMatcherPolicy : Microsoft.AspNetCore.Routing.MatcherPolicy, Microsoft.AspNetCore.Routing.Matching.IEndpointComparerPolicy, Microsoft.AspNetCore.Routing.Matching.IEndpointSelectorPolicy, Microsoft.AspNetCore.Routing.Matching.INodeBuilderPolicy { public HttpMethodMatcherPolicy() { } public System.Collections.Generic.IComparer Comparer { get { throw null; } } public override int Order { get { throw null; } } - public bool AppliesToEndpoints(System.Collections.Generic.IReadOnlyList endpoints) { throw null; } + public System.Threading.Tasks.Task ApplyAsync(Microsoft.AspNetCore.Http.HttpContext httpContext, Microsoft.AspNetCore.Routing.EndpointSelectorContext context, Microsoft.AspNetCore.Routing.Matching.CandidateSet candidates) { throw null; } public Microsoft.AspNetCore.Routing.Matching.PolicyJumpTable BuildJumpTable(int exitDestination, System.Collections.Generic.IReadOnlyList edges) { throw null; } public System.Collections.Generic.IReadOnlyList GetEdges(System.Collections.Generic.IReadOnlyList endpoints) { throw null; } + bool Microsoft.AspNetCore.Routing.Matching.IEndpointSelectorPolicy.AppliesToEndpoints(System.Collections.Generic.IReadOnlyList endpoints) { throw null; } + bool Microsoft.AspNetCore.Routing.Matching.INodeBuilderPolicy.AppliesToEndpoints(System.Collections.Generic.IReadOnlyList endpoints) { throw null; } } public partial interface IEndpointComparerPolicy { diff --git a/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs b/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs index af3981521a..d6ef3bbda2 100644 --- a/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs +++ b/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Internal; @@ -14,7 +15,7 @@ namespace Microsoft.AspNetCore.Routing.Matching /// An that implements filtering and selection by /// the HTTP method of a request. /// - public sealed class HttpMethodMatcherPolicy : MatcherPolicy, IEndpointComparerPolicy, INodeBuilderPolicy + public sealed class HttpMethodMatcherPolicy : MatcherPolicy, IEndpointComparerPolicy, INodeBuilderPolicy, IEndpointSelectorPolicy { // Used in tests internal static readonly string OriginHeader = "Origin"; @@ -39,18 +40,34 @@ namespace Microsoft.AspNetCore.Routing.Matching /// public override int Order => -1000; - /// - /// For framework use only. - /// - /// - /// - public bool AppliesToEndpoints(IReadOnlyList endpoints) + bool INodeBuilderPolicy.AppliesToEndpoints(IReadOnlyList endpoints) { if (endpoints == null) { throw new ArgumentNullException(nameof(endpoints)); } + if (ContainsDynamicEndpoints(endpoints)) + { + return false; + } + + return AppliesToEndpointsCore(endpoints); + } + + bool IEndpointSelectorPolicy.AppliesToEndpoints(IReadOnlyList endpoints) + { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + // When the node contains dynamic endpoints we can't make any assumptions. + return ContainsDynamicEndpoints(endpoints); + } + + private bool AppliesToEndpointsCore(IReadOnlyList endpoints) + { for (var i = 0; i < endpoints.Count; i++) { if (endpoints[i].Metadata.GetMetadata()?.HttpMethods.Count > 0) @@ -62,6 +79,104 @@ namespace Microsoft.AspNetCore.Routing.Matching return false; } + /// + /// For framework use only. + /// + /// + /// + /// + /// + public Task ApplyAsync(HttpContext httpContext, EndpointSelectorContext context, CandidateSet candidates) + { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (candidates == null) + { + throw new ArgumentNullException(nameof(candidates)); + } + + // Returning a 405 here requires us to return keep track of all 'seen' HTTP methods. We allocate to + // keep track of this beause we either need to keep track of the HTTP methods or keep track of the + // endpoints - both allocate. + // + // Those code only runs in the presence of dynamic endpoints anyway. + // + // We want to return a 405 iff we eliminated ALL of the currently valid endpoints due to HTTP method + // mismatch. + bool? needs405Endpoint = null; + HashSet methods = null; + + for (var i = 0; i < candidates.Count; i++) + { + // We do this check first for consistency with how 405 is implemented for the graph version + // of this code. We still want to know if any endpoints in this set require an HTTP method + // even if those endpoints are already invalid. + var metadata = candidates[i].Endpoint.Metadata.GetMetadata(); + if (metadata == null || metadata.HttpMethods.Count == 0) + { + // Can match any method. + needs405Endpoint = false; + continue; + } + + // Saw a valid endpoint. + needs405Endpoint = needs405Endpoint ?? true; + + if (!candidates.IsValidCandidate(i)) + { + continue; + } + + var httpMethod = httpContext.Request.Method; + if (metadata.AcceptCorsPreflight && + string.Equals(httpMethod, PreflightHttpMethod, StringComparison.OrdinalIgnoreCase) && + httpContext.Request.Headers.ContainsKey(OriginHeader) && + httpContext.Request.Headers.TryGetValue(AccessControlRequestMethod, out var accessControlRequestMethod) && + !StringValues.IsNullOrEmpty(accessControlRequestMethod)) + { + needs405Endpoint = false; // We don't return a 405 for a CORS preflight request when the endpoints accept CORS preflight. + httpMethod = accessControlRequestMethod; + } + + var matched = false; + for (var j = 0; j < metadata.HttpMethods.Count; j++) + { + var candidateMethod = metadata.HttpMethods[j]; + if (!string.Equals(httpMethod, candidateMethod, StringComparison.OrdinalIgnoreCase)) + { + methods = methods ?? new HashSet(StringComparer.OrdinalIgnoreCase); + methods.Add(candidateMethod); + continue; + } + + matched = true; + needs405Endpoint = false; + break; + } + + if (!matched) + { + candidates.SetValidity(i, false); + } + } + + if (needs405Endpoint == true) + { + // We saw some endpoints coming in, and we eliminated them all. + context.Endpoint = CreateRejectionEndpoint(methods.OrderBy(m => m, StringComparer.OrdinalIgnoreCase)); + } + + return Task.CompletedTask; + } + /// /// For framework use only. /// diff --git a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIEndpointSelectorPolicyIntegrationTest.cs b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIEndpointSelectorPolicyIntegrationTest.cs new file mode 100644 index 0000000000..b9d6f4c6bb --- /dev/null +++ b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIEndpointSelectorPolicyIntegrationTest.cs @@ -0,0 +1,11 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Routing.Matching +{ + // End-to-end tests for the HTTP method matching functionality + public class HttpMethodMatcherPolicyIEndpointSelectorPolicyIntegrationTestBase : HttpMethodMatcherPolicyIntegrationTestBase + { + protected override bool HasDynamicMetadata => true; + } +} diff --git a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyINodeBuilderPolicyIntegrationTest.cs b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyINodeBuilderPolicyIntegrationTest.cs new file mode 100644 index 0000000000..4e18493c11 --- /dev/null +++ b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyINodeBuilderPolicyIntegrationTest.cs @@ -0,0 +1,11 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Routing.Matching +{ + // End-to-end tests for the HTTP method matching functionality + public class HttpMethodMatcherPolicyINodeBuilderPolicyIntegrationTestBase : HttpMethodMatcherPolicyIntegrationTestBase + { + protected override bool HasDynamicMetadata => false; + } +} diff --git a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTest.cs b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTestBase.cs similarity index 96% rename from src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTest.cs rename to src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTestBase.cs index 1bdbcd0e2f..258e9b0a92 100644 --- a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTestBase.cs @@ -14,8 +14,10 @@ using static Microsoft.AspNetCore.Routing.Matching.HttpMethodMatcherPolicy; namespace Microsoft.AspNetCore.Routing.Matching { // End-to-end tests for the HTTP method matching functionality - public class HttpMethodMatcherPolicyIntegrationTest + public abstract class HttpMethodMatcherPolicyIntegrationTestBase { + protected abstract bool HasDynamicMetadata { get; } + [Fact] public async Task Match_HttpMethod() { @@ -203,7 +205,7 @@ namespace Microsoft.AspNetCore.Routing.Matching Assert.Equal("DELETE, GET, PUT", httpContext.Response.Headers["Allow"]); } - [Fact] // When all of the candidates handles specific verbs, use a 405 endpoint + [Fact] public async Task NotMatch_HttpMethod_CORS_DoesNotReturn405() { // Arrange @@ -357,7 +359,7 @@ namespace Microsoft.AspNetCore.Routing.Matching return (httpContext, context); } - internal static RouteEndpoint CreateEndpoint( + internal RouteEndpoint CreateEndpoint( string template, object defaults = null, object constraints = null, @@ -371,6 +373,11 @@ namespace Microsoft.AspNetCore.Routing.Matching metadata.Add(new HttpMethodMetadata(httpMethods ?? Array.Empty(), acceptCorsPreflight)); } + if (HasDynamicMetadata) + { + metadata.Add(new DynamicEndpointMetadata()); + } + var displayName = "endpoint: " + template + " " + string.Join(", ", httpMethods ?? new[] { "(any)" }); return new RouteEndpoint( TestConstants.EmptyRequestDelegate, @@ -385,5 +392,10 @@ namespace Microsoft.AspNetCore.Routing.Matching var endpoint = CreateEndpoint(template); return (CreateMatcher(endpoint), endpoint); } + + private class DynamicEndpointMetadata : IDynamicEndpointMetadata + { + public bool IsDynamic => true; + } } } diff --git a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyTest.cs b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyTest.cs index 37353903d0..c3742f1840 100644 --- a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -14,12 +14,12 @@ namespace Microsoft.AspNetCore.Routing.Matching public class HttpMethodMatcherPolicyTest { [Fact] - public void AppliesToNode_EndpointWithoutMetadata_ReturnsFalse() + public void INodeBuilderPolicy_AppliesToNode_EndpointWithoutMetadata_ReturnsFalse() { // Arrange var endpoints = new[] { CreateEndpoint("/", null), }; - var policy = CreatePolicy(); + var policy = (INodeBuilderPolicy)CreatePolicy(); // Act var result = policy.AppliesToEndpoints(endpoints); @@ -29,7 +29,7 @@ namespace Microsoft.AspNetCore.Routing.Matching } [Fact] - public void AppliesToNode_EndpointWithoutHttpMethods_ReturnsFalse() + public void INodeBuilderPolicy_AppliesToNode_EndpointWithoutHttpMethods_ReturnsFalse() { // Arrange var endpoints = new[] @@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Routing.Matching CreateEndpoint("/", new HttpMethodMetadata(Array.Empty())), }; - var policy = CreatePolicy(); + var policy = (INodeBuilderPolicy)CreatePolicy(); // Act var result = policy.AppliesToEndpoints(endpoints); @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Routing.Matching } [Fact] - public void AppliesToNode_EndpointHasHttpMethods_ReturnsTrue() + public void INodeBuilderPolicy_AppliesToNode_EndpointHasHttpMethods_ReturnsTrue() { // Arrange var endpoints = new[] @@ -56,7 +56,7 @@ namespace Microsoft.AspNetCore.Routing.Matching CreateEndpoint("/", new HttpMethodMetadata(new[] { "GET", })), }; - var policy = CreatePolicy(); + var policy = (INodeBuilderPolicy)CreatePolicy(); // Act var result = policy.AppliesToEndpoints(endpoints); @@ -65,6 +65,96 @@ namespace Microsoft.AspNetCore.Routing.Matching Assert.True(result); } + [Fact] + public void INodeBuilderPolicy_AppliesToNode_EndpointIsDynamic_ReturnsFalse() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new HttpMethodMetadata(Array.Empty())), + CreateEndpoint("/", new HttpMethodMetadata(new[] { "GET", }), new DynamicEndpointMetadata()), + }; + + var policy = (INodeBuilderPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.False(result); + } + + [Fact] + public void IEndpointSelectorPolicy_AppliesToNode_EndpointWithoutMetadata_ReturnsTrue() + { + // Arrange + var endpoints = new[] { CreateEndpoint("/", null, new DynamicEndpointMetadata()), }; + + var policy = (IEndpointSelectorPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.True(result); + } + + [Fact] + public void IEndpointSelectorPolicy_AppliesToNode_EndpointWithoutHttpMethods_ReturnsTrue() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new HttpMethodMetadata(Array.Empty()), new DynamicEndpointMetadata()), + }; + + var policy = (IEndpointSelectorPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.True(result); + } + + [Fact] + public void IEndpointSelectorPolicy_AppliesToNode_EndpointHasHttpMethods_ReturnsTrue() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new HttpMethodMetadata(Array.Empty()), new DynamicEndpointMetadata()), + CreateEndpoint("/", new HttpMethodMetadata(new[] { "GET", })), + }; + + var policy = (IEndpointSelectorPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.True(result); + } + + [Fact] + public void IEndpointSelectorPolicy_AppliesToNode_EndpointIsNotDynamic_ReturnsFalse() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new HttpMethodMetadata(Array.Empty())), + CreateEndpoint("/", new HttpMethodMetadata(new[] { "GET", })), + }; + + var policy = (IEndpointSelectorPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.False(result); + } + [Fact] public void GetEdges_GroupsByHttpMethod() { @@ -277,7 +367,7 @@ namespace Microsoft.AspNetCore.Routing.Matching }); } - private static RouteEndpoint CreateEndpoint(string template, HttpMethodMetadata httpMethodMetadata) + private static RouteEndpoint CreateEndpoint(string template, HttpMethodMetadata httpMethodMetadata, params object[] more) { var metadata = new List(); if (httpMethodMetadata != null) @@ -285,6 +375,11 @@ namespace Microsoft.AspNetCore.Routing.Matching metadata.Add(httpMethodMetadata); } + if (more != null) + { + metadata.AddRange(more); + } + return new RouteEndpoint( TestConstants.EmptyRequestDelegate, RoutePatternFactory.Parse(template), @@ -297,5 +392,10 @@ namespace Microsoft.AspNetCore.Routing.Matching { return new HttpMethodMatcherPolicy(); } + + private class DynamicEndpointMetadata : IDynamicEndpointMetadata + { + public bool IsDynamic => true; + } } } diff --git a/src/Http/Routing/test/UnitTests/Matching/MatcherAssert.cs b/src/Http/Routing/test/UnitTests/Matching/MatcherAssert.cs index 6053946310..28ec4c68a4 100644 --- a/src/Http/Routing/test/UnitTests/Matching/MatcherAssert.cs +++ b/src/Http/Routing/test/UnitTests/Matching/MatcherAssert.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -93,7 +93,7 @@ namespace Microsoft.AspNetCore.Routing.Matching private static string FormatRouteValues(RouteValueDictionary values) { - return "{" + string.Join(", ", values.Select(kvp => $"{kvp.Key} = '{kvp.Value}'")) + "}"; + return values == null ? "{}" : "{" + string.Join(", ", values.Select(kvp => $"{kvp.Key} = '{kvp.Value}'")) + "}"; } } -} \ No newline at end of file +}