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 dotnet#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 dotnet#38720
pranavkm · Jan 26, 2022 · dd8d233 · dd8d233
1 parent 98bb246
commit dd8d233
Show file tree

Hide file tree

Showing 6 changed files with 244 additions and 98 deletions.
diff --git 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);
     }
 
     /// <summary>

diff --git 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<string, string, bool> WriteCorrectCharacterEncoding
-    {
-        get
-        {
-            var data = new TheoryData<string, string, bool>
-                {
-                    { "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 <b>Wörld</b>\"}";
-        var content = new { key = "Hello \n <b>Wörld</b>" };
-
-        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
@@ -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 <b>Wörld</b>" };
+
+        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 <b>Wörld</b>\"}";
+        var content = new { key = "Hello \n <b>Wörld</b>" };
+
+        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<int> AsyncEnumerableClosedConnection([EnumeratorCancellat
         }
     }
 
+    public static TheoryData<string, string, bool> WriteCorrectCharacterEncoding
+    {
+        get
+        {
+            var data = new TheoryData<string, string, bool>
+            {
+                { "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
@@ -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 <b>Wörld</b>\"}";
+        var content = new { key = "Hello \n <b>Wörld</b>" };
+
+        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<int> AsyncEnumerableThrows([EnumeratorCancellation] Cance
         }
     }
 
+    public static TheoryData<string, string, bool> WriteCorrectCharacterEncoding
+    {
+        get
+        {
+            var data = new TheoryData<string, string, bool>
+            {
+                { "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
@@ -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]