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

System.Text.Json polymorphism serialized with ReferenceHandler.IgnoreCycles #111790

Open
yesmey opened this issue Jan 24, 2025 · 4 comments · May be fixed by #111808
Open

System.Text.Json polymorphism serialized with ReferenceHandler.IgnoreCycles #111790

yesmey opened this issue Jan 24, 2025 · 4 comments · May be fixed by #111808
Labels
area-System.Text.Json bug in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@yesmey
Copy link
Contributor

yesmey commented Jan 24, 2025

Description

Under very specific conditions (using ReferenceHandler.IgnoreCycles, JsonDerivedType, and objects contained within an array), the serializer incorrectly applies the discriminator to a unrelated object that is serialized after.

After serializing a polymorphic object within an array - which is removed because its a circular reference - the serializer adds the discriminator to the next coming object.

Reproduction Steps

var person = new Employee
{
    Office = new Office(),
    Foo = new Dummy()
};

person.Office.People = [person];

var options = new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.IgnoreCycles };
var json = JsonSerializer.Serialize(person, options);
Console.WriteLine(json);

[JsonDerivedType(typeof(Employee), "Employee")]
abstract class Person
{
}

class Employee : Person
{
    // public Dummy Foo { get; set; } Issue does not occur here

    public Office Office { get; set; }

    public Dummy Foo { get; set; } // Issue occurs here
}

class Office
{
    // Doesnt matter what type of collection is being used, IEnumerable/List/Array/Dictionary
    public List<Person> People { get; set; }
    // public Dummy Foo { get; set; } Issue also occurs here
}

class Dummy
{
}

Expected behavior

Expected output:

{
    "Foo": {},
    "Office": {
        "People": [
            null
        ]
    }
}

Actual behavior

{
    "Foo": {
        "$type": "Employee"
    },
    "Office": {
        "People": [
            null
        ]
    }
}

Regression?

No response

Known Workarounds

No response

Configuration

Tested on .NET version 8.0.112 and 9.0.101

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 24, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

@eiriktsarpalis
Copy link
Member

Oh wow. Thanks for reporting this.

@eiriktsarpalis eiriktsarpalis added bug and removed untriaged New issue has not been triaged by the area owner labels Jan 24, 2025
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Jan 24, 2025
@yesmey
Copy link
Contributor Author

yesmey commented Jan 24, 2025

Don't have the ability to test the code right now, but it seems to me the problem lies here:

if (!isContinuation && options.ReferenceHandlingStrategy != JsonKnownReferenceHandler.Unspecified &&
TryHandleSerializedObjectReference(writer, value, options, polymorphicConverter, ref state))
{
// The reference handler wrote reference metadata, serialization complete.
return true;
}

The object in the array enters ResolvePolymorphicConverter which will change the polymorphic state to PolymorphicReEntryStarted. We then enter TryHandleSerializedObjectReference and return early without ever changing back the PolymorphicState, so whatever comes next is going to inherit it.

This might fix it

if (!isContinuation && options.ReferenceHandlingStrategy != JsonKnownReferenceHandler.Unspecified &&
    TryHandleSerializedObjectReference(writer, value, options, polymorphicConverter, ref state))
{
+     if (polymorphicConverter is not null)
+     {
+         state.Current.ExitPolymorphicConverter(true);
+     } 
    // The reference handler wrote reference metadata, serialization complete.
    return true;
}

Just guessing though :)

@eiriktsarpalis
Copy link
Member

I won't be able to take a look at this soon, so if you have a good idea on how to fix this may I suggest submitting a PR?

yesmey added a commit to yesmey/runtime that referenced this issue Jan 24, 2025
Clear the polymorphic state after ignoring a reference to prevent subsequent object writes from inheriting it.
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json bug in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants