Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Breaking change]: The System.Text.Json reflection-based deserializer is resolving all property metadata eagerly #37041

Closed
1 of 3 tasks
eiriktsarpalis opened this issue Sep 11, 2023 · 0 comments · Fixed by #37112
Assignees
Labels
breaking-change Indicates a .NET Core breaking change doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.

Comments

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Sep 11, 2023

Description

The System.Text.Json reflection-based serializer has been using a lazy loading approach when it comes to resolving property metadata. This has made it possible for POCOs containing unsupported property types to deserialize successfully, provided that the underlying JSON doesn't bind to any of the unsupported properties. This is despite the fact that instances of the same type would fail to serialize.

Starting with .NET 8, the serializer has been changed so that all properties are resolved eagerly in both serialization and deserialization. This has been done to add better support for combining multiple resolvers (cf. dotnet/runtime#71933), which necessitates early analysis of the serialized type graph. A side-effect of this change is that users currently depending on the previous behaviour could start seeing runtime deserialization errors where it previously was working.

Version

.NET 8 Preview 4

Previous behavior

The code

var result = JsonSerializer.Deserialize<MyPoco>("""{ "Value": 1 }""");//, MyContext.Default.MyPoco);
Console.WriteLine(result.Value);

public class MyPoco
{
    public int Value { get; set; }

    public NestedValue Unsupported { get; set; }
}

public class NestedValue
{
    public ReadOnlySpan<byte> Span => Array.Empty<byte>();
}

Will work without issue in .NET 7 applications.

New behavior

The same code will throw the following runtime exception in .NET 8:

System.InvalidOperationException: The type 'System.ReadOnlySpan`1[System.Byte]' of property 'Span' on type 'NestedValue' is invalid for serialization or deserialization because it is a pointer type, is a ref struct, or contains generic parameters that have not been replaced by specific types.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(Type typeToConvert, Type declaringType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.CreatePropertyInfo(JsonTypeInfo typeInfo, Type typeToConvert, MemberInfo memberInfo, JsonSerializerOptions options, Boolean shouldCheckForRequiredKeyword, Boolean hasJsonIncludeAttribute)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.AddMembersDeclaredBySuperType(JsonTypeInfo typeInfo, Type currentType, Boolean constructorHasSetsRequiredMembersAttribute, PropertyHierarchyResolutionState& state)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.PopulateProperties(JsonTypeInfo typeInfo)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.CreateTypeInfoCore(Type type, JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoNoCaching(Type type)
   at System.Text.Json.JsonSerializerOptions.CachingContext.CreateCacheEntry(Type type, CachingContext context)

Which is consistent with the error thrown when attempting to serialize instances of the same type in .NET 7 and the source generator (which produces a compile-time error).

Type of breaking change

  • Binary incompatible: Existing binaries may encounter a breaking change in behavior, such as failure to load or execute, and if so, require recompilation.
  • Source incompatible: When recompiled using the new SDK or component or to target the new runtime, existing source code may require source changes to compile successfully.
  • Behavioral change: Existing binaries may behave differently at run time.

Reason for change

Necessitated by new requirements related to fast-path serialization support in combined source generated contexts (cf. dotnet/runtime#71933)

Recommended action

Removing the unsupported property, authoring a custom converter for the unsupported type, or adding the JsonIgnoreAttribute like so:

public class MyPoco
{
    public int Value { get; set; }

    [JsonIgnore]
    public NestedValue Unsupported { get; set; }
}

Feature area

Serialization

Affected APIs

No response


Associated WorkItem - 159034

@eiriktsarpalis eiriktsarpalis added doc-idea Indicates issues that are suggestions for new topics [org][type][category] breaking-change Indicates a .NET Core breaking change Pri1 High priority, do before Pri2 and Pri3 labels Sep 11, 2023
@dotnet-bot dotnet-bot added the ⌚ Not Triaged Not triaged label Sep 11, 2023
@gewarren gewarren added the 🗺️ reQUEST Triggers an issue to be imported into Quest. label Sep 11, 2023
@github-actions github-actions bot added 📌 seQUESTered Identifies that an issue has been imported into Quest. and removed 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Sep 12, 2023
@gewarren gewarren removed the ⌚ Not Triaged Not triaged label Sep 13, 2023
@ghost ghost added the in-pr This issue will be closed (fixed) by an active pull request. label Sep 14, 2023
@ghost ghost removed the in-pr This issue will be closed (fixed) by an active pull request. label Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Indicates a .NET Core breaking change doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants