diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index d9ee2e8f4e..cac2b904f0 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -32,6 +32,7 @@ Later on, this will be checked using this condition: Microsoft.AspNetCore.Authentication.Google; Microsoft.AspNetCore.Http; Microsoft.AspNetCore.Mvc.Core; + Microsoft.AspNetCore.Routing; Microsoft.AspNetCore.Server.IIS; Microsoft.AspNetCore.Server.Kestrel.Core; java:signalr; diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs index 30a103a531..ed4893046e 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs @@ -77,7 +77,10 @@ namespace Microsoft.AspNetCore.Mvc // Confirm the request's content type is more specific than a media type this action supports e.g. OK // if client sent "text/plain" data and this action supports "text/*". - if (requestContentType == null || !IsSubsetOfAnyContentType(requestContentType)) + // + // Requests without a content type do not return a 415. It is a common pattern to place [Consumes] on + // a controller and have GET actions + if (requestContentType != null && !IsSubsetOfAnyContentType(requestContentType)) { context.Result = new UnsupportedMediaTypeResult(); } diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs index c0f0dd35bd..fcd959225d 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs @@ -10,6 +10,7 @@ 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 { @@ -120,13 +121,23 @@ namespace Microsoft.AspNetCore.Mvc.Routing // If after we're done there isn't any endpoint that accepts */*, then we'll synthesize an // endpoint that always returns a 415. - if (!edges.ContainsKey(AnyContentType)) + if (!edges.TryGetValue(AnyContentType, out var anyEndpoints)) { edges.Add(AnyContentType, new List() { CreateRejectionEndpoint(), }); + + // Add a node to use when there is no request content type. + // When there is no content type we want the policy to no-op + edges.Add(string.Empty, endpoints.ToList()); } + else + { + // If there is an endpoint that accepts */* then it is also used when there is no content type + edges.Add(string.Empty, anyEndpoints.ToList()); + } + return edges .Select(kvp => new PolicyNodeEdge(kvp.Key, kvp.Value)) @@ -155,7 +166,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing // Since our 'edges' can have wildcards, we do a sort based on how wildcard-ey they // are then then execute them in linear order. var ordered = edges - .Select(e => (mediaType: new MediaType((string)e.State), destination: e.Destination)) + .Select(e => (mediaType: CreateEdgeMediaType(ref e), destination: e.Destination)) .OrderBy(e => GetScore(e.mediaType)) .ToArray(); @@ -170,7 +181,28 @@ namespace Microsoft.AspNetCore.Mvc.Routing } } - return new ConsumesPolicyJumpTable(exitDestination, ordered); + var noContentTypeDestination = GetNoContentTypeDestination(ordered); + + return new ConsumesPolicyJumpTable(exitDestination, noContentTypeDestination, ordered); + } + + private static int GetNoContentTypeDestination((MediaType mediaType, int destination)[] destinations) + { + for (var i = 0; i < destinations.Length; i++) + { + if (!destinations[i].mediaType.Type.HasValue) + { + return destinations[i].destination; + } + } + + throw new InvalidOperationException("Could not find destination for no content type."); + } + + private static MediaType CreateEdgeMediaType(ref PolicyJumpTableEdge e) + { + var mediaType = (string)e.State; + return !string.IsNullOrEmpty(mediaType) ? new MediaType(mediaType) : default; } private int GetScore(in MediaType mediaType) @@ -207,21 +239,24 @@ namespace Microsoft.AspNetCore.Mvc.Routing private class ConsumesPolicyJumpTable : PolicyJumpTable { - private (MediaType mediaType, int destination)[] _destinations; - private int _exitDestination; + private readonly (MediaType mediaType, int destination)[] _destinations; + private readonly int _exitDestination; + private readonly int _noContentTypeDestination; - public ConsumesPolicyJumpTable(int exitDestination, (MediaType mediaType, int destination)[] destinations) + public ConsumesPolicyJumpTable(int exitDestination, int noContentTypeDestination, (MediaType mediaType, int destination)[] destinations) { _exitDestination = exitDestination; + _noContentTypeDestination = noContentTypeDestination; _destinations = destinations; } public override int GetDestination(HttpContext httpContext) { var contentType = httpContext.Request.ContentType; + if (string.IsNullOrEmpty(contentType)) { - return _exitDestination; + return _noContentTypeDestination; } var requestMediaType = new MediaType(contentType); diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ConsumesAttributeTests.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ConsumesAttributeTests.cs index 1ecf35fb8e..5ee4e4178a 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ConsumesAttributeTests.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ConsumesAttributeTests.cs @@ -326,7 +326,7 @@ namespace Microsoft.AspNetCore.Mvc [Theory] [InlineData("")] [InlineData(null)] - public void OnResourceExecuting_NullOrEmptyRequestContentType_SetsUnsupportedMediaTypeResult(string contentType) + public void OnResourceExecuting_NullOrEmptyRequestContentType_IsNoOp(string contentType) { // Arrange var httpContext = new DefaultHttpContext(); @@ -349,8 +349,7 @@ namespace Microsoft.AspNetCore.Mvc consumesFilter.OnResourceExecuting(resourceExecutingContext); // Assert - Assert.NotNull(resourceExecutingContext.Result); - Assert.IsType(resourceExecutingContext.Result); + Assert.Null(resourceExecutingContext.Result); } [Theory] diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs index b0a59ce678..6a3876b1ae 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs @@ -91,6 +91,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Collection( edges.OrderBy(e => e.State), e => + { + Assert.Equal(string.Empty, e.State); + Assert.Equal(new[] { endpoints[1], endpoints[4], }, e.Endpoints.ToArray()); + }, + e => { Assert.Equal("*/*", e.State); Assert.Equal(new[] { endpoints[1], endpoints[4], }, e.Endpoints.ToArray()); @@ -123,7 +128,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing } [Fact] // See explanation in GetEdges for how this case is different - public void GetEdges_GroupsByContentType_CreatesHttp405Endpoint() + public void GetEdges_GroupsByContentType_CreatesHttp415Endpoint() { // Arrange var endpoints = new[] @@ -144,6 +149,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Collection( edges.OrderBy(e => e.State), e => + { + Assert.Equal(string.Empty, e.State); + Assert.Equal(new[] { endpoints[0], endpoints[1], endpoints[2], }, e.Endpoints.ToArray()); + }, + e => { Assert.Equal("*/*", e.State); Assert.Equal(ConsumesMatcherPolicy.Http415EndpointDisplayName, Assert.Single(e.Endpoints).DisplayName); @@ -190,6 +200,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing var edges = new PolicyJumpTableEdge[] { // In reverse order of how they should be processed + new PolicyJumpTableEdge(string.Empty, 0), new PolicyJumpTableEdge("*/*", 1), new PolicyJumpTableEdge("application/*", 2), new PolicyJumpTableEdge("text/*", 3), diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs index 9a978d2662..9836ff8dff 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs @@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange var request = new HttpRequestMessage( HttpMethod.Post, - "http://localhost/ConsumesAttribute_Company/CreateProduct"); + "http://localhost/ConsumesAttribute_WithFallbackActionController/CreateProduct"); // Act var response = await Client.SendAsync(request); @@ -49,7 +49,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent_ReturnsUnsupported() + public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent() { // Arrange var request = new HttpRequestMessage( @@ -58,7 +58,26 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Act var response = await Client.SendAsync(request); - await response.AssertStatusCodeAsync(HttpStatusCode.UnsupportedMediaType); + var body = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("ConsumesAttribute_PassThrough_Product_Json", body); + } + + [Fact] + public async Task NoRequestContentType_MultipleMatches_IfAMultipleActionWithConstraintIsPresent() + { + // Arrange + var request = new HttpRequestMessage( + HttpMethod.Post, + "http://localhost/ConsumesAttribute_PassThrough/CreateProductMultiple"); + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); } [Theory] diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_PassThroughController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_PassThroughController.cs index ce405047c8..a689368d0a 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_PassThroughController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_PassThroughController.cs @@ -14,5 +14,17 @@ namespace BasicWebSite.Controllers.ActionConstraints { return Content("ConsumesAttribute_PassThrough_Product_Json"); } + + [Consumes("application/json")] + public IActionResult CreateProductMultiple(Product_Json jsonInput) + { + return Content("ConsumesAttribute_PassThrough_Product_Json"); + } + + [Consumes("application/xml")] + public IActionResult CreateProductMultiple(Product_Xml jsonInput) + { + return Content("ConsumesAttribute_PassThrough_Product_Xml"); + } } } \ No newline at end of file diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_WithFallbackActionController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_WithFallbackActionController.cs index eb92870096..c2332699ad 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_WithFallbackActionController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_WithFallbackActionController.cs @@ -6,7 +6,7 @@ using Microsoft.AspNetCore.Mvc; namespace BasicWebSite.Controllers.ActionConstraints { - [Route("ConsumesAttribute_Company/[action]")] + [Route("ConsumesAttribute_WithFallbackActionController/[action]")] public class ConsumesAttribute_WithFallbackActionController : Controller { [Consumes("application/json")] diff --git a/src/Mvc/test/WebSites/BasicWebSite/StartupWithEndpointRouting.cs b/src/Mvc/test/WebSites/BasicWebSite/StartupWithEndpointRouting.cs index 01726c53e5..bf1e8ff394 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/StartupWithEndpointRouting.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/StartupWithEndpointRouting.cs @@ -29,6 +29,8 @@ namespace BasicWebSite public void Configure(IApplicationBuilder app) { + app.UseDeveloperExceptionPage(); + // Initializes the RequestId service for each request app.UseMiddleware();