MVC Controller Response - Wrong ContentType #3245

src/Microsoft.AspNet.Mvc.Core/Infrastructure/ObjectResultExecutor.cs

@@ -48,7 +48,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             Logger = loggerFactory.CreateLogger<ObjectResultExecutor>();
             WriterFactory = writerFactory.CreateWriter;
         }
-        
+
         /// <summary>
         /// Gets the <see cref="ILogger"/>.
         /// </summary>
@@ -89,6 +89,22 @@ namespace Microsoft.AspNet.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 List<MediaTypeHeaderValue>();
+                    }
+
+                    result.ContentTypes.Add(MediaTypeHeaderValue.Parse(responseContentType));
+                }
+            }
+
             ValidateContentTypes(result.ContentTypes);
 
             var formatters = result.Formatters;
@@ -108,7 +124,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
                 WriterFactory,
                 objectType,
                 result.Value);
-            
+
             var selectedFormatter = SelectFormatter(formatterContext, result.ContentTypes, formatters);
             if (selectedFormatter == null)
             {
@@ -121,7 +137,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
 
             Logger.FormatterSelected(selectedFormatter, formatterContext);
             Logger.ObjectResultExecuting(context);
-            
+
             result.OnFormatting(context);
             return selectedFormatter.WriteAsync(formatterContext);
         }
@@ -324,7 +340,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             {
                 throw new ArgumentNullException(nameof(sortedAcceptHeaders));
             }
-            
+
             foreach (var contentType in sortedAcceptHeaders)
             {
                 foreach (var formatter in formatters)
@@ -466,7 +482,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
                     }
                 }
             }
-            
+
             // We want a descending sort, but BinarySearch does ascending
             sorted.Reverse();
             return sorted;

test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs

@@ -51,6 +51,30 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             Assert.Equal(new MediaTypeHeaderValue("application/json"), context.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]
+        public async Task ExecuteAsync_ContentTypeProvidedFromResponseAndObjectResult_UsesResponseContentType()
+        {
+            // Arrange
+            var executor = CreateCustomObjectResultExecutor();
+            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/plain";
+            var result = new ObjectResult("input");
+            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
+            Assert.IsType<TestStringOutputFormatter>(executor.SelectedOutputFormatter);
+            Assert.Equal("text/plain; charset=utf-8", httpContext.Response.ContentType);
+        }
+
         [Fact]
         public void SelectFormatter_WithOneProvidedContentType_IgnoresAcceptHeader()
         {
@@ -82,6 +106,27 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             Assert.Equal(new MediaTypeHeaderValue("application/json"), context.ContentType);
         }
 
+        [Fact]
+        public async Task ExecuteAsync_WithOneProvidedContentType_FromResponseContentType_IgnoresAcceptHeader()
+        {
+            // Arrange
+            var executor = CreateCustomObjectResultExecutor();
+            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 = "application/json";
+            var result = new ObjectResult("input");
+            result.Formatters.Add(new TestXmlOutputFormatter());
+            result.Formatters.Add(new TestJsonOutputFormatter()); // This will be chosen based on the content type
+
+            // Act
+            await executor.ExecuteAsync(actionContext, result);
+
+            // Assert
+            Assert.IsType<TestJsonOutputFormatter>(executor.SelectedOutputFormatter);
+            Assert.Equal("application/json; charset=utf-8", httpContext.Response.ContentType);
+        }
+
         [Fact]
         public void SelectFormatter_WithOneProvidedContentType_NoFallback()
         {
@@ -111,6 +156,25 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             Assert.Null(formatter);
         }
 
+        [Fact]
+        public async Task ExecuteAsync_WithOneProvidedContentType_FromResponseContentType_NoFallback()
+        {
+            // Arrange
+            var executor = CreateCustomObjectResultExecutor();
+            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 = "application/json";
+            var result = new ObjectResult("input");
+            result.Formatters.Add(new TestXmlOutputFormatter());
+
+            // Act
+            await executor.ExecuteAsync(actionContext, result);
+
+            // Assert
+            Assert.Null(executor.SelectedOutputFormatter);
+        }
+
         // ObjectResult.ContentTypes, Accept header, expected content type
         public static TheoryData<string[], string, string> ContentTypes
         {
@@ -158,7 +222,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
         {
             // Arrange
             var executor = CreateExecutor();
-            
+
             var formatters = new List<IOutputFormatter>
             {
                 new CannotWriteFormatter(),
@@ -229,7 +293,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
 
             var context = new OutputFormatterWriteContext(
                 new DefaultHttpContext(),
-                new TestHttpResponseStreamWriterFactory().CreateWriter, 
+                new TestHttpResponseStreamWriterFactory().CreateWriter,
                 objectType: null,
                 @object: null);
 
@@ -315,7 +379,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
 
             var executor = CreateExecutor();
 
-            var actionContext = new ActionContext();
+            var actionContext = new ActionContext() { HttpContext = new DefaultHttpContext() };
 
             // Act & Assert
             var exception = await Assert.ThrowsAsync<InvalidOperationException>(
@@ -331,7 +395,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
         // Chrome & Opera
         [InlineData("text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8", "application/json; charset=utf-8")]
         // IE
-        [InlineData("text/html, application/xhtml+xml, */*", "application/json; charset=utf-8")] 
+        [InlineData("text/html, application/xhtml+xml, */*", "application/json; charset=utf-8")]
         // Firefox & Safari
         [InlineData("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", "application/json; charset=utf-8")]
         // Misc
@@ -428,6 +492,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
                 NullLoggerFactory.Instance);
         }
 
+        private static CustomObjectResultExecutor CreateCustomObjectResultExecutor()
+        {
+            return new CustomObjectResultExecutor(
+                new TestOptionsManager<MvcOptions>(),
+                new TestHttpResponseStreamWriterFactory(),
+                NullLoggerFactory.Instance);
+        }
+
         private class CannotWriteFormatter : IOutputFormatter
         {
             public virtual bool CanWriteResult(OutputFormatterCanWriteContext context)
@@ -473,17 +545,32 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             }
         }
 
+        private class TestStringOutputFormatter : OutputFormatter
+        {
+            public TestStringOutputFormatter()
+            {
+                SupportedMediaTypes.Add(new MediaTypeHeaderValue("text/plain"));
+
+                SupportedEncodings.Add(Encoding.UTF8);
+            }
+
+            public override Task WriteResponseBodyAsync(OutputFormatterWriteContext context)
+            {
+                return Task.FromResult(0);
+            }
+        }
+
         private class TestObjectResultExecutor : ObjectResultExecutor
         {
             public TestObjectResultExecutor(
-                IOptions<MvcOptions> options, 
+                IOptions<MvcOptions> options,
                 IHttpResponseStreamWriterFactory writerFactory,
-                ILoggerFactory loggerFactory) 
+                ILoggerFactory loggerFactory)
                 : base(options, writerFactory, loggerFactory)
             {
             }
 
-            public new IOutputFormatter SelectFormatter(
+            new public IOutputFormatter SelectFormatter(
                 OutputFormatterWriteContext formatterContext,
                 IList<MediaTypeHeaderValue> contentTypes,
                 IList<IOutputFormatter> formatters)
@@ -491,5 +578,27 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
                 return base.SelectFormatter(formatterContext, contentTypes, formatters);
             }
         }
+
+        private class CustomObjectResultExecutor : ObjectResultExecutor
+        {
+            public CustomObjectResultExecutor(
+                IOptions<MvcOptions> options,
+                IHttpResponseStreamWriterFactory writerFactory,
+                ILoggerFactory loggerFactory)
+                : base(options, writerFactory, loggerFactory)
+            {
+            }
+
+            public IOutputFormatter SelectedOutputFormatter { get; private set; }
+
+            protected override IOutputFormatter SelectFormatter(
+                OutputFormatterWriteContext formatterContext,
+                IList<MediaTypeHeaderValue> contentTypes,
+                IList<IOutputFormatter> formatters)
+            {
+                SelectedOutputFormatter = base.SelectFormatter(formatterContext, contentTypes, formatters);
+                return SelectedOutputFormatter;
+            }
+        }
     }
 }

test/Microsoft.AspNet.Mvc.FunctionalTests/StreamOutputFormatterTest.cs

@@ -21,7 +21,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
         [InlineData("MemoryStreamWithContentType", "text/html")]
         [InlineData("MemoryStreamWithContentTypeFromProduces", "text/plain")]
         [InlineData("MemoryStreamWithContentTypeFromProducesWithMultipleValues", "text/html")]
-        [InlineData("MemoryStreamOverridesContentTypeWithProduces", "text/plain")]
+        [InlineData("MemoryStreamOverridesProducesContentTypeWithResponseContentType", "text/plain")]
         public async Task StreamOutputFormatter_ReturnsAppropriateContentAndContentType(string actionName, string contentType)
         {
             // Arrange & Act

test/WebSites/FormatterWebSite/Controllers/StreamController.cs

@@ -37,7 +37,7 @@ namespace FormatterWebSite
 
         [HttpGet]
         [Produces("text/plain")]
-        public Stream MemoryStreamOverridesContentTypeWithProduces()
+        public Stream MemoryStreamOverridesProducesContentTypeWithResponseContentType()
         {
             // Produces will set a ContentType on the implicit ObjectResult and
             // ContentType on response are overriden by content types from ObjectResult.