Remove Base64 value length limit from Utf8JsonWriter (#85334)

* Remove Base64 value length limit from Utf8JsonWriter

* Address PR feedback by adding some more asserts

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs
index bfe99c3..cbdaaed 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs
@@ -70,7 +70,6 @@ namespace System.Text.Json
 
         public const int MaxEscapedTokenSize = 1_000_000_000;   // Max size for already escaped value.
         public const int MaxUnescapedTokenSize = MaxEscapedTokenSize / MaxExpansionFactorWhileEscaping;  // 166_666_666 bytes
-        public const int MaxBase64ValueTokenSize = (MaxEscapedTokenSize >> 2) * 3 / MaxExpansionFactorWhileEscaping;  // 125_000_000 bytes
         public const int MaxCharacterTokenSize = MaxEscapedTokenSize / MaxExpansionFactorWhileEscaping; // 166_666_666 characters
 
         public const int MaximumFormatBooleanLength = 5;

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
index 7537f72..417ed4c 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
@@ -88,6 +88,12 @@ namespace System.Text.Json
         }
 
         [DoesNotReturn]
+        public static void ThrowPropertyNameTooLargeArgumentException(int length)
+        {
+            throw GetArgumentException(SR.Format(SR.PropertyNameTooLarge, length));
+        }
+
+        [DoesNotReturn]
         public static void ThrowArgumentException(ReadOnlySpan<byte> propertyName, ReadOnlySpan<byte> value)
         {
             if (propertyName.Length > JsonConstants.MaxUnescapedTokenSize)

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs
index ffb522c..9271bb4 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs
@@ -47,13 +47,6 @@ namespace System.Text.Json
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static void ValidateBytes(ReadOnlySpan<byte> bytes)
-        {
-            if (bytes.Length > JsonConstants.MaxBase64ValueTokenSize)
-                ThrowHelper.ThrowArgumentException_ValueTooLarge(bytes.Length);
-        }
-
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static void ValidateDouble(double value)
         {
             if (!JsonHelpers.IsFinite(value))
@@ -114,17 +107,17 @@ namespace System.Text.Json
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static void ValidatePropertyAndBytes(ReadOnlySpan<char> propertyName, ReadOnlySpan<byte> bytes)
+        public static void ValidatePropertyNameLength(ReadOnlySpan<char> propertyName)
         {
-            if (propertyName.Length > JsonConstants.MaxCharacterTokenSize || bytes.Length > JsonConstants.MaxBase64ValueTokenSize)
-                ThrowHelper.ThrowArgumentException(propertyName, bytes);
+            if (propertyName.Length > JsonConstants.MaxCharacterTokenSize)
+                ThrowHelper.ThrowPropertyNameTooLargeArgumentException(propertyName.Length);
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static void ValidatePropertyAndBytes(ReadOnlySpan<byte> propertyName, ReadOnlySpan<byte> bytes)
+        public static void ValidatePropertyNameLength(ReadOnlySpan<byte> propertyName)
         {
-            if (propertyName.Length > JsonConstants.MaxUnescapedTokenSize || bytes.Length > JsonConstants.MaxBase64ValueTokenSize)
-                ThrowHelper.ThrowArgumentException(propertyName, bytes);
+            if (propertyName.Length > JsonConstants.MaxUnescapedTokenSize)
+                ThrowHelper.ThrowPropertyNameTooLargeArgumentException(propertyName.Length);
         }
 
         internal static void ValidateNumber(ReadOnlySpan<byte> utf8FormattedNumber)

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Bytes.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Bytes.cs
index 1c95db2..9c4785e 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Bytes.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Bytes.cs
@@ -22,8 +22,6 @@ namespace System.Text.Json
             ReadOnlySpan<byte> utf8PropertyName = propertyName.EncodedUtf8Bytes;
             Debug.Assert(utf8PropertyName.Length <= JsonConstants.MaxUnescapedTokenSize);
 
-            JsonWriterHelper.ValidateBytes(bytes);
-
             WriteBase64ByOptions(utf8PropertyName, bytes);
 
             SetFlagToAddListSeparatorBeforeNextItem();
@@ -72,7 +70,7 @@ namespace System.Text.Json
         /// </remarks>
         public void WriteBase64String(ReadOnlySpan<char> propertyName, ReadOnlySpan<byte> bytes)
         {
-            JsonWriterHelper.ValidatePropertyAndBytes(propertyName, bytes);
+            JsonWriterHelper.ValidatePropertyNameLength(propertyName);
 
             WriteBase64Escape(propertyName, bytes);
 
@@ -96,7 +94,7 @@ namespace System.Text.Json
         /// </remarks>
         public void WriteBase64String(ReadOnlySpan<byte> utf8PropertyName, ReadOnlySpan<byte> bytes)
         {
-            JsonWriterHelper.ValidatePropertyAndBytes(utf8PropertyName, bytes);
+            JsonWriterHelper.ValidatePropertyNameLength(utf8PropertyName);
 
             WriteBase64Escape(utf8PropertyName, bytes);
 

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Bytes.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Bytes.cs
index fb6376f..6d04df1 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Bytes.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Bytes.cs
@@ -23,8 +23,6 @@ namespace System.Text.Json
         /// </remarks>
         public void WriteBase64StringValue(ReadOnlySpan<byte> bytes)
         {
-            JsonWriterHelper.ValidateBytes(bytes);
-
             WriteBase64ByOptions(bytes);
 
             SetFlagToAddListSeparatorBeforeNextItem();
@@ -51,13 +49,22 @@ namespace System.Text.Json
         // TODO: https://github.com/dotnet/runtime/issues/29293
         private void WriteBase64Minimized(ReadOnlySpan<byte> bytes)
         {
-            int encodingLength = Base64.GetMaxEncodedToUtf8Length(bytes.Length);
+            // Base64.GetMaxEncodedToUtf8Length checks to make sure the length is <= int.MaxValue / 4 * 3,
+            // as a length longer than that would overflow int.MaxValue when Base64 encoded. To ensure we
+            // throw an appropriate exception, we check the same condition here first.
+            const int MaxLengthAllowed = int.MaxValue / 4 * 3;
+            if (bytes.Length > MaxLengthAllowed)
+            {
+                ThrowHelper.ThrowArgumentException_ValueTooLarge(bytes.Length);
+            }
 
-            Debug.Assert(encodingLength < int.MaxValue - 3);
+            int encodingLength = Base64.GetMaxEncodedToUtf8Length(bytes.Length);
+            Debug.Assert(encodingLength <= int.MaxValue - 3);
 
             // 2 quotes to surround the base-64 encoded string value.
             // Optionally, 1 list separator
             int maxRequired = encodingLength + 3;
+            Debug.Assert((uint)maxRequired <= int.MaxValue);
 
             if (_memory.Length - BytesPending < maxRequired)
             {
@@ -83,13 +90,21 @@ namespace System.Text.Json
             int indent = Indentation;
             Debug.Assert(indent <= 2 * _options.MaxDepth);
 
-            int encodingLength = Base64.GetMaxEncodedToUtf8Length(bytes.Length);
+            // Base64.GetMaxEncodedToUtf8Length checks to make sure the length is <= int.MaxValue / 4 * 3,
+            // as a length longer than that would overflow int.MaxValue when Base64 encoded. However, we
+            // also need the indentation + 2 quotes, and optionally a list separate and 1-2 bytes for a new line.
+            // Validate the encoded bytes length won't overflow with all of the length.
+            int extraSpaceRequired = indent + 3 + s_newLineLength;
+            int maxLengthAllowed = int.MaxValue / 4 * 3 - extraSpaceRequired;
+            if (bytes.Length > maxLengthAllowed)
+            {
+                ThrowHelper.ThrowArgumentException_ValueTooLarge(bytes.Length);
+            }
 
-            Debug.Assert(encodingLength < int.MaxValue - indent - 3 - s_newLineLength);
+            int encodingLength = Base64.GetMaxEncodedToUtf8Length(bytes.Length);
 
-            // indentation + 2 quotes to surround the base-64 encoded string value.
-            // Optionally, 1 list separator, and 1-2 bytes for new line
-            int maxRequired = indent + encodingLength + 3 + s_newLineLength;
+            int maxRequired = encodingLength + extraSpaceRequired;
+            Debug.Assert((uint)maxRequired <= int.MaxValue - 3);
 
             if (_memory.Length - BytesPending < maxRequired)
             {

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Utf8JsonWriterTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Utf8JsonWriterTests.cs
index a6689a0..e625db9 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Utf8JsonWriterTests.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Utf8JsonWriterTests.cs
@@ -12,6 +12,7 @@ using System.Text.Encodings.Web;
 using System.Text.Unicode;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.DotNet.XUnitExtensions;
 using Newtonsoft.Json;
 using Xunit;
 
@@ -3100,64 +3101,73 @@ namespace System.Text.Json.Tests
         [InlineData(false, false)]
         public void WritingTooLargeBase64Bytes(bool formatted, bool skipValidation)
         {
-            byte[] value;
-
             try
             {
-                value = new byte[200_000_000];
-            }
-            catch (OutOfMemoryException)
-            {
-                return;
-            }
+                byte[] value = new byte[200_000_000];
+                value.AsSpan().Fill((byte)'a');
 
-            value.AsSpan().Fill(255);
+                var options = new JsonWriterOptions { Indented = formatted, SkipValidation = skipValidation };
+                var output = new ArrayBufferWriter<byte>(value.Length);
 
-            var options = new JsonWriterOptions { Indented = formatted, SkipValidation = skipValidation };
-            var output = new ArrayBufferWriter<byte>(1024);
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteBase64StringValue(value.AsSpan(0, 125_000_001));
+                }
+                Assert.InRange(output.WrittenCount, 125_000_001, int.MaxValue);
+                output.Clear();
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64StringValue(value.AsSpan(0, 125_000_001)));
-            }
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String(value.AsSpan(0, 166_666_667), value.AsSpan(0, 1)));
+                }
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String(value.AsSpan(0, 166_666_667), value.AsSpan(0, 1)));
-            }
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String(Encoding.UTF8.GetString(value).ToCharArray().AsSpan(0, 166_666_667), value.AsSpan(0, 1)));
+                }
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String(Encoding.UTF8.GetString(value).ToCharArray().AsSpan(0, 166_666_667), value.AsSpan(0, 1)));
-            }
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteBase64StringValue(value);
+                }
+                Assert.InRange(output.WrittenCount, Base64.GetMaxEncodedToUtf8Length(value.Length), int.MaxValue);
+                output.Clear();
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64StringValue(value));
-            }
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String("foo", value);
+                }
+                Assert.InRange(output.WrittenCount, Base64.GetMaxEncodedToUtf8Length(value.Length), int.MaxValue);
+                output.Clear();
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                jsonUtf8.WriteStartObject();
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String("foo", value));
-            }
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String("foo"u8, value);
+                }
+                Assert.InRange(output.WrittenCount, Base64.GetMaxEncodedToUtf8Length(value.Length), int.MaxValue);
+                output.Clear();
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                jsonUtf8.WriteStartObject();
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String("foo"u8, value));
-            }
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String("foo".AsSpan(), value);
+                }
+                Assert.InRange(output.WrittenCount, Base64.GetMaxEncodedToUtf8Length(value.Length), int.MaxValue);
+                output.Clear();
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                jsonUtf8.WriteStartObject();
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String("foo".AsSpan(), value));
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String(JsonEncodedText.Encode("foo"), value);
+                }
+                Assert.InRange(output.WrittenCount, Base64.GetMaxEncodedToUtf8Length(value.Length), int.MaxValue);
+                output.Clear();
             }
-
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+            catch (OutOfMemoryException)
             {
-                jsonUtf8.WriteStartObject();
-                Assert.Throws<ArgumentException>(() => jsonUtf8.WriteBase64String(JsonEncodedText.Encode("foo"), value));
+                throw new SkipTestException("Out of memory allocating large objects");
             }
         }
 
@@ -3172,59 +3182,57 @@ namespace System.Text.Json.Tests
         [InlineData(true, false)]
         [InlineData(false, true)]
         [InlineData(false, false)]
-        public void WritingLargestPossibleBase64Bytes(bool formatted, bool skipValidation)
+        public void WritingHugeBase64Bytes(bool formatted, bool skipValidation)
         {
-            byte[] value;
-
             try
             {
-                value = new byte[125_000_000];
-            }
-            catch (OutOfMemoryException)
-            {
-                return;
-            }
+                byte[] value = new byte[1_000_000_000];
 
-            value.AsSpan().Fill(168);
+                value.AsSpan().Fill(168);
 
-            var options = new JsonWriterOptions { Indented = formatted, SkipValidation = skipValidation };
-            var output = new ArrayBufferWriter<byte>(1024);
+                var options = new JsonWriterOptions { Indented = formatted, SkipValidation = skipValidation };
+                var output = new ArrayBufferWriter<byte>(1024);
 
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                jsonUtf8.WriteBase64StringValue(value);
-            }
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteBase64StringValue(value);
+                }
 
-            output.Clear();
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                jsonUtf8.WriteStartObject();
-                jsonUtf8.WriteBase64String("foo", value);
-                jsonUtf8.WriteEndObject();
-            }
+                output.Clear();
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String("foo", value);
+                    jsonUtf8.WriteEndObject();
+                }
 
-            output.Clear();
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                jsonUtf8.WriteStartObject();
-                jsonUtf8.WriteBase64String("foo"u8, value);
-                jsonUtf8.WriteEndObject();
-            }
+                output.Clear();
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String("foo"u8, value);
+                    jsonUtf8.WriteEndObject();
+                }
 
-            output.Clear();
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
-            {
-                jsonUtf8.WriteStartObject();
-                jsonUtf8.WriteBase64String("foo".AsSpan(), value);
-                jsonUtf8.WriteEndObject();
-            }
+                output.Clear();
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String("foo".AsSpan(), value);
+                    jsonUtf8.WriteEndObject();
+                }
 
-            output.Clear();
-            using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                output.Clear();
+                using (var jsonUtf8 = new Utf8JsonWriter(output, options))
+                {
+                    jsonUtf8.WriteStartObject();
+                    jsonUtf8.WriteBase64String(JsonEncodedText.Encode("foo"), value);
+                    jsonUtf8.WriteEndObject();
+                }
+            }
+            catch (OutOfMemoryException)
             {
-                jsonUtf8.WriteStartObject();
-                jsonUtf8.WriteBase64String(JsonEncodedText.Encode("foo"), value);
-                jsonUtf8.WriteEndObject();
+                throw new SkipTestException("Out of memory allocating large objects");
             }
         }