Skip to content

Commit bfa0276

Browse files
System.Text.Json: Fix polymorphic state bug when using ReferenceHandler.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 <[email protected]>
1 parent e66d834 commit bfa0276

File tree

5 files changed

+113
-0
lines changed

5 files changed

+113
-0
lines changed

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.MetadataHandling.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,13 @@ internal bool TryHandleSerializedObjectReference(Utf8JsonWriter writer, object v
148148
if (resolver.ContainsReferenceForCycleDetection(value))
149149
{
150150
writer.WriteNullValue();
151+
152+
if (polymorphicConverter is not null)
153+
{
154+
// Clear out any polymorphic state.
155+
state.PolymorphicTypeDiscriminator = null;
156+
state.PolymorphicTypeResolver = null;
157+
}
151158
return true;
152159
}
153160

src/libraries/System.Text.Json/tests/Common/ReferenceHandlerTests/ReferenceHandlerTests.IgnoreCycles.cs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,48 @@ public async Task IgnoreCycles_BoxedValueShouldNotBeIgnored()
429429
await Test_Serialize_And_SerializeAsync_Contains(root, expectedSubstring: @"""DayOfBirth"":15", expectedTimes: 2, s_optionsIgnoreCycles);
430430
}
431431

432+
[Fact]
433+
public async Task IgnoreCycles_DerivedType_InArray()
434+
{
435+
var worker = new OfficeWorker
436+
{
437+
Office = new Office
438+
{
439+
Dummy = new()
440+
}
441+
};
442+
443+
worker.Office.Staff = [worker, new RemoteWorker()];
444+
445+
await Test_Serialize_And_SerializeAsync(worker, """{"Office":{"Staff":[null,{"$type":"remote"}],"Dummy":{}}}""", s_optionsIgnoreCycles);
446+
447+
worker.Office.Staff = [worker];
448+
449+
await Test_Serialize_And_SerializeAsync(worker, """{"Office":{"Staff":[null],"Dummy":{}}}""", s_optionsIgnoreCycles);
450+
}
451+
452+
[JsonDerivedType(typeof(OfficeWorker), "office")]
453+
[JsonDerivedType(typeof(RemoteWorker), "remote")]
454+
public abstract class EmployeeLocation
455+
{
456+
}
457+
458+
public class OfficeWorker : EmployeeLocation
459+
{
460+
public Office Office { get; set; }
461+
}
462+
463+
public class RemoteWorker : EmployeeLocation
464+
{
465+
}
466+
467+
public class Office
468+
{
469+
public EmployeeLocation[] Staff { get; set; }
470+
471+
public EmptyClass Dummy { get; set; }
472+
}
473+
432474
[Fact]
433475
public async Task CycleDetectionStatePersistsAcrossContinuations()
434476
{

src/libraries/System.Text.Json/tests/Common/ReferenceHandlerTests/ReferenceHandlerTests.Serialize.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,5 +224,49 @@ public async Task CustomHashCode()
224224
// otherwise objects would not be correctly identified when searching for them in the dictionary.
225225
Assert.Same(listCopy[0], listCopy[1]);
226226
}
227+
228+
[Fact]
229+
public async Task Preserve_DerivedType_InArray()
230+
{
231+
var worker = new OfficeWorker
232+
{
233+
Office = new Office
234+
{
235+
Dummy = new()
236+
}
237+
};
238+
239+
worker.Office.Staff = [worker, new RemoteWorker()];
240+
241+
string json = await Serializer.SerializeWrapper(worker, s_serializerOptionsPreserve);
242+
Assert.Equal("""{"$id":"1","Office":{"$id":"2","Staff":[{"$ref":"1"},{"$id":"3","$type":"remote"}],"Dummy":{"$id":"4"}}}""", json);
243+
244+
worker.Office.Staff = [worker];
245+
246+
json = await Serializer.SerializeWrapper(worker, s_serializerOptionsPreserve);
247+
Assert.Equal("""{"$id":"1","Office":{"$id":"2","Staff":[{"$ref":"1"}],"Dummy":{"$id":"3"}}}""", json);
248+
}
249+
250+
[JsonDerivedType(typeof(OfficeWorker), "office")]
251+
[JsonDerivedType(typeof(RemoteWorker), "remote")]
252+
public abstract class EmployeeLocation
253+
{
254+
}
255+
256+
public class OfficeWorker : EmployeeLocation
257+
{
258+
public Office Office { get; set; }
259+
}
260+
261+
public class RemoteWorker : EmployeeLocation
262+
{
263+
}
264+
265+
public class Office
266+
{
267+
public EmployeeLocation[] Staff { get; set; }
268+
269+
public EmptyClass Dummy { get; set; }
270+
}
227271
}
228272
}

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ public ReferenceHandlerTests_IgnoreCycles_Metadata(JsonSerializerWrapper seriali
8989
[JsonSerializable(typeof(TreeNode<List<string>>))]
9090
[JsonSerializable(typeof(TreeNode<object>))]
9191
[JsonSerializable(typeof(int))]
92+
[JsonSerializable(typeof(EmployeeLocation))]
93+
[JsonSerializable(typeof(EmployeeLocation[]))]
94+
[JsonSerializable(typeof(OfficeWorker))]
95+
[JsonSerializable(typeof(Office))]
96+
[JsonSerializable(typeof(RemoteWorker))]
9297
internal sealed partial class ReferenceHandlerTests_IgnoreCyclesContext_Metadata : JsonSerializerContext
9398
{
9499
}
@@ -172,6 +177,11 @@ public ReferenceHandlerTests_IgnoreCycles_Default(JsonSerializerWrapper serializ
172177
[JsonSerializable(typeof(TreeNode<List<string>>))]
173178
[JsonSerializable(typeof(TreeNode<object>))]
174179
[JsonSerializable(typeof(int))]
180+
[JsonSerializable(typeof(EmployeeLocation))]
181+
[JsonSerializable(typeof(EmployeeLocation[]))]
182+
[JsonSerializable(typeof(OfficeWorker))]
183+
[JsonSerializable(typeof(Office))]
184+
[JsonSerializable(typeof(RemoteWorker))]
175185
internal sealed partial class ReferenceHandlerTests_IgnoreCyclesContext_Default : JsonSerializerContext
176186
{
177187
}

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/ReferenceHandlerTests.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,11 @@ public ReferenceHandlerTests_Metadata(JsonSerializerWrapper serializer)
139139
[JsonSerializable(typeof(ClassWithConflictingIdProperty))]
140140
[JsonSerializable(typeof(ClassWithIgnoredConflictingProperty))]
141141
[JsonSerializable(typeof(ClassWithExtensionDataConflictingProperty))]
142+
[JsonSerializable(typeof(EmployeeLocation))]
143+
[JsonSerializable(typeof(EmployeeLocation[]))]
144+
[JsonSerializable(typeof(OfficeWorker))]
145+
[JsonSerializable(typeof(Office))]
146+
[JsonSerializable(typeof(RemoteWorker))]
142147
internal sealed partial class ReferenceHandlerTestsContext_Metadata : JsonSerializerContext
143148
{
144149
}
@@ -281,6 +286,11 @@ public ReferenceHandlerTests_Default(JsonSerializerWrapper serializer)
281286
[JsonSerializable(typeof(ClassWithConflictingIdProperty))]
282287
[JsonSerializable(typeof(ClassWithIgnoredConflictingProperty))]
283288
[JsonSerializable(typeof(ClassWithExtensionDataConflictingProperty))]
289+
[JsonSerializable(typeof(EmployeeLocation))]
290+
[JsonSerializable(typeof(EmployeeLocation[]))]
291+
[JsonSerializable(typeof(OfficeWorker))]
292+
[JsonSerializable(typeof(Office))]
293+
[JsonSerializable(typeof(RemoteWorker))]
284294
internal sealed partial class ReferenceHandlerTestsContext_Default : JsonSerializerContext
285295
{
286296
}

0 commit comments

Comments
 (0)