From 35ee0e3a4958db6e33d92b5f71506066b6fd842e Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 9 Jul 2015 13:42:07 -0700 Subject: [PATCH] Fix for #2739 This change removes the validation that forces an OutputFormatter to set an encoding, so that you can use the OutputFormatter base class for non-text. The check that we had would pretty much only be hit when you didn't have any SupportedEncodings. If you have anything in SupportedEncodings, then one of them will be picked as a default. So removing the check is to do, because for a text-based formatter you'd never run into this issue in the first place. --- .../OutputFormatter.cs | 23 ++++++++---- .../Properties/Resources.Designer.cs | 16 -------- src/Microsoft.AspNet.Mvc.Core/Resources.resx | 3 -- .../OutputFormatterTests.cs | 37 +++++++++++-------- 4 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/OutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/OutputFormatter.cs index 18af391512..dc41744ed6 100644 --- a/src/Microsoft.AspNet.Mvc.Core/OutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/OutputFormatter.cs @@ -182,19 +182,26 @@ namespace Microsoft.AspNet.Mvc // Copy the media type as we don't want it to affect the next request selectedMediaType = selectedMediaType.Copy(); + // Not text-based media types will use an encoding/charset - binary formats just ignore it. We want to + // make this class work with media types that use encodings, and those that don't. + // + // The default implementation of SelectCharacterEncoding will read from the list of SupportedEncodings + // and will always choose a default encoding if any are supported. So, the only cases where the + // selectedEncoding can be null are: + // + // 1). No supported encodings - we assume this is a non-text format + // 2). Custom implementation of SelectCharacterEncoding - trust the user and give them what they want. var selectedEncoding = SelectCharacterEncoding(context); - if (selectedEncoding == null) + if (selectedEncoding != null) { - // No supported encoding was found so there is no way for us to start writing. - throw new InvalidOperationException(Resources.FormatOutputFormatterNoEncoding(GetType().FullName)); + context.SelectedEncoding = selectedEncoding; + + // Override the content type value even if one already existed. + selectedMediaType.Charset = selectedEncoding.WebName; } - context.SelectedEncoding = selectedEncoding; - - // Override the content type value even if one already existed. - selectedMediaType.Charset = selectedEncoding.WebName; - context.SelectedContentType = context.SelectedContentType ?? selectedMediaType; + var response = context.HttpContext.Response; response.ContentType = selectedMediaType.ToString(); } diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index ca87ea3fb5..ca9ccc6dd3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -362,22 +362,6 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("TypeMustDeriveFromType"), p0, p1); } - /// - /// No encoding found for output formatter '{0}'. There must be at least one supported encoding registered in order for the output formatter to write content. - /// - internal static string OutputFormatterNoEncoding - { - get { return GetString("OutputFormatterNoEncoding"); } - } - - /// - /// No encoding found for output formatter '{0}'. There must be at least one supported encoding registered in order for the output formatter to write content. - /// - internal static string FormatOutputFormatterNoEncoding(object p0) - { - return string.Format(CultureInfo.CurrentCulture, GetString("OutputFormatterNoEncoding"), p0); - } - /// /// No encoding found for input formatter '{0}'. There must be at least one supported encoding registered in order for the formatter to read content. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index c5cbbd164c..bb5aa0eaa8 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -183,9 +183,6 @@ The type '{0}' must derive from '{1}'. - - No encoding found for output formatter '{0}'. There must be at least one supported encoding registered in order for the output formatter to write content. - No encoding found for input formatter '{0}'. There must be at least one supported encoding registered in order for the formatter to read content. diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/OutputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/OutputFormatterTests.cs index a398cd43ff..71abc7d07f 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/OutputFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/OutputFormatterTests.cs @@ -76,24 +76,31 @@ namespace Microsoft.AspNet.Mvc.Test } [Fact] - public void WriteResponseContentHeaders_FormatterWithNoEncoding_Throws() + public void WriteResponseContentHeaders_NoSupportedEncodings_NoEncodingIsSet() { // Arrange - var testFormatter = new TestOutputFormatter(); - var testContentType = MediaTypeHeaderValue.Parse("text/invalid"); - var formatterContext = new OutputFormatterContext(); - var mockHttpContext = new Mock(); - var httpRequest = new DefaultHttpContext().Request; - mockHttpContext.SetupGet(o => o.Request).Returns(httpRequest); - formatterContext.HttpContext = mockHttpContext.Object; + var formatter = new TestOutputFormatter(); - // Act & Assert - var ex = Assert.Throws( - () => testFormatter.WriteResponseHeaders(formatterContext)); - Assert.Equal("No encoding found for output formatter " + - "'Microsoft.AspNet.Mvc.Test.OutputFormatterTests+TestOutputFormatter'." + - " There must be at least one supported encoding registered in order for the" + - " output formatter to write content.", ex.Message); + var testContentType = MediaTypeHeaderValue.Parse("text/json"); + + formatter.SupportedEncodings.Clear(); + formatter.SupportedMediaTypes.Clear(); + formatter.SupportedMediaTypes.Add(testContentType); + + var formatterContext = new OutputFormatterContext() + { + HttpContext = new DefaultHttpContext(), + }; + + // Act + formatter.WriteResponseHeaders(formatterContext); + + // Assert + Assert.Null(formatterContext.SelectedEncoding); + Assert.Equal(testContentType, formatterContext.SelectedContentType); + + // If we had set an encoding, it would be part of the content type header + Assert.Equal(testContentType, formatterContext.HttpContext.Response.GetTypedHeaders().ContentType); } [Fact]