Skip to content

Commit d783bad

Browse files
eiriktsarpaliseiriktsarpalis
authored
Ensure source generated metadata properties are read-only. (#76540)
* Ensure source generated metadata properties are read-only. * Add JsonTypeInfo.IsReadOnly/MakeReadOnly() APIs.
1 parent 3eae628 commit d783bad

File tree

8 files changed

+127
-28
lines changed

8 files changed

+127
-28
lines changed

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

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,8 @@
33

44
using System.Collections.Generic;
55
using System.Diagnostics;
6-
using System.Diagnostics.CodeAnalysis;
76
using System.Globalization;
87
using System.Reflection;
9-
using System.Reflection.Metadata;
10-
using System.Text.Json;
118
using System.Text.Json.Reflection;
129
using System.Text.Json.Serialization;
1310
using Microsoft.CodeAnalysis;
@@ -28,6 +25,7 @@ private sealed partial class Emitter
2825
private const string DefaultOptionsStaticVarName = "s_defaultOptions";
2926
private const string DefaultContextBackingStaticVarName = "s_defaultContext";
3027
internal const string GetConverterFromFactoryMethodName = "GetConverterFromFactory";
28+
private const string MakeReadOnlyMethodName = "MakeReadOnly";
3129
private const string InfoVarName = "info";
3230
private const string PropertyInfoVarName = "propertyInfo";
3331
internal const string JsonContextVarName = "jsonContext";
@@ -1010,6 +1008,8 @@ private static string GenerateFastPathFuncForType(TypeGenerationSpec typeGenSpec
10101008

10111009
return $@"
10121010
1011+
// Intentionally not a static method because we create a delegate to it. Invoking delegates to instance
1012+
// methods is almost as fast as virtual calls. Static methods need to go through a shuffle thunk.
10131013
private void {serializeMethodName}({Utf8JsonWriterTypeRef} {WriterVarName}, {valueTypeRef} {ValueVarName})
10141014
{{
10151015
{GetEarlyNullCheckSource(emitNullCheck)}
@@ -1085,16 +1085,19 @@ private static string GenerateForType(TypeGenerationSpec typeMetadata, string me
10851085
/// </summary>
10861086
public {typeInfoPropertyTypeRef} {typeFriendlyName}
10871087
{{
1088-
get => _{typeFriendlyName} ??= {typeMetadata.CreateTypeInfoMethodName}({OptionsInstanceVariableName});
1088+
get => _{typeFriendlyName} ??= {typeMetadata.CreateTypeInfoMethodName}({OptionsInstanceVariableName}, makeReadOnly: true);
10891089
}}
10901090

1091-
// Intentionally not a static method because we create a delegate to it. Invoking delegates to instance
1092-
// methods is almost as fast as virtual calls. Static methods need to go through a shuffle thunk.
1093-
private {typeInfoPropertyTypeRef} {typeMetadata.CreateTypeInfoMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName})
1091+
private {typeInfoPropertyTypeRef} {typeMetadata.CreateTypeInfoMethodName}({JsonSerializerOptionsTypeRef} {OptionsLocalVariableName}, bool makeReadOnly)
10941092
{{
10951093
{typeInfoPropertyTypeRef}? {JsonTypeInfoReturnValueLocalVariableName} = null;
10961094
{WrapWithCheckForCustomConverter(metadataInitSource, typeCompilableName)}
10971095

1096+
if (makeReadOnly)
1097+
{{
1098+
{JsonTypeInfoReturnValueLocalVariableName}.{MakeReadOnlyMethodName}();
1099+
}}
1100+
10981101
return {JsonTypeInfoReturnValueLocalVariableName};
10991102
}}
11001103
{additionalSource}";
@@ -1271,30 +1274,23 @@ private string GetGetTypeInfoImplementation()
12711274
// Explicit IJsonTypeInfoResolver implementation
12721275
sb.AppendLine();
12731276
sb.Append(@$"{JsonTypeInfoTypeRef}? {JsonTypeInfoResolverTypeRef}.GetTypeInfo({TypeTypeRef} type, {JsonSerializerOptionsTypeRef} {OptionsLocalVariableName})
1274-
{{
1275-
if ({OptionsInstanceVariableName} == {OptionsLocalVariableName})
1276-
{{
1277-
return this.GetTypeInfo(type);
1278-
}}
1279-
else
1280-
{{");
1277+
{{");
12811278
// TODO (https://github.com/dotnet/runtime/issues/52218): Make this Dictionary-lookup-based if root-serializable type count > 64.
12821279
foreach (TypeGenerationSpec metadata in types)
12831280
{
12841281
if (metadata.ClassType != ClassType.TypeUnsupportedBySourceGen)
12851282
{
12861283
sb.Append($@"
1287-
if (type == typeof({metadata.TypeRef}))
1288-
{{
1289-
return {metadata.CreateTypeInfoMethodName}({OptionsLocalVariableName});
1290-
}}
1284+
if (type == typeof({metadata.TypeRef}))
1285+
{{
1286+
return {metadata.CreateTypeInfoMethodName}({OptionsLocalVariableName}, makeReadOnly: false);
1287+
}}
12911288
");
12921289
}
12931290
}
12941291

12951292
sb.Append($@"
1296-
return null;
1297-
}}
1293+
return null;
12981294
}}
12991295
");
13001296

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,7 +1195,9 @@ public abstract partial class JsonTypeInfo
11951195
internal JsonTypeInfo() { }
11961196
public System.Text.Json.Serialization.JsonConverter Converter { get { throw null; } }
11971197
public System.Func<object>? CreateObject { get { throw null; } set { } }
1198+
public bool IsReadOnly { get { throw null; } }
11981199
public System.Text.Json.Serialization.Metadata.JsonTypeInfoKind Kind { get { throw null; } }
1200+
public void MakeReadOnly() { throw null; }
11991201
public System.Text.Json.Serialization.JsonNumberHandling? NumberHandling { get { throw null; } set { } }
12001202
public System.Action<object>? OnDeserialized { get { throw null; } set { } }
12011203
public System.Action<object>? OnDeserializing { get { throw null; } set { } }

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,10 @@
247247
<value>Cannot add callbacks to the 'Modifiers' property after the resolver has been used for the first time.</value>
248248
</data>
249249
<data name="TypeInfoImmutable" xml:space="preserve">
250-
<value>JsonTypeInfo cannot be changed after first usage.</value>
250+
<value>This JsonTypeInfo instance is marked read-only or has already been used in serialization or deserialization.</value>
251251
</data>
252252
<data name="PropertyInfoImmutable" xml:space="preserve">
253-
<value>JsonPropertyInfo cannot be changed after first usage.</value>
253+
<value>This JsonTypeInfo instance is marked read-only or has already been used in serialization or deserialization.</value>
254254
</data>
255255
<data name="MaxDepthMustBePositive" xml:space="preserve">
256256
<value>Max depth must be positive.</value>
@@ -337,7 +337,7 @@
337337
<value>The JSON object contains a trailing comma at the end which is not supported in this mode. Change the reader options.</value>
338338
</data>
339339
<data name="SerializerOptionsReadOnly" xml:space="preserve">
340-
<value>Serializer options cannot be changed once serialization or deserialization has occurred.</value>
340+
<value>This JsonSerializerOptions instance is read-only or has already been used in serialization or deserialization.</value>
341341
</data>
342342
<data name="StreamNotWritable" xml:space="preserve">
343343
<value>Stream is not writable.</value>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ internal static JsonPropertyInfo GetPropertyPlaceholder()
266266

267267
private protected void VerifyMutable()
268268
{
269-
if (_isConfigured)
269+
if (ParentTypeInfo?.IsReadOnly == true)
270270
{
271271
ThrowHelper.ThrowInvalidOperationException_PropertyInfoImmutable();
272272
}

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,23 @@ public JsonPolymorphismOptions? PolymorphismOptions
256256
}
257257
}
258258

259+
/// <summary>
260+
/// Specifies whether the current instance has been locked for modification.
261+
/// </summary>
262+
/// <remarks>
263+
/// A <see cref="JsonTypeInfo"/> instance can be locked either if
264+
/// it has been passed to one of the <see cref="JsonSerializer"/> methods,
265+
/// has been associated with a <see cref="JsonSerializerContext"/> instance,
266+
/// or a user explicitly called the <see cref="MakeReadOnly"/> method on the instance.
267+
/// </remarks>
268+
public bool IsReadOnly { get; private set; }
269+
270+
/// <summary>
271+
/// Locks the current instance for further modification.
272+
/// </summary>
273+
/// <remarks>This method is idempotent.</remarks>
274+
public void MakeReadOnly() => IsReadOnly = true;
275+
259276
private protected JsonPolymorphismOptions? _polymorphismOptions;
260277

261278
internal object? CreateObjectWithArgs { get; set; }
@@ -477,7 +494,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, JsonSerializerOptions
477494

478495
internal void VerifyMutable()
479496
{
480-
if (_isConfigured)
497+
if (IsReadOnly)
481498
{
482499
ThrowHelper.ThrowInvalidOperationException_TypeInfoImmutable();
483500
}
@@ -511,6 +528,7 @@ void ConfigureLocked()
511528
{
512529
Configure();
513530

531+
IsReadOnly = true;
514532
_isConfigured = true;
515533
}
516534
catch (Exception e)
@@ -693,6 +711,7 @@ public static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions o
693711
/// <returns>A blank <see cref="JsonPropertyInfo"/> instance.</returns>
694712
/// <exception cref="ArgumentNullException"><paramref name="propertyType"/> or <paramref name="name"/> is null.</exception>
695713
/// <exception cref="ArgumentException"><paramref name="propertyType"/> cannot be used for serialization.</exception>
714+
/// <exception cref="InvalidOperationException">The <see cref="JsonTypeInfo"/> instance has been locked for further modification.</exception>
696715
[RequiresUnreferencedCode(MetadataFactoryRequiresUnreferencedCode)]
697716
[RequiresDynamicCode(MetadataFactoryRequiresUnreferencedCode)]
698717
public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name)
@@ -712,6 +731,7 @@ public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name)
712731
ThrowHelper.ThrowArgumentException_CannotSerializeInvalidType(nameof(propertyType), propertyType, Type, name);
713732
}
714733

734+
VerifyMutable();
715735
JsonPropertyInfo propertyInfo = CreatePropertyUsingReflection(propertyType);
716736
propertyInfo.Name = name;
717737

@@ -748,6 +768,8 @@ internal void CacheMember(JsonPropertyInfo jsonPropertyInfo, JsonPropertyDiction
748768
Debug.Assert(jsonPropertyInfo.MemberName != null, "MemberName can be null in custom JsonPropertyInfo instances and should never be passed in this method");
749769
string memberName = jsonPropertyInfo.MemberName;
750770

771+
jsonPropertyInfo.EnsureChildOf(this);
772+
751773
if (jsonPropertyInfo.IsExtensionData)
752774
{
753775
if (ExtensionDataProperty != null)

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,70 @@ public static void VariousNestingAndVisibilityLevelsAreSupported()
2121
Assert.NotNull(NestedPublicContext.NestedProtectedInternalClass.Default);
2222
}
2323

24+
[Fact]
25+
public static void PropertyMetadataIsImmutable()
26+
{
27+
JsonTypeInfo<Person> typeInfo = PersonJsonContext.Default.Person;
28+
29+
Assert.True(typeInfo.IsReadOnly);
30+
Assert.Throws<InvalidOperationException>(() => typeInfo.CreateObject = null);
31+
Assert.Throws<InvalidOperationException>(() => typeInfo.OnDeserializing = obj => { });
32+
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Clear());
33+
34+
JsonPropertyInfo propertyInfo = typeInfo.Properties[0];
35+
Assert.Throws<InvalidOperationException>(() => propertyInfo.Name = "differentName");
36+
Assert.Throws<InvalidOperationException>(() => propertyInfo.NumberHandling = JsonNumberHandling.AllowReadingFromString);
37+
Assert.Throws<InvalidOperationException>(() => propertyInfo.IsRequired = true);
38+
Assert.Throws<InvalidOperationException>(() => propertyInfo.Order = -1);
39+
}
40+
41+
[Fact]
42+
public static void JsonSerializerContext_GetTypeInfo_MetadataIsImmutable()
43+
{
44+
JsonTypeInfo<Person> typeInfo = (JsonTypeInfo<Person>)PersonJsonContext.Default.GetTypeInfo(typeof(Person));
45+
46+
Assert.True(typeInfo.IsReadOnly);
47+
Assert.Throws<InvalidOperationException>(() => typeInfo.CreateObject = null);
48+
Assert.Throws<InvalidOperationException>(() => typeInfo.OnDeserializing = obj => { });
49+
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Clear());
50+
51+
JsonPropertyInfo propertyInfo = typeInfo.Properties[0];
52+
Assert.Throws<InvalidOperationException>(() => propertyInfo.Name = "differentName");
53+
Assert.Throws<InvalidOperationException>(() => propertyInfo.NumberHandling = JsonNumberHandling.AllowReadingFromString);
54+
Assert.Throws<InvalidOperationException>(() => propertyInfo.IsRequired = true);
55+
Assert.Throws<InvalidOperationException>(() => propertyInfo.Order = -1);
56+
}
57+
58+
[Fact]
59+
public static void IJsonTypeInfoResolver_GetTypeInfo_MetadataIsMutable()
60+
{
61+
IJsonTypeInfoResolver resolver = PersonJsonContext.Default;
62+
JsonTypeInfo<Person> typeInfo = (JsonTypeInfo<Person>)resolver.GetTypeInfo(typeof(Person), PersonJsonContext.Default.Options);
63+
64+
Assert.NotSame(typeInfo, PersonJsonContext.Default.Person);
65+
Assert.False(typeInfo.IsReadOnly);
66+
67+
JsonTypeInfo<Person> typeInfo2 = (JsonTypeInfo<Person>)resolver.GetTypeInfo(typeof(Person), PersonJsonContext.Default.Options);
68+
Assert.NotSame(typeInfo, typeInfo2);
69+
Assert.False(typeInfo.IsReadOnly);
70+
71+
typeInfo.CreateObject = null;
72+
typeInfo.OnDeserializing = obj => { };
73+
74+
JsonPropertyInfo propertyInfo = typeInfo.Properties[0];
75+
propertyInfo.Name = "differentName";
76+
propertyInfo.NumberHandling = JsonNumberHandling.AllowReadingFromString;
77+
propertyInfo.IsRequired = true;
78+
propertyInfo.Order = -1;
79+
80+
typeInfo.Properties.Clear();
81+
Assert.Equal(0, typeInfo.Properties.Count);
82+
83+
// Changes should not impact other metadata instances
84+
Assert.Equal(2, typeInfo2.Properties.Count);
85+
Assert.Equal(2, PersonJsonContext.Default.Person.Properties.Count);
86+
}
87+
2488
[Fact]
2589
public static void VariousGenericsAreSupported()
2690
{

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public static void TypeInfoPropertiesDefaults(Type type)
3636

3737
JsonTypeInfo ti = r.GetTypeInfo(type, o);
3838

39+
Assert.False(ti.IsReadOnly);
3940
Assert.Same(o, ti.Options);
4041
Assert.NotNull(ti.Properties);
4142

@@ -400,17 +401,25 @@ private static void TestTypeInfoImmutability<T>(JsonTypeInfo<T> typeInfo)
400401
Assert.Equal(typeof(T), typeInfo.Type);
401402
Assert.True(typeInfo.Converter.CanConvert(typeof(T)));
402403

403-
JsonPropertyInfo prop = typeInfo.CreateJsonPropertyInfo(typeof(string), "foo");
404+
Assert.True(typeInfo.IsReadOnly);
404405
Assert.True(typeInfo.Properties.IsReadOnly);
406+
Assert.Throws<InvalidOperationException>(() => typeInfo.CreateJsonPropertyInfo(typeof(string), "foo"));
405407
Assert.Throws<InvalidOperationException>(() => untyped.CreateObject = untyped.CreateObject);
406408
Assert.Throws<InvalidOperationException>(() => typeInfo.CreateObject = typeInfo.CreateObject);
407409
Assert.Throws<InvalidOperationException>(() => typeInfo.NumberHandling = typeInfo.NumberHandling);
410+
Assert.Throws<InvalidOperationException>(() => typeInfo.CreateJsonPropertyInfo(typeof(string), "foo"));
408411
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Clear());
409-
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Add(prop));
410-
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Insert(0, prop));
411412
Assert.Throws<InvalidOperationException>(() => typeInfo.PolymorphismOptions = null);
412413
Assert.Throws<InvalidOperationException>(() => typeInfo.PolymorphismOptions = new());
413414

415+
if (typeInfo.Properties.Count > 0)
416+
{
417+
JsonPropertyInfo prop = typeInfo.Properties[0];
418+
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Add(prop));
419+
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.Insert(0, prop));
420+
Assert.Throws<InvalidOperationException>(() => typeInfo.Properties.RemoveAt(0));
421+
}
422+
414423
if (typeInfo.PolymorphismOptions is JsonPolymorphismOptions jpo)
415424
{
416425
Assert.True(jpo.DerivedTypes.IsReadOnly);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,9 +1045,11 @@ public static void GetTypeInfo_MutableOptionsInstance(Type type)
10451045
options.TypeInfoResolver = new DefaultJsonTypeInfoResolver();
10461046
JsonTypeInfo typeInfo = options.GetTypeInfo(type);
10471047
Assert.Equal(type, typeInfo.Type);
1048+
Assert.False(typeInfo.IsReadOnly);
10481049

10491050
JsonTypeInfo typeInfo2 = options.GetTypeInfo(type);
10501051
Assert.Equal(type, typeInfo2.Type);
1052+
Assert.False(typeInfo2.IsReadOnly);
10511053

10521054
Assert.NotSame(typeInfo, typeInfo2);
10531055

@@ -1066,6 +1068,7 @@ public static void GetTypeInfo_ImmutableOptionsInstance(Type type)
10661068

10671069
JsonTypeInfo typeInfo = options.GetTypeInfo(type);
10681070
Assert.Equal(type, typeInfo.Type);
1071+
Assert.True(typeInfo.IsReadOnly);
10691072

10701073
JsonTypeInfo typeInfo2 = options.GetTypeInfo(type);
10711074
Assert.Same(typeInfo, typeInfo2);
@@ -1077,6 +1080,7 @@ public static void GetTypeInfo_MutableOptions_CanModifyMetadata()
10771080
var options = new JsonSerializerOptions { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };
10781081
JsonTypeInfo<TestClassForEncoding> jti = (JsonTypeInfo<TestClassForEncoding>)options.GetTypeInfo(typeof(TestClassForEncoding));
10791082

1083+
Assert.False(jti.IsReadOnly);
10801084
Assert.Equal(1, jti.Properties.Count);
10811085
jti.Properties.Clear();
10821086

@@ -1088,11 +1092,13 @@ public static void GetTypeInfo_MutableOptions_CanModifyMetadata()
10881092

10891093
// Using JsonTypeInfo will lock JsonSerializerOptions
10901094
Assert.True(options.IsReadOnly);
1095+
Assert.True(jti.IsReadOnly);
10911096
Assert.Throws<InvalidOperationException>(() => options.IncludeFields = false);
10921097

10931098
// Getting JsonTypeInfo now should return a fresh immutable instance
10941099
JsonTypeInfo<TestClassForEncoding> jti2 = (JsonTypeInfo<TestClassForEncoding>)options.GetTypeInfo(typeof(TestClassForEncoding));
10951100
Assert.NotSame(jti, jti2);
1101+
Assert.True(jti2.IsReadOnly);
10961102
Assert.Equal(1, jti2.Properties.Count);
10971103
Assert.Throws<InvalidOperationException>(() => jti2.Properties.Clear());
10981104

0 commit comments

Comments
 (0)