Skip to content

Commit 397f924

Browse files
pranavkmpranavkm
authored
Ensure SystemTextJsonHelper always HTML encodes output. (#12808)
* Ensure JsonSerializer always HTML encodes output. * Update JsonOptions.JsonSerializerOptions to use encoder scheme that does not encode non-ASCII characters by default. This makes the encoding comparable to Json.NET's defaults * In SystemTextJsonHelper, ensure that the content is always HTML-encoded * Unskip skipped test Fixes #9946 Fixes #11459
1 parent 709b390 commit 397f924

18 files changed

+248
-75
lines changed

‎src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2210,7 +2210,7 @@ public SystemTextJsonInputFormatter(Microsoft.AspNetCore.Mvc.JsonOptions options
22102210
}
22112211
public partial class SystemTextJsonOutputFormatter : Microsoft.AspNetCore.Mvc.Formatters.TextOutputFormatter
22122212
{
2213-
public SystemTextJsonOutputFormatter(Microsoft.AspNetCore.Mvc.JsonOptions options) { }
2213+
public SystemTextJsonOutputFormatter(System.Text.Json.JsonSerializerOptions jsonSerializerOptions) { }
22142214
public System.Text.Json.JsonSerializerOptions SerializerOptions { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
22152215
[System.Diagnostics.DebuggerStepThroughAttribute]
22162216
public sealed override System.Threading.Tasks.Task WriteResponseBodyAsync(Microsoft.AspNetCore.Mvc.Formatters.OutputFormatterWriteContext context, System.Text.Encoding selectedEncoding) { throw null; }

‎src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.IO;
66
using System.Text;
7+
using System.Text.Encodings.Web;
78
using System.Text.Json;
89
using System.Threading;
910
using System.Threading.Tasks;
@@ -17,14 +18,13 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
1718
/// </summary>
1819
public class SystemTextJsonOutputFormatter : TextOutputFormatter
1920
{
20-
2121
/// <summary>
2222
/// Initializes a new <see cref="SystemTextJsonOutputFormatter"/> instance.
2323
/// </summary>
24-
/// <param name="options">The <see cref="JsonOptions"/>.</param>
25-
public SystemTextJsonOutputFormatter(JsonOptions options)
24+
/// <param name="jsonSerializerOptions">The <see cref="JsonSerializerOptions"/>.</param>
25+
public SystemTextJsonOutputFormatter(JsonSerializerOptions jsonSerializerOptions)
2626
{
27-
SerializerOptions = options.JsonSerializerOptions;
27+
SerializerOptions = jsonSerializerOptions;
2828

2929
SupportedEncodings.Add(Encoding.UTF8);
3030
SupportedEncodings.Add(Encoding.Unicode);
@@ -33,6 +33,19 @@ public SystemTextJsonOutputFormatter(JsonOptions options)
3333
SupportedMediaTypes.Add(MediaTypeHeaderValues.ApplicationAnyJsonSyntax);
3434
}
3535

36+
internal static SystemTextJsonOutputFormatter CreateFormatter(JsonOptions jsonOptions)
37+
{
38+
var jsonSerializerOptions = jsonOptions.JsonSerializerOptions;
39+
40+
if (jsonSerializerOptions.Encoder is null)
41+
{
42+
// If the user hasn't explicitly configured the encoder, use the less strict encoder that does not encode all non-ASCII characters.
43+
jsonSerializerOptions = jsonSerializerOptions.Copy(JavaScriptEncoder.UnsafeRelaxedJsonEscaping);
44+
}
45+
46+
return new SystemTextJsonOutputFormatter(jsonSerializerOptions);
47+
}
48+
3649
/// <summary>
3750
/// Gets the <see cref="JsonSerializerOptions"/> used to configure the <see cref="JsonSerializer"/>.
3851
/// </summary>

‎src/Mvc/Mvc.Core/src/Infrastructure/JsonSerializerOptionsCopyConstructor.cs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System.Text.Encodings.Web;
5+
6+
namespace System.Text.Json
7+
{
8+
internal static class JsonSerializerOptionsCopyConstructor
9+
{
10+
public static JsonSerializerOptions Copy(this JsonSerializerOptions serializerOptions, JavaScriptEncoder encoder)
11+
{
12+
var copiedOptions = new JsonSerializerOptions
13+
{
14+
AllowTrailingCommas = serializerOptions.AllowTrailingCommas,
15+
DefaultBufferSize = serializerOptions.DefaultBufferSize,
16+
DictionaryKeyPolicy = serializerOptions.DictionaryKeyPolicy,
17+
IgnoreNullValues = serializerOptions.IgnoreNullValues,
18+
IgnoreReadOnlyProperties = serializerOptions.IgnoreReadOnlyProperties,
19+
MaxDepth = serializerOptions.MaxDepth,
20+
PropertyNameCaseInsensitive = serializerOptions.PropertyNameCaseInsensitive,
21+
PropertyNamingPolicy = serializerOptions.PropertyNamingPolicy,
22+
ReadCommentHandling = serializerOptions.ReadCommentHandling,
23+
WriteIndented = serializerOptions.WriteIndented
24+
};
25+
26+
for (var i = 0; i < serializerOptions.Converters.Count; i++)
27+
{
28+
copiedOptions.Converters.Add(serializerOptions.Converters[i]);
29+
}
30+
31+
copiedOptions.Encoder = encoder;
32+
33+
return copiedOptions;
34+
}
35+
}
36+
}

‎src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ public void Configure(MvcOptions options)
8787
options.OutputFormatters.Add(new HttpNoContentOutputFormatter());
8888
options.OutputFormatters.Add(new StringOutputFormatter());
8989
options.OutputFormatters.Add(new StreamOutputFormatter());
90-
options.OutputFormatters.Add(new SystemTextJsonOutputFormatter(_jsonOptions.Value));
90+
91+
var jsonOutputFormatter = SystemTextJsonOutputFormatter.CreateFormatter(_jsonOptions.Value);
92+
options.OutputFormatters.Add(jsonOutputFormatter);
9193

9294
// Set up ValueProviders
9395
options.ValueProviderFactories.Add(new FormValueProviderFactory());

‎src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ private static IServiceProvider CreateServices()
9191
{
9292
var options = Options.Create(new MvcOptions());
9393
options.Value.OutputFormatters.Add(new StringOutputFormatter());
94-
options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new JsonOptions()));
94+
options.Value.OutputFormatters.Add(SystemTextJsonOutputFormatter.CreateFormatter(new JsonOptions()));
9595

9696
var services = new ServiceCollection();
9797
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(

‎src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ private static IServiceProvider CreateServices()
104104
{
105105
var options = Options.Create(new MvcOptions());
106106
options.Value.OutputFormatters.Add(new StringOutputFormatter());
107-
options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new JsonOptions()));
107+
options.Value.OutputFormatters.Add(SystemTextJsonOutputFormatter.CreateFormatter(new JsonOptions()));
108108

109109
var services = new ServiceCollection();
110110
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(

‎src/Mvc/Mvc.Core/test/CreatedResultTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ private static IServiceProvider CreateServices()
9292
{
9393
var options = Options.Create(new MvcOptions());
9494
options.Value.OutputFormatters.Add(new StringOutputFormatter());
95-
options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new JsonOptions()));
95+
options.Value.OutputFormatters.Add(SystemTextJsonOutputFormatter.CreateFormatter(new JsonOptions()));
9696

9797
var services = new ServiceCollection();
9898
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(

‎src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ private void Initialize(
465465
// Set up default output formatters.
466466
MvcOptions.OutputFormatters.Add(new HttpNoContentOutputFormatter());
467467
MvcOptions.OutputFormatters.Add(new StringOutputFormatter());
468-
MvcOptions.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new JsonOptions()));
468+
MvcOptions.OutputFormatters.Add(SystemTextJsonOutputFormatter.CreateFormatter(new JsonOptions()));
469469

470470
// Set up default mapping for json extensions to content type
471471
MvcOptions.FormatterMappings.SetMediaTypeMappingForFormat(

‎src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,71 @@ public static TheoryData<string, string, bool> WriteCorrectCharacterEncoding
7676
}
7777
}
7878

79+
[Theory]
80+
[MemberData(nameof(WriteCorrectCharacterEncoding))]
81+
public async Task WriteToStreamAsync_UsesCorrectCharacterEncoding(
82+
string content,
83+
string encodingAsString,
84+
bool isDefaultEncoding)
85+
{
86+
// Arrange
87+
var formatter = GetOutputFormatter();
88+
var expectedContent = "\"" + content + "\"";
89+
var mediaType = MediaTypeHeaderValue.Parse(string.Format("application/json; charset={0}", encodingAsString));
90+
var encoding = CreateOrGetSupportedEncoding(formatter, encodingAsString, isDefaultEncoding);
91+
92+
93+
var body = new MemoryStream();
94+
var actionContext = GetActionContext(mediaType, body);
95+
96+
var outputFormatterContext = new OutputFormatterWriteContext(
97+
actionContext.HttpContext,
98+
new TestHttpResponseStreamWriterFactory().CreateWriter,
99+
typeof(string),
100+
content)
101+
{
102+
ContentType = new StringSegment(mediaType.ToString()),
103+
};
104+
105+
// Act
106+
await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding(encodingAsString));
107+
108+
// Assert
109+
var actualContent = encoding.GetString(body.ToArray());
110+
Assert.Equal(expectedContent, actualContent, StringComparer.OrdinalIgnoreCase);
111+
}
112+
113+
[Fact]
114+
public async Task WriteResponseBodyAsync_Encodes()
115+
{
116+
// Arrange
117+
var formatter = GetOutputFormatter();
118+
var expectedContent = "{\"key\":\"Hello \\n <b>Wörld</b>\"}";
119+
var content = new { key = "Hello \n <b>Wörld</b>" };
120+
121+
var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8");
122+
var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true);
123+
124+
var body = new MemoryStream();
125+
var actionContext = GetActionContext(mediaType, body);
126+
127+
var outputFormatterContext = new OutputFormatterWriteContext(
128+
actionContext.HttpContext,
129+
new TestHttpResponseStreamWriterFactory().CreateWriter,
130+
typeof(string),
131+
content)
132+
{
133+
ContentType = new StringSegment(mediaType.ToString()),
134+
};
135+
136+
// Act
137+
await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8"));
138+
139+
// Assert
140+
var actualContent = encoding.GetString(body.ToArray());
141+
Assert.Equal(expectedContent, actualContent);
142+
}
143+
79144
[Fact]
80145
public async Task ErrorDuringSerialization_DoesNotCloseTheBrackets()
81146
{

‎src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs

Lines changed: 1 addition & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,13 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
5-
using System.IO;
6-
using System.Text;
7-
using System.Text.Encodings.Web;
8-
using System.Threading.Tasks;
9-
using Microsoft.Extensions.Primitives;
10-
using Microsoft.Net.Http.Headers;
11-
using Xunit;
12-
134
namespace Microsoft.AspNetCore.Mvc.Formatters
145
{
156
public class SystemTextJsonOutputFormatterTest : JsonOutputFormatterTestBase
167
{
178
protected override TextOutputFormatter GetOutputFormatter()
189
{
19-
return new SystemTextJsonOutputFormatter(new JsonOptions());
20-
}
21-
22-
[Theory]
23-
[MemberData(nameof(WriteCorrectCharacterEncoding))]
24-
public async Task WriteToStreamAsync_UsesCorrectCharacterEncoding(
25-
string content,
26-
string encodingAsString,
27-
bool isDefaultEncoding)
28-
{
29-
// Arrange
30-
var formatter = GetOutputFormatter();
31-
var expectedContent = "\"" + JavaScriptEncoder.Default.Encode(content) + "\"";
32-
var mediaType = MediaTypeHeaderValue.Parse(string.Format("application/json; charset={0}", encodingAsString));
33-
var encoding = CreateOrGetSupportedEncoding(formatter, encodingAsString, isDefaultEncoding);
34-
35-
36-
var body = new MemoryStream();
37-
var actionContext = GetActionContext(mediaType, body);
38-
39-
var outputFormatterContext = new OutputFormatterWriteContext(
40-
actionContext.HttpContext,
41-
new TestHttpResponseStreamWriterFactory().CreateWriter,
42-
typeof(string),
43-
content)
44-
{
45-
ContentType = new StringSegment(mediaType.ToString()),
46-
};
47-
48-
// Act
49-
await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding(encodingAsString));
50-
51-
// Assert
52-
var actualContent = encoding.GetString(body.ToArray());
53-
Assert.Equal(expectedContent, actualContent, StringComparer.OrdinalIgnoreCase);
10+
return SystemTextJsonOutputFormatter.CreateFormatter(new JsonOptions());
5411
}
5512
}
5613
}

‎src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ private static IServiceProvider CreateServices()
6969
{
7070
var options = Options.Create(new MvcOptions());
7171
options.Value.OutputFormatters.Add(new StringOutputFormatter());
72-
options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new JsonOptions()));
72+
options.Value.OutputFormatters.Add(SystemTextJsonOutputFormatter.CreateFormatter(new JsonOptions()));
7373

7474
var services = new ServiceCollection();
7575
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(

‎src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private static IServiceProvider CreateServices()
7070
{
7171
var options = Options.Create(new MvcOptions());
7272
options.Value.OutputFormatters.Add(new StringOutputFormatter());
73-
options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new JsonOptions()));
73+
options.Value.OutputFormatters.Add(SystemTextJsonOutputFormatter.CreateFormatter(new JsonOptions()));
7474

7575
var services = new ServiceCollection();
7676
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(

‎src/Mvc/Mvc.ViewFeatures/src/Rendering/SystemTextJsonHelper.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Copyright (c) .NET Foundation. All rights reserved.
33
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
44

5+
using System.Text.Encodings.Web;
56
using System.Text.Json;
67
using Microsoft.AspNetCore.Html;
78
using Microsoft.Extensions.Options;
@@ -10,20 +11,30 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
1011
{
1112
internal class SystemTextJsonHelper : IJsonHelper
1213
{
13-
private readonly JsonOptions _options;
14+
private readonly JsonSerializerOptions _htmlSafeJsonSerializerOptions;
1415

1516
public SystemTextJsonHelper(IOptions<JsonOptions> options)
1617
{
17-
_options = options.Value;
18+
_htmlSafeJsonSerializerOptions = GetHtmlSafeSerializerOptions(options.Value.JsonSerializerOptions);
1819
}
1920

2021
/// <inheritdoc />
2122
public IHtmlContent Serialize(object value)
2223
{
2324
// JsonSerializer always encodes non-ASCII chars, so we do not need
2425
// to do anything special with the SerializerOptions
25-
var json = JsonSerializer.Serialize(value, _options.JsonSerializerOptions);
26+
var json = JsonSerializer.Serialize(value, _htmlSafeJsonSerializerOptions);
2627
return new HtmlString(json);
2728
}
29+
30+
private static JsonSerializerOptions GetHtmlSafeSerializerOptions(JsonSerializerOptions serializerOptions)
31+
{
32+
if (serializerOptions.Encoder is null || serializerOptions.Encoder == JavaScriptEncoder.Default)
33+
{
34+
return serializerOptions;
35+
}
36+
37+
return serializerOptions.Copy(JavaScriptEncoder.Default);
38+
}
2839
}
2940
}

‎src/Mvc/Mvc.ViewFeatures/test/Rendering/JsonHelperTestBase.cs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public virtual void Serialize_EscapesHtmlByDefault()
2525

2626
// Assert
2727
var htmlString = Assert.IsType<HtmlString>(result);
28-
Assert.Equal(expectedOutput, htmlString.ToString());
28+
Assert.Equal(expectedOutput, htmlString.ToString(), ignoreCase: true);
2929
}
3030

3131
[Fact]
@@ -71,14 +71,14 @@ public virtual void Serialize_WithNonAsciiChars()
7171
{
7272
HTML = $"Hello pingüino"
7373
};
74-
var expectedOutput = "{\"html\":\"Hello ping\\u00fcino\"}";
74+
var expectedOutput = "{\"html\":\"Hello pingüino\"}";
7575

7676
// Act
7777
var result = helper.Serialize(obj);
7878

7979
// Assert
8080
var htmlString = Assert.IsType<HtmlString>(result);
81-
Assert.Equal(expectedOutput, htmlString.ToString());
81+
Assert.Equal(expectedOutput, htmlString.ToString(), ignoreCase: true);
8282
}
8383

8484
[Fact]
@@ -99,5 +99,25 @@ public void Serialize_WithHTMLEntities()
9999
var htmlString = Assert.IsType<HtmlString>(result);
100100
Assert.Equal(expectedOutput, htmlString.ToString());
101101
}
102+
103+
104+
[Fact]
105+
public virtual void Serialize_WithHTMLNonAsciiAndControlChars()
106+
{
107+
// Arrange
108+
var helper = GetJsonHelper();
109+
var obj = new
110+
{
111+
HTML = "<b>Hello \n pingüino</b>"
112+
};
113+
var expectedOutput = "{\"html\":\"\\u003cb\\u003eHello \\n pingüino\\u003c/b\\u003e\"}";
114+
115+
// Act
116+
var result = helper.Serialize(obj);
117+
118+
// Assert
119+
var htmlString = Assert.IsType<HtmlString>(result);
120+
Assert.Equal(expectedOutput, htmlString.ToString());
121+
}
102122
}
103123
}

0 commit comments

Comments
 (0)