diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs index 2993fac38ab548..023fa4b1342afc 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs @@ -32,6 +32,15 @@ private JsonEncodedText(byte[] utf8Value) _utf8Value = utf8Value; } + private JsonEncodedText(string stringValue, byte[] utf8Value) + { + Debug.Assert(stringValue != null); + Debug.Assert(utf8Value != null); + + _value = stringValue; + _utf8Value = utf8Value; + } + /// /// Encodes the string text value as a JSON string. /// @@ -125,6 +134,37 @@ private static JsonEncodedText EncodeHelper(ReadOnlySpan utf8Value, JavaSc } } + /// + /// Internal version that keeps the existing string and byte[] references if there is no escaping required. + /// + internal static JsonEncodedText Encode(string stringValue, byte[] utf8Value, JavaScriptEncoder? encoder = null) + { + Debug.Assert(stringValue.Equals(JsonHelpers.Utf8GetString(utf8Value))); + + if (utf8Value.Length == 0) + { + return new JsonEncodedText(stringValue, utf8Value); + } + + JsonWriterHelper.ValidateValue(utf8Value); + return EncodeHelper(stringValue, utf8Value, encoder); + } + + private static JsonEncodedText EncodeHelper(string stringValue, byte[] utf8Value, JavaScriptEncoder? encoder) + { + int idx = JsonWriterHelper.NeedsEscaping(utf8Value, encoder); + + if (idx != -1) + { + return new JsonEncodedText(GetEscapedString(utf8Value, idx, encoder)); + } + else + { + // Encoding is not necessary; use the same stringValue and utf8Value references. + return new JsonEncodedText(stringValue, utf8Value); + } + } + private static byte[] GetEscapedString(ReadOnlySpan utf8Value, int firstEscapeIndexVal, JavaScriptEncoder? encoder) { Debug.Assert(int.MaxValue / JsonConstants.MaxExpansionFactorWhileEscaping >= utf8Value.Length); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs index af02cc267b5d1f..a89d1910c365b1 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs @@ -14,11 +14,11 @@ internal sealed class LargeObjectWithParameterizedConstructorConverter : Obje { protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo) { - bool success = jsonParameterInfo.ReadJson(ref state, ref reader, out object? arg0); + bool success = jsonParameterInfo.ReadJson(ref state, ref reader, out object? arg); if (success) { - ((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.Position] = arg0!; + ((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.Position] = arg!; } return success; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs index 7842a0b8b7b3b0..7977458c421623 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs @@ -454,41 +454,19 @@ private bool TryLookupConstructorParameter( ReadOnlySpan unescapedPropertyName = JsonSerializer.GetPropertyName(ref state, ref reader, options); - if (!state.Current.JsonClassInfo.TryGetParameter(unescapedPropertyName, ref state.Current, out jsonParameterInfo)) - { - return false; - } - - Debug.Assert(jsonParameterInfo != null); + jsonParameterInfo = state.Current.JsonClassInfo.GetParameter( + unescapedPropertyName, + ref state.Current, + out byte[] utf8PropertyName); - // Increment ConstructorParameterIndex so GetProperty() starts with the next parameter the next time this function is called. + // Increment ConstructorParameterIndex so GetParameter() checks the next parameter first when called again. state.Current.CtorArgumentState!.ParameterIndex++; - // Support JsonException.Path. - Debug.Assert( - jsonParameterInfo.JsonPropertyName == null || - options.PropertyNameCaseInsensitive || - unescapedPropertyName.SequenceEqual(jsonParameterInfo.JsonPropertyName)); - - if (jsonParameterInfo.JsonPropertyName == null) - { - byte[] propertyNameArray = unescapedPropertyName.ToArray(); - if (options.PropertyNameCaseInsensitive) - { - // Each payload can have a different name here; remember the value on the temporary stack. - state.Current.JsonPropertyName = propertyNameArray; - } - else - { - //Prevent future allocs by caching globally on the JsonPropertyInfo which is specific to a Type+PropertyName - // so it will match the incoming payload except when case insensitivity is enabled(which is handled above). - jsonParameterInfo.JsonPropertyName = propertyNameArray; - } - } + // For case insensitive and missing property support of JsonPath, remember the value on the temporary stack. + state.Current.JsonPropertyName = utf8PropertyName; state.Current.CtorArgumentState.JsonParameterInfo = jsonParameterInfo; - - return true; + return jsonParameterInfo != null; } internal override bool ConstructorIsParameterized => true; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs index 889471432accd3..e9b99527cbc9f8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -145,21 +144,24 @@ internal static JsonPropertyInfo CreatePropertyInfoForClassInfo( runtimePropertyType: runtimePropertyType, propertyInfo: null, // Not a real property so this is null. parentClassType: typeof(object), // a dummy value (not used) - converter : converter, + converter: converter, options); } // AggressiveInlining used although a large method it is only called from one location and is on a hot path. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadStackFrame frame) + public JsonPropertyInfo GetProperty( + ReadOnlySpan propertyName, + ref ReadStackFrame frame, + out byte[] utf8PropertyName) { - JsonPropertyInfo? info = null; + PropertyRef propertyRef; + + ulong key = GetKey(propertyName); // Keep a local copy of the cache in case it changes by another thread. PropertyRef[]? localPropertyRefsSorted = _propertyRefsSorted; - ulong key = GetKey(propertyName); - // If there is an existing cache, then use it. if (localPropertyRefsSorted != null) { @@ -174,10 +176,11 @@ public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadSta { if (iForward < count) { - PropertyRef propertyRef = localPropertyRefsSorted[iForward]; - if (TryIsPropertyRefEqual(propertyRef, propertyName, key, ref info)) + propertyRef = localPropertyRefsSorted[iForward]; + if (IsPropertyRefEqual(propertyRef, propertyName, key)) { - return info; + utf8PropertyName = propertyRef.NameFromJson; + return propertyRef.Info; } ++iForward; @@ -185,9 +188,10 @@ public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadSta if (iBackward >= 0) { propertyRef = localPropertyRefsSorted[iBackward]; - if (TryIsPropertyRefEqual(propertyRef, propertyName, key, ref info)) + if (IsPropertyRefEqual(propertyRef, propertyName, key)) { - return info; + utf8PropertyName = propertyRef.NameFromJson; + return propertyRef.Info; } --iBackward; @@ -195,10 +199,11 @@ public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadSta } else if (iBackward >= 0) { - PropertyRef propertyRef = localPropertyRefsSorted[iBackward]; - if (TryIsPropertyRefEqual(propertyRef, propertyName, key, ref info)) + propertyRef = localPropertyRefsSorted[iBackward]; + if (IsPropertyRefEqual(propertyRef, propertyName, key)) { - return info; + utf8PropertyName = propertyRef.NameFromJson; + return propertyRef.Info; } --iBackward; @@ -211,24 +216,39 @@ public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadSta } } - // No cached item was found. Try the main list which has all of the properties. - - string stringPropertyName = JsonHelpers.Utf8GetString(propertyName); - + // No cached item was found. Try the main dictionary which has all of the properties. Debug.Assert(PropertyCache != null); - if (!PropertyCache.TryGetValue(stringPropertyName, out info)) + if (PropertyCache.TryGetValue(JsonHelpers.Utf8GetString(propertyName), out JsonPropertyInfo? info)) { - info = JsonPropertyInfo.s_missingProperty; - } + if (Options.PropertyNameCaseInsensitive) + { + if (propertyName.SequenceEqual(info.NameAsUtf8Bytes)) + { + Debug.Assert(key == GetKey(info.NameAsUtf8Bytes.AsSpan())); - Debug.Assert(info != null); + // Use the existing byte[] reference instead of creating another one. + utf8PropertyName = info.NameAsUtf8Bytes!; + } + else + { + // Make a copy of the original Span. + utf8PropertyName = propertyName.ToArray(); + } + } + else + { + Debug.Assert(key == GetKey(info.NameAsUtf8Bytes!.AsSpan())); + utf8PropertyName = info.NameAsUtf8Bytes!; + } + } + else + { + info = JsonPropertyInfo.s_missingProperty; - // Three code paths to get here: - // 1) info == s_missingProperty. Property not found. - // 2) key == info.PropertyNameKey. Exact match found. - // 3) key != info.PropertyNameKey. Match found due to case insensitivity. - Debug.Assert(info == JsonPropertyInfo.s_missingProperty || key == info.PropertyNameKey || Options.PropertyNameCaseInsensitive); + // Make a copy of the original Span. + utf8PropertyName = propertyName.ToArray(); + } // Check if we should add this to the cache. // Only cache up to a threshold length and then just use the dictionary when an item is not found in the cache. @@ -255,7 +275,9 @@ public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadSta frame.PropertyRefCache = new List(); } - PropertyRef propertyRef = new PropertyRef(key, info); + Debug.Assert(info != null); + + propertyRef = new PropertyRef(key, info, utf8PropertyName); frame.PropertyRefCache.Add(propertyRef); } } @@ -265,18 +287,18 @@ public JsonPropertyInfo GetProperty(ReadOnlySpan propertyName, ref ReadSta // AggressiveInlining used although a large method it is only called from one location and is on a hot path. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool TryGetParameter( + public JsonParameterInfo? GetParameter( ReadOnlySpan propertyName, ref ReadStackFrame frame, - out JsonParameterInfo? jsonParameterInfo) + out byte[] utf8PropertyName) { - JsonParameterInfo? info = null; + ParameterRef parameterRef; + + ulong key = GetKey(propertyName); // Keep a local copy of the cache in case it changes by another thread. ParameterRef[]? localParameterRefsSorted = _parameterRefsSorted; - ulong key = GetKey(propertyName); - // If there is an existing cache, then use it. if (localParameterRefsSorted != null) { @@ -291,11 +313,11 @@ public bool TryGetParameter( { if (iForward < count) { - ParameterRef parameterRef = localParameterRefsSorted[iForward]; - if (TryIsParameterRefEqual(parameterRef, propertyName, key, ref info)) + parameterRef = localParameterRefsSorted[iForward]; + if (IsParameterRefEqual(parameterRef, propertyName, key)) { - jsonParameterInfo = info; - return true; + utf8PropertyName = parameterRef.NameFromJson; + return parameterRef.Info; } ++iForward; @@ -303,10 +325,10 @@ public bool TryGetParameter( if (iBackward >= 0) { parameterRef = localParameterRefsSorted[iBackward]; - if (TryIsParameterRefEqual(parameterRef, propertyName, key, ref info)) + if (IsParameterRefEqual(parameterRef, propertyName, key)) { - jsonParameterInfo = info; - return true; + utf8PropertyName = parameterRef.NameFromJson; + return parameterRef.Info; } --iBackward; @@ -314,11 +336,11 @@ public bool TryGetParameter( } else if (iBackward >= 0) { - ParameterRef parameterRef = localParameterRefsSorted[iBackward]; - if (TryIsParameterRefEqual(parameterRef, propertyName, key, ref info)) + parameterRef = localParameterRefsSorted[iBackward]; + if (IsParameterRefEqual(parameterRef, propertyName, key)) { - jsonParameterInfo = info; - return true; + utf8PropertyName = parameterRef.NameFromJson; + return parameterRef.Info; } --iBackward; @@ -331,26 +353,39 @@ public bool TryGetParameter( } } - string propertyNameAsString = JsonHelpers.Utf8GetString(propertyName); - + // No cached item was found. Try the main dictionary which has all of the parameters. Debug.Assert(ParameterCache != null); - if (!ParameterCache.TryGetValue(propertyNameAsString, out info)) + if (ParameterCache.TryGetValue(JsonHelpers.Utf8GetString(propertyName), out JsonParameterInfo? info)) { - // Constructor parameter not found. We'll check if it's a property next. - jsonParameterInfo = null; - return false; - } + if (Options.PropertyNameCaseInsensitive) + { + if (propertyName.SequenceEqual(info.NameAsUtf8Bytes)) + { + Debug.Assert(key == GetKey(info.NameAsUtf8Bytes.AsSpan())); - jsonParameterInfo = info; - Debug.Assert(info != null); + // Use the existing byte[] reference instead of creating another one. + utf8PropertyName = info.NameAsUtf8Bytes!; + } + else + { + // Make a copy of the original Span. + utf8PropertyName = propertyName.ToArray(); + } + } + else + { + Debug.Assert(key == GetKey(info.NameAsUtf8Bytes!.AsSpan())); + utf8PropertyName = info.NameAsUtf8Bytes!; + } + } + else + { + Debug.Assert(info == null); - // Two code paths to get here: - // 1) key == info.PropertyNameKey. Exact match found. - // 2) key != info.PropertyNameKey. Match found due to case insensitivity. - // TODO: recheck these conditions - Debug.Assert(key == info.ParameterNameKey || - propertyNameAsString.Equals(info.NameAsString, StringComparison.OrdinalIgnoreCase)); + // Make a copy of the original Span. + utf8PropertyName = propertyName.ToArray(); + } // Check if we should add this to the cache. // Only cache up to a threshold length and then just use the dictionary when an item is not found in the cache. @@ -377,24 +412,23 @@ public bool TryGetParameter( frame.CtorArgumentState.ParameterRefCache = new List(); } - ParameterRef parameterRef = new ParameterRef(key, jsonParameterInfo); + parameterRef = new ParameterRef(key, info!, utf8PropertyName); frame.CtorArgumentState.ParameterRefCache.Add(parameterRef); } } - return true; + return info; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool TryIsPropertyRefEqual(in PropertyRef propertyRef, ReadOnlySpan propertyName, ulong key, [NotNullWhen(true)] ref JsonPropertyInfo? info) + private static bool IsPropertyRefEqual(in PropertyRef propertyRef, ReadOnlySpan propertyName, ulong key) { if (key == propertyRef.Key) { // We compare the whole name, although we could skip the first 7 bytes (but it's not any faster) if (propertyName.Length <= PropertyNameKeyLength || - propertyName.SequenceEqual(propertyRef.Info.Name)) + propertyName.SequenceEqual(propertyRef.NameFromJson)) { - info = propertyRef.Info; return true; } } @@ -403,15 +437,14 @@ private static bool TryIsPropertyRefEqual(in PropertyRef propertyRef, ReadOnlySp } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool TryIsParameterRefEqual(in ParameterRef parameterRef, ReadOnlySpan parameterName, ulong key, [NotNullWhen(true)] ref JsonParameterInfo? info) + private static bool IsParameterRefEqual(in ParameterRef parameterRef, ReadOnlySpan parameterName, ulong key) { if (key == parameterRef.Key) { // We compare the whole name, although we could skip the first 7 bytes (but it's not any faster) if (parameterName.Length <= PropertyNameKeyLength || - parameterName.SequenceEqual(parameterRef.Info.ParameterName)) + parameterName.SequenceEqual(parameterRef.NameFromJson)) { - info = parameterRef.Info; return true; } } 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 ddb731c24c91dc..f0adf7412981ff 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 @@ -168,15 +168,23 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) cacheArray = new JsonPropertyInfo[cache.Count]; } - // Set fields when finished to avoid concurrency issues. - PropertyCache = cache; + // Copy the dictionary cache to the array cache. cache.Values.CopyTo(cacheArray, 0); + + // Set the array cache field at this point since it is completely initialized. + // It can now be safely accessed by other threads. PropertyCacheArray = cacheArray; + // Allow constructor parameter logic to remove items from the dictionary since the JSON + // property values will be passed to the constructor and do not call a property setter. if (converter.ConstructorIsParameterized) { - InitializeConstructorParameters(converter.ConstructorInfo!); + InitializeConstructorParameters(cache, converter.ConstructorInfo!); } + + // Set the dictionary cache field at this point since it is completely initialized. + // It can now be safely accessed by other threads. + PropertyCache = cache; } break; case ClassType.Enumerable: @@ -210,13 +218,11 @@ private static bool IsNonPublicProperty(PropertyInfo propertyInfo) return !((getMethod != null && getMethod.IsPublic) || (setMethod != null && setMethod.IsPublic)); } - private void InitializeConstructorParameters(ConstructorInfo constructorInfo) + private void InitializeConstructorParameters(Dictionary propertyCache, ConstructorInfo constructorInfo) { ParameterInfo[] parameters = constructorInfo!.GetParameters(); Dictionary parameterCache = CreateParameterCache(parameters.Length, Options); - Dictionary propertyCache = PropertyCache!; - foreach (ParameterInfo parameterInfo in parameters) { PropertyInfo? firstMatch = null; @@ -251,7 +257,7 @@ private void InitializeConstructorParameters(ConstructorInfo constructorInfo) // One object property cannot map to multiple constructor // parameters (ConvertName above can't return multiple strings). - parameterCache.Add(jsonParameterInfo.NameAsString, jsonParameterInfo); + parameterCache.Add(jsonPropertyInfo.NameAsString!, jsonParameterInfo); // Remove property from deserialization cache to reduce the number of JsonPropertyInfos considered during JSON matching. propertyCache.Remove(jsonPropertyInfo.NameAsString!); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfo.cs index d531d9f9c2d279..f2d8408205d312 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonParameterInfo.cs @@ -22,22 +22,13 @@ internal abstract class JsonParameterInfo // The default value of the parameter. This is `DefaultValue` of the `ParameterInfo`, if specified, or the CLR `default` for the `ParameterType`. public object? DefaultValue { get; protected set; } - // The name from a Json value. This is cached for performance on first deserialize. - public byte[]? JsonPropertyName { get; set; } - // Options can be referenced here since all JsonPropertyInfos originate from a JsonClassInfo that is cached on JsonSerializerOptions. protected JsonSerializerOptions Options { get; set; } = null!; // initialized in Init method public ParameterInfo ParameterInfo { get; private set; } = null!; // The name of the parameter as UTF-8 bytes. - public byte[] ParameterName { get; private set; } = null!; - - // The name of the parameter. - public string NameAsString { get; private set; } = null!; - - // Key for fast property name lookup. - public ulong ParameterNameKey { get; private set; } + public byte[] NameAsUtf8Bytes { get; private set; } = null!; // The zero-based position of the parameter in the formal parameter list. public int Position { get; private set; } @@ -77,12 +68,7 @@ public virtual void Initialize( private void DetermineParameterName(JsonPropertyInfo matchingProperty) { - NameAsString = matchingProperty.NameAsString!; - - // `NameAsString` is valid UTF16, so just call the simple UTF16->UTF8 encoder. - ParameterName = Encoding.UTF8.GetBytes(NameAsString); - - ParameterNameKey = JsonClassInfo.GetKey(ParameterName); + NameAsUtf8Bytes = matchingProperty.NameAsUtf8Bytes!; } // Create a parameter that is ignored at run-time. It uses the same type (typeof(sbyte)) to help 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 5c43c5d8bfb0fc..81455209d2b115 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 @@ -83,13 +83,10 @@ private void DeterminePropertyName() Debug.Assert(NameAsString != null); // At this point propertyName is valid UTF16, so just call the simple UTF16->UTF8 encoder. - Name = Encoding.UTF8.GetBytes(NameAsString); + NameAsUtf8Bytes = Encoding.UTF8.GetBytes(NameAsString); // Cache the escaped property name. - EscapedName = JsonEncodedText.Encode(Name, Options.Encoder); - - ulong key = JsonClassInfo.GetKey(Name); - PropertyNameKey = key; + EscapedName = JsonEncodedText.Encode(NameAsString, NameAsUtf8Bytes, Options.Encoder); } private void DetermineSerializationCapabilities(JsonIgnoreCondition? ignoreCondition) @@ -145,10 +142,6 @@ private void DetermineIgnoreCondition(JsonIgnoreCondition? ignoreCondition) } } - // The escaped name passed to the writer. - // Use a field here (not a property) to avoid value semantics. - public JsonEncodedText? EscapedName; - public static TAttribute? GetAttribute(PropertyInfo propertyInfo) where TAttribute : Attribute { return (TAttribute?)propertyInfo.GetCustomAttribute(typeof(TAttribute), inherit: false); @@ -194,15 +187,32 @@ public virtual void Initialize( public bool IsPropertyPolicy { get; protected set; } - // The name from a Json value. This is cached for performance on first deserialize. - public byte[]? JsonPropertyName { get; set; } - - // The name of the property with any casing policy or the name specified from JsonPropertyNameAttribute. - public byte[]? Name { get; private set; } + // There are 3 copies of the property name: + // 1) NameAsString. The unescaped property name. + // 2) NameAsUtf8Bytes. The Utf8 version of NameAsString. Used during during deserialization for property lookup. + // 3) EscapedName. The escaped verson of NameAsString and NameAsUtf8Bytes written during serialization. Internally shares + // the same instances of NameAsString and NameAsUtf8Bytes if there is no escaping. + + /// + /// The unescaped name of the property. + /// Is either the actual CLR property name, + /// the value specified in JsonPropertyNameAttribute, + /// or the value returned from PropertyNamingPolicy(clrPropertyName). + /// public string? NameAsString { get; private set; } - // Key for fast property name lookup. - public ulong PropertyNameKey { get; set; } + /// + /// Utf8 version of NameAsString. + /// + public byte[]? NameAsUtf8Bytes { get; private set; } + + /// + /// The escaped name passed to the writer. + /// + /// + /// JsonEncodedText is a value type so a field is used (not a property) to avoid unnecessary copies. + /// + public JsonEncodedText? EscapedName; // Options can be referenced here since all JsonPropertyInfos originate from a JsonClassInfo that is cached on JsonSerializerOptions. protected JsonSerializerOptions Options { get; set; } = null!; // initialized in Init method 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 a59a14fd634898..6996f765619308 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 @@ -27,13 +27,21 @@ internal static JsonPropertyInfo LookupProperty( { Debug.Assert(state.Current.JsonClassInfo.ClassType == ClassType.Object); + useExtensionProperty = false; + ReadOnlySpan unescapedPropertyName = GetPropertyName(ref state, ref reader, options); - JsonPropertyInfo jsonPropertyInfo = state.Current.JsonClassInfo.GetProperty(unescapedPropertyName, ref state.Current); + JsonPropertyInfo jsonPropertyInfo = state.Current.JsonClassInfo.GetProperty( + unescapedPropertyName, + ref state.Current, + out byte[] utf8PropertyName); - // Increment PropertyIndex so GetProperty() starts with the next property the next time this function is called. + // Increment PropertyIndex so GetProperty() checks the next property first when called again. state.Current.PropertyIndex++; + // For case insensitive and missing property support of JsonPath, remember the value on the temporary stack. + state.Current.JsonPropertyName = utf8PropertyName; + // Determine if we should use the extension property. if (jsonPropertyInfo == JsonPropertyInfo.s_missingProperty) { @@ -50,41 +58,9 @@ internal static JsonPropertyInfo LookupProperty( jsonPropertyInfo = dataExtProperty; useExtensionProperty = true; } - else - { - useExtensionProperty = false; - } - - state.Current.JsonPropertyInfo = jsonPropertyInfo; - return jsonPropertyInfo; } - // Support JsonException.Path. - Debug.Assert( - jsonPropertyInfo.JsonPropertyName == null || - options.PropertyNameCaseInsensitive || - unescapedPropertyName.SequenceEqual(jsonPropertyInfo.JsonPropertyName)); - state.Current.JsonPropertyInfo = jsonPropertyInfo; - - if (jsonPropertyInfo.JsonPropertyName == null) - { - byte[] propertyNameArray = unescapedPropertyName.ToArray(); - if (options.PropertyNameCaseInsensitive) - { - // Each payload can have a different name here; remember the value on the temporary stack. - state.Current.JsonPropertyName = propertyNameArray; - } - else - { - // Prevent future allocs by caching globally on the JsonPropertyInfo which is specific to a Type+PropertyName - // so it will match the incoming payload except when case insensitivity is enabled (which is handled above). - state.Current.JsonPropertyInfo.JsonPropertyName = propertyNameArray; - } - } - - state.Current.JsonPropertyInfo = jsonPropertyInfo; - useExtensionProperty = false; return jsonPropertyInfo; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ParameterRef.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ParameterRef.cs index 9f2c654b5a9134..f0ee05b6436ae6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ParameterRef.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ParameterRef.cs @@ -6,15 +6,18 @@ namespace System.Text.Json { internal readonly struct ParameterRef { - public ParameterRef(ulong key, JsonParameterInfo info) + public ParameterRef(ulong key, JsonParameterInfo info, byte[] nameFromJson) { Key = key; Info = info; + NameFromJson = nameFromJson; } - // The first 6 bytes are the first part of the name and last 2 bytes are the name's length. public readonly ulong Key; public readonly JsonParameterInfo Info; + + // NameFromJson may be different than Info.NameAsUtf8Bytes when case insensitive is enabled. + public readonly byte[] NameFromJson; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PropertyRef.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PropertyRef.cs index 41dbbe5e411e26..e092a7cfe5cdde 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PropertyRef.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PropertyRef.cs @@ -6,15 +6,17 @@ namespace System.Text.Json { internal readonly struct PropertyRef { - public PropertyRef(ulong key, JsonPropertyInfo info) + public PropertyRef(ulong key, JsonPropertyInfo info, byte[] nameFromJson) { Key = key; Info = info; + NameFromJson = nameFromJson; } - // The first 6 bytes are the first part of the name and last 2 bytes are the name's length. public readonly ulong Key; - public readonly JsonPropertyInfo Info; + + // NameFromJson may be different than Info.NameAsUtf8Bytes when case insensitive is enabled. + public readonly byte[] NameFromJson; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs index 3a9e0dddf145dd..7e7f497a536cb9 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs @@ -298,8 +298,9 @@ static void AppendPropertyName(StringBuilder sb, string? propertyName) if (utf8PropertyName == null) { // Attempt to get the JSON property name from the JsonPropertyInfo or JsonParameterInfo. - utf8PropertyName = frame.JsonPropertyInfo?.JsonPropertyName ?? - frame.CtorArgumentState?.JsonParameterInfo?.JsonPropertyName; + utf8PropertyName = frame.JsonPropertyInfo?.NameAsUtf8Bytes ?? + frame.CtorArgumentState?.JsonParameterInfo?.NameAsUtf8Bytes; + if (utf8PropertyName == null) { // Attempt to get the JSON property name set manually for dictionary diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandling.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandling.cs index 2928b0f2758141..269e0cb84b1bb1 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandling.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandling.cs @@ -53,8 +53,8 @@ public sealed class ReferenceHandling /// public static ReferenceHandling Preserve { get; } = new ReferenceHandling(PreserveReferencesHandling.All); - private readonly PreserveReferencesHandling _preserveHandlingOnSerialize; - private readonly PreserveReferencesHandling _preserveHandlingOnDeserialize; + private readonly bool _shouldReadPreservedReferences; + private readonly bool _shouldWritePreservedReferences; /// /// Creates a new instance of using the specified @@ -65,18 +65,18 @@ private ReferenceHandling(PreserveReferencesHandling handling) : this(handling, // For future, someone may want to define their own custom Handler with different behaviors of PreserveReferenceHandling on Serialize vs Deserialize. private ReferenceHandling(PreserveReferencesHandling preserveHandlingOnSerialize, PreserveReferencesHandling preserveHandlingOnDeserialize) { - _preserveHandlingOnSerialize = preserveHandlingOnSerialize; - _preserveHandlingOnDeserialize = preserveHandlingOnDeserialize; + _shouldReadPreservedReferences = preserveHandlingOnDeserialize == PreserveReferencesHandling.All; + _shouldWritePreservedReferences = preserveHandlingOnSerialize == PreserveReferencesHandling.All; } internal bool ShouldReadPreservedReferences() { - return _preserveHandlingOnDeserialize == PreserveReferencesHandling.All; + return _shouldReadPreservedReferences; } internal bool ShouldWritePreservedReferences() { - return _preserveHandlingOnSerialize == PreserveReferencesHandling.All; + return _shouldWritePreservedReferences; } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/ExtensionDataTests.cs b/src/libraries/System.Text.Json/tests/Serialization/ExtensionDataTests.cs index b0e27027971c84..1e0b46b7e1b7ea 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/ExtensionDataTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/ExtensionDataTests.cs @@ -99,6 +99,18 @@ public static void MultipleExtensionPropertyIgnoredWhenNull() Assert.Equal("{\"ActualDictionary\":{},\"test\":\"value\"}", actual); } + [Fact] + public static void ExtensionPropertyInvalidJsonFail() + { + const string BadJson = @"{""Good"":""OK"",""Bad"":!}"; + + JsonException jsonException = Assert.Throws(() => JsonSerializer.Deserialize(BadJson)); + Assert.Contains("Path: $.Bad | LineNumber: 0 | BytePositionInLine: 19.", jsonException.ToString()); + Assert.NotNull(jsonException.InnerException); + Assert.IsAssignableFrom(jsonException.InnerException); + Assert.Contains("!", jsonException.InnerException.ToString()); + } + [Fact] public static void ExtensionPropertyAlreadyInstantiated() { diff --git a/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs b/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs index a13f36661798b4..1b0ff3e70ae12b 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs @@ -231,6 +231,89 @@ public static void EmptyPropertyName() } } + [Fact] + public static void EmptyPropertyNameInExtensionData() + { + { + string json = @"{"""":42}"; + EmptyClassWithExtensionProperty obj = JsonSerializer.Deserialize(json); + Assert.Equal(1, obj.MyOverflow.Count); + Assert.Equal(42, obj.MyOverflow[""].GetInt32()); + } + + { + // Verify that last-in wins. + string json = @"{"""":42, """":43}"; + EmptyClassWithExtensionProperty obj = JsonSerializer.Deserialize(json); + Assert.Equal(1, obj.MyOverflow.Count); + Assert.Equal(43, obj.MyOverflow[""].GetInt32()); + } + } + + [Fact] + public static void EmptyPropertyName_WinsOver_ExtensionDataEmptyPropertyName() + { + string json = @"{"""":1}"; + + ClassWithEmptyPropertyNameAndExtensionProperty obj; + + // Create a new options instances to re-set any caches. + JsonSerializerOptions options = new JsonSerializerOptions(); + + // Verify the real property wins over the extension data property. + obj = JsonSerializer.Deserialize(json, options); + Assert.Equal(1, obj.MyInt1); + Assert.Null(obj.MyOverflow); + } + + [Fact] + public static void EmptyPropertyNameAndExtensionData_ExtDataFirst() + { + // Verify any caching treats real property (with empty name) differently than a missing property. + + ClassWithEmptyPropertyNameAndExtensionProperty obj; + + // Create a new options instances to re-set any caches. + JsonSerializerOptions options = new JsonSerializerOptions(); + + // First populate cache with a missing property name. + string json = @"{""DoesNotExist"":42}"; + obj = JsonSerializer.Deserialize(json, options); + Assert.Equal(0, obj.MyInt1); + Assert.Equal(1, obj.MyOverflow.Count); + Assert.Equal(42, obj.MyOverflow["DoesNotExist"].GetInt32()); + + // Then use an empty property. + json = @"{"""":43}"; + obj = JsonSerializer.Deserialize(json, options); + Assert.Equal(43, obj.MyInt1); + Assert.Null(obj.MyOverflow); + } + + [Fact] + public static void EmptyPropertyAndExtensionData_PropertyFirst() + { + // Verify any caching treats real property (with empty name) differently than a missing property. + + ClassWithEmptyPropertyNameAndExtensionProperty obj; + + // Create a new options instances to re-set any caches. + JsonSerializerOptions options = new JsonSerializerOptions(); + + // First use an empty property. + string json = @"{"""":43}"; + obj = JsonSerializer.Deserialize(json, options); + Assert.Equal(43, obj.MyInt1); + Assert.Null(obj.MyOverflow); + + // Then populate cache with a missing property name. + json = @"{""DoesNotExist"":42}"; + obj = JsonSerializer.Deserialize(json, options); + Assert.Equal(0, obj.MyInt1); + Assert.Equal(1, obj.MyOverflow.Count); + Assert.Equal(42, obj.MyOverflow["DoesNotExist"].GetInt32()); + } + [Fact] public static void UnicodePropertyNames() { @@ -515,4 +598,13 @@ public class EmptyClassWithExtensionProperty [JsonExtensionData] public IDictionary MyOverflow { get; set; } } + + public class ClassWithEmptyPropertyNameAndExtensionProperty + { + [JsonPropertyName("")] + public int MyInt1 { get; set; } + + [JsonExtensionData] + public IDictionary MyOverflow { get; set; } + } }