[Fixes #2108] StringOutputFormatter fails when HttpNotAcceptableOutputFormatter is used

src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs

@@ -68,101 +68,77 @@ namespace Microsoft.AspNet.Mvc
             await selectedFormatter.WriteAsync(formatterContext);
         }
 
-        public virtual IOutputFormatter SelectFormatter(OutputFormatterContext formatterContext,
+        public virtual IOutputFormatter SelectFormatter(
-                                                       IEnumerable<IOutputFormatter> formatters)
+            OutputFormatterContext formatterContext,
+            IEnumerable<IOutputFormatter> formatters)
         {
+            // Check if any content-type was explicitly set (for example, via ProducesAttribute 
+            // or Url path extension mapping). If yes, then ignore content-negotiation and use this content-type.
             if (ContentTypes.Count == 1)
             {
-                // There is only one content type specified so we can skip looking at the accept headers.
                 return SelectFormatterUsingAnyAcceptableContentType(formatterContext,
                                                                     formatters,
                                                                     ContentTypes);
             }
 
-            var incomingAcceptHeaderMediaTypes = formatterContext.ActionContext.HttpContext.Request.GetTypedHeaders().Accept ??
+            var sortedAcceptHeaderMediaTypes = GetSortedAcceptHeaderMediaTypes(formatterContext);
-                                                    new MediaTypeHeaderValue[] { };
-
-            // By default we want to ignore considering accept headers for content negotiation when
-            // they have a media type like */* in them. Browsers typically have these media types.
-            // In these cases we would want the first formatter in the list of output formatters to
-            // write the response. This default behavior can be changed through options, so checking here.
-            var options = formatterContext.ActionContext.HttpContext
-                                                        .RequestServices
-                                                        .GetRequiredService<IOptions<MvcOptions>>()
-                                                        .Options;
-
-            var respectAcceptHeader = true;
-            if (options.RespectBrowserAcceptHeader == false
-                && incomingAcceptHeaderMediaTypes.Any(mediaType => mediaType.MatchesAllTypes))
-            {
-                respectAcceptHeader = false;
-            }
-            
-            IEnumerable<MediaTypeHeaderValue> sortedAcceptHeaderMediaTypes = null;
-            if (respectAcceptHeader)
-            {
-                sortedAcceptHeaderMediaTypes = SortMediaTypeHeaderValues(incomingAcceptHeaderMediaTypes)
-                                                .Where(header => header.Quality != HeaderQuality.NoMatch);
-            }
 
             IOutputFormatter selectedFormatter = null;
             if (ContentTypes == null || ContentTypes.Count == 0)
             {
-                if (respectAcceptHeader)
+                // Check if we have enough information to do content-negotiation, otherwise get the first formatter
+                // which can write the type.
+                MediaTypeHeaderValue requestContentType = null;
+                MediaTypeHeaderValue.TryParse(
+                    formatterContext.ActionContext.HttpContext.Request.ContentType,
+                    out requestContentType);
+                if (!sortedAcceptHeaderMediaTypes.Any() && requestContentType == null)
+                {
+                    return SelectFormatterBasedOnTypeMatch(formatterContext, formatters);
+                }
+
+                //
+                // Content-Negotiation starts from this point on.
+                //
+
+                // 1. Select based on sorted accept headers.
+                if (sortedAcceptHeaderMediaTypes.Any())
                 {
-                    // Select based on sorted accept headers.
                     selectedFormatter = SelectFormatterUsingSortedAcceptHeaders(
                                                                             formatterContext,
                                                                             formatters,
                                                                             sortedAcceptHeaderMediaTypes);
                 }
 
-                if (selectedFormatter == null)
+                // 2. No formatter was found based on accept headers, fall back on request Content-Type header.
+                if (selectedFormatter == null && requestContentType != null)
                 {
-                    var requestContentType = formatterContext.ActionContext.HttpContext.Request.ContentType;
-
-                    // No formatter found based on accept headers, fall back on request contentType.
-                    MediaTypeHeaderValue incomingContentType = null;
-                    MediaTypeHeaderValue.TryParse(requestContentType, out incomingContentType);
-
-                    // In case the incomingContentType is null (as can be the case with get requests),
-                    // we need to pick the first formatter which
-                    // can support writing this type.
-                    var contentTypes = new[] { incomingContentType };
                     selectedFormatter = SelectFormatterUsingAnyAcceptableContentType(
                                                                                 formatterContext,
                                                                                 formatters,
-                                                                                contentTypes);
+                                                                                new[] { requestContentType });
+                }
 
-                    // This would be the case when no formatter could write the type base on the
+                // 3. No formatter was found based on Accept and request Content-Type headers, so
-                    // accept headers and the request content type. Fallback on type based match.
+                // fallback on type based match.
-                    if (selectedFormatter == null)
+                if (selectedFormatter == null)
-                    {
+                {
-                        foreach (var formatter in formatters)
+                    // Set this flag to indicate that content-negotiation has failed to let formatters decide
-                        {
+                    // if they want to write the response or not.
-                            var supportedContentTypes = formatter.GetSupportedContentTypes(
+                    formatterContext.FailedContentNegotiation = true;
-                                                                         formatterContext.DeclaredType,
-                                                                         formatterContext.Object?.GetType(),
-                                                                         contentType: null);
 
-                            if (formatter.CanWriteResult(formatterContext, supportedContentTypes?.FirstOrDefault()))
+                    return SelectFormatterBasedOnTypeMatch(formatterContext, formatters);
-                            {
-                                return formatter;
-                            }
-                        }
-                    }
                 }
             }
             else
             {
-                if (respectAcceptHeader)
+                if (sortedAcceptHeaderMediaTypes.Any())
                 {
                     // Filter and remove accept headers which cannot support any of the user specified content types.
                     var filteredAndSortedAcceptHeaders = sortedAcceptHeaderMediaTypes
-                                                                    .Where(acceptHeader =>
+                                                                    .Where(acceptHeader => 
-                                                                            ContentTypes
+                                                                        ContentTypes.Any(contentType => 
-                                                                                .Any(contentType =>
+                                                                            contentType.IsSubsetOf(acceptHeader)));
-                                                                                       contentType.IsSubsetOf(acceptHeader)));
 
                     selectedFormatter = SelectFormatterUsingSortedAcceptHeaders(
                                                                         formatterContext,
@@ -188,6 +164,26 @@ namespace Microsoft.AspNet.Mvc
             return selectedFormatter;
         }
 
+        public virtual IOutputFormatter SelectFormatterBasedOnTypeMatch(
+            OutputFormatterContext formatterContext,
+            IEnumerable<IOutputFormatter> formatters)
+        {
+            foreach (var formatter in formatters)
+            {
+                var supportedContentTypes = formatter.GetSupportedContentTypes(
+                                                             formatterContext.DeclaredType,
+                                                             formatterContext.Object?.GetType(),
+                                                             contentType: null);
+
+                if (formatter.CanWriteResult(formatterContext, supportedContentTypes?.FirstOrDefault()))
+                {
+                    return formatter;
+                }
+            }
+
+            return null;
+        }
+
         public virtual IOutputFormatter SelectFormatterUsingSortedAcceptHeaders(
                                                             OutputFormatterContext formatterContext,
                                                             IEnumerable<IOutputFormatter> formatters,
@@ -224,6 +220,38 @@ namespace Microsoft.AspNet.Mvc
             return selectedFormatter;
         }
 
+        private IEnumerable<MediaTypeHeaderValue> GetSortedAcceptHeaderMediaTypes(
+            OutputFormatterContext formatterContext)
+        {
+            var request = formatterContext.ActionContext.HttpContext.Request;
+            var incomingAcceptHeaderMediaTypes = request.GetTypedHeaders().Accept ?? new MediaTypeHeaderValue[] { };
+
+            // By default we want to ignore considering accept headers for content negotiation when
+            // they have a media type like */* in them. Browsers typically have these media types.
+            // In these cases we would want the first formatter in the list of output formatters to
+            // write the response. This default behavior can be changed through options, so checking here.
+            var options = formatterContext.ActionContext.HttpContext
+                                                        .RequestServices
+                                                        .GetRequiredService<IOptions<MvcOptions>>()
+                                                        .Options;
+
+            var respectAcceptHeader = true;
+            if (options.RespectBrowserAcceptHeader == false
+                && incomingAcceptHeaderMediaTypes.Any(mediaType => mediaType.MatchesAllTypes))
+            {
+                respectAcceptHeader = false;
+            }
+
+            var sortedAcceptHeaderMediaTypes = Enumerable.Empty<MediaTypeHeaderValue>();
+            if (respectAcceptHeader)
+            {
+                sortedAcceptHeaderMediaTypes = SortMediaTypeHeaderValues(incomingAcceptHeaderMediaTypes)
+                                                .Where(header => header.Quality != HeaderQuality.NoMatch);
+            }
+
+            return sortedAcceptHeaderMediaTypes;
+        }
+
         private void ThrowIfUnsupportedContentType()
         {
             var matchAllContentType = ContentTypes?.FirstOrDefault(

src/Microsoft.AspNet.Mvc.Core/Formatters/HttpNotAcceptableOutputFormatter.cs

@@ -10,15 +10,14 @@ using Microsoft.Net.Http.Headers;
 namespace Microsoft.AspNet.Mvc
 {
     /// <summary>
-    /// A formatter which does not have a supported content type and selects itself if no content type is passed to it.
+    /// A formatter which selects itself when content-negotiation has failed and writes a 406 Not Acceptable response.
-    /// Sets the status code to 406 if selected.
     /// </summary>
     public class HttpNotAcceptableOutputFormatter : IOutputFormatter
     {
         /// <inheritdoc />
         public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
         {
-            return contentType == null;
+            return context.FailedContentNegotiation ?? false;
         }
 
         /// <inheritdoc />
@@ -37,4 +36,4 @@ namespace Microsoft.AspNet.Mvc
             return Task.FromResult(true);
         }
     }
-}
+}

src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatterContext.cs

@@ -45,5 +45,16 @@ namespace Microsoft.AspNet.Mvc
         /// Null indicates no value set by the <see cref="ObjectResult"/>.
         /// </remarks>
         public int? StatusCode { get; set; }
+
+        /// <summary>
+        /// Gets or sets a flag to indicate that content-negotiation could not find a formatter based on the 
+        /// information on the <see cref="Http.HttpRequest"/>.
+        /// </summary>
+        /// <remarks>
+        /// A <see cref="IOutputFormatter"/> can use this information to decide how to write a response. 
+        /// For example, the <see cref="HttpNotAcceptableOutputFormatter"/> sets a 406 Not Acceptable response 
+        /// when content negotiation has failed.
+        /// </remarks>
+        public bool? FailedContentNegotiation { get; set; }
     }
 }

test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs

@@ -107,6 +107,37 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults
             Assert.Equal(input, result.Value);
         }
 
+        [Fact]
+        public async Task NoAcceptAndContentTypeHeaders_406Formatter_DoesNotTakeEffect()
+        {
+            // Arrange
+            var expectedContentType = "application/json; charset=utf-8";
+
+            var input = 123;
+            var httpResponse = new DefaultHttpContext().Response;
+            httpResponse.Body = new MemoryStream();
+            var actionContext = CreateMockActionContext(
+                outputFormatters: new IOutputFormatter[] 
+                {
+                    new HttpNotAcceptableOutputFormatter(),
+                    new JsonOutputFormatter()
+                },
+                response: httpResponse,
+                requestAcceptHeader: null,
+                requestContentType: null,
+                requestAcceptCharsetHeader: null);
+
+            var result = new ObjectResult(input);
+            result.ContentTypes = new List<MediaTypeHeaderValue>();
+            result.ContentTypes.Add(MediaTypeHeaderValue.Parse(expectedContentType));
+
+            // Act
+            await result.ExecuteResultAsync(actionContext);
+
+            // Assert
+            Assert.Equal(expectedContentType, httpResponse.ContentType);
+        }
+
         [Fact]
         public async Task ObjectResult_WithSingleContentType_TheGivenContentTypeIsSelected()
         {
@@ -643,6 +674,103 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults
             Assert.Equal(expectedMessage, exception.Message);
         }
 
+        [Fact]
+        public async Task ObjectResult_WithStringType_WritesTextPlain_Ignoring406Formatter()
+        {
+            // Arrange
+            var expectedData = "Hello World!";
+            var objectResult = new ObjectResult(expectedData);
+            var outputFormatters = new IOutputFormatter[] 
+            {
+                new HttpNotAcceptableOutputFormatter(),
+                new StringOutputFormatter(),
+                new JsonOutputFormatter()
+            };
+
+            var response = new Mock<HttpResponse>();
+            var responseStream = new MemoryStream();
+            response.SetupGet(r => r.Body).Returns(responseStream);
+
+            var actionContext = CreateMockActionContext(
+                                    outputFormatters,
+                                    response.Object,
+                                    requestAcceptHeader: "application/json");
+
+            // Act
+            await objectResult.ExecuteResultAsync(actionContext);
+
+            // Assert
+            response.VerifySet(r => r.ContentType = "text/plain; charset=utf-8");
+            responseStream.Position = 0;
+            var actual = new StreamReader(responseStream).ReadToEnd();
+            Assert.Equal(expectedData, actual);
+        }
+
+        [Fact]
+        public async Task ObjectResult_WithSingleContentType_Ignores406Formatter()
+        {
+            // Arrange
+            var objectResult = new ObjectResult(new Person() { Name = "John" });
+            objectResult.ContentTypes.Add(new MediaTypeHeaderValue("application/json"));
+            var outputFormatters = new IOutputFormatter[] 
+            {
+                new HttpNotAcceptableOutputFormatter(),
+                new JsonOutputFormatter()
+            };
+            var response = new Mock<HttpResponse>();
+            var responseStream = new MemoryStream();
+            response.SetupGet(r => r.Body).Returns(responseStream);
+            var expectedData = "{\"Name\":\"John\"}";
+
+            var actionContext = CreateMockActionContext(
+                                    outputFormatters,
+                                    response.Object,
+                                    requestAcceptHeader: "application/non-existing",
+                                    requestContentType: "application/non-existing");
+
+            // Act
+            await objectResult.ExecuteResultAsync(actionContext);
+
+            // Assert
+            response.VerifySet(r => r.ContentType = "application/json; charset=utf-8");
+            responseStream.Position = 0;
+            var actual = new StreamReader(responseStream).ReadToEnd();
+            Assert.Equal(expectedData, actual);
+        }
+
+        [Fact]
+        public async Task ObjectResult_WithMultipleContentTypes_Ignores406Formatter()
+        {
+            // Arrange
+            var objectResult = new ObjectResult(new Person() { Name = "John" });
+            objectResult.ContentTypes.Add(new MediaTypeHeaderValue("application/foo"));
+            objectResult.ContentTypes.Add(new MediaTypeHeaderValue("application/json"));
+            var outputFormatters = new IOutputFormatter[] 
+            {
+                new HttpNotAcceptableOutputFormatter(),
+                new JsonOutputFormatter()
+            };
+            var response = new Mock<HttpResponse>();
+            var responseStream = new MemoryStream();
+            response.SetupGet(r => r.Body).Returns(responseStream);
+            var expectedData = "{\"Name\":\"John\"}";
+
+            var actionContext = CreateMockActionContext(
+                                    outputFormatters,
+                                    response.Object,
+                                    requestAcceptHeader: "application/non-existing",
+                                    requestContentType: "application/non-existing");
+
+            // Act
+            await objectResult.ExecuteResultAsync(actionContext);
+
+            // Assert
+            response.VerifySet(r => r.ContentType = "application/json; charset=utf-8");
+            responseStream.Position = 0;
+            var actual = new StreamReader(responseStream).ReadToEnd();
+            Assert.Equal(expectedData, actual);
+        }
+        
         private static ActionContext CreateMockActionContext(
                                                              HttpResponse response = null,
                                                              string requestAcceptHeader = "application/*",

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

@@ -132,6 +132,26 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             XmlAssert.Equal(expectedOutput, actual);
         }
 
+        [Theory]
+        [InlineData("http://localhost/FallbackOnTypeBasedMatch/UseTheFallback_WithDefaultFormatters")]
+        [InlineData("http://localhost/FallbackOnTypeBasedMatch/OverrideTheFallback_WithDefaultFormatters")]
+        public async Task NoAcceptAndRequestContentTypeHeaders_UsesFirstFormatterWhichCanWriteType(string url)
+        {
+            // Arrange
+            var server = TestHelper.CreateServer(_app, SiteName);
+            var client = server.CreateClient();
+            var expectedContentType = MediaTypeHeaderValue.Parse("application/json;charset=utf-8");
+
+            // Act
+            var response = await client.GetAsync(url + "?input=100");
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.Equal(expectedContentType, response.Content.Headers.ContentType);
+            var actual = await response.Content.ReadAsStringAsync();
+            Assert.Equal("100", actual);
+        }
+
         [Fact]
         public async Task NoMatchingFormatter_ForTheGivenContentType_Returns406()
         {
@@ -408,6 +428,29 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             Assert.Equal(expectedBody, body);
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task ObjectResult_WithStringReturnType_WritesTextPlainFormat(bool matchFormatterOnObjectType)
+        {
+            // Arrange
+            var server = TestHelper.CreateServer(_app, SiteName);
+            var client = server.CreateClient();
+            var targetUri = "http://localhost/FallbackOnTypeBasedMatch/ReturnString?matchFormatterOnObjectType=" +
+                matchFormatterOnObjectType;
+            var request = new HttpRequestMessage(HttpMethod.Get, targetUri);
+            request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json"));
+
+            // Act
+            var response = await client.SendAsync(request);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.Equal("text/plain", response.Content.Headers.ContentType.MediaType);
+            var actualBody = await response.Content.ReadAsStringAsync();
+            Assert.Equal("Hello World!", actualBody);
+        }
+
         [Theory]
         [InlineData("OverrideTheFallback_WithDefaultFormatters")]
         [InlineData("OverrideTheFallback_UsingCustomFormatters")]

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

@@ -69,6 +69,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             var server = TestHelper.CreateServer(_app, SiteName, SamplesFolder);
             var client = server.CreateClient();
             var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/FormUrlEncoded/IsValidPerson");
+            request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
             request.Content = new StringContent(input, Encoding.UTF8, "application/x-www-form-urlencoded");
 
             // Act

test/WebSites/ContentNegotiationWebSite/Controllers/FallbackOnTypeBasedMatchController.cs

@@ -58,5 +58,23 @@ namespace ContentNegotiationWebSite
 
             return objectResult;
         }
+
+        public IActionResult ReturnString(
+            bool matchFormatterOnObjectType, 
+            [FromServices] IOutputFormattersProvider outputFormattersProvider)
+        {
+            var objectResult = new ObjectResult("Hello World!");
+            if (matchFormatterOnObjectType)
+            {
+                objectResult.Formatters.Add(new HttpNotAcceptableOutputFormatter());
+            }
+
+            foreach (var formatter in outputFormattersProvider.OutputFormatters)
+            {
+                objectResult.Formatters.Add(formatter);
+            }
+
+            return objectResult;
+        }
     }
 }