Change consumes behavior to ignore requests with no content type (#6645)

Author: JamesNK

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;

src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs

@@ -77,7 +77,10 @@ public void OnResourceExecuting(ResourceExecutingContext context)
 
                 // 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();
                 }

src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs

@@ -10,6 +10,7 @@
 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 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
 
             // 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<Endpoint>()
                 {
                     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 @@ public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJ
             // 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 @@ public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJ
                 }
             }
 
-            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 @@ protected override int CompareMetadata(IConsumesMetadata x, IConsumesMetadata y)
 
         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);

src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ConsumesAttributeTests.cs

@@ -326,7 +326,7 @@ public void OnResourceExecuting_ForNoContentTypeMatch_SetsUnsupportedMediaTypeRe
         [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 @@ public void OnResourceExecuting_NullOrEmptyRequestContentType_SetsUnsupportedMed
             consumesFilter.OnResourceExecuting(resourceExecutingContext);
 
             // Assert
-            Assert.NotNull(resourceExecutingContext.Result);
-            Assert.IsType<UnsupportedMediaTypeResult>(resourceExecutingContext.Result);
+            Assert.Null(resourceExecutingContext.Result);
         }
 
         [Theory]

src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs

@@ -91,6 +91,11 @@ public void GetEdges_GroupsByContentType()
             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 @@ public void GetEdges_GroupsByContentType()
         }
 
         [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 @@ public void GetEdges_GroupsByContentType_CreatesHttp405Endpoint()
             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 @@ public void BuildJumpTable_SortsEdgesByPriority(string contentType, int expected
             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),

src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs

@@ -37,7 +37,7 @@ public async Task NoRequestContentType_SelectsActionWithoutConstraint()
             // 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 @@ public async Task NoRequestContentType_SelectsActionWithoutConstraint()
         }
 
         [Fact]
-        public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent_ReturnsUnsupported()
+        public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent()
         {
             // Arrange
             var request = new HttpRequestMessage(
@@ -58,7 +58,26 @@ public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPr
 
             // 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]

src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_PassThroughController.cs

@@ -14,5 +14,17 @@ public IActionResult CreateProduct(Product_Json jsonInput)
         {
             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");
+        }
     }
 }

src/Mvc/test/WebSites/BasicWebSite/Controllers/ActionConstraints/ConsumesAttribute_WithFallbackActionController.cs

@@ -6,7 +6,7 @@
 
 namespace BasicWebSite.Controllers.ActionConstraints
 {
-    [Route("ConsumesAttribute_Company/[action]")]
+    [Route("ConsumesAttribute_WithFallbackActionController/[action]")]
     public class ConsumesAttribute_WithFallbackActionController : Controller
     {
         [Consumes("application/json")]

src/Mvc/test/WebSites/BasicWebSite/StartupWithEndpointRouting.cs

@@ -29,6 +29,8 @@ public void ConfigureServices(IServiceCollection services)
 
         public void Configure(IApplicationBuilder app)
         {
+            app.UseDeveloperExceptionPage();
+
             // Initializes the RequestId service for each request
             app.UseMiddleware<RequestIdMiddleware>();