Disallow types with property names conflicting with metadata. (dotnet…

…#106460)

src/libraries/System.Text.Json/src/Resources/Strings.resx

@@ -662,6 +662,9 @@
   <data name="Polymorphism_InvalidCustomTypeDiscriminatorPropertyName" xml:space="preserve">
     <value>The metadata property names '$id', '$ref', and '$values' are reserved and cannot be used as custom type discriminator property names.</value>
   </data>
+  <data name="Polymorphism_PropertyConflictsWithMetadataPropertyName" xml:space="preserve">
+    <value>The type '{0}' contains property '{1}' that conflicts with an existing metadata property name. Consider either renaming it or ignoring it with JsonIgnoreAttribute.</value>
+  </data>
   <data name="Polymorphism_ConfigurationDoesNotSpecifyDerivedTypes" xml:space="preserve">
     <value>Polymorphic configuration for type '{0}' should specify at least one derived type.</value>
   </data>

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

@@ -443,7 +443,7 @@ internal void Configure()
             }
             else
             {
-                CacheNameAsUtf8BytesAndEscapedNameSection();
+                ValidateAndCachePropertyName();
             }
 
             if (IsRequired)
@@ -472,10 +472,21 @@ internal void Configure()
         [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
         internal abstract void DetermineReflectionPropertyAccessors(MemberInfo memberInfo, bool useNonPublicAccessors);
 
-        private void CacheNameAsUtf8BytesAndEscapedNameSection()
+        private void ValidateAndCachePropertyName()
         {
             Debug.Assert(Name != null);
 
+            if (Options.ReferenceHandlingStrategy is ReferenceHandlingStrategy.Preserve &&
+                this is { DeclaringType.IsValueType: false, IsIgnored: false, IsExtensionData: false } &&
+                Name is JsonSerializer.IdPropertyName or JsonSerializer.RefPropertyName)
+            {
+                // Validate potential conflicts with reference preservation metadata property names.
+                // Conflicts with polymorphic type discriminators are contextual and need to be
+                // handled separately by the PolymorphicTypeResolver type.
+
+                ThrowHelper.ThrowInvalidOperationException_PropertyConflictsWithMetadataPropertyName(DeclaringType, Name);
+            }
+
             NameAsUtf8Bytes = Encoding.UTF8.GetBytes(Name);
             EscapedNameSection = JsonHelpers.GetEscapedPropertyNameSection(NameAsUtf8Bytes, Options.Encoder);
         }

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs

@@ -42,16 +42,17 @@ public PolymorphicTypeResolver(JsonSerializerOptions options, JsonPolymorphismOp
                     ThrowHelper.ThrowInvalidOperationException_DerivedTypeNotSupported(BaseType, derivedType);
                 }
 
-                var derivedJsonTypeInfo = new DerivedJsonTypeInfo(derivedType, typeDiscriminator);
+                JsonTypeInfo derivedTypeInfo = options.GetTypeInfoInternal(derivedType);
+                DerivedJsonTypeInfo derivedTypeInfoHolder = new(typeDiscriminator, derivedTypeInfo);
 
-                if (!_typeToDiscriminatorId.TryAdd(derivedType, derivedJsonTypeInfo))
+                if (!_typeToDiscriminatorId.TryAdd(derivedType, derivedTypeInfoHolder))
                 {
                     ThrowHelper.ThrowInvalidOperationException_DerivedTypeIsAlreadySpecified(BaseType, derivedType);
                 }
 
                 if (typeDiscriminator is not null)
                 {
-                    if (!(_discriminatorIdtoType ??= new()).TryAdd(typeDiscriminator, derivedJsonTypeInfo))
+                    if (!(_discriminatorIdtoType ??= new()).TryAdd(typeDiscriminator, derivedTypeInfoHolder))
                     {
                         ThrowHelper.ThrowInvalidOperationException_TypeDicriminatorIdIsAlreadySpecified(BaseType, typeDiscriminator);
                     }
@@ -69,6 +70,8 @@ public PolymorphicTypeResolver(JsonSerializerOptions options, JsonPolymorphismOp
 
             if (UsesTypeDiscriminators)
             {
+                Debug.Assert(_discriminatorIdtoType != null, "Discriminator index must have been populated.");
+
                 if (!converterCanHaveMetadata)
                 {
                     ThrowHelper.ThrowNotSupportedException_BaseConverterDoesNotSupportMetadata(BaseType);
@@ -88,6 +91,21 @@ public PolymorphicTypeResolver(JsonSerializerOptions options, JsonPolymorphismOp
                     CustomTypeDiscriminatorPropertyNameUtf8 = utf8EncodedName;
                     CustomTypeDiscriminatorPropertyNameJsonEncoded = JsonEncodedText.Encode(propertyName, options.Encoder);
                 }
+
+                // Check if the discriminator property name conflicts with any derived property names.
+                foreach (DerivedJsonTypeInfo derivedTypeInfo in _discriminatorIdtoType.Values)
+                {
+                    if (derivedTypeInfo.JsonTypeInfo.Kind is JsonTypeInfoKind.Object)
+                    {
+                        foreach (JsonPropertyInfo property in derivedTypeInfo.JsonTypeInfo.Properties)
+                        {
+                            if (property is { IsIgnored: false, IsExtensionData: false } && property.Name == propertyName)
+                            {
+                                ThrowHelper.ThrowInvalidOperationException_PropertyConflictsWithMetadataPropertyName(derivedTypeInfo.JsonTypeInfo.Type, propertyName);
+                            }
+                        }
+                    }
+                }
             }
         }
 
@@ -136,7 +154,7 @@ public bool TryGetDerivedJsonTypeInfo(Type runtimeType, [NotNullWhen(true)] out
             }
             else
             {
-                jsonTypeInfo = result.GetJsonTypeInfo(_options);
+                jsonTypeInfo = result.JsonTypeInfo;
                 typeDiscriminator = result.TypeDiscriminator;
                 return true;
             }
@@ -151,7 +169,7 @@ public bool TryGetDerivedJsonTypeInfo(object typeDiscriminator, [NotNullWhen(tru
             if (_discriminatorIdtoType.TryGetValue(typeDiscriminator, out DerivedJsonTypeInfo? result))
             {
                 Debug.Assert(typeDiscriminator.Equals(result.TypeDiscriminator));
-                jsonTypeInfo = result.GetJsonTypeInfo(_options);
+                jsonTypeInfo = result.JsonTypeInfo;
                 return true;
             }
 
@@ -218,7 +236,7 @@ public static bool IsSupportedDerivedType(Type baseType, Type? derivedType) =>
                         }
                         else
                         {
-                            ThrowHelper.ThrowNotSupportedException_RuntimeTypeDiamondAmbiguity(BaseType, type, result.DerivedType, interfaceResult.DerivedType);
+                            ThrowHelper.ThrowNotSupportedException_RuntimeTypeDiamondAmbiguity(BaseType, type, result.JsonTypeInfo.Type, interfaceResult.JsonTypeInfo.Type);
                         }
                     }
                 }
@@ -307,24 +325,20 @@ public static bool IsSupportedDerivedType(Type baseType, Type? derivedType) =>
         }
 
         /// <summary>
-        /// Lazy JsonTypeInfo result holder for a derived type.
+        /// JsonTypeInfo result holder for a derived type.
         /// </summary>
         private sealed class DerivedJsonTypeInfo
         {
-            private volatile JsonTypeInfo? _jsonTypeInfo;
-
-            public DerivedJsonTypeInfo(Type type, object? typeDiscriminator)
+            public DerivedJsonTypeInfo(object? typeDiscriminator, JsonTypeInfo derivedTypeInfo)
             {
                 Debug.Assert(typeDiscriminator is null or int or string);
 
-                DerivedType = type;
                 TypeDiscriminator = typeDiscriminator;
+                JsonTypeInfo = derivedTypeInfo;
             }
 
-            public Type DerivedType { get; }
             public object? TypeDiscriminator { get; }
-            public JsonTypeInfo GetJsonTypeInfo(JsonSerializerOptions options)
-                => _jsonTypeInfo ??= options.GetTypeInfoInternal(DerivedType);
+            public JsonTypeInfo JsonTypeInfo { get; }
         }
     }
 }

src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs

@@ -903,6 +903,12 @@ public static void ThrowInvalidOperationException_InvalidCustomTypeDiscriminator
             throw new InvalidOperationException(SR.Polymorphism_InvalidCustomTypeDiscriminatorPropertyName);
         }
 
+        [DoesNotReturn]
+        public static void ThrowInvalidOperationException_PropertyConflictsWithMetadataPropertyName(Type type, string propertyName)
+        {
+            throw new InvalidOperationException(SR.Format(SR.Polymorphism_PropertyConflictsWithMetadataPropertyName, type, propertyName));
+        }
+
         [DoesNotReturn]
         public static void ThrowInvalidOperationException_PolymorphicTypeConfigurationDoesNotSpecifyDerivedTypes(Type baseType)
         {

src/libraries/System.Text.Json/tests/Common/ReferenceHandlerTests/ReferenceHandlerTests.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Text.Encodings.Web;
+using System.Text.Json.Nodes;
 using System.Threading.Tasks;
 using Newtonsoft.Json;
 using Xunit;
@@ -849,5 +850,76 @@ public IEnumerator<object> GetEnumerator()
 
             IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
         }
+
+        [Theory]
+        [InlineData(typeof(ClassWithConflictingRefProperty), "$ref")]
+        [InlineData(typeof(ClassWithConflictingIdProperty), "$id")]
+        public async Task ClassWithConflictingMetadataProperties_ThrowsInvalidOperationException(Type type, string propertyName)
+        {
+            InvalidOperationException ex;
+            object value = Activator.CreateInstance(type);
+
+            ex = Assert.Throws<InvalidOperationException>(() => Serializer.GetTypeInfo(type, s_serializerOptionsPreserve));
+            ValidateException(ex);
+
+            ex = await Assert.ThrowsAsync<InvalidOperationException>(() => Serializer.SerializeWrapper(value, type, s_serializerOptionsPreserve));
+            ValidateException(ex);
+
+            ex = await Assert.ThrowsAsync<InvalidOperationException>(() => Serializer.DeserializeWrapper("{}", type, s_serializerOptionsPreserve));
+            ValidateException(ex);
+
+            void ValidateException(InvalidOperationException ex)
+            {
+                Assert.Contains($"The type '{type}' contains property '{propertyName}' that conflicts with an existing metadata property name.", ex.Message);
+            }
+        }
+
+        public class ClassWithConflictingRefProperty
+        {
+            [JsonPropertyName("$ref")]
+            public int Value { get; set; }
+        }
+
+        public class ClassWithConflictingIdProperty
+        {
+            [JsonPropertyName("$id")]
+            public int Value { get; set; }
+        }
+
+        [Fact]
+        public async Task ClassWithIgnoredConflictingProperty_Supported()
+        {
+            ClassWithIgnoredConflictingProperty value = new();
+            string json = await Serializer.SerializeWrapper(value, s_serializerOptionsPreserve);
+            Assert.Equal("""{"$id":"1"}""", json);
+
+            value = await Serializer.DeserializeWrapper<ClassWithIgnoredConflictingProperty>(json, s_serializerOptionsPreserve);
+            Assert.NotNull(value);
+        }
+
+        public class ClassWithIgnoredConflictingProperty
+        {
+            [JsonPropertyName("$id"), JsonIgnore]
+            public int Value { get; set; }
+        }
+
+        [Fact]
+        public async Task ClassWithExtensionDataConflictingProperty_Supported()
+        {
+            ClassWithExtensionDataConflictingProperty value = new();
+            string json = await Serializer.SerializeWrapper(value, s_serializerOptionsPreserve);
+            Assert.Equal("""{"$id":"1"}""", json);
+
+            value = await Serializer.DeserializeWrapper<ClassWithExtensionDataConflictingProperty>("""{"$id":"1","extraProp":null}""", s_serializerOptionsPreserve);
+            Assert.NotNull(value);
+            Assert.Equal(1, value.Value.Count);
+            Assert.Contains("extraProp", value.Value);
+        }
+
+        public class ClassWithExtensionDataConflictingProperty
+        {
+            [JsonPropertyName("$id"), JsonExtensionData]
+            public JsonObject Value { get; set; }
+        }
     }
 }

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ReferenceHandlerTests.cs

@@ -135,6 +135,10 @@ public ReferenceHandlerTests_Metadata(JsonSerializerWrapper serializer)
         [JsonSerializable(typeof(LinkedList<object[]>))]
         [JsonSerializable(typeof(LinkedList<Base[]>))]
         [JsonSerializable(typeof(LinkedList<Base[][]>))]
+        [JsonSerializable(typeof(ClassWithConflictingRefProperty))]
+        [JsonSerializable(typeof(ClassWithConflictingIdProperty))]
+        [JsonSerializable(typeof(ClassWithIgnoredConflictingProperty))]
+        [JsonSerializable(typeof(ClassWithExtensionDataConflictingProperty))]
         internal sealed partial class ReferenceHandlerTestsContext_Metadata : JsonSerializerContext
         {
         }
@@ -273,6 +277,10 @@ public ReferenceHandlerTests_Default(JsonSerializerWrapper serializer)
         [JsonSerializable(typeof(LinkedList<object[]>))]
         [JsonSerializable(typeof(LinkedList<Base[]>))]
         [JsonSerializable(typeof(LinkedList<Base[][]>))]
+        [JsonSerializable(typeof(ClassWithConflictingRefProperty))]
+        [JsonSerializable(typeof(ClassWithConflictingIdProperty))]
+        [JsonSerializable(typeof(ClassWithIgnoredConflictingProperty))]
+        [JsonSerializable(typeof(ClassWithExtensionDataConflictingProperty))]
         internal sealed partial class ReferenceHandlerTestsContext_Default : JsonSerializerContext
         {
         }

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PolymorphicTests.cs

@@ -4,6 +4,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Text.Json.Nodes;
 using System.Text.Json.Serialization.Metadata;
 using System.Threading.Tasks;
 using Xunit;
@@ -577,5 +578,102 @@ class MyDerivedThing : MyThing
         class MyThingCollection : List<IThing> { }
 
         class MyThingDictionary : Dictionary<string, IThing> { }
+
+        [Theory]
+        [InlineData(typeof(PolymorphicTypeWithConflictingPropertyNameAtBase), typeof(PolymorphicTypeWithConflictingPropertyNameAtBase.Derived))]
+        [InlineData(typeof(PolymorphicTypeWithConflictingPropertyNameAtDerived), typeof(PolymorphicTypeWithConflictingPropertyNameAtDerived.Derived))]
+        public async Task PolymorphicTypesWithConflictingPropertyNames_ThrowsInvalidOperationException(Type baseType, Type derivedType)
+        {
+            InvalidOperationException ex;
+            object value = Activator.CreateInstance(derivedType);
+
+            ex = Assert.Throws<InvalidOperationException>(() => Serializer.GetTypeInfo(baseType));
+            ValidateException(ex);
+
+            ex = await Assert.ThrowsAsync<InvalidOperationException>(() => Serializer.SerializeWrapper(value, baseType));
+            ValidateException(ex);
+
+            ex = await Assert.ThrowsAsync<InvalidOperationException>(() => Serializer.DeserializeWrapper("{}", baseType));
+            ValidateException(ex);
+
+            void ValidateException(InvalidOperationException ex)
+            {
+                Assert.Contains($"The type '{derivedType}' contains property 'Type' that conflicts with an existing metadata property name.", ex.Message);
+            }
+        }
+
+        [JsonPolymorphic(TypeDiscriminatorPropertyName = "Type")]
+        [JsonDerivedType(typeof(Derived), nameof(Derived))]
+        public abstract class PolymorphicTypeWithConflictingPropertyNameAtBase
+        {
+            public string Type { get; set; }
+
+            public class Derived : PolymorphicTypeWithConflictingPropertyNameAtBase
+            {
+                public string Name { get; set; }
+            }
+        }
+
+        [JsonPolymorphic(TypeDiscriminatorPropertyName = "Type")]
+        [JsonDerivedType(typeof(Derived), nameof(Derived))]
+        public abstract class PolymorphicTypeWithConflictingPropertyNameAtDerived
+        {
+            public class Derived : PolymorphicTypeWithConflictingPropertyNameAtDerived
+            {
+                public string Type { get; set; }
+            }
+        }
+
+        [Fact]
+        public async Task PolymorphicTypeWithIgnoredConflictingPropertyName_Supported()
+        {
+            PolymorphicTypeWithIgnoredConflictingPropertyName value = new PolymorphicTypeWithIgnoredConflictingPropertyName.Derived();
+
+            JsonTypeInfo typeInfo = Serializer.GetTypeInfo(typeof(PolymorphicTypeWithIgnoredConflictingPropertyName));
+            Assert.NotNull(typeInfo);
+
+            string json = await Serializer.SerializeWrapper(value);
+            Assert.Equal("""{"Type":"Derived"}""", json);
+
+            value = await Serializer.DeserializeWrapper<PolymorphicTypeWithIgnoredConflictingPropertyName>(json);
+            Assert.IsType<PolymorphicTypeWithIgnoredConflictingPropertyName.Derived>(value);
+        }
+
+        [JsonPolymorphic(TypeDiscriminatorPropertyName = "Type")]
+        [JsonDerivedType(typeof(Derived), nameof(Derived))]
+        public abstract class PolymorphicTypeWithIgnoredConflictingPropertyName
+        {
+            [JsonIgnore]
+            public string Type { get; set; }
+
+            public class Derived : PolymorphicTypeWithIgnoredConflictingPropertyName;
+        }
+
+        [Fact]
+        public async Task PolymorphicTypeWithExtensionDataConflictingPropertyName_Supported()
+        {
+            PolymorphicTypeWithExtensionDataConflictingPropertyName value = new PolymorphicTypeWithExtensionDataConflictingPropertyName.Derived();
+
+            JsonTypeInfo typeInfo = Serializer.GetTypeInfo(typeof(PolymorphicTypeWithIgnoredConflictingPropertyName));
+            Assert.NotNull(typeInfo);
+
+            string json = await Serializer.SerializeWrapper(value);
+            Assert.Equal("""{"Type":"Derived"}""", json);
+
+            value = await Serializer.DeserializeWrapper<PolymorphicTypeWithExtensionDataConflictingPropertyName>("""{"Type":"Derived","extraProp":null}""");
+            Assert.IsType<PolymorphicTypeWithExtensionDataConflictingPropertyName.Derived>(value);
+            Assert.Equal(1, value.Type.Count);
+            Assert.Contains("extraProp", value.Type);
+        }
+
+        [JsonPolymorphic(TypeDiscriminatorPropertyName = "Type")]
+        [JsonDerivedType(typeof(Derived), nameof(Derived))]
+        public abstract class PolymorphicTypeWithExtensionDataConflictingPropertyName
+        {
+            [JsonExtensionData]
+            public JsonObject Type { get; set; }
+
+            public class Derived : PolymorphicTypeWithExtensionDataConflictingPropertyName;
+        }
     }
 }