diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/OutputFormatterCanWriteContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/OutputFormatterCanWriteContext.cs index 15b931dd02..9d8a68c7ce 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/OutputFormatterCanWriteContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/OutputFormatterCanWriteContext.cs @@ -22,12 +22,6 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// public virtual StringSegment ContentType { get; set; } - /// - /// Gets or sets a value indicating that content-negotiation could not find a formatter based on the - /// information on the . - /// - public virtual bool? FailedContentNegotiation { get; set; } - /// /// Gets or sets the object to write to the response. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/HttpNotAcceptableOutputFormatter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Formatters/HttpNotAcceptableOutputFormatter.cs deleted file mode 100644 index 44b3f3a80a..0000000000 --- a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/HttpNotAcceptableOutputFormatter.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc.Internal; - -namespace Microsoft.AspNetCore.Mvc.Formatters -{ - /// - /// A formatter which selects itself when content-negotiation has failed and writes a 406 Not Acceptable response. - /// - public class HttpNotAcceptableOutputFormatter : IOutputFormatter - { - /// - public bool CanWriteResult(OutputFormatterCanWriteContext context) - { - return context.FailedContentNegotiation ?? false; - } - - /// - public Task WriteAsync(OutputFormatterWriteContext context) - { - var response = context.HttpContext.Response; - response.StatusCode = StatusCodes.Status406NotAcceptable; - return TaskCache.CompletedTask; - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs index 9772f3aa38..cb0306c18a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs @@ -45,6 +45,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal OptionsFormatters = options.Value.OutputFormatters; RespectBrowserAcceptHeader = options.Value.RespectBrowserAcceptHeader; + ReturnHttpNotAcceptable = options.Value.ReturnHttpNotAcceptable; Logger = loggerFactory.CreateLogger(); WriterFactory = writerFactory.CreateWriter; } @@ -64,6 +65,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal /// protected bool RespectBrowserAcceptHeader { get; } + /// + /// Gets the value of . + /// + protected bool ReturnHttpNotAcceptable { get; } + /// /// Gets the writer factory delegate. /// @@ -130,7 +136,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // No formatter supports this. Logger.NoFormatter(formatterContext); - + context.HttpContext.Response.StatusCode = StatusCodes.Status406NotAcceptable; return TaskCache.CompletedTask; } @@ -175,71 +181,57 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(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) - { - Logger.SkippedContentNegotiation(contentTypes[0]); - - return SelectFormatterUsingAnyAcceptableContentType(formatterContext, formatters, contentTypes); - } - var request = formatterContext.HttpContext.Request; - - var mediaTypes = GetMediaTypes(contentTypes, request); + var acceptableMediaTypes = GetAcceptableMediaTypes(contentTypes, request); + var selectFormatterWithoutRegardingAcceptHeader = false; IOutputFormatter selectedFormatter = null; - if (contentTypes.Count == 0) + + if (acceptableMediaTypes.Count == 0) { - // Check if we have enough information to do content-negotiation, otherwise get the first formatter - // which can write the type. Let the formatter choose the Content-Type. - if (!(mediaTypes.Count > 0)) - { - Logger.NoAcceptForNegotiation(); + // There is either no Accept header value, or it contained */* and we + // are not currently respecting the 'browser accept header'. + Logger.NoAcceptForNegotiation(); - return SelectFormatterNotUsingAcceptHeaders(formatterContext, formatters); - } - - // - // Content-Negotiation starts from this point on. - // - - // 1. Select based on sorted accept headers. - selectedFormatter = SelectFormatterUsingSortedAcceptHeaders( - formatterContext, - formatters, - mediaTypes); - - // 2. No formatter was found based on Accept header. Fallback to the first formatter which can write - // the type. Let the formatter choose the Content-Type. - if (selectedFormatter == null) - { - Logger.NoFormatterFromNegotiation(mediaTypes); - - // Set this flag to indicate that content-negotiation has failed to let formatters decide - // if they want to write the response or not. - formatterContext.FailedContentNegotiation = true; - - return SelectFormatterNotUsingAcceptHeaders(formatterContext, formatters); - } + selectFormatterWithoutRegardingAcceptHeader = true; } else { - if (mediaTypes.Count > 0) + if (contentTypes.Count == 0) { + // Use whatever formatter can meet the client's request selectedFormatter = SelectFormatterUsingSortedAcceptHeaders( formatterContext, formatters, - mediaTypes); + acceptableMediaTypes); + } + else + { + // Verify that a content type from the context is compatible with the client's request + selectedFormatter = SelectFormatterUsingSortedAcceptHeadersAndContentTypes( + formatterContext, + formatters, + acceptableMediaTypes, + contentTypes); } - if (selectedFormatter == null) + if (selectedFormatter == null && !ReturnHttpNotAcceptable) + { + Logger.NoFormatterFromNegotiation(acceptableMediaTypes); + + selectFormatterWithoutRegardingAcceptHeader = true; + } + } + + if (selectFormatterWithoutRegardingAcceptHeader) + { + if (contentTypes.Count == 0) + { + selectedFormatter = SelectFormatterNotUsingContentType( + formatterContext, + formatters); + } + else { - // Either there were no acceptHeaders that were present OR - // There were no accept headers which matched OR - // There were acceptHeaders which matched but there was no formatter - // which supported any of them. - // In any of these cases, if the user has specified content types, - // do a last effort to find a formatter which can write any of the user specified content type. selectedFormatter = SelectFormatterUsingAnyAcceptableContentType( formatterContext, formatters, @@ -250,7 +242,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal return selectedFormatter; } - private List GetMediaTypes( + private List GetAcceptableMediaTypes( MediaTypeCollection contentTypes, HttpRequest request) { @@ -264,11 +256,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal result.Clear(); return result; } - - if (!InAcceptableMediaTypes(result[i].MediaType, contentTypes)) - { - result.RemoveAt(i); - } } result.Sort((left, right) => left.Quality > right.Quality ? -1 : (left.Quality == right.Quality ? 0 : 1)); @@ -276,26 +263,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal return result; } - private static bool InAcceptableMediaTypes(StringSegment mediaType, MediaTypeCollection acceptableMediaTypes) - { - if (acceptableMediaTypes.Count == 0) - { - return true; - } - - var parsedMediaType = new MediaType(mediaType); - for (int i = 0; i < acceptableMediaTypes.Count; i++) - { - var acceptableMediaType = new MediaType(acceptableMediaTypes[i]); - if (acceptableMediaType.IsSubsetOf(parsedMediaType)) - { - return true; - } - } - - return false; - } - /// /// Selects the to write the response. The first formatter which /// can write the response should be chosen without any consideration for content type. @@ -307,7 +274,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal /// /// The selected or null if no formatter can write the response. /// - protected virtual IOutputFormatter SelectFormatterNotUsingAcceptHeaders( + protected virtual IOutputFormatter SelectFormatterNotUsingContentType( OutputFormatterWriteContext formatterContext, IList formatters) { @@ -432,6 +399,53 @@ namespace Microsoft.AspNetCore.Mvc.Internal return null; } + + /// + /// Selects the to write the response based on the content type values + /// present in and . + /// + /// The . + /// + /// The list of instances to consider. + /// + /// + /// The ordered content types from the Accept header, sorted by descending q-value. + /// + /// + /// The ordered content types from in descending priority order. + /// + /// + /// The selected or null if no formatter can write the response. + /// + protected virtual IOutputFormatter SelectFormatterUsingSortedAcceptHeadersAndContentTypes( + OutputFormatterWriteContext formatterContext, + IList formatters, + IList sortedAcceptableContentTypes, + MediaTypeCollection possibleOutputContentTypes) + { + for (var i = 0; i < sortedAcceptableContentTypes.Count; i++) + { + var acceptableContentType = new MediaType(sortedAcceptableContentTypes[i].MediaType); + for (var j = 0; j < possibleOutputContentTypes.Count; j++) + { + var candidateContentType = new MediaType(possibleOutputContentTypes[j]); + if (candidateContentType.IsSubsetOf(acceptableContentType)) + { + for (var k = 0; k < formatters.Count; k++) + { + var formatter = formatters[k]; + formatterContext.ContentType = new StringSegment(possibleOutputContentTypes[j]); + if (formatter.CanWriteResult(formatterContext)) + { + return formatter; + } + } + } + } + } + + return null; + } private void ValidateContentTypes(MediaTypeCollection contentTypes) { diff --git a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs index 2d0768fb72..b314d360d3 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs @@ -123,6 +123,13 @@ namespace Microsoft.AspNetCore.Mvc /// public bool RespectBrowserAcceptHeader { get; set; } + /// + /// Gets or sets the flag which decides whether an HTTP 406 Not Acceptable response + /// will be returned if no formatter has been selected to format the response. + /// by default. + /// + public bool ReturnHttpNotAcceptable { get; set; } + /// /// Gets a list of used by this application. /// diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/HttpNotAcceptableOutputFormatterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/HttpNotAcceptableOutputFormatterTest.cs deleted file mode 100644 index e7811c0ae7..0000000000 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/HttpNotAcceptableOutputFormatterTest.cs +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.Formatters -{ - public class HttpNotAcceptableOutputFormatterTest - { - [Theory] - [InlineData(false)] - [InlineData(null)] - public void CanWriteResult_ReturnsFalse_WhenConnegHasntFailed(bool? failedContentNegotiation) - { - // Arrange - var formatter = new HttpNotAcceptableOutputFormatter(); - - var context = new OutputFormatterWriteContext( - new DefaultHttpContext(), - new TestHttpResponseStreamWriterFactory().CreateWriter, - objectType: null, - @object: null) - { - FailedContentNegotiation = failedContentNegotiation, - }; - - // Act - var result = formatter.CanWriteResult(context); - - // Assert - Assert.False(result); - } - - [Fact] - public void CanWriteResult_ReturnsTrue_WhenConnegHasFailed() - { - // Arrange - var formatter = new HttpNotAcceptableOutputFormatter(); - - var context = new OutputFormatterWriteContext( - new DefaultHttpContext(), - new TestHttpResponseStreamWriterFactory().CreateWriter, - objectType: null, - @object: null) - { - FailedContentNegotiation = true, - }; - - // Act - var result = formatter.CanWriteResult(context); - - // Assert - Assert.True(result); - } - - [Fact] - public async Task WriteAsync_Sets406NotAcceptable() - { - // Arrange - var formatter = new HttpNotAcceptableOutputFormatter(); - - var context = new OutputFormatterWriteContext( - new DefaultHttpContext(), - new TestHttpResponseStreamWriterFactory().CreateWriter, - objectType: null, - @object: null); - - // Act - await formatter.WriteAsync(context); - - // Assert - Assert.Equal(StatusCodes.Status406NotAcceptable, context.HttpContext.Response.StatusCode); - } - } -} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs index dc314c46fd..55912a2483 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ObjectResultExecutorTest.cs @@ -249,7 +249,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public void SelectFormatter_NoProvidedContentTypesAndNoAcceptHeader_ChoosesFirstFormattterThatCanWrite() + public void SelectFormatter_NoProvidedContentTypesAndNoAcceptHeader_ChoosesFirstFormatterThatCanWrite() { // Arrange var executor = CreateExecutor(); @@ -276,11 +276,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert Assert.Same(formatters[1], formatter); Assert.Equal(new StringSegment("application/json"), context.ContentType); - Assert.Null(context.FailedContentNegotiation); } [Fact] - public void SelectFormatter_WithAcceptHeader_ConnegFails() + public void SelectFormatter_WithAcceptHeader_UsesFallback() { // Arrange var executor = CreateExecutor(); @@ -308,7 +307,39 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert Assert.Same(formatters[0], formatter); Assert.Equal(new StringSegment("application/xml"), context.ContentType); - Assert.True(context.FailedContentNegotiation); + } + + [Fact] + public void SelectFormatter_WithAcceptHeaderAndReturnHttpNotAcceptable_DoesNotUseFallback() + { + // Arrange + var options = new TestOptionsManager(); + options.Value.ReturnHttpNotAcceptable = true; + + var executor = CreateExecutor(options); + + var formatters = new List + { + new TestXmlOutputFormatter(), + new TestJsonOutputFormatter(), + }; + + var context = new OutputFormatterWriteContext( + new DefaultHttpContext(), + new TestHttpResponseStreamWriterFactory().CreateWriter, + objectType: null, + @object: null); + + context.HttpContext.Request.Headers[HeaderNames.Accept] = "text/custom,application/custom"; + + // Act + var formatter = executor.SelectFormatter( + context, + new MediaTypeCollection { }, + formatters); + + // Assert + Assert.Null(formatter); } [Fact] diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ContentNegotiationTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ContentNegotiationTest.cs index f03bd99478..81306a53b8 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ContentNegotiationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ContentNegotiationTest.cs @@ -324,29 +324,6 @@ END:VCARD } } - [Theory] - [InlineData("UseTheFallback_WithDefaultFormatters")] - [InlineData("UseTheFallback_UsingCustomFormatters")] - public async Task NoMatchOn_RequestContentType_FallsBackOnTypeBasedMatch_MatchFound(string actionName) - { - // Arrange - var expectedContentType = MediaTypeHeaderValue.Parse("application/json;charset=utf-8"); - var expectedBody = "1234"; - var targetUri = "http://localhost/FallbackOnTypeBasedMatch/" + actionName + "/?input=1234"; - var content = new StringContent("1234", Encoding.UTF8, "application/custom"); - var request = new HttpRequestMessage(HttpMethod.Post, targetUri); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/custom1")); - request.Content = content; - - // Act - var response = await Client.SendAsync(request); - - // Assert - Assert.Equal(expectedContentType, response.Content.Headers.ContentType); - var body = await response.Content.ReadAsStringAsync(); - Assert.Equal(expectedBody, body); - } - [Theory] [InlineData(true)] [InlineData(false)] @@ -388,25 +365,6 @@ END:VCARD Assert.Equal("Hello World!", actualBody); } - [Theory] - [InlineData("OverrideTheFallback_WithDefaultFormatters")] - [InlineData("OverrideTheFallback_UsingCustomFormatters")] - public async Task NoMatchOn_RequestContentType_SkipTypeMatchByAddingACustomFormatter(string actionName) - { - // Arrange - var targetUri = "http://localhost/FallbackOnTypeBasedMatch/" + actionName + "/?input=1234"; - var content = new StringContent("1234", Encoding.UTF8, "application/custom"); - var request = new HttpRequestMessage(HttpMethod.Post, targetUri); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/custom1")); - request.Content = content; - - // Act - var response = await Client.SendAsync(request); - - // Assert - Assert.Equal(HttpStatusCode.NotAcceptable, response.StatusCode); - } - [Fact] public async Task NoMatchOn_RequestContentType_FallsBackOnTypeBasedMatch_NoMatchFound_Returns406() { diff --git a/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/FallbackOnTypeBasedMatchController.cs b/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/FallbackOnTypeBasedMatchController.cs index 8987c8220a..a232df7585 100644 --- a/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/FallbackOnTypeBasedMatchController.cs +++ b/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/FallbackOnTypeBasedMatchController.cs @@ -53,7 +53,6 @@ namespace BasicWebSite.Controllers.ContentNegotiation public IActionResult OverrideTheFallback_UsingCustomFormatters(int input) { var objectResult = new ObjectResult(input); - objectResult.Formatters.Add(new HttpNotAcceptableOutputFormatter()); objectResult.Formatters.Add(new PlainTextFormatter()); objectResult.Formatters.Add(_jsonOutputFormatter); @@ -63,7 +62,6 @@ namespace BasicWebSite.Controllers.ContentNegotiation public IActionResult OverrideTheFallback_WithDefaultFormatters(int input) { var objectResult = new ObjectResult(input); - objectResult.Formatters.Add(new HttpNotAcceptableOutputFormatter()); foreach (var formatter in _mvcOptions.Value.OutputFormatters) { objectResult.Formatters.Add(formatter); @@ -73,14 +71,9 @@ namespace BasicWebSite.Controllers.ContentNegotiation } public IActionResult ReturnString( - bool matchFormatterOnObjectType, [FromServices] IOptions optionsAccessor) { var objectResult = new ObjectResult("Hello World!"); - if (matchFormatterOnObjectType) - { - objectResult.Formatters.Add(new HttpNotAcceptableOutputFormatter()); - } foreach (var formatter in optionsAccessor.Value.OutputFormatters) {