From: Steve Harter Date: Tue, 8 Oct 2019 20:02:15 +0000 (-0500) Subject: Address misc feedback and issues from recent perf changes (dotnet/corefx#41414) X-Git-Tag: submit/tizen/20210909.063632~11031^2~332 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d69397eb532b36675528f4a80f818eda51202872;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Address misc feedback and issues from recent perf changes (dotnet/corefx#41414) Commit migrated from https://github.com/dotnet/corefx/commit/8655ef98455f8a8924a4ab4ff62009c1a23d4033 --- diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 1b44e05..6d36478 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -297,9 +297,6 @@ Expected depth to be zero at the end of the JSON payload. There is an open JSON object or array that should be closed. - - The provided data of length {0} has remaining bytes {1}. - The JSON value could not be converted to {0}. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs index 1923347..aefe60a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs @@ -438,9 +438,15 @@ namespace System.Text.Json key = MemoryMarshal.Read(propertyName); // Max out the length byte. - // The comparison logic tests for equality against the full contents instead of just - // the key if the property name length is >7. + // This will cause the comparison logic to always test for equality against the full contents + // when the first 7 bytes are the same. key |= 0xFF00000000000000; + + // It is also possible to include the length up to 0xFF in order to prevent false positives + // when the first 7 bytes match but a different length (up to 0xFF). However the extra logic + // slows key generation in the majority of cases: + // key &= 0x00FFFFFFFFFFFFFF; + // key |= (ulong) 7 << Math.Max(length, 0xFF); } else if (length > 3) { @@ -448,25 +454,25 @@ namespace System.Text.Json if (length == 7) { - key |= (ulong) propertyName[6] << (6 * BitsInByte) - | (ulong) propertyName[5] << (5 * BitsInByte) - | (ulong) propertyName[4] << (4 * BitsInByte) - | (ulong) 7 << (7 * BitsInByte); + key |= (ulong)propertyName[6] << (6 * BitsInByte) + | (ulong)propertyName[5] << (5 * BitsInByte) + | (ulong)propertyName[4] << (4 * BitsInByte) + | (ulong)7 << (7 * BitsInByte); } else if (length == 6) { - key |= (ulong) propertyName[5] << (5 * BitsInByte) - | (ulong) propertyName[4] << (4 * BitsInByte) - | (ulong) 6 << (7 * BitsInByte); + key |= (ulong)propertyName[5] << (5 * BitsInByte) + | (ulong)propertyName[4] << (4 * BitsInByte) + | (ulong)6 << (7 * BitsInByte); } else if (length == 5) { - key |= (ulong) propertyName[4] << (4 * BitsInByte) - | (ulong) 5 << (7 * BitsInByte); + key |= (ulong)propertyName[4] << (4 * BitsInByte) + | (ulong)5 << (7 * BitsInByte); } else { - key |= (ulong) 4 << (7 * BitsInByte); + key |= (ulong)4 << (7 * BitsInByte); } } else if (length > 1) @@ -475,18 +481,18 @@ namespace System.Text.Json if (length == 3) { - key |= (ulong) propertyName[2] << (2 * BitsInByte) - | (ulong) 3 << (7 * BitsInByte); + key |= (ulong)propertyName[2] << (2 * BitsInByte) + | (ulong)3 << (7 * BitsInByte); } else { - key |= (ulong) 2 << (7 * BitsInByte); + key |= (ulong)2 << (7 * BitsInByte); } } else if (length == 1) { key = propertyName[0] - | (ulong) 1 << (7 * BitsInByte); + | (ulong)1 << (7 * BitsInByte); } else { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs index 59c6096..9604b4a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs @@ -353,7 +353,8 @@ namespace System.Text.Json Debug.Assert(ShouldDeserialize); JsonPropertyInfo propertyInfo; - if (ElementClassInfo != null && (propertyInfo = ElementClassInfo.PolicyProperty) != null) + JsonClassInfo elementClassInfo = ElementClassInfo; + if (elementClassInfo != null && (propertyInfo = elementClassInfo.PolicyProperty) != null) { // Forward the setter to the value-based JsonPropertyInfo. propertyInfo.ReadEnumerable(tokenType, ref state, ref reader); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs index e845153..2d56b20 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs @@ -54,7 +54,7 @@ namespace System.Text.Json state.Current.CollectionPropertyInitialized = true; - // We should not be processing custom converters here. + // We should not be processing custom converters here (converters are of ClassType.Value). Debug.Assert(state.Current.JsonClassInfo.ClassType != ClassType.Value); // Set or replace the existing enumerable value. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs index f434ed4..8eef116 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs @@ -31,11 +31,13 @@ namespace System.Text.Json state.Current.CollectionPropertyInitialized = true; ClassType classType = state.Current.JsonClassInfo.ClassType; - if (classType == ClassType.Value && - jsonPropertyInfo.ElementClassInfo.Type != typeof(object) && - jsonPropertyInfo.ElementClassInfo.Type != typeof(JsonElement)) + if (classType == ClassType.Value) { - ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(state.Current.JsonClassInfo.Type); + Type elementClassInfoType = jsonPropertyInfo.ElementClassInfo.Type; + if (elementClassInfoType != typeof(object) && elementClassInfoType != typeof(JsonElement)) + { + ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(state.Current.JsonClassInfo.Type); + } } JsonClassInfo classInfo = state.Current.JsonClassInfo; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs index a6fee2b..1aec551 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs @@ -26,10 +26,11 @@ namespace System.Text.Json Debug.Assert(state.Current.ReturnValue != default || state.Current.TempDictionaryValues != default); Debug.Assert(state.Current.JsonClassInfo != default); - if (state.Current.IsProcessingDictionaryOrIDictionaryConstructible() && + bool isProcessingDictObject = state.Current.IsProcessingDictionaryOrIDictionaryConstructibleObject(); + if ((isProcessingDictObject || state.Current.IsProcessingDictionaryOrIDictionaryConstructibleProperty()) && state.Current.JsonClassInfo.DataExtensionProperty != state.Current.JsonPropertyInfo) { - if (state.Current.IsProcessingObject(ClassType.Dictionary | ClassType.IDictionaryConstructible)) + if (isProcessingDictObject) { state.Current.JsonPropertyInfo = state.Current.JsonClassInfo.PolicyProperty; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Span.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Span.cs index d88d3de..7f9f13f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Span.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Span.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics; + namespace System.Text.Json { public static partial class JsonSerializer @@ -56,10 +58,8 @@ namespace System.Text.Json var reader = new Utf8JsonReader(utf8Json, isFinalBlock: true, readerState); object result = ReadCore(returnType, options, ref reader); - if (reader.BytesConsumed != utf8Json.Length) - { - ThrowHelper.ThrowJsonException_DeserializeDataRemaining(utf8Json.Length, utf8Json.Length - reader.BytesConsumed); - } + // The reader should have thrown if we have remaining bytes. + Debug.Assert(reader.BytesConsumed == utf8Json.Length); return result; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs index be77078..ba6b2fa 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs @@ -194,10 +194,8 @@ namespace System.Text.Json ArrayPool.Shared.Return(buffer); } - if (bytesInBuffer != 0) - { - ThrowHelper.ThrowJsonException_DeserializeDataRemaining(totalBytesRead, bytesInBuffer); - } + // The reader should have thrown if we have remaining bytes. + Debug.Assert(bytesInBuffer == 0); return (TValue)readStack.Current.ReturnValue; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.String.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.String.cs index 0e262fee..3fed716 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.String.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.String.cs @@ -3,16 +3,12 @@ // See the LICENSE file in the project root for more information. using System.Buffers; +using System.Diagnostics; namespace System.Text.Json { public static partial class JsonSerializer { - // The maximum length of a byte array before we ask for the actual transcoded byte size. - // The "int.MaxValue / 2" is used because max byte[] length is a bit less than int.MaxValue - // and because we don't want to allocate such a large buffer if not necessary. - private const int MaxArrayLengthBeforeCalculatingSize = int.MaxValue / 2 / JsonConstants.MaxExpansionFactorWhileTranscoding; - /// /// Parse the text representing a single JSON value into a . /// @@ -55,6 +51,8 @@ namespace System.Text.Json /// public static object Deserialize(string json, Type returnType, JsonSerializerOptions options = null) { + const long ArrayPoolMaxSizeBeforeUsingNormalAlloc = 1024 * 1024; + if (json == null) { throw new ArgumentNullException(nameof(json)); @@ -73,22 +71,13 @@ namespace System.Text.Json object result; byte[] tempArray = null; - int maxBytes; - - // For performance, avoid asking for the actual byte count unless necessary. - if (json.Length > MaxArrayLengthBeforeCalculatingSize) - { - // Get the actual byte count in order to handle large input. - maxBytes = JsonReaderHelper.GetUtf8ByteCount(json.AsSpan()); - } - else - { - maxBytes = json.Length * JsonConstants.MaxExpansionFactorWhileTranscoding; - } - - Span utf8 = maxBytes <= JsonConstants.StackallocThreshold ? - stackalloc byte[maxBytes] : - (tempArray = ArrayPool.Shared.Rent(maxBytes)); + // For performance, avoid obtaining actual byte count unless memory usage is higher than the threshold. + Span utf8 = json.Length <= (ArrayPoolMaxSizeBeforeUsingNormalAlloc / JsonConstants.MaxExpansionFactorWhileTranscoding) ? + // Use a pooled alloc. + tempArray = ArrayPool.Shared.Rent(json.Length * JsonConstants.MaxExpansionFactorWhileTranscoding) : + // Use a normal alloc since the pool would create a normal alloc anyway based on the threshold (per current implementation) + // and by using a normal alloc we can avoid the Clear(). + new byte[JsonReaderHelper.GetUtf8ByteCount(json.AsSpan())]; try { @@ -99,11 +88,8 @@ namespace System.Text.Json var reader = new Utf8JsonReader(utf8, isFinalBlock: true, readerState); result = ReadCore(returnType, options, ref reader); - if (reader.BytesConsumed != actualByteCount) - { - ThrowHelper.ThrowJsonException_DeserializeDataRemaining( - actualByteCount, actualByteCount - reader.BytesConsumed); - } + // The reader should have thrown if we have remaining bytes. + Debug.Assert(reader.BytesConsumed == actualByteCount); } finally { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs index 3add101..7db0f4a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs @@ -303,10 +303,8 @@ namespace System.Text.Json ReadCore(options, ref newReader, ref readStack); - if (newReader.BytesConsumed != length) - { - ThrowHelper.ThrowJsonException_DeserializeDataRemaining(length, length - newReader.BytesConsumed); - } + // The reader should have thrown if we have remaining bytes. + Debug.Assert(newReader.BytesConsumed == length); } catch (JsonException) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleObject.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleObject.cs index 2ec63b8..0b35eb2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleObject.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleObject.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Runtime.CompilerServices; +using System.Text.Json.Serialization; namespace System.Text.Json { @@ -28,7 +29,6 @@ namespace System.Text.Json } state.Current.WriteObjectOrArrayStart(ClassType.Object, writer, options); - state.Current.PropertyEnumeratorActive = true; state.Current.MoveToNextProperty = true; } @@ -38,7 +38,7 @@ namespace System.Text.Json } // Determine if we are done enumerating properties. - if (state.Current.PropertyEnumeratorActive) + if (state.Current.ExtensionDataStatus != ExtensionDataWriteStatus.Finished) { // If ClassType.Unknown at this point, we are typeof(object) which should not have any properties. Debug.Assert(state.Current.JsonClassInfo.ClassType != ClassType.Unknown); @@ -59,10 +59,6 @@ namespace System.Text.Json { state.Pop(); } - else - { - state.Current.EndObject(); - } return true; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs index accb266..70a0438 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs @@ -88,6 +88,22 @@ namespace System.Text.Json } /// + /// Is the current object a Dictionary or IDictionaryConstructible. + /// + public bool IsProcessingDictionaryOrIDictionaryConstructibleObject() + { + return IsProcessingObject(ClassType.Dictionary | ClassType.IDictionaryConstructible); + } + + /// + /// Is the current property a Dictionary or IDictionaryConstructible. + /// + public bool IsProcessingDictionaryOrIDictionaryConstructibleProperty() + { + return IsProcessingProperty(ClassType.Dictionary | ClassType.IDictionaryConstructible); + } + + /// /// Is the current object or property a Dictionary or IDictionaryConstructible. /// public bool IsProcessingDictionaryOrIDictionaryConstructible() @@ -205,9 +221,10 @@ namespace System.Text.Json if (jsonPropertyInfo.EnumerableConverter != null) { IList converterList; - if (jsonPropertyInfo.ElementClassInfo.ClassType == ClassType.Value) + JsonClassInfo elementClassInfo = jsonPropertyInfo.ElementClassInfo; + if (elementClassInfo.ClassType == ClassType.Value) { - converterList = jsonPropertyInfo.ElementClassInfo.PolicyProperty.CreateConverterList(); + converterList = elementClassInfo.PolicyProperty.CreateConverterList(); } else { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs index f2b7913..740702f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs @@ -31,7 +31,6 @@ namespace System.Text.Json public bool MoveToNextProperty; // The current property. - public bool PropertyEnumeratorActive; public int PropertyEnumeratorIndex; public ExtensionDataWriteStatus ExtensionDataStatus; public JsonPropertyInfo JsonPropertyInfo; @@ -102,20 +101,15 @@ namespace System.Text.Json public void Reset() { CurrentValue = null; - EndObject(); - } - - public void EndObject() - { CollectionEnumerator = null; ExtensionDataStatus = ExtensionDataWriteStatus.NotStarted; IsIDictionaryConstructible = false; JsonClassInfo = null; PropertyEnumeratorIndex = 0; - PropertyEnumeratorActive = false; PopStackOnEndCollection = false; PopStackOnEndObject = false; StartObjectWritten = false; + EndProperty(); } @@ -146,28 +140,20 @@ namespace System.Text.Json { EndProperty(); - if (PropertyEnumeratorActive) + int maxPropertyIndex = JsonClassInfo.PropertyCacheArray.Length; + + ++PropertyEnumeratorIndex; + if (PropertyEnumeratorIndex >= maxPropertyIndex) { - int len = JsonClassInfo.PropertyCacheArray.Length; - if (PropertyEnumeratorIndex < len) + if (PropertyEnumeratorIndex > maxPropertyIndex) { - if ((PropertyEnumeratorIndex == len - 1) && JsonClassInfo.DataExtensionProperty != null) - { - ExtensionDataStatus = ExtensionDataWriteStatus.Writing; - } - - PropertyEnumeratorIndex++; - PropertyEnumeratorActive = true; + ExtensionDataStatus = ExtensionDataWriteStatus.Finished; } - else + else if (JsonClassInfo.DataExtensionProperty != null) { - PropertyEnumeratorActive = false; + ExtensionDataStatus = ExtensionDataWriteStatus.Writing; } } - else - { - ExtensionDataStatus = ExtensionDataWriteStatus.Finished; - } } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index e39b8fd..efa1ffb 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -131,11 +131,6 @@ namespace System.Text.Json throw new InvalidOperationException(SR.Format(SR.SerializerDictionaryKeyNull, policyType)); } - public static void ThrowJsonException_DeserializeDataRemaining(long length, long bytesRemaining) - { - throw new JsonException(SR.Format(SR.DeserializeDataRemaining, length, bytesRemaining), path: null, lineNumber: null, bytePositionInLine: null); - } - [MethodImpl(MethodImplOptions.NoInlining)] public static void ReThrowWithPath(in ReadStack readStack, JsonReaderException ex) { @@ -179,7 +174,6 @@ namespace System.Text.Json string message = ex.Message; - // If the message is empty or contains EmbedPathInfoFlag then append the Path, LineNumber and BytePositionInLine. if (string.IsNullOrEmpty(message)) { // Use a default message. @@ -213,10 +207,10 @@ namespace System.Text.Json string path = writeStack.PropertyPath(); ex.Path = path; - // If the message is empty, use a default message with the Path. string message = ex.Message; if (string.IsNullOrEmpty(message)) { + // Use a default message. message = SR.Format(SR.SerializeUnableToSerialize); ex.AppendPathInformation = true; } diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.cs index 12d7afb..5cdf6aa 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.cs @@ -65,5 +65,41 @@ namespace System.Text.Json.Serialization.Tests => writer.WriteStringValue(value); } } + + private class ConverterReturningNull : JsonConverter + { + public override Customer Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + Assert.Equal(JsonTokenType.StartObject, reader.TokenType); + + bool rc = reader.Read(); + Assert.True(rc); + + Assert.Equal(JsonTokenType.EndObject, reader.TokenType); + + return null; + } + + public override void Write(Utf8JsonWriter writer, Customer value, JsonSerializerOptions options) + { + throw new NotSupportedException(); + } + } + + [Fact] + public static void VerifyConverterWithTrailingWhitespace() + { + string json = "{} "; + + var options = new JsonSerializerOptions(); + options.Converters.Add(new ConverterReturningNull()); + + byte[] utf8 = Encoding.UTF8.GetBytes(json); + + // The serializer will finish reading the whitespace and no exception will be thrown. + Customer c = JsonSerializer.Deserialize(utf8, options); + + Assert.Null(c); + } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/Object.ReadTests.cs b/src/libraries/System.Text.Json/tests/Serialization/Object.ReadTests.cs index e2a6918..56dc04e 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/Object.ReadTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/Object.ReadTests.cs @@ -2,8 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections; using System.Collections.Generic; +using System.IO; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -469,5 +469,55 @@ namespace System.Text.Json.Serialization.Tests Assert.Equal(1, parsedObject.ParsedSubMixedTypeParsedClass.ParsedDictionary.Count); Assert.Equal(18, parsedObject.ParsedSubMixedTypeParsedClass.ParsedDictionary["Key1"]); } + + private class POCO { } + + [Theory] + [InlineData("{}{}")] + [InlineData("{}1")] + [InlineData("{}/**/")] + [InlineData("{} /**/")] + public static void TooMuchJsonFails(string json) + { + byte[] jsonBytes = Encoding.UTF8.GetBytes(json); + + Assert.Throws(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Deserialize(jsonBytes)); + Assert.Throws(() => JsonSerializer.DeserializeAsync(new MemoryStream(jsonBytes)).Result); + + // Using a reader directly doesn't throw since it stops once POCO is read. + Utf8JsonReader reader = new Utf8JsonReader(jsonBytes); + JsonSerializer.Deserialize(ref reader); + + Assert.True(reader.BytesConsumed > 0); + Assert.NotEqual(jsonBytes.Length, reader.BytesConsumed); + } + + [Theory] + [InlineData("{")] + [InlineData(@"{""")] + [InlineData(@"{""a""")] + [InlineData(@"{""a"":")] + [InlineData(@"{""a"":1")] + [InlineData(@"{""a"":1,")] + public static void TooLittleJsonFails(string json) + { + byte[] jsonBytes = Encoding.UTF8.GetBytes(json); + + Assert.Throws(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Deserialize(jsonBytes)); + Assert.Throws(() => JsonSerializer.DeserializeAsync(new MemoryStream(jsonBytes)).Result); + + // Using a reader directly throws since it can't read full object. + Utf8JsonReader reader = new Utf8JsonReader(jsonBytes); + try + { + JsonSerializer.Deserialize(ref reader); + Assert.True(false, "Expected exception."); + } + catch (JsonException) { } + + Assert.Equal(0, reader.BytesConsumed); + } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/ReadValueTests.cs b/src/libraries/System.Text.Json/tests/Serialization/ReadValueTests.cs index ece5193..1e29ceb 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/ReadValueTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/ReadValueTests.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information. using System.Buffers; +using System.IO; +using System.Threading.Tasks; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -493,5 +495,76 @@ namespace System.Text.Json.Serialization.Tests obj = JsonSerializer.Deserialize(ref reader, typeof(int)); Assert.Equal(1, obj); } + + [Theory] + [InlineData("0,1")] + [InlineData("0 1")] + [InlineData("0 {}")] + public static void TooMuchJson(string json) + { + byte[] jsonBytes = Encoding.UTF8.GetBytes(json); + + Assert.Throws(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Deserialize(jsonBytes)); + Assert.Throws(() => JsonSerializer.DeserializeAsync(new MemoryStream(jsonBytes)).Result); + + // Using a reader directly doesn't throw. + Utf8JsonReader reader = new Utf8JsonReader(jsonBytes); + JsonSerializer.Deserialize(ref reader); + Assert.Equal(1, reader.BytesConsumed); + } + + [Theory] + [InlineData("0/**/")] + [InlineData("0 /**/")] + public static void TooMuchJsonWithComments(string json) + { + byte[] jsonBytes = Encoding.UTF8.GetBytes(json); + Assert.Throws(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Deserialize(jsonBytes)); + Assert.Throws(() => JsonSerializer.DeserializeAsync(new MemoryStream(jsonBytes)).Result); + + // Using a reader directly doesn't throw. + Utf8JsonReader reader = new Utf8JsonReader(jsonBytes); + JsonSerializer.Deserialize(ref reader); + Assert.Equal(1, reader.BytesConsumed); + + // Use JsonCommentHandling.Skip + + var options = new JsonSerializerOptions(); + options.ReadCommentHandling = JsonCommentHandling.Skip; + JsonSerializer.Deserialize(json, options); + JsonSerializer.Deserialize(jsonBytes, options); + int result = JsonSerializer.DeserializeAsync(new MemoryStream(jsonBytes), options).Result; + + // Using a reader directly doesn't throw. + reader = new Utf8JsonReader(jsonBytes); + JsonSerializer.Deserialize(ref reader, options); + Assert.Equal(1, reader.BytesConsumed); + } + + [Theory] + [InlineData("[")] + [InlineData("[0")] + [InlineData("[0,")] + public static void TooLittleJsonForIntArray(string json) + { + byte[] jsonBytes = Encoding.UTF8.GetBytes(json); + + Assert.Throws(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Deserialize(jsonBytes)); + Assert.Throws(() => JsonSerializer.DeserializeAsync(new MemoryStream(jsonBytes)).Result); + + // Using a reader directly throws since it can't read full int[]. + Utf8JsonReader reader = new Utf8JsonReader(jsonBytes); + try + { + JsonSerializer.Deserialize(ref reader); + Assert.True(false, "Expected exception."); + } + catch (JsonException) { } + + Assert.Equal(0, reader.BytesConsumed); + } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/Value.ReadTests.cs b/src/libraries/System.Text.Json/tests/Serialization/Value.ReadTests.cs index f889b0d..4236cca 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/Value.ReadTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/Value.ReadTests.cs @@ -365,22 +365,18 @@ namespace System.Text.Json.Serialization.Tests } } - // NOTE: LongInputString test is constrained to run on Windows and MacOSX because it causes - // problems on Linux due to the way deferred memory allocation works. On Linux, the allocation can - // succeed even if there is not enough memory but then the test may get killed by the OOM killer at the - // time the memory is accessed which triggers the full memory allocation. + private const long ArrayPoolMaxSizeBeforeUsingNormalAlloc = 1024 * 1024; private const int MaxExpansionFactorWhileTranscoding = 3; - private const int MaxArrayLengthBeforeCalculatingSize = int.MaxValue / 2 / MaxExpansionFactorWhileTranscoding; - [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)] - [ConditionalTheory(nameof(IsX64))] - [InlineData(MaxArrayLengthBeforeCalculatingSize - 3)] - [InlineData(MaxArrayLengthBeforeCalculatingSize - 2)] - [InlineData(MaxArrayLengthBeforeCalculatingSize - 1)] - [InlineData(MaxArrayLengthBeforeCalculatingSize)] - [InlineData(MaxArrayLengthBeforeCalculatingSize + 1)] - [InlineData(MaxArrayLengthBeforeCalculatingSize + 2)] - [InlineData(MaxArrayLengthBeforeCalculatingSize + 3)] - [OuterLoop] + private const long Threshold = ArrayPoolMaxSizeBeforeUsingNormalAlloc / MaxExpansionFactorWhileTranscoding; + + [Theory] + [InlineData(Threshold - 3)] + [InlineData(Threshold - 2)] + [InlineData(Threshold - 1)] + [InlineData(Threshold)] + [InlineData(Threshold + 1)] + [InlineData(Threshold + 2)] + [InlineData(Threshold + 3)] public static void LongInputString(int length) { // Verify boundary conditions in Deserialize() that inspect the size to determine allocation strategy.