Skip to content

Commit 3ac5592

Browse files
committed
Address review feedback
1 parent 2b9a5ec commit 3ac5592

File tree

5 files changed

+234
-50
lines changed

5 files changed

+234
-50
lines changed

src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ internal static class JsonConstants
6767
// When transcoding from UTF8 -> UTF16, the byte count threshold where we rent from the array pool before performing a normal alloc.
6868
public const long ArrayPoolMaxSizeBeforeUsingNormalAlloc = 1024 * 1024;
6969

70+
public const int MaxRawValueLength = int.MaxValue / MaxExpansionFactorWhileTranscoding;
71+
7072
public const int MaxEscapedTokenSize = 1_000_000_000; // Max size for already escaped value.
7173
public const int MaxUnescapedTokenSize = MaxEscapedTokenSize / MaxExpansionFactorWhileEscaping; // 166_666_666 bytes
7274
public const int MaxBase64ValueTokenSize = (MaxEscapedTokenSize >> 2) * 3 / MaxExpansionFactorWhileEscaping; // 125_000_000 bytes

src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Raw.cs

Lines changed: 98 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,93 @@ namespace System.Text.Json
88
public sealed partial class Utf8JsonWriter
99
{
1010
/// <summary>
11-
/// Writes the input as JSON content.
11+
/// Writes the input as JSON content. It is expected that the input content is a single complete JSON value.
1212
/// </summary>
1313
/// <param name="json">The raw JSON content to write.</param>
14-
/// <param name="skipInputValidation">Whether to skip validation of the input JSON content.</param>
14+
/// <param name="skipInputValidation">Whether to validate if the input is an RFC 8259-compliant JSON payload.</param>
15+
/// <exception cref="ArgumentNullException">Thrown if <paramref name="json"/> is <see langword="null"/>.</exception>
16+
/// <exception cref="ArgumentException">Thrown if the length of the input is zero or greater than 715,827,882.</exception>
17+
/// <exception cref="JsonException">Thrown if <paramref name="skipInputValidation"/> is <see langword="false"/>, and the input is not RFC 8259-compliant.</exception>
18+
/// <remarks>
19+
/// When writing untrused JSON values, do not set <paramref name="skipInputValidation"/> to <see langword="true"/> as this can result in invalid JSON
20+
/// being written, and/or the overall payload being written to the writer instance being invalid.
21+
///
22+
/// When using this method, the input content will be written to the writer destination as-is, unless validation fails.
23+
///
24+
/// The <see cref="JsonWriterOptions.SkipValidation"/> value for the writer instance is honored when using this method.
25+
///
26+
/// The <see cref="JsonWriterOptions.Indented"/> and <see cref="JsonWriterOptions.Encoder"/> values for the writer instance are not applied when using this method.
27+
/// </remarks>
1528
public void WriteRawValue(string json, bool skipInputValidation = false)
1629
{
30+
if (!_options.SkipValidation)
31+
{
32+
ValidateWritingValue();
33+
}
34+
1735
if (json == null)
1836
{
1937
throw new ArgumentNullException(nameof(json));
2038
}
2139

22-
WriteRawValue(json.AsSpan(), skipInputValidation);
40+
TranscodeAndWriteRawValue(json.AsSpan(), skipInputValidation);
2341
}
2442

2543
/// <summary>
26-
/// Writes the input as JSON content.
44+
/// Writes the input as JSON content. It is expected that the input content is a single complete JSON value.
2745
/// </summary>
2846
/// <param name="json">The raw JSON content to write.</param>
29-
/// <param name="skipInputValidation">Whether to skip validation of the input JSON content.</param>
47+
/// <param name="skipInputValidation">Whether to validate if the input is an RFC 8259-compliant JSON payload.</param>
48+
/// <exception cref="ArgumentException">Thrown if the length of the input is zero or greater than 715,827,882.</exception>
49+
/// <exception cref="JsonException">Thrown if <paramref name="skipInputValidation"/> is <see langword="false"/>, and the input is not RFC 8259-compliant.</exception>
50+
/// <remarks>
51+
/// When writing untrused JSON values, do not set <paramref name="skipInputValidation"/> to <see langword="true"/> as this can result in invalid JSON
52+
/// being written, and/or the overall payload being written to the writer instance being invalid.
53+
///
54+
/// When using this method, the input content will be written to the writer destination as-is, unless validation fails.
55+
///
56+
/// The <see cref="JsonWriterOptions.SkipValidation"/> value for the writer instance is honored when using this method.
57+
///
58+
/// The <see cref="JsonWriterOptions.Indented"/> and <see cref="JsonWriterOptions.Encoder"/> values for the writer instance are not applied when using this method.
59+
/// </remarks>
3060
public void WriteRawValue(ReadOnlySpan<char> json, bool skipInputValidation = false)
61+
{
62+
if (!_options.SkipValidation)
63+
{
64+
ValidateWritingValue();
65+
}
66+
67+
TranscodeAndWriteRawValue(json, skipInputValidation);
68+
}
69+
70+
/// <summary>
71+
/// Writes the input as JSON content. It is expected that the input content is a single complete JSON value.
72+
/// </summary>
73+
/// <param name="utf8Json">The raw JSON content to write.</param>
74+
/// <param name="skipInputValidation">Whether to validate if the input is an RFC 8259-compliant JSON payload.</param>
75+
/// <exception cref="ArgumentException">Thrown if the length of the input is zero or greater than 715,827,882.</exception>
76+
/// <exception cref="JsonException">Thrown if <paramref name="skipInputValidation"/> is <see langword="false"/>, and the input is not RFC 8259-compliant.</exception>
77+
/// <remarks>
78+
/// When writing untrused JSON values, do not set <paramref name="skipInputValidation"/> to <see langword="true"/> as this can result in invalid JSON
79+
/// being written, and/or the overall payload being written to the writer instance being invalid.
80+
///
81+
/// When using this method, the input content will be written to the writer destination as-is, unless validation fails.
82+
///
83+
/// The <see cref="JsonWriterOptions.SkipValidation"/> value for the writer instance is honored when using this method.
84+
///
85+
/// The <see cref="JsonWriterOptions.Indented"/> and <see cref="JsonWriterOptions.Encoder"/> values for the writer instance are not applied when using this method.
86+
/// </remarks>
87+
public void WriteRawValue(ReadOnlySpan<byte> utf8Json, bool skipInputValidation = false)
88+
{
89+
if (!_options.SkipValidation)
90+
{
91+
ValidateWritingValue();
92+
}
93+
94+
WriteRawValueInternal(utf8Json, skipInputValidation);
95+
}
96+
97+
private void TranscodeAndWriteRawValue(ReadOnlySpan<char> json, bool skipInputValidation)
3198
{
3299
byte[]? tempArray = null;
33100

@@ -55,33 +122,40 @@ public void WriteRawValue(ReadOnlySpan<char> json, bool skipInputValidation = fa
55122
}
56123
}
57124

58-
/// <summary>
59-
/// Writes the input as JSON content.
60-
/// </summary>
61-
/// <param name="utf8Json">The raw JSON content to write.</param>
62-
/// <param name="skipInputValidation">Whether to skip validation of the input JSON content.</param>
63-
public void WriteRawValue(ReadOnlySpan<byte> utf8Json, bool skipInputValidation = false)
125+
private void WriteRawValueInternal(ReadOnlySpan<byte> utf8Json, bool skipInputValidation)
64126
{
65-
if (utf8Json.Length == 0)
127+
int len = utf8Json.Length;
128+
129+
if (len == 0)
66130
{
67131
ThrowHelper.ThrowArgumentException(SR.ExpectedJsonTokens);
68132
}
69-
70-
if (!skipInputValidation)
133+
else if (len > JsonConstants.MaxRawValueLength)
71134
{
72-
Utf8JsonReader reader = new Utf8JsonReader(utf8Json);
135+
ThrowHelper.ThrowArgumentException_ValueTooLarge(len);
136+
}
73137

74-
try
75-
{
76-
while (reader.Read());
77-
}
78-
catch (JsonReaderException ex)
138+
if (skipInputValidation)
139+
{
140+
// Treat all unvalidated raw JSON value writes as string. If the payload is valid, this approach does
141+
// not affect structural validation since a string token is equivalent to a complete object, array,
142+
// or other complete JSON tokens when considering structural validation on subsequent writer calls.
143+
// If the payload is not valid, then we make no guarantees about the structural validation of the final payload.
144+
_tokenType = JsonTokenType.String;
145+
}
146+
else
147+
{
148+
// Utilize reader validation.
149+
Utf8JsonReader reader = new(utf8Json);
150+
while (reader.Read())
79151
{
80-
ThrowHelper.ThrowArgumentException(ex.Message);
152+
_tokenType = reader.TokenType;
81153
}
82154
}
83155

84-
int maxRequired = utf8Json.Length + 1; // Optionally, 1 list separator
156+
// TODO (https://github.com/dotnet/runtime/issues/29293):
157+
// investigate writing this in chunks, rather than requesting one potentially long, contiguous buffer.
158+
int maxRequired = len + 1; // Optionally, 1 list separator
85159

86160
if (_memory.Length - BytesPending < maxRequired)
87161
{
@@ -96,12 +170,10 @@ public void WriteRawValue(ReadOnlySpan<byte> utf8Json, bool skipInputValidation
96170
}
97171

98172
utf8Json.CopyTo(output.Slice(BytesPending));
99-
BytesPending += utf8Json.Length;
173+
BytesPending += len;
100174

101-
SetFlagToAddListSeparatorBeforeNextItem();
102175

103-
// Treat all raw JSON value writes as string.
104-
_tokenType = JsonTokenType.String;
176+
SetFlagToAddListSeparatorBeforeNextItem();
105177
}
106178
}
107179
}

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,10 @@
186186
<Compile Include="Utf8JsonReaderTests.TryGet.Date.cs" />
187187
<Compile Include="Utf8JsonReaderTests.ValueTextEquals.cs" />
188188
<Compile Include="Utf8JsonWriterTests.cs" />
189+
<Compile Include="Utf8JsonWriterTests.WriteRaw.cs" />
189190
</ItemGroup>
190191
<ItemGroup>
191192
<Compile Include="..\..\src\System\Text\Json\BitStack.cs" Link="BitStack.cs" />
192-
<Compile Include="Utf8JsonWriterTests.WriteRaw.cs" />
193193
</ItemGroup>
194194
<ItemGroup Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)'">
195195
<Compile Include="$(CommonPath)System\Runtime\CompilerServices\IsExternalInit.cs" Link="Common\System\Runtime\CompilerServices\IsExternalInit.cs" />

0 commit comments

Comments
 (0)