[release/7.0] Ensure source generated metadata properties are read-on…

…ly. (#76540) (#76899)

* Ensure source generated metadata properties are read-only. (#76540)

* Add JsonTypeInfo.IsReadOnly/MakeReadOnly() APIs.

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

@@ -3,11 +3,8 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
-using System.Reflection.Metadata;
-using System.Text.Json;
 using System.Text.Json.Reflection;
 using System.Text.Json.Serialization;
 using Microsoft.CodeAnalysis;
@@ -28,6 +25,7 @@ private sealed partial class Emitter
             private const string DefaultOptionsStaticVarName = "s_defaultOptions";
             private const string DefaultContextBackingStaticVarName = "s_defaultContext";
             internal const string GetConverterFromFactoryMethodName = "GetConverterFromFactory";
+            private const string MakeReadOnlyMethodName = "MakeReadOnly";
             private const string InfoVarName = "info";
             private const string PropertyInfoVarName = "propertyInfo";
             internal const string JsonContextVarName = "jsonContext";
@@ -1015,6 +1013,8 @@ private static string GenerateFastPathFuncForType(TypeGenerationSpec typeGenSpec
 
                 return $@"
 
+// Intentionally not a static method because we create a delegate to it. Invoking delegates to instance
+// methods is almost as fast as virtual calls. Static methods need to go through a shuffle thunk.
 private void {serializeMethodName}({Utf8JsonWriterTypeRef} {WriterVarName}, {valueTypeRef} {ValueVarName})
 {{
     {GetEarlyNullCheckSource(emitNullCheck)}
@@ -1090,16 +1090,19 @@ private static string GenerateForType(TypeGenerationSpec typeMetadata, string me
 /// </summary>
 public {typeInfoPropertyTypeRef} {typeFriendlyName}
 {{
-    get => _{typeFriendlyName} ??= {typeMetadata.CreateTypeInfoMethodName}({OptionsInstanceVariableName});
+    get => _{typeFriendlyName} ??= {typeMetadata.CreateTypeInfoMethodName}({OptionsInstanceVariableName}, makeReadOnly: true);
 }}
 
-// Intentionally not a static method because we create a delegate to it. Invoking delegates to instance
-// methods is almost as fast as virtual calls. Static methods need to go through a shuffle thunk.
-private {typeInfoPropertyTypeRef} {typeMetadata.CreateTypeInfoMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName})
+private {typeInfoPropertyTypeRef} {typeMetadata.CreateTypeInfoMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName}, bool makeReadOnly)
 {{
     {typeInfoPropertyTypeRef}? {JsonTypeInfoReturnValueLocalVariableName} = null;
     {WrapWithCheckForCustomConverter(metadataInitSource, typeCompilableName)}
 
+    if (makeReadOnly)
+    {{
+        {JsonTypeInfoReturnValueLocalVariableName}.{MakeReadOnlyMethodName}();
+    }}
+
     return {JsonTypeInfoReturnValueLocalVariableName};
 }}
 {additionalSource}";
@@ -1276,30 +1279,23 @@ private string GetGetTypeInfoImplementation()
                 // Explicit IJsonTypeInfoResolver implementation
                 sb.AppendLine();
                 sb.Append(@$"{JsonTypeInfoTypeRef}? {JsonTypeInfoResolverTypeRef}.GetTypeInfo({TypeTypeRef} type, {JsonSerializerOptionsTypeRef} {OptionsLocalVariableName})
-{{
-    if ({OptionsInstanceVariableName} == {OptionsLocalVariableName})
-    {{
-        return this.GetTypeInfo(type);
-    }}
-    else
-    {{");
+{{");
                 // TODO (https://github.com/dotnet/runtime/issues/52218): Make this Dictionary-lookup-based if root-serializable type count > 64.
                 foreach (TypeGenerationSpec metadata in types)
                 {
                     if (metadata.ClassType != ClassType.TypeUnsupportedBySourceGen)
                     {
                         sb.Append($@"
-        if (type == typeof({metadata.TypeRef}))
-        {{
-            return {metadata.CreateTypeInfoMethodName}({OptionsLocalVariableName});
-        }}
+    if (type == typeof({metadata.TypeRef}))
+    {{
+        return {metadata.CreateTypeInfoMethodName}({OptionsLocalVariableName}, makeReadOnly: false);
+    }}
 ");
                     }
                 }
 
                 sb.Append($@"
-        return null;
-    }}
+    return null;
 }}
 ");
 

src/libraries/System.Text.Json/ref/System.Text.Json.cs

@@ -1190,7 +1190,9 @@ public abstract partial class JsonTypeInfo
         internal JsonTypeInfo() { }
         public System.Text.Json.Serialization.JsonConverter Converter { get { throw null; } }
         public System.Func<object>? CreateObject { get { throw null; } set { } }
+        public bool IsReadOnly { get { throw null; } }
         public System.Text.Json.Serialization.Metadata.JsonTypeInfoKind Kind { get { throw null; } }
+        public void MakeReadOnly() { throw null; }
         public System.Text.Json.Serialization.JsonNumberHandling? NumberHandling { get { throw null; } set { } }
         public System.Action<object>? OnDeserialized { get { throw null; } set { } }
         public System.Action<object>? OnDeserializing { get { throw null; } set { } }

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

@@ -247,10 +247,10 @@
     <value>Cannot add callbacks to the 'Modifiers' property after the resolver has been used for the first time.</value>
   </data>
   <data name="TypeInfoImmutable" xml:space="preserve">
-    <value>JsonTypeInfo cannot be changed after first usage.</value>
+    <value>This JsonTypeInfo instance is marked read-only or has already been used in serialization or deserialization.</value>
   </data>
   <data name="PropertyInfoImmutable" xml:space="preserve">
-    <value>JsonPropertyInfo cannot be changed after first usage.</value>
+    <value>This JsonTypeInfo instance is marked read-only or has already been used in serialization or deserialization.</value>
   </data>
   <data name="MaxDepthMustBePositive" xml:space="preserve">
     <value>Max depth must be positive.</value>
@@ -337,7 +337,7 @@
     <value>The JSON object contains a trailing comma at the end which is not supported in this mode. Change the reader options.</value>
   </data>
   <data name="SerializerOptionsImmutable" xml:space="preserve">
-    <value>Serializer options cannot be changed once serialization or deserialization has occurred.</value>
+    <value>This JsonSerializerOptions instance is read-only or has already been used in serialization or deserialization.</value>
   </data>
   <data name="StreamNotWritable" xml:space="preserve">
     <value>Stream is not writable.</value>

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

@@ -268,7 +268,7 @@ internal static JsonPropertyInfo GetPropertyPlaceholder()
 
         private protected void VerifyMutable()
         {
-            if (_isConfigured)
+            if (ParentTypeInfo?.IsReadOnly == true)
             {
                 ThrowHelper.ThrowInvalidOperationException_PropertyInfoImmutable();
             }

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

@@ -256,6 +256,23 @@ public JsonPolymorphismOptions? PolymorphismOptions
             }
         }
 
+        /// <summary>
+        /// Specifies whether the current instance has been locked for modification.
+        /// </summary>
+        /// <remarks>
+        /// A <see cref="JsonTypeInfo"/> instance can be locked either if
+        /// it has been passed to one of the <see cref="JsonSerializer"/> methods,
+        /// has been associated with a <see cref="JsonSerializerContext"/> instance,
+        /// or a user explicitly called the <see cref="MakeReadOnly"/> method on the instance.
+        /// </remarks>
+        public bool IsReadOnly { get; private set; }
+
+        /// <summary>
+        /// Locks the current instance for further modification.
+        /// </summary>
+        /// <remarks>This method is idempotent.</remarks>
+        public void MakeReadOnly() => IsReadOnly = true;
+
         private protected JsonPolymorphismOptions? _polymorphismOptions;
 
         internal object? CreateObjectWithArgs { get; set; }
@@ -482,7 +499,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, JsonSerializerOptions
 
         internal void VerifyMutable()
         {
-            if (_isConfigured)
+            if (IsReadOnly)
             {
                 ThrowHelper.ThrowInvalidOperationException_TypeInfoImmutable();
             }
@@ -516,6 +533,7 @@ void ConfigureLocked()
                     {
                         Configure();
 
+                        IsReadOnly = true;
                         _isConfigured = true;
                     }
                     catch (Exception e)
@@ -699,6 +717,7 @@ public static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions o
         /// <returns>A blank <see cref="JsonPropertyInfo"/> instance.</returns>
         /// <exception cref="ArgumentNullException"><paramref name="propertyType"/> or <paramref name="name"/> is null.</exception>
         /// <exception cref="ArgumentException"><paramref name="propertyType"/> cannot be used for serialization.</exception>
+        /// <exception cref="InvalidOperationException">The <see cref="JsonTypeInfo"/> instance has been locked for further modification.</exception>
         [RequiresUnreferencedCode(MetadataFactoryRequiresUnreferencedCode)]
         [RequiresDynamicCode(MetadataFactoryRequiresUnreferencedCode)]
         public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name)
@@ -718,6 +737,7 @@ public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name)
                 ThrowHelper.ThrowArgumentException_CannotSerializeInvalidType(nameof(propertyType), propertyType, Type, name);
             }
 
+            VerifyMutable();
             JsonPropertyInfo propertyInfo = CreatePropertyUsingReflection(propertyType);
             propertyInfo.Name = name;
 
@@ -754,6 +774,8 @@ internal void CacheMember(JsonPropertyInfo jsonPropertyInfo, JsonPropertyDiction
             Debug.Assert(jsonPropertyInfo.MemberName != null, "MemberName can be null in custom JsonPropertyInfo instances and should never be passed in this method");
             string memberName = jsonPropertyInfo.MemberName;
 
+            jsonPropertyInfo.EnsureChildOf(this);
+
             if (jsonPropertyInfo.IsExtensionData)
             {
                 if (ExtensionDataProperty != null)

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs

@@ -21,6 +21,70 @@ public static void VariousNestingAndVisibilityLevelsAreSupported()
             Assert.NotNull(NestedPublicContext.NestedProtectedInternalClass.Default);
         }
 
+        [Fact]
+        public static void PropertyMetadataIsImmutable()
+        {
+            JsonTypeInfo<Person> typeInfo = PersonJsonContext.Default.Person;
+
+            Assert.True(typeInfo.IsReadOnly);
+            Assert.Throws<InvalidOperationException>(() => typeInfo.CreateObject = null);
+            Assert.Throws<InvalidOperationException>(() => typeInfo.OnDeserializing = obj => { });
+            Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Clear());
+
+            JsonPropertyInfo propertyInfo = typeInfo.Properties[0];
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.Name = "differentName");
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.NumberHandling = JsonNumberHandling.AllowReadingFromString);
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.IsRequired = true);
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.Order = -1);
+        }
+
+        [Fact]
+        public static void JsonSerializerContext_GetTypeInfo_MetadataIsImmutable()
+        {
+            JsonTypeInfo<Person> typeInfo = (JsonTypeInfo<Person>)PersonJsonContext.Default.GetTypeInfo(typeof(Person));
+
+            Assert.True(typeInfo.IsReadOnly);
+            Assert.Throws<InvalidOperationException>(() => typeInfo.CreateObject = null);
+            Assert.Throws<InvalidOperationException>(() => typeInfo.OnDeserializing = obj => { });
+            Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Clear());
+
+            JsonPropertyInfo propertyInfo = typeInfo.Properties[0];
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.Name = "differentName");
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.NumberHandling = JsonNumberHandling.AllowReadingFromString);
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.IsRequired = true);
+            Assert.Throws<InvalidOperationException>(() => propertyInfo.Order = -1);
+        }
+
+        [Fact]
+        public static void IJsonTypeInfoResolver_GetTypeInfo_MetadataIsMutable()
+        {
+            IJsonTypeInfoResolver resolver = PersonJsonContext.Default;
+            JsonTypeInfo<Person> typeInfo = (JsonTypeInfo<Person>)resolver.GetTypeInfo(typeof(Person), PersonJsonContext.Default.Options);
+
+            Assert.NotSame(typeInfo, PersonJsonContext.Default.Person);
+            Assert.False(typeInfo.IsReadOnly);
+
+            JsonTypeInfo<Person> typeInfo2 = (JsonTypeInfo<Person>)resolver.GetTypeInfo(typeof(Person), PersonJsonContext.Default.Options);
+            Assert.NotSame(typeInfo, typeInfo2);
+            Assert.False(typeInfo.IsReadOnly);
+
+            typeInfo.CreateObject = null;
+            typeInfo.OnDeserializing = obj => { };
+
+            JsonPropertyInfo propertyInfo = typeInfo.Properties[0];
+            propertyInfo.Name = "differentName";
+            propertyInfo.NumberHandling = JsonNumberHandling.AllowReadingFromString;
+            propertyInfo.IsRequired = true;
+            propertyInfo.Order = -1;
+
+            typeInfo.Properties.Clear();
+            Assert.Equal(0, typeInfo.Properties.Count);
+
+            // Changes should not impact other metadata instances
+            Assert.Equal(2, typeInfo2.Properties.Count);
+            Assert.Equal(2, PersonJsonContext.Default.Person.Properties.Count);
+        }
+
         [Fact]
         public static void VariousGenericsAreSupported()
         {

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs

@@ -36,6 +36,7 @@ public static void TypeInfoPropertiesDefaults(Type type)
 
             JsonTypeInfo ti = r.GetTypeInfo(type, o);
 
+            Assert.False(ti.IsReadOnly);
             Assert.Same(o, ti.Options);
             Assert.NotNull(ti.Properties);
 
@@ -400,17 +401,25 @@ private static void TestTypeInfoImmutability<T>(JsonTypeInfo<T> typeInfo)
             Assert.Equal(typeof(T), typeInfo.Type);
             Assert.True(typeInfo.Converter.CanConvert(typeof(T)));
 
-            JsonPropertyInfo prop = typeInfo.CreateJsonPropertyInfo(typeof(string), "foo");
+            Assert.True(typeInfo.IsReadOnly);
             Assert.True(typeInfo.Properties.IsReadOnly);
+            Assert.Throws<InvalidOperationException>(() => typeInfo.CreateJsonPropertyInfo(typeof(string), "foo"));
             Assert.Throws<InvalidOperationException>(() => untyped.CreateObject = untyped.CreateObject);
             Assert.Throws<InvalidOperationException>(() => typeInfo.CreateObject = typeInfo.CreateObject);
             Assert.Throws<InvalidOperationException>(() => typeInfo.NumberHandling = typeInfo.NumberHandling);
+            Assert.Throws<InvalidOperationException>(() => typeInfo.CreateJsonPropertyInfo(typeof(string), "foo"));
             Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Clear());
-            Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Add(prop));
-            Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Insert(0, prop));
             Assert.Throws<InvalidOperationException>(() => typeInfo.PolymorphismOptions = null);
             Assert.Throws<InvalidOperationException>(() => typeInfo.PolymorphismOptions = new());
 
+            if (typeInfo.Properties.Count > 0)
+            {
+                JsonPropertyInfo prop = typeInfo.Properties[0];
+                Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Add(prop));
+                Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Insert(0, prop));
+                Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.RemoveAt(0));
+            }
+
             if (typeInfo.PolymorphismOptions is JsonPolymorphismOptions jpo)
             {
                 Assert.True(jpo.DerivedTypes.IsReadOnly);

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

@@ -994,9 +994,11 @@ public static void GetTypeInfo_MutableOptionsInstance(Type type)
             options.TypeInfoResolver = new DefaultJsonTypeInfoResolver();
             JsonTypeInfo typeInfo = options.GetTypeInfo(type);
             Assert.Equal(type, typeInfo.Type);
+            Assert.False(typeInfo.IsReadOnly);
 
             JsonTypeInfo typeInfo2 = options.GetTypeInfo(type);
             Assert.Equal(type, typeInfo2.Type);
+            Assert.False(typeInfo2.IsReadOnly);
 
             Assert.NotSame(typeInfo, typeInfo2);
 
@@ -1015,6 +1017,7 @@ public static void GetTypeInfo_ImmutableOptionsInstance(Type type)
 
             JsonTypeInfo typeInfo = options.GetTypeInfo(type);
             Assert.Equal(type, typeInfo.Type);
+            Assert.True(typeInfo.IsReadOnly);
 
             JsonTypeInfo typeInfo2 = options.GetTypeInfo(type);
             Assert.Same(typeInfo, typeInfo2);
@@ -1026,6 +1029,7 @@ public static void GetTypeInfo_MutableOptions_CanModifyMetadata()
             var options = new JsonSerializerOptions { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };
             JsonTypeInfo<TestClassForEncoding> jti = (JsonTypeInfo<TestClassForEncoding>)options.GetTypeInfo(typeof(TestClassForEncoding));
 
+            Assert.False(jti.IsReadOnly);
             Assert.Equal(1, jti.Properties.Count);
             jti.Properties.Clear();
 
@@ -1034,11 +1038,13 @@ public static void GetTypeInfo_MutableOptions_CanModifyMetadata()
             Assert.Equal("{}", json);
 
             // Using JsonTypeInfo will lock JsonSerializerOptions
+            Assert.True(jti.IsReadOnly);
             Assert.Throws<InvalidOperationException>(() => options.IncludeFields = false);
 
             // Getting JsonTypeInfo now should return a fresh immutable instance
             JsonTypeInfo<TestClassForEncoding> jti2 = (JsonTypeInfo<TestClassForEncoding>)options.GetTypeInfo(typeof(TestClassForEncoding));
             Assert.NotSame(jti, jti2);
+            Assert.True(jti2.IsReadOnly);
             Assert.Equal(1, jti2.Properties.Count);
             Assert.Throws<InvalidOperationException>(() => jti2.Properties.Clear());