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 bfe99c3a1249ca1f705e962fe98d286f6a86fb8e..cbdaaed8a3e298c91d176a2f3c91636b010e7b6f 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 7537f720abf11c161ffdef03865a998203719409..417ed4c520b988fae674e6e17814307b88e001a4 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
@@ -87,6 +87,12 @@ namespace System.Text.Json
             throw GetInvalidOperationException(SR.FailedToGetLargerSpan);
         }
 
+        [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)
         {

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 ffb522cf876a49a6673d76834e7cf2c357afc860..9271bb4745795bbe3dd7896658c6014814159cff 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
@@ -46,13 +46,6 @@ namespace System.Text.Json
                 ThrowHelper.ThrowArgumentException_ValueTooLarge(value.Length);
         }
 
-        [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)
         {
@@ -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 1c95db2f6cce3c649f421340bb8fa9f4f1de57d7..9c4785e56bfab166af7b21bc6ef2d12240da7b16 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 fb6376fe4d9691897db685024296a7b036df0920..6d04df1c3ce6e610293eba959a855b768f30c7e0 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 a6689a0b01aa51c4cd98492c95383109875f502d..e625db93ab674246ab9e959b24988424af0dcf36 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");
             }
         }