From dd8d233f84bbaec18f9fb646825c0a24926aaa8e Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 26 Jan 2022 07:13:45 -0800 Subject: [PATCH] Avoid modifying the JavaScriptEncoder on SystemTextJsonOutputFormatter By default, STJ encodes most non-ASCII characters which is different from Newtonsoft.Json's defaults. When we first defaulted to STJ in 3.1, MVC attempted to minimize this difference by using a more compatible (unsafe-relaxed) encoding scheme if a user hadn't explicitly configured one via JsonOptions. As noted in https://github.com/dotnet/aspnetcore/issues/38720, this causes issues if a JsonSerializerContext is configured. This PR changes the output formatter to no longer change the JavaScriptEncoder. Users can manually configure the unsafe-relaxed encoding globally if they understand the consequences of doing so. Contributes to https://github.com/dotnet/aspnetcore/issues/38720 --- .../SystemTextJsonOutputFormatter.cs | 14 +- .../Formatters/JsonOutputFormatterTestBase.cs | 82 ------------ .../SystemTextJsonOutputFormatterTest.cs | 121 ++++++++++++++++++ .../test/NewtonsoftJsonOutputFormatterTest.cs | 81 ++++++++++++ .../JsonOutputFormatterTestBase.cs | 4 +- .../SystemTextJsonOutputFormatterTest.cs | 40 +++++- 6 files changed, 244 insertions(+), 98 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 54e14704e7e9..2e5662714056 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -3,7 +3,6 @@ using System.Runtime.ExceptionServices; using System.Text; -using System.Text.Encodings.Web; using System.Text.Json; namespace Microsoft.AspNetCore.Mvc.Formatters; @@ -30,18 +29,7 @@ public SystemTextJsonOutputFormatter(JsonSerializerOptions jsonSerializerOptions internal static SystemTextJsonOutputFormatter CreateFormatter(JsonOptions jsonOptions) { - var jsonSerializerOptions = jsonOptions.JsonSerializerOptions; - - if (jsonSerializerOptions.Encoder is null) - { - // If the user hasn't explicitly configured the encoder, use the less strict encoder that does not encode all non-ASCII characters. - jsonSerializerOptions = new JsonSerializerOptions(jsonSerializerOptions) - { - Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, - }; - } - - return new SystemTextJsonOutputFormatter(jsonSerializerOptions); + return new SystemTextJsonOutputFormatter(jsonOptions.JsonSerializerOptions); } /// diff --git a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs index 48663036ffce..881a64d9462f 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs @@ -56,88 +56,6 @@ public void CanWriteResult_ReturnsExpectedValueForMediaType( Assert.Equal(new StringSegment(expectedContentType), outputFormatterContext.ContentType); } - public static TheoryData WriteCorrectCharacterEncoding - { - get - { - var data = new TheoryData - { - { "This is a test 激光這兩個字是甚麼意思 string written using utf-8", "utf-8", true }, - { "This is a test 激光這兩個字是甚麼意思 string written using utf-16", "utf-16", true }, - { "This is a test 激光這兩個字是甚麼意思 string written using utf-32", "utf-32", false }, - { "This is a test æøå string written using iso-8859-1", "iso-8859-1", false }, - }; - - return data; - } - } - - [Theory] - [MemberData(nameof(WriteCorrectCharacterEncoding))] - public async Task WriteToStreamAsync_UsesCorrectCharacterEncoding( - string content, - string encodingAsString, - bool isDefaultEncoding) - { - // Arrange - var formatter = GetOutputFormatter(); - var expectedContent = "\"" + content + "\""; - var mediaType = MediaTypeHeaderValue.Parse(string.Format(CultureInfo.InvariantCulture, "application/json; charset={0}", encodingAsString)); - var encoding = CreateOrGetSupportedEncoding(formatter, encodingAsString, isDefaultEncoding); - - - var body = new MemoryStream(); - var actionContext = GetActionContext(mediaType, body); - - var outputFormatterContext = new OutputFormatterWriteContext( - actionContext.HttpContext, - new TestHttpResponseStreamWriterFactory().CreateWriter, - typeof(string), - content) - { - ContentType = new StringSegment(mediaType.ToString()), - }; - - // Act - await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding(encodingAsString)); - - // Assert - var actualContent = encoding.GetString(body.ToArray()); - Assert.Equal(expectedContent, actualContent, StringComparer.OrdinalIgnoreCase); - Assert.True(body.CanWrite, "Response body should not be disposed."); - } - - [Fact] - public async Task WriteResponseBodyAsync_Encodes() - { - // Arrange - var formatter = GetOutputFormatter(); - var expectedContent = "{\"key\":\"Hello \\n Wörld\"}"; - var content = new { key = "Hello \n Wörld" }; - - var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8"); - var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true); - - var body = new MemoryStream(); - var actionContext = GetActionContext(mediaType, body); - - var outputFormatterContext = new OutputFormatterWriteContext( - actionContext.HttpContext, - new TestHttpResponseStreamWriterFactory().CreateWriter, - typeof(string), - content) - { - ContentType = new StringSegment(mediaType.ToString()), - }; - - // Act - await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8")); - - // Assert - var actualContent = encoding.GetString(body.ToArray()); - Assert.Equal(expectedContent, actualContent); - } - [Fact] public async Task ErrorDuringSerialization_DoesNotCloseTheBrackets() { diff --git a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs index 71fd6d97ac52..91f790f7bdcb 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs @@ -3,6 +3,7 @@ using System.Runtime.CompilerServices; using System.Text; +using System.Text.Encodings.Web; using System.Text.Json; using System.Text.Json.Serialization; using Microsoft.Extensions.Primitives; @@ -54,6 +55,76 @@ public async Task WriteResponseBodyAsync_AllowsConfiguringPreserveReferenceHandl Assert.Equal(expectedContent, actualContent); } + [Fact] + public async Task WriteResponseBodyAsync_Encodes() + { + // Arrange + var formatter = GetOutputFormatter(); + + var expectedContent = "{\"key\":\"Hello \\n \\u003Cb\\u003EW\\u00F6rld\\u003C/b\\u003E\"}"; + var content = new { key = "Hello \n Wörld" }; + + var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8"); + var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true); + + var body = new MemoryStream(); + var actionContext = GetActionContext(mediaType, body); + + var outputFormatterContext = new OutputFormatterWriteContext( + actionContext.HttpContext, + new TestHttpResponseStreamWriterFactory().CreateWriter, + typeof(string), + content) + { + ContentType = new StringSegment(mediaType.ToString()), + }; + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8")); + + // Assert + var actualContent = encoding.GetString(body.ToArray()); + Assert.Equal(expectedContent, actualContent); + } + + [Fact] + public async Task WriteResponseBodyAsync_WithUnsafeRelaxedEncoding_Encodes() + { + // Arrange + var formatter = SystemTextJsonOutputFormatter.CreateFormatter(new() + { + JsonSerializerOptions = + { + Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, + }, + }); + + var expectedContent = "{\"key\":\"Hello \\n Wörld\"}"; + var content = new { key = "Hello \n Wörld" }; + + var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8"); + var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true); + + var body = new MemoryStream(); + var actionContext = GetActionContext(mediaType, body); + + var outputFormatterContext = new OutputFormatterWriteContext( + actionContext.HttpContext, + new TestHttpResponseStreamWriterFactory().CreateWriter, + typeof(string), + content) + { + ContentType = new StringSegment(mediaType.ToString()), + }; + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8")); + + // Assert + var actualContent = encoding.GetString(body.ToArray()); + Assert.Equal(expectedContent, actualContent); + } + [Fact] public async Task WriteResponseBodyAsync_WithNonUtf8Encoding_FormattingErrorsAreThrown() { @@ -156,6 +227,56 @@ async IAsyncEnumerable AsyncEnumerableClosedConnection([EnumeratorCancellat } } + public static TheoryData WriteCorrectCharacterEncoding + { + get + { + var data = new TheoryData + { + { "This is a test 激光這兩個字是甚麼意思 string written using utf-8", "utf-8", true }, + { "This is a test 激光這兩個字是甚麼意思 string written using utf-16", "utf-16", true }, + { "This is a test 激光這兩個字是甚麼意思 string written using utf-32", "utf-32", false }, + { "This is a test æøå string written using iso-8859-1", "iso-8859-1", false }, + }; + + return data; + } + } + + [Theory] + [MemberData(nameof(WriteCorrectCharacterEncoding))] + public async Task WriteToStreamAsync_UsesCorrectCharacterEncoding( + string content, + string encodingAsString, + bool isDefaultEncoding) + { + // Arrange + var formatter = GetOutputFormatter(); + var expectedContent = "\"" + JavaScriptEncoder.Default.Encode(content) + "\""; + var mediaType = MediaTypeHeaderValue.Parse($"application/json; charset={encodingAsString}"); + var encoding = CreateOrGetSupportedEncoding(formatter, encodingAsString, isDefaultEncoding); + + var body = new MemoryStream(); + var actionContext = GetActionContext(mediaType, body); + + var outputFormatterContext = new OutputFormatterWriteContext( + actionContext.HttpContext, + new TestHttpResponseStreamWriterFactory().CreateWriter, + typeof(string), + content) + { + ContentType = new StringSegment(mediaType.ToString()), + }; + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding(encodingAsString)); + + // Assert + var actualContent = encoding.GetString(body.ToArray()); + Assert.Equal(expectedContent, actualContent, StringComparer.OrdinalIgnoreCase); + Assert.True(body.CanWrite, "Response body should not be disposed."); + } + private class Person { public string Name { get; set; } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs index ceed0fca2c01..081c13d05a80 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs @@ -117,6 +117,37 @@ public async Task MvcJsonOptionsAreUsedToSetBufferThreshold() Assert.Equal(2, ((FileBufferingWriteStream)writeStream).MemoryThreshold); } + [Fact] + public async Task WriteResponseBodyAsync_Encodes() + { + // Arrange + var formatter = GetOutputFormatter(); + var expectedContent = "{\"key\":\"Hello \\n Wörld\"}"; + var content = new { key = "Hello \n Wörld" }; + + var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8"); + var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true); + + var body = new MemoryStream(); + var actionContext = GetActionContext(mediaType, body); + + var outputFormatterContext = new OutputFormatterWriteContext( + actionContext.HttpContext, + new TestHttpResponseStreamWriterFactory().CreateWriter, + typeof(string), + content) + { + ContentType = new StringSegment(mediaType.ToString()), + }; + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8")); + + // Assert + var actualContent = encoding.GetString(body.ToArray()); + Assert.Equal(expectedContent, actualContent); + } + [Fact] public async Task ChangesTo_SerializerSettings_AffectSerialization() { @@ -549,6 +580,56 @@ async IAsyncEnumerable AsyncEnumerableThrows([EnumeratorCancellation] Cance } } + public static TheoryData WriteCorrectCharacterEncoding + { + get + { + var data = new TheoryData + { + { "This is a test 激光這兩個字是甚麼意思 string written using utf-8", "utf-8", true }, + { "This is a test 激光這兩個字是甚麼意思 string written using utf-16", "utf-16", true }, + { "This is a test 激光這兩個字是甚麼意思 string written using utf-32", "utf-32", false }, + { "This is a test æøå string written using iso-8859-1", "iso-8859-1", false }, + }; + + return data; + } + } + + [Theory] + [MemberData(nameof(WriteCorrectCharacterEncoding))] + public async Task WriteToStreamAsync_UsesCorrectCharacterEncoding( + string content, + string encodingAsString, + bool isDefaultEncoding) + { + // Arrange + var formatter = GetOutputFormatter(); + var expectedContent = "\"" + content + "\""; + var mediaType = MediaTypeHeaderValue.Parse($"application/json; charset={encodingAsString}"); + var encoding = CreateOrGetSupportedEncoding(formatter, encodingAsString, isDefaultEncoding); + + var body = new MemoryStream(); + var actionContext = GetActionContext(mediaType, body); + + var outputFormatterContext = new OutputFormatterWriteContext( + actionContext.HttpContext, + new TestHttpResponseStreamWriterFactory().CreateWriter, + typeof(string), + content) + { + ContentType = new StringSegment(mediaType.ToString()), + }; + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding(encodingAsString)); + + // Assert + var actualContent = encoding.GetString(body.ToArray()); + Assert.Equal(expectedContent, actualContent, StringComparer.OrdinalIgnoreCase); + Assert.True(body.CanWrite, "Response body should not be disposed."); + } + private class TestableJsonOutputFormatter : NewtonsoftJsonOutputFormatter { public TestableJsonOutputFormatter(JsonSerializerSettings serializerSettings) diff --git a/src/Mvc/test/Mvc.FunctionalTests/JsonOutputFormatterTestBase.cs b/src/Mvc/test/Mvc.FunctionalTests/JsonOutputFormatterTestBase.cs index cb6c44c72723..cf86b693eb9e 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/JsonOutputFormatterTestBase.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/JsonOutputFormatterTestBase.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Net; @@ -105,7 +105,7 @@ public virtual async Task Formatting_StringValueWithNonAsciiCharacters() // Assert await response.AssertStatusCodeAsync(HttpStatusCode.OK); - Assert.Equal("\"Une bête de cirque\"", await response.Content.ReadAsStringAsync()); + Assert.Equal("\"Une b\\u00EAte de cirque\"", await response.Content.ReadAsStringAsync()); } [Fact] diff --git a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs index 9d333536a7d5..ae3885367617 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs @@ -2,7 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Net; +using System.Net.Http; +using System.Net.Http.Headers; +using System.Text; using System.Text.Encodings.Web; +using System.Text.Json; using FormatterWebSite.Controllers; using Microsoft.Extensions.DependencyInjection; @@ -16,7 +20,41 @@ public SystemTextJsonOutputFormatterTest(MvcTestFixture base.SerializableErrorIsReturnedInExpectedFormat(); + public override async Task SerializableErrorIsReturnedInExpectedFormat() + { + // Arrange + var input = "" + + "" + + "2foo"; + + var expectedOutput = "{\"Id\":[\"The field Id must be between 10 and 100." + + "\"],\"Name\":[\"The field Name must be a string or array type with" + + " a minimum length of \\u002715\\u0027.\"]}"; + var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/SerializableError/CreateEmployee"); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json")); + request.Content = new StringContent(input, Encoding.UTF8, "application/xml"); + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + + var actualContent = await response.Content.ReadAsStringAsync(); + Assert.Equal(expectedOutput, actualContent); + + var modelStateErrors = JsonSerializer.Deserialize>(actualContent); + Assert.Equal(2, modelStateErrors.Count); + + var errors = Assert.Single(modelStateErrors, kvp => kvp.Key == "Id").Value; + + var error = Assert.Single(errors); + Assert.Equal("The field Id must be between 10 and 100.", error); + + errors = Assert.Single(modelStateErrors, kvp => kvp.Key == "Name").Value; + error = Assert.Single(errors); + Assert.Equal("The field Name must be a string or array type with a minimum length of '15'.", error); + } [Fact] public override async Task Formatting_StringValueWithUnicodeContent()