Merge pull request #32107 from YohDeadfall/fix-hidden-props

Fixed serialization of hidden base class members
dotnet · May 8, 2020 · 32120db · 32120db
2 parents c1aab32 + d600e91
commit 32120db
Show file tree

Hide file tree

Showing 2 changed files with 503 additions and 34 deletions.
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
@@ -96,52 +96,58 @@ public JsonClassInfo(Type type, JsonSerializerOptions options)
                 case ClassType.Object:
                     {
                         CreateObject = options.MemberAccessorStrategy.CreateConstructor(type);
+                        Dictionary<string, JsonPropertyInfo> cache = new Dictionary<string, JsonPropertyInfo>(
+                            Options.PropertyNameCaseInsensitive
+                                ? StringComparer.OrdinalIgnoreCase
+                                : StringComparer.Ordinal);
 
-                        PropertyInfo[] properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
-
-                        Dictionary<string, JsonPropertyInfo> cache = CreatePropertyCache(properties.Length);
-
-                        foreach (PropertyInfo propertyInfo in properties)
+                        for (Type? currentType = type; currentType != null; currentType = currentType.BaseType)
                         {
-                            // Ignore indexers
-                            if (propertyInfo.GetIndexParameters().Length > 0)
+                            foreach (PropertyInfo propertyInfo in currentType.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly))
                             {
-                                continue;
-                            }
-
-                            if (IsNonPublicProperty(propertyInfo))
-                            {
-                                if (JsonPropertyInfo.GetAttribute<JsonIncludeAttribute>(propertyInfo) != null)
+                                // Ignore indexers
+                                if (propertyInfo.GetIndexParameters().Length > 0)
                                 {
-                                    ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, Type);
+                                    continue;
                                 }
 
-                                // Non-public properties should not be included for (de)serialization.
-                                continue;
-                            }
+                                if (IsNonPublicProperty(propertyInfo))
+                                {
+                                    if (JsonPropertyInfo.GetAttribute<JsonIncludeAttribute>(propertyInfo) != null)
+                                    {
+                                        ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(propertyInfo, currentType);
+                                    }
 
-                            // For now we only support public getters\setters
-                            if (propertyInfo.GetMethod?.IsPublic == true ||
-                                propertyInfo.SetMethod?.IsPublic == true)
-                            {
-                                JsonPropertyInfo jsonPropertyInfo = AddProperty(propertyInfo, type, options);
-                                Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null);
+                                    // Non-public properties should not be included for (de)serialization.
+                                    continue;
+                                }
 
-                                // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception.
-                                if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo))
+                                // For now we only support public getters\setters
+                                if (propertyInfo.GetMethod?.IsPublic == true ||
+                                    propertyInfo.SetMethod?.IsPublic == true)
                                 {
-                                    JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString];
+                                    JsonPropertyInfo jsonPropertyInfo = AddProperty(propertyInfo, currentType, options);
+                                    Debug.Assert(jsonPropertyInfo != null && jsonPropertyInfo.NameAsString != null);
 
-                                    if (other.ShouldDeserialize == false && other.ShouldSerialize == false)
-                                    {
-                                        // Overwrite the one just added since it has [JsonIgnore].
-                                        cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo;
-                                    }
-                                    else if (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true)
+                                    // If the JsonPropertyNameAttribute or naming policy results in collisions, throw an exception.
+                                    if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo))
                                     {
-                                        ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo);
+                                        JsonPropertyInfo other = cache[jsonPropertyInfo.NameAsString];
+
+                                        if (other.ShouldDeserialize == false && other.ShouldSerialize == false)
+                                        {
+                                            // Overwrite the one just added since it has [JsonIgnore].
+                                            cache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo;
+                                        }
+                                        else if (other.PropertyInfo?.Name != jsonPropertyInfo.PropertyInfo?.Name &&
+                                            (jsonPropertyInfo.ShouldDeserialize == true || jsonPropertyInfo.ShouldSerialize == true))
+                                        {
+                                            // Check for name equality is required to determine when a new slot is used for the member.
+                                            // Therefore, if names are not the same, there is conflict due to the name policy or attributes.
+                                            ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo);
+                                        }
+                                        // else ignore jsonPropertyInfo since it has [JsonIgnore] or it's hidden by a new slot.
                                     }
-                                    // else ignore jsonPropertyInfo since it has [JsonIgnore].
                                 }
                             }
                         }