Skip to content

Commit

Permalink
Fix STJ support for System.Reflection.NullabilityInfoContext.IsSuppor…
Browse files Browse the repository at this point in the history 
…ted = false (dotnet#102852)

* Fix STJ support for System.Reflection.NullabilityInfoContext.IsSupported = false

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

* Bring back exception in cases where RespectNullability has been turned on.

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs

---------

Co-authored-by: David Cantú <dacantu@microsoft.com>
  • Loading branch information
@eiriktsarpalis @jozkee
eiriktsarpalis and jozkee authored May 30, 2024
1 parent cdd1aca commit 9a3a278
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 13 deletions.
39 changes: 26 additions & 13 deletions ...t.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ namespace System.Text.Json.Serialization.Metadata
{
public partial class DefaultJsonTypeInfoResolver
{
private static readonly bool s_isNullabilityInfoContextSupported =
AppContext.TryGetSwitch("System.Reflection.NullabilityInfoContext.IsSupported", out bool isSupported) ? isSupported : true;

internal static MemberAccessor MemberAccessor
{
[RequiresUnreferencedCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
Expand Down Expand Up @@ -71,7 +74,10 @@ private static JsonTypeInfo CreateTypeInfoCore(Type type, JsonConverter converte

if (typeInfo.Kind is JsonTypeInfoKind.Object)
{
NullabilityInfoContext nullabilityCtx = new();
// If the System.Reflection.NullabilityInfoContext.IsSupported feature switch has been disabled,
// we want to avoid resolving nullability information for properties and parameters unless the
// user has explicitly opted into nullability enforcement in which case an exception will be surfaced.
NullabilityInfoContext? nullabilityCtx = s_isNullabilityInfoContextSupported || options.RespectNullableAnnotations ? new() : null;

if (converter.ConstructorIsParameterized)
{
Expand All @@ -91,7 +97,7 @@ private static JsonTypeInfo CreateTypeInfoCore(Type type, JsonConverter converte

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
private static void PopulateProperties(JsonTypeInfo typeInfo, NullabilityInfoContext nullabilityCtx)
private static void PopulateProperties(JsonTypeInfo typeInfo, NullabilityInfoContext? nullabilityCtx)
{
Debug.Assert(!typeInfo.IsReadOnly);
Debug.Assert(typeInfo.Kind is JsonTypeInfoKind.Object);
Expand Down Expand Up @@ -158,7 +164,7 @@ private static void PopulateProperties(JsonTypeInfo typeInfo, NullabilityInfoCon
private static void AddMembersDeclaredBySuperType(
JsonTypeInfo typeInfo,
Type currentType,
NullabilityInfoContext nullabilityCtx,
NullabilityInfoContext? nullabilityCtx,
bool constructorHasSetsRequiredMembersAttribute,
ref JsonTypeInfo.PropertyHierarchyResolutionState state)
{
Expand Down Expand Up @@ -219,7 +225,7 @@ private static void AddMember(
JsonTypeInfo typeInfo,
Type typeToConvert,
MemberInfo memberInfo,
NullabilityInfoContext nullabilityCtx,
NullabilityInfoContext? nullabilityCtx,
bool shouldCheckForRequiredKeyword,
bool hasJsonIncludeAttribute,
ref JsonTypeInfo.PropertyHierarchyResolutionState state)
Expand All @@ -241,7 +247,7 @@ private static void AddMember(
JsonTypeInfo typeInfo,
Type typeToConvert,
MemberInfo memberInfo,
NullabilityInfoContext nullabilityCtx,
NullabilityInfoContext? nullabilityCtx,
JsonSerializerOptions options,
bool shouldCheckForRequiredKeyword,
bool hasJsonIncludeAttribute)
Expand Down Expand Up @@ -301,7 +307,7 @@ private static bool PropertyIsOverriddenAndIgnored(PropertyInfo propertyInfo, Di

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
private static void PopulateParameterInfoValues(JsonTypeInfo typeInfo, NullabilityInfoContext nullabilityCtx)
private static void PopulateParameterInfoValues(JsonTypeInfo typeInfo, NullabilityInfoContext? nullabilityCtx)
{
Debug.Assert(typeInfo.Converter.ConstructorInfo != null);
ParameterInfo[] parameters = typeInfo.Converter.ConstructorInfo.GetParameters();
Expand All @@ -326,9 +332,7 @@ private static void PopulateParameterInfoValues(JsonTypeInfo typeInfo, Nullabili
Position = reflectionInfo.Position,
HasDefaultValue = reflectionInfo.HasDefaultValue,
DefaultValue = reflectionInfo.GetDefaultValue(),
IsNullable =
reflectionInfo.ParameterType.IsNullableType() &&
DetermineParameterNullability(reflectionInfo, nullabilityCtx) is not NullabilityState.NotNull,
IsNullable = DetermineParameterNullability(reflectionInfo, nullabilityCtx) is not NullabilityState.NotNull,
};

jsonParameters[i] = jsonInfo;
Expand All @@ -344,7 +348,7 @@ private static void PopulatePropertyInfo(
MemberInfo memberInfo,
JsonConverter? customConverter,
JsonIgnoreCondition? ignoreCondition,
NullabilityInfoContext nullabilityCtx,
NullabilityInfoContext? nullabilityCtx,
bool shouldCheckForRequiredKeyword,
bool hasJsonIncludeAttribute)
{
Expand Down Expand Up @@ -487,9 +491,9 @@ internal static void DeterminePropertyAccessors<T>(JsonPropertyInfo<T> jsonPrope

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
private static void DeterminePropertyNullability(JsonPropertyInfo propertyInfo, MemberInfo memberInfo, NullabilityInfoContext nullabilityCtx)
private static void DeterminePropertyNullability(JsonPropertyInfo propertyInfo, MemberInfo memberInfo, NullabilityInfoContext? nullabilityCtx)
{
if (!propertyInfo.PropertyTypeCanBeNull)
if (!propertyInfo.PropertyTypeCanBeNull || nullabilityCtx is null)
{
return;
}
Expand All @@ -511,8 +515,17 @@ private static void DeterminePropertyNullability(JsonPropertyInfo propertyInfo,

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
private static NullabilityState DetermineParameterNullability(ParameterInfo parameterInfo, NullabilityInfoContext nullabilityCtx)
private static NullabilityState DetermineParameterNullability(ParameterInfo parameterInfo, NullabilityInfoContext? nullabilityCtx)
{
if (!parameterInfo.ParameterType.IsNullableType())
{
return NullabilityState.NotNull;
}

if (nullabilityCtx is null)
{
return NullabilityState.Unknown;
}
#if NET8_0
// Workaround for https://github.com/dotnet/runtime/issues/92487
// The fix has been incorporated into .NET 9 (and the polyfilled implementations in netfx).
Expand Down
54 changes: 54 additions & 0 deletions src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,60 @@ public static void Options_RespectNullableAnnotationsDefault_FeatureSwitch(bool?
}, arg, options).Dispose();
}

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void Options_NullabilityInfoFeatureSwitchDisabled_ReportsPropertiesAsNullable()
{
var options = new RemoteInvokeOptions()
{
RuntimeConfigurationOptions =
{
["System.Reflection.NullabilityInfoContext.IsSupported"] = false
}
};

RemoteExecutor.Invoke(static () =>
{
var value = new NullableAnnotationsTests.NotNullablePropertyClass();
string expectedJson = """{"Property":null}""";
Assert.Null(value.Property);
string json = JsonSerializer.Serialize(value);
Assert.Equal(expectedJson, json);
value = JsonSerializer.Deserialize<NullableAnnotationsTests.NotNullablePropertyClass>(json);
Assert.Null(value.Property);
}, options).Dispose();
}

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void Options_NullabilityInfoFeatureSwitchDisabled_RespectNullabilityAnnotationsEnabled_ThrowsInvalidOperationException()
{
var options = new RemoteInvokeOptions()
{
RuntimeConfigurationOptions =
{
["System.Reflection.NullabilityInfoContext.IsSupported"] = false
}
};

RemoteExecutor.Invoke(static () =>
{
var jsonOptions = new JsonSerializerOptions { RespectNullableAnnotations = true };
var value = new NullableAnnotationsTests.NotNullablePropertyClass();
string expectedJson = """{"Property":null}""";
InvalidOperationException ex;
ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(value, jsonOptions));
Assert.Contains("System.Reflection.NullabilityInfoContext.IsSupported", ex.Message);
ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<NullableAnnotationsTests.NotNullablePropertyClass>(expectedJson, jsonOptions));
Assert.Contains("System.Reflection.NullabilityInfoContext.IsSupported", ex.Message);
}, options).Dispose();
}

private static void GenericObjectOrJsonElementConverterTestHelper<T>(string converterName, object objectValue, string stringValue)
{
var options = new JsonSerializerOptions();
Expand Down

0 comments on commit 9a3a278

Please sign in to comment.