From 33173d3031c251714fefcbc0f6fbafccb95c4e03 Mon Sep 17 00:00:00 2001 From: harshgMSFT Date: Tue, 26 Aug 2014 16:08:14 -0700 Subject: [PATCH] Fix for Content negotiation should fallback to the first formatter that can write the type #1033 - Includes functional and unit tests. --- .../ActionResults/ObjectResult.cs | 14 +++ .../Formatters/IOutputFormatter.cs | 13 +++ .../Formatters/NoContentFormatter.cs | 7 ++ .../Formatters/OutputFormatter.cs | 19 ++-- ...entResultTests.cs => ObjectResultTests.cs} | 90 +++++++++++++++- .../Formatters/OutputFormatterTests.cs | 18 ++++ .../JsonResultTest.cs | 16 +++ .../OutputFormatterDescriptorTest.cs | 18 ++++ .../ConnegTests.cs | 62 +++++++++++ .../FallbackOnTypeBasedMatchController.cs | 101 ++++++++++++++++++ 10 files changed, 341 insertions(+), 17 deletions(-) rename test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/{ObjectContentResultTests.cs => ObjectResultTests.cs} (86%) create mode 100644 test/WebSites/ConnegWebSite/Controllers/FallbackOnTypeBasedMatchController.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs index 3cbbc2524d..67d9273b58 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs @@ -86,6 +86,20 @@ namespace Microsoft.AspNet.Mvc formatterContext, formatters, contentTypes); + + // This would be the case when no formatter could write the type base on the + // accept headers and the request content type. Fallback on type based match. + if(selectedFormatter == null) + { + foreach (var formatter in formatters) + { + if (formatter.CanWriteResult(formatterContext, + formatter.SupportedMediaTypes?.FirstOrDefault())) + { + return formatter; + } + } + } } } else if (ContentTypes.Count == 1) diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs index a4eb3fbc12..4651dbe735 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs @@ -18,6 +18,19 @@ namespace Microsoft.AspNet.Mvc /// public interface IOutputFormatter { + /// + /// Gets the mutable collection of character encodings supported by + /// this . The encodings are + /// used when writing the data. + /// + IList SupportedEncodings { get; } + + /// + /// Gets the mutable collection of elements supported by + /// this . + /// + IList SupportedMediaTypes { get; } + /// /// Determines whether this can serialize /// an object of the specified type. diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/NoContentFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/NoContentFormatter.cs index 19179e7506..ae65b159c5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/NoContentFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/NoContentFormatter.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.Collections.Generic; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.HeaderValueAbstractions; @@ -11,6 +14,10 @@ namespace Microsoft.AspNet.Mvc /// public class NoContentFormatter : IOutputFormatter { + public IList SupportedEncodings { get; private set; } + + public IList SupportedMediaTypes { get; private set; } + public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) { // ignore the contentType and just look at the content. diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs index cc6f2d8efb..1811c3a495 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs @@ -17,19 +17,6 @@ namespace Microsoft.AspNet.Mvc /// public abstract class OutputFormatter : IOutputFormatter { - /// - /// Gets the mutable collection of character encodings supported by - /// this instance. The encodings are - /// used when writing the data. - /// - public IList SupportedEncodings { get; private set; } - - /// - /// Gets the mutable collection of elements supported by - /// this instance. - /// - public IList SupportedMediaTypes { get; private set; } - /// /// Initializes a new instance of the class. /// @@ -39,6 +26,12 @@ namespace Microsoft.AspNet.Mvc SupportedMediaTypes = new List(); } + /// + public IList SupportedEncodings { get; private set; } + + /// + public IList SupportedMediaTypes { get; private set; } + /// /// Determines the best amongst the supported encodings /// for reading or writing an HTTP entity body based on the provided . diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectContentResultTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs similarity index 86% rename from test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectContentResultTests.cs rename to test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs index f6607ea96d..2551fb93e3 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectContentResultTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs @@ -274,6 +274,52 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults httpResponse.VerifySet(r => r.ContentType = expectedContentType); } + [Theory] + [InlineData("", 2)] + [InlineData(null, 2)] + [InlineData("application/xml", 3)] + [InlineData("application/custom", 3)] + [InlineData("application/xml;q=1, application/custom;q=0.8", 4)] + public void SelectFormatter_WithNoMatchingAcceptHeadersAndRequestContentType_PicksFormatterBasedOnObjectType + (string acceptHeader, int attemptedCountForCanWrite) + { + // For no accept headers, + //can write is called twice once for the request media type and once for the type match pass. + // For each adduaccept header, it is called once. + // Arrange + var stream = new MemoryStream(); + var httpResponse = new Mock(); + httpResponse.SetupProperty(o => o.ContentType); + httpResponse.SetupGet(r => r.Body).Returns(stream); + + var actionContext = CreateMockActionContext(httpResponse.Object, + requestAcceptHeader: acceptHeader, + requestContentType: "application/xml"); + var input = "testInput"; + var result = new ObjectResult(input); + + // Set more than one formatters. The test output formatter throws on write. + result.Formatters = new List + { + new CannotWriteFormatter(), + new CountingFormatter(), + }; + + var context = new OutputFormatterContext() + { + ActionContext = actionContext, + Object = input, + DeclaredType = typeof(string) + }; + + // Act + var formatter = result.SelectFormatter(context, result.Formatters); + + // Assert + var countingFormatter = Assert.IsType(formatter); + Assert.Equal(attemptedCountForCanWrite, countingFormatter.GetCanWriteCallCount()); + } + [Fact] public async Task ObjectResult_NoContentTypeSetWithNoAcceptHeadersAndNoRequestContentType_PicksFirstFormatterWhichCanWrite() @@ -461,21 +507,57 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults return serviceCollection.BuildServiceProvider(); } + public class CountingFormatter : OutputFormatter + { + private int _canWriteCallsCount = 0; + + public CountingFormatter() + { + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("text/plain")); + SupportedEncodings.Add(Encoding.GetEncoding("utf-8")); + } + + public override bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) + { + _canWriteCallsCount++; + if (base.CanWriteResult(context, contentType)) + { + var actionReturnString = context.Object as string; + if (actionReturnString != null) + { + return true; + } + } + + return false; + } + + public int GetCanWriteCallCount() + { + return _canWriteCallsCount; + } + + public override Task WriteResponseBodyAsync(OutputFormatterContext context) + { + throw new NotImplementedException(); + } + } + public class CannotWriteFormatter : IOutputFormatter { - public List SupportedEncodings + public IList SupportedEncodings { get { - throw new NotImplementedException(); + return null; } } - public List SupportedMediaTypes + public IList SupportedMediaTypes { get { - throw new NotImplementedException(); + return null; } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs index 091186da42..0751e73865 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs @@ -120,6 +120,24 @@ namespace Microsoft.AspNet.Mvc.Test formatterContext.SelectedContentType.RawValue); } + [Fact] + public void CanWriteResult_ForNullContentType_UsesFirstEntryInSupportedContentTypes() + { + // For no accept headers, + //can write is called twice once for the request media type and once for the type match pass. + // For each adduaccept header, it is called once. + // Arrange + var context = new OutputFormatterContext(); + var formatter = new TestOutputFormatter(); + + // Act + var result = formatter.CanWriteResult(context, null); + + // Assert + Assert.True(result); + Assert.Equal(formatter.SupportedMediaTypes[0].RawValue, context.SelectedContentType.RawValue); + } + private class TestOutputFormatter : OutputFormatter { public TestOutputFormatter() diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs index 982b816a94..6e6e2a68b8 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs @@ -113,6 +113,22 @@ namespace Microsoft.AspNet.Mvc { public Encoding Encoding { get; set; } + public IList SupportedEncodings + { + get + { + return null; + } + } + + public IList SupportedMediaTypes + { + get + { + return null; + } + } + public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) { return true; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/OutputFormatterDescriptorTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/OutputFormatterDescriptorTest.cs index 3d50106498..f426ab80df 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/OutputFormatterDescriptorTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/OutputFormatterDescriptorTest.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.HeaderValueAbstractions; using Microsoft.AspNet.Mvc.OptionDescriptors; @@ -55,6 +57,22 @@ namespace Microsoft.AspNet.Mvc.Core private class TestOutputFormatter : IOutputFormatter { + public IList SupportedEncodings + { + get + { + return null; + } + } + + public IList SupportedMediaTypes + { + get + { + return null; + } + } + public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) { throw new NotImplementedException(); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ConnegTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ConnegTests.cs index a58a73baf3..0c4e5ca663 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ConnegTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ConnegTests.cs @@ -266,5 +266,67 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var body = await response.Content.ReadAsStringAsync(); Assert.Equal(expectedBody, body); } + + [Theory] + [InlineData("UseTheFallback_WithDefaultFormatters")] + [InlineData("UseTheFallback_UsingCustomFormatters")] + public async Task NoMatchOn_RequestContentType_FallsBackOnTypeBasedMatch_MatchFound(string actionName) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.Handler; + var expectedContentType = "application/json;charset=utf-8"; + var expectedBody = "1234"; + var targetUri = "http://localhost/FallbackOnTypeBasedMatch/" + actionName + "/?input=1234"; + // Act + + var result = await client.PostAsync(targetUri, + "1234", + "application/custom", + (request) => request.Accept = "application/custom1"); + + // Assert + Assert.Equal(expectedContentType, result.HttpContext.Response.ContentType); + var body = await result.HttpContext.Response.ReadBodyAsStringAsync(); + Assert.Equal(expectedBody, body); + } + + [Theory] + [InlineData("OverrideTheFallback_WithDefaultFormatters")] + [InlineData("OverrideTheFallback_UsingCustomFormatters")] + public async Task NoMatchOn_RequestContentType_SkipTypeMatchByAddingACustomFormatter(string actionName) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.Handler; + var targetUri = "http://localhost/FallbackOnTypeBasedMatch/" + actionName + "/?input=1234"; + + // Act + var result = await client.PostAsync(targetUri, + "1234", + "application/custom", + (request) => request.Accept = "application/custom1"); + + // Assert + Assert.Equal(406, result.HttpContext.Response.StatusCode); + } + + [Fact] + public async Task NoMatchOn_RequestContentType_FallsBackOnTypeBasedMatch_NoMatchFound_Returns406() + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.Handler; + var targetUri = "http://localhost/FallbackOnTypeBasedMatch/FallbackGivesNoMatch/?input=1234"; + + // Act + var result = await client.PostAsync(targetUri, + "1234", + "application/custom", + (request) => request.Accept = "application/custom1"); + + // Assert + Assert.Equal(406, result.HttpContext.Response.StatusCode); + } } } \ No newline at end of file diff --git a/test/WebSites/ConnegWebSite/Controllers/FallbackOnTypeBasedMatchController.cs b/test/WebSites/ConnegWebSite/Controllers/FallbackOnTypeBasedMatchController.cs new file mode 100644 index 0000000000..330682001c --- /dev/null +++ b/test/WebSites/ConnegWebSite/Controllers/FallbackOnTypeBasedMatchController.cs @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Mvc; +using Microsoft.AspNet.Mvc.HeaderValueAbstractions; +using Microsoft.Framework.DependencyInjection; +namespace ConnegWebsite +{ + public class FallbackOnTypeBasedMatchController : Controller + { + public int UseTheFallback_WithDefaultFormatters(int input) + { + return input; + } + + public IActionResult UseTheFallback_UsingCustomFormatters(int input) + { + var objectResult = new ObjectResult(input); + + // Request content type is application/custom. + // PlainTextFormatter cannot write because it does not support the type. + // JsonOutputFormatter cannot write in the first attempt because it does not support the + // request content type. + objectResult.Formatters.Add(new PlainTextFormatter()); + objectResult.Formatters.Add(new JsonOutputFormatter(JsonOutputFormatter.CreateDefaultSettings(), false)); + + return objectResult; + } + + public IActionResult FallbackGivesNoMatch(int input) + { + var objectResult = new ObjectResult(input); + + // Request content type is application/custom. + // PlainTextFormatter cannot write because it does not support the type. + objectResult.Formatters.Add(new PlainTextFormatter()); + + return objectResult; + } + + public IActionResult OverrideTheFallback_UsingCustomFormatters(int input) + { + var objectResult = new ObjectResult(input); + objectResult.Formatters.Add(new StopIfNoMatchOutputFormatter()); + objectResult.Formatters.Add(new PlainTextFormatter()); + objectResult.Formatters.Add(new JsonOutputFormatter(JsonOutputFormatter.CreateDefaultSettings(), false)); + return objectResult; + } + + public IActionResult OverrideTheFallback_WithDefaultFormatters(int input) + { + var objectResult = new ObjectResult(input); + var formattersProvider = ActionContext.HttpContext.RequestServices.GetService(); + objectResult.Formatters.Add(new StopIfNoMatchOutputFormatter()); + foreach (var formatter in formattersProvider.OutputFormatters) + { + objectResult.Formatters.Add(formatter); + } + + return objectResult; + } + + public class StopIfNoMatchOutputFormatter : IOutputFormatter + { + public IList SupportedEncodings + { + get + { + return null; + } + } + + public IList SupportedMediaTypes + { + get + { + return null; + } + } + + // Select if no Registered content type. + public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) + { + return contentType == null; + } + + public Task WriteAsync(OutputFormatterContext context) + { + var response = context.ActionContext.HttpContext.Response; + response.StatusCode = 406; + return Task.FromResult(true); + } + } + } +} \ No newline at end of file