Skip to content

Commit

Permalink
System.Text.Json: Fix polymorphic state bug when using ReferenceHandl…
Browse files Browse the repository at this point in the history
…er.IgnoreCycles (#111808)

* Fix issue #111790

Clear the polymorphic state after ignoring a reference to prevent subsequent object writes from inheriting it.

* Copy tests to cover preserve references

* Update metadata

* Add classes to referencehandlertest.ignorecycles

---------

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
  • Loading branch information
yesmey and eiriktsarpalis authored Jan 29, 2025
1 parent e66d834 commit bfa0276
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ internal bool TryHandleSerializedObjectReference(Utf8JsonWriter writer, object v
if (resolver.ContainsReferenceForCycleDetection(value))
{
writer.WriteNullValue();

if (polymorphicConverter is not null)
{
// Clear out any polymorphic state.
state.PolymorphicTypeDiscriminator = null;
state.PolymorphicTypeResolver = null;
}
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,48 @@ public async Task IgnoreCycles_BoxedValueShouldNotBeIgnored()
await Test_Serialize_And_SerializeAsync_Contains(root, expectedSubstring: @"""DayOfBirth"":15", expectedTimes: 2, s_optionsIgnoreCycles);
}

[Fact]
public async Task IgnoreCycles_DerivedType_InArray()
{
var worker = new OfficeWorker
{
Office = new Office
{
Dummy = new()
}
};

worker.Office.Staff = [worker, new RemoteWorker()];

await Test_Serialize_And_SerializeAsync(worker, """{"Office":{"Staff":[null,{"$type":"remote"}],"Dummy":{}}}""", s_optionsIgnoreCycles);

worker.Office.Staff = [worker];

await Test_Serialize_And_SerializeAsync(worker, """{"Office":{"Staff":[null],"Dummy":{}}}""", s_optionsIgnoreCycles);
}

[JsonDerivedType(typeof(OfficeWorker), "office")]
[JsonDerivedType(typeof(RemoteWorker), "remote")]
public abstract class EmployeeLocation
{
}

public class OfficeWorker : EmployeeLocation
{
public Office Office { get; set; }
}

public class RemoteWorker : EmployeeLocation
{
}

public class Office
{
public EmployeeLocation[] Staff { get; set; }

public EmptyClass Dummy { get; set; }
}

[Fact]
public async Task CycleDetectionStatePersistsAcrossContinuations()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,5 +224,49 @@ public async Task CustomHashCode()
// otherwise objects would not be correctly identified when searching for them in the dictionary.
Assert.Same(listCopy[0], listCopy[1]);
}

[Fact]
public async Task Preserve_DerivedType_InArray()
{
var worker = new OfficeWorker
{
Office = new Office
{
Dummy = new()
}
};

worker.Office.Staff = [worker, new RemoteWorker()];

string json = await Serializer.SerializeWrapper(worker, s_serializerOptionsPreserve);
Assert.Equal("""{"$id":"1","Office":{"$id":"2","Staff":[{"$ref":"1"},{"$id":"3","$type":"remote"}],"Dummy":{"$id":"4"}}}""", json);

worker.Office.Staff = [worker];

json = await Serializer.SerializeWrapper(worker, s_serializerOptionsPreserve);
Assert.Equal("""{"$id":"1","Office":{"$id":"2","Staff":[{"$ref":"1"}],"Dummy":{"$id":"3"}}}""", json);
}

[JsonDerivedType(typeof(OfficeWorker), "office")]
[JsonDerivedType(typeof(RemoteWorker), "remote")]
public abstract class EmployeeLocation
{
}

public class OfficeWorker : EmployeeLocation
{
public Office Office { get; set; }
}

public class RemoteWorker : EmployeeLocation
{
}

public class Office
{
public EmployeeLocation[] Staff { get; set; }

public EmptyClass Dummy { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ public ReferenceHandlerTests_IgnoreCycles_Metadata(JsonSerializerWrapper seriali
[JsonSerializable(typeof(TreeNode<List<string>>))]
[JsonSerializable(typeof(TreeNode<object>))]
[JsonSerializable(typeof(int))]
[JsonSerializable(typeof(EmployeeLocation))]
[JsonSerializable(typeof(EmployeeLocation[]))]
[JsonSerializable(typeof(OfficeWorker))]
[JsonSerializable(typeof(Office))]
[JsonSerializable(typeof(RemoteWorker))]
internal sealed partial class ReferenceHandlerTests_IgnoreCyclesContext_Metadata : JsonSerializerContext
{
}
Expand Down Expand Up @@ -172,6 +177,11 @@ public ReferenceHandlerTests_IgnoreCycles_Default(JsonSerializerWrapper serializ
[JsonSerializable(typeof(TreeNode<List<string>>))]
[JsonSerializable(typeof(TreeNode<object>))]
[JsonSerializable(typeof(int))]
[JsonSerializable(typeof(EmployeeLocation))]
[JsonSerializable(typeof(EmployeeLocation[]))]
[JsonSerializable(typeof(OfficeWorker))]
[JsonSerializable(typeof(Office))]
[JsonSerializable(typeof(RemoteWorker))]
internal sealed partial class ReferenceHandlerTests_IgnoreCyclesContext_Default : JsonSerializerContext
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ public ReferenceHandlerTests_Metadata(JsonSerializerWrapper serializer)
[JsonSerializable(typeof(ClassWithConflictingIdProperty))]
[JsonSerializable(typeof(ClassWithIgnoredConflictingProperty))]
[JsonSerializable(typeof(ClassWithExtensionDataConflictingProperty))]
[JsonSerializable(typeof(EmployeeLocation))]
[JsonSerializable(typeof(EmployeeLocation[]))]
[JsonSerializable(typeof(OfficeWorker))]
[JsonSerializable(typeof(Office))]
[JsonSerializable(typeof(RemoteWorker))]
internal sealed partial class ReferenceHandlerTestsContext_Metadata : JsonSerializerContext
{
}
Expand Down Expand Up @@ -281,6 +286,11 @@ public ReferenceHandlerTests_Default(JsonSerializerWrapper serializer)
[JsonSerializable(typeof(ClassWithConflictingIdProperty))]
[JsonSerializable(typeof(ClassWithIgnoredConflictingProperty))]
[JsonSerializable(typeof(ClassWithExtensionDataConflictingProperty))]
[JsonSerializable(typeof(EmployeeLocation))]
[JsonSerializable(typeof(EmployeeLocation[]))]
[JsonSerializable(typeof(OfficeWorker))]
[JsonSerializable(typeof(Office))]
[JsonSerializable(typeof(RemoteWorker))]
internal sealed partial class ReferenceHandlerTestsContext_Default : JsonSerializerContext
{
}
Expand Down

0 comments on commit bfa0276

Please sign in to comment.