diff --git a/src/Mvc/Mvc.Core/src/Routing/ConsumesMatcherPolicy.cs b/src/Mvc/Mvc.Core/src/Routing/ConsumesMatcherPolicy.cs index fcd959225d..ca1ce032a9 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ConsumesMatcherPolicy.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ConsumesMatcherPolicy.cs @@ -9,12 +9,10 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Matching; -using Microsoft.AspNetCore.Routing.Patterns; -using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Mvc.Routing { - internal class ConsumesMatcherPolicy : MatcherPolicy, IEndpointComparerPolicy, INodeBuilderPolicy + internal class ConsumesMatcherPolicy : MatcherPolicy, IEndpointComparerPolicy, INodeBuilderPolicy, IEndpointSelectorPolicy { internal const string Http415EndpointDisplayName = "415 HTTP Unsupported Media Type"; internal const string AnyContentType = "*/*"; @@ -24,16 +22,140 @@ namespace Microsoft.AspNetCore.Mvc.Routing public IComparer Comparer { get; } = new ConsumesMetadataEndpointComparer(); - 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) + { return endpoints.Any(e => e.Metadata.GetMetadata()?.ContentTypes.Count > 0); } + 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)); + } + + // We want to return a 415 iff we eliminated ALL of the currently valid endpoints due to content type + // mismatch. + bool? needs415Endpoint = null; + + for (var i = 0; i < candidates.Count; i++) + { + // We do this check first for consistency with how 415 is implemented for the graph version + // of this code. We still want to know if any endpoints in this set require an a ContentType + // even if those endpoints are already invalid. + var metadata = candidates[i].Endpoint.Metadata.GetMetadata(); + if (metadata == null || metadata.ContentTypes.Count == 0) + { + // Can match any content type. + needs415Endpoint = false; + continue; + } + + // Saw a valid endpoint. + needs415Endpoint = needs415Endpoint ?? true; + + if (!candidates.IsValidCandidate(i)) + { + // If the candidate is already invalid, then do a search to see if it has a wildcard content type. + // + // We don't want to return a 415 if any content type could be accepted depending on other parameters. + if (metadata != null) + { + for (var j = 0; j < metadata.ContentTypes.Count; j++) + { + if (string.Equals("*/*", metadata.ContentTypes[j], StringComparison.Ordinal)) + { + needs415Endpoint = false; + break; + } + } + } + + continue; + } + + var contentType = httpContext.Request.ContentType; + var mediaType = string.IsNullOrEmpty(contentType) ? (MediaType?)null : new MediaType(contentType); + + var matched = false; + for (var j = 0; j < metadata.ContentTypes.Count; j++) + { + var candidateMediaType = new MediaType(metadata.ContentTypes[j]); + if (candidateMediaType.MatchesAllTypes) + { + // We don't need a 415 response because there's an endpoint that would accept any type. + needs415Endpoint = false; + } + + // If there's no ContentType, then then can only matched by a wildcard `*/*`. + if (mediaType == null && !candidateMediaType.MatchesAllTypes) + { + continue; + } + + // We have a ContentType but it's not a match. + else if (mediaType != null && !mediaType.Value.IsSubsetOf(candidateMediaType)) + { + continue; + } + + // We have a ContentType and we accept any value OR we have a ContentType and it's a match. + matched = true; + needs415Endpoint = false; + break; + } + + if (!matched) + { + candidates.SetValidity(i, false); + } + } + + if (needs415Endpoint == true) + { + // We saw some endpoints coming in, and we eliminated them all. + context.Endpoint = CreateRejectionEndpoint(); + } + + return Task.CompletedTask; + } + public IReadOnlyList GetEdges(IReadOnlyList endpoints) { if (endpoints == null) diff --git a/src/Mvc/Mvc.Core/test/Routing/ConsumesMatcherPolicyTest.cs b/src/Mvc/Mvc.Core/test/Routing/ConsumesMatcherPolicyTest.cs index 6a3876b1ae..7d63eaacc4 100644 --- a/src/Mvc/Mvc.Core/test/Routing/ConsumesMatcherPolicyTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/ConsumesMatcherPolicyTest.cs @@ -13,15 +13,17 @@ using Xunit; namespace Microsoft.AspNetCore.Mvc.Routing { + // There are some unit tests here for the IEndpointSelectorPolicy implementation. + // The INodeBuilderPolicy implementation is well-tested by functional tests. public class ConsumesMatcherPolicyTest { [Fact] - public void AppliesToEndpoints_EndpointWithoutMetadata_ReturnsFalse() + public void INodeBuilderPolicy_AppliesToEndpoints_EndpointWithoutMetadata_ReturnsFalse() { // Arrange var endpoints = new[] { CreateEndpoint("/", null), }; - var policy = CreatePolicy(); + var policy = (INodeBuilderPolicy)CreatePolicy(); // Act var result = policy.AppliesToEndpoints(endpoints); @@ -31,7 +33,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing } [Fact] - public void AppliesToEndpoints_EndpointWithoutContentTypes_ReturnsFalse() + public void INodeBuilderPolicy_AppliesToEndpoints_EndpointWithoutContentTypes_ReturnsFalse() { // Arrange var endpoints = new[] @@ -39,7 +41,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing CreateEndpoint("/", new ConsumesMetadata(Array.Empty())), }; - var policy = CreatePolicy(); + var policy = (INodeBuilderPolicy)CreatePolicy(); // Act var result = policy.AppliesToEndpoints(endpoints); @@ -49,7 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing } [Fact] - public void AppliesToEndpoints_EndpointHasContentTypes_ReturnsTrue() + public void INodeBuilderPolicy_AppliesToEndpoints_EndpointHasContentTypes_ReturnsTrue() { // Arrange var endpoints = new[] @@ -58,7 +60,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing CreateEndpoint("/", new ConsumesMetadata(new[] { "application/json", })), }; - var policy = CreatePolicy(); + var policy = (INodeBuilderPolicy)CreatePolicy(); // Act var result = policy.AppliesToEndpoints(endpoints); @@ -67,6 +69,96 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.True(result); } + [Fact] + public void INodeBuilderPolicy_AppliesToEndpoints_WithDynamicMetadata_ReturnsFalse() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty()), new DynamicEndpointMetadata()), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/json", })), + }; + + var policy = (INodeBuilderPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.False(result); + } + + [Fact] + public void IEndpointSelectorPolicy_AppliesToEndpoints_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_AppliesToEndpoints_EndpointWithoutContentTypes_ReturnsTrue() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty()), new DynamicEndpointMetadata()), + }; + + var policy = (IEndpointSelectorPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.True(result); + } + + [Fact] + public void IEndpointSelectorPolicy_AppliesToEndpoints_EndpointHasContentTypes_ReturnsTrue() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty()), new DynamicEndpointMetadata()), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/json", })), + }; + + var policy = (IEndpointSelectorPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.True(result); + } + + [Fact] + public void IEndpointSelectorPolicy_AppliesToEndpoints_WithoutDynamicMetadata_ReturnsFalse() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty())), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/json", })), + }; + + var policy = (IEndpointSelectorPolicy)CreatePolicy(); + + // Act + var result = policy.AppliesToEndpoints(endpoints); + + // Assert + Assert.False(result); + } + [Fact] public void GetEdges_GroupsByContentType() { @@ -224,7 +316,303 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Equal(expected, actual); } - private static RouteEndpoint CreateEndpoint(string template, ConsumesMetadata consumesMetadata) + [Fact] + public async Task ApplyAsync_EndpointWithoutMetadata_MatchWithoutContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", null), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext(); + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointAllowsAnyContentType_MatchWithoutContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty())), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext(); + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointHasWildcardContentType_MatchWithoutContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(new string[] { "*/*" })), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext(); + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointWithoutMetadata_MatchWithAnyContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", null), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "text/plain", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointAllowsAnyContentType_MatchWithAnyContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty())), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "text/plain", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointHasWildcardContentType_MatchWithAnyContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(new string[] { "*/*" })), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "text/plain", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointHasSubTypeWildcard_MatchWithValidContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(new string[] { "application/*+json", })), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "application/project+json", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointHasMultipleContentType_MatchWithValidContentType() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(new string[] { "text/xml", "application/xml", })), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "application/xml", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_EndpointDoesNotMatch_Returns415() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(new string[] { "text/xml", "application/xml", })), + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "application/json", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.False(candidates.IsValidCandidate(0)); + Assert.NotNull(context.Endpoint); + } + + [Fact] + public async Task ApplyAsync_EndpointDoesNotMatch_DoesNotReturns415WithContentTypeObliviousEndpoint() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(new string[] { "text/xml", "application/xml", })), + CreateEndpoint("/", null) + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "application/json", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.False(candidates.IsValidCandidate(0)); + Assert.Null(context.Endpoint); + } + + [Fact] + public async Task ApplyAsync_EndpointDoesNotMatch_DoesNotReturns415WithContentTypeWildcardEndpoint() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(new string[] { "text/xml", "application/xml", })), + CreateEndpoint("/", new ConsumesMetadata(new string[] { "*/*", })) + }; + + var candidates = CreateCandidateSet(endpoints); + var context = new EndpointSelectorContext(); + var httpContext = new DefaultHttpContext() + { + Request = + { + ContentType = "application/json", + }, + }; + + var policy = CreatePolicy(); + + // Act + await policy.ApplyAsync(httpContext, context, candidates); + + // Assert + Assert.False(candidates.IsValidCandidate(0)); + Assert.True(candidates.IsValidCandidate(1)); + Assert.Null(context.Endpoint); + } + + private static RouteEndpoint CreateEndpoint(string template, ConsumesMetadata consumesMetadata, params object[] more) { var metadata = new List(); if (consumesMetadata != null) @@ -232,6 +620,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing metadata.Add(consumesMetadata); } + if (more != null) + { + metadata.AddRange(more); + } + return new RouteEndpoint( (context) => Task.CompletedTask, RoutePatternFactory.Parse(template), @@ -240,9 +633,19 @@ namespace Microsoft.AspNetCore.Mvc.Routing $"test: {template} - {string.Join(", ", consumesMetadata?.ContentTypes ?? Array.Empty())}"); } + private static CandidateSet CreateCandidateSet(Endpoint[] endpoints) + { + return new CandidateSet(endpoints, new RouteValueDictionary[endpoints.Length], new int[endpoints.Length]); + } + private static ConsumesMatcherPolicy CreatePolicy() { return new ConsumesMatcherPolicy(); } + + private class DynamicEndpointMetadata : IDynamicEndpointMetadata + { + public bool IsDynamic => true; + } } }