From a48e75dfb475a2fab28e555df531c83dda31cc2c Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 2 Oct 2018 10:24:38 -0700 Subject: [PATCH 1/2] Implicitly set content type for ObjectResults containing ProblemDetails (#8512) * Implicitly set content type for ObjectResults containing ProblemDetails Fixes #8467 --- .../Infrastructure/ObjectResultExecutor.cs | 43 +++++---- .../ObjectResult.cs | 10 +- .../ObjectResultExecutorTest.cs | 96 +++++++++++++++++++ 3 files changed, 130 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ObjectResultExecutor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ObjectResultExecutor.cs index 5d5fd81b23..d48d213532 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ObjectResultExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ObjectResultExecutor.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Text; using System.Threading.Tasks; @@ -84,21 +85,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure throw new ArgumentNullException(nameof(result)); } - // If the user sets the content type both on the ObjectResult (example: by Produces) and Response object, - // then the one set on ObjectResult takes precedence over the Response object - if (result.ContentTypes == null || result.ContentTypes.Count == 0) - { - var responseContentType = context.HttpContext.Response.ContentType; - if (!string.IsNullOrEmpty(responseContentType)) - { - if (result.ContentTypes == null) - { - result.ContentTypes = new MediaTypeCollection(); - } - - result.ContentTypes.Add(responseContentType); - } - } + InferContentTypes(context, result); var objectType = result.DeclaredType; if (objectType == null || objectType == typeof(object)) @@ -113,8 +100,8 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure result.Value); var selectedFormatter = FormatterSelector.SelectFormatter( - formatterContext, - (IList)result.Formatters ?? Array.Empty(), + formatterContext, + (IList)result.Formatters ?? Array.Empty(), result.ContentTypes); if (selectedFormatter == null) { @@ -130,5 +117,27 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure result.OnFormatting(context); return selectedFormatter.WriteAsync(formatterContext); } + + private static void InferContentTypes(ActionContext context, ObjectResult result) + { + Debug.Assert(result.ContentTypes != null); + if (result.ContentTypes.Count != 0) + { + return; + } + + // If the user sets the content type both on the ObjectResult (example: by Produces) and Response object, + // then the one set on ObjectResult takes precedence over the Response object + var responseContentType = context.HttpContext.Response.ContentType; + if (!string.IsNullOrEmpty(responseContentType)) + { + result.ContentTypes.Add(responseContentType); + } + else if (result.Value is ProblemDetails) + { + result.ContentTypes.Add("application/problem+json"); + result.ContentTypes.Add("application/problem+xml"); + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs index bb7fc0522d..a5a0323089 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs @@ -11,11 +11,13 @@ namespace Microsoft.AspNetCore.Mvc { public class ObjectResult : ActionResult, IStatusCodeActionResult { + private MediaTypeCollection _contentTypes; + public ObjectResult(object value) { Value = value; Formatters = new FormatterCollection(); - ContentTypes = new MediaTypeCollection(); + _contentTypes = new MediaTypeCollection(); } [ActionResultObjectValue] @@ -23,7 +25,11 @@ namespace Microsoft.AspNetCore.Mvc public FormatterCollection Formatters { get; set; } - public MediaTypeCollection ContentTypes { get; set; } + public MediaTypeCollection ContentTypes + { + get => _contentTypes; + set => _contentTypes = value ?? throw new ArgumentNullException(nameof(value)); + } public Type DeclaredType { get; set; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs index 04013644df..207d04161f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs @@ -17,6 +17,32 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { public class ObjectResultExecutorTest { + [Fact] + public async Task ExecuteAsync_UsesSpecifiedContentType() + { + // Arrange + var executor = CreateExecutor(); + + var httpContext = new DefaultHttpContext(); + var actionContext = new ActionContext() { HttpContext = httpContext }; + httpContext.Request.Headers[HeaderNames.Accept] = "application/xml"; // This will not be used + httpContext.Response.ContentType = "text/json"; + + var result = new ObjectResult("input") + { + ContentTypes = { "text/xml", }, + }; + result.Formatters.Add(new TestXmlOutputFormatter()); + result.Formatters.Add(new TestJsonOutputFormatter()); + result.Formatters.Add(new TestStringOutputFormatter()); // This will be chosen based on the content type + + // Act + await executor.ExecuteAsync(actionContext, result); + + // Assert + MediaTypeAssert.Equal("text/xml; charset=utf-8", httpContext.Response.ContentType); + } + // For this test case probably the most common use case is when there is a format mapping based // content type selected but the developer had set the content type on the Response.ContentType [Fact] @@ -85,6 +111,74 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure Assert.Equal(406, httpContext.Response.StatusCode); } + [Fact] + public async Task ExecuteAsync_ForProblemDetailsValue_UsesSpecifiedContentType() + { + // Arrange + var executor = CreateExecutor(); + + var httpContext = new DefaultHttpContext(); + var actionContext = new ActionContext() { HttpContext = httpContext }; + httpContext.Response.ContentType = "application/json"; + + var result = new ObjectResult(new ProblemDetails()) + { + ContentTypes = { "text/plain" }, + }; + result.Formatters.Add(new TestXmlOutputFormatter()); + result.Formatters.Add(new TestJsonOutputFormatter()); + result.Formatters.Add(new TestStringOutputFormatter()); // This will be chosen based on the content type + + // Act + await executor.ExecuteAsync(actionContext, result); + + // Assert + MediaTypeAssert.Equal("text/plain; charset=utf-8", httpContext.Response.ContentType); + } + + [Fact] + public async Task ExecuteAsync_ForProblemDetailsValue_UsesResponseContentType() + { + // Arrange + var executor = CreateExecutor(); + + var httpContext = new DefaultHttpContext(); + var actionContext = new ActionContext() { HttpContext = httpContext }; + httpContext.Response.ContentType = "application/json"; + + var result = new ObjectResult(new ProblemDetails()); + result.Formatters.Add(new TestXmlOutputFormatter()); + result.Formatters.Add(new TestJsonOutputFormatter()); // This will be chosen based on the response content type + result.Formatters.Add(new TestStringOutputFormatter()); + + // Act + await executor.ExecuteAsync(actionContext, result); + + // Assert + MediaTypeAssert.Equal("application/json; charset=utf-8", httpContext.Response.ContentType); + } + + [Fact] + public async Task ExecuteAsync_NoContentTypeProvidedForProblemDetails_UsesDefaultContentTypes() + { + // Arrange + var executor = CreateExecutor(); + + var httpContext = new DefaultHttpContext(); + var actionContext = new ActionContext() { HttpContext = httpContext }; + + var result = new ObjectResult(new ProblemDetails()); + result.Formatters.Add(new TestXmlOutputFormatter()); // This will be chosen based on the implicitly added content type + result.Formatters.Add(new TestJsonOutputFormatter()); + result.Formatters.Add(new TestStringOutputFormatter()); + + // Act + await executor.ExecuteAsync(actionContext, result); + + // Assert + MediaTypeAssert.Equal("application/problem+xml; charset=utf-8", httpContext.Response.ContentType); + } + [Fact] public async Task ExecuteAsync_NoFormatterFound_Returns406() { @@ -310,6 +404,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/json")); SupportedMediaTypes.Add(new MediaTypeHeaderValue("text/json")); + SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/*+json")); SupportedEncodings.Add(Encoding.UTF8); } @@ -326,6 +421,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/xml")); SupportedMediaTypes.Add(new MediaTypeHeaderValue("text/xml")); + SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/*+xml")); SupportedEncodings.Add(Encoding.UTF8); } From 70ddf15cbc988a39048e696595a2e86e7c6c2d80 Mon Sep 17 00:00:00 2001 From: Alexej Timonin Date: Tue, 2 Oct 2018 20:49:46 +0200 Subject: [PATCH 2/2] MethodMatches :shower: --- .../ApiExplorer/ApiConventionMatcher.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApiExplorer/ApiConventionMatcher.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApiExplorer/ApiConventionMatcher.cs index b9e6f593c3..e080ce4ef4 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApiExplorer/ApiConventionMatcher.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApiExplorer/ApiConventionMatcher.cs @@ -15,12 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer bool MethodMatches() { var methodNameMatchBehavior = GetNameMatchBehavior(conventionMethod); - if (!IsNameMatch(methodInfo.Name, conventionMethod.Name, methodNameMatchBehavior)) - { - return false; - } - - return true; + return IsNameMatch(methodInfo.Name, conventionMethod.Name, methodNameMatchBehavior); } bool ParametersMatch()