From fd9886d8f070c7b6713ff5411d1465f209e758d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Wed, 17 Feb 2021 20:04:19 -0800 Subject: [PATCH] Add option to ignore reference cycles on serialization (#46101) * Add option to ignore reference cycles on serialization * Fix whitespace * Combine IsValueType check on WriteCoreAsObject * Add missing null-forgiving operator * Fix perf regression by using ReferenceHandlerStrategy field in JsonSerializerOptions * Address suggestions * Rename API to IgnoreCycles * Add missing letter to test class name * Fix CI issue with nullable annotation --- .../System.Text.Json/ref/System.Text.Json.cs | 1 + .../src/System.Text.Json.csproj | 4 + .../Collection/DictionaryDefaultConverter.cs | 5 +- .../Collection/IEnumerableDefaultConverter.cs | 13 +- .../Object/ObjectDefaultConverter.cs | 8 +- .../Serialization/IgnoreReferenceHandler.cs | 12 + .../Serialization/IgnoreReferenceResolver.cs | 42 ++ .../JsonConverterOfT.WriteCore.cs | 15 +- .../Json/Serialization/JsonConverterOfT.cs | 81 +++- .../Json/Serialization/JsonPropertyInfoOfT.cs | 9 + .../JsonSerializer.Read.HandlePropertyName.cs | 2 +- .../Serialization/JsonSerializerOptions.cs | 5 + .../Text/Json/Serialization/ReadStack.cs | 2 +- .../Serialization/ReferenceEqualsWrapper.cs | 16 + .../Json/Serialization/ReferenceHandler.cs | 11 + .../ReferenceHandlingStrategy.cs | 12 + .../Json/Serialization/ReferenceResolver.cs | 8 + .../Text/Json/Serialization/WriteStack.cs | 5 +- .../ReferenceHandlerTests.IgnoreCycles.cs | 442 ++++++++++++++++++ .../tests/System.Text.Json.Tests.csproj | 3 +- 20 files changed, 652 insertions(+), 44 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceHandler.cs create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceResolver.cs create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceEqualsWrapper.cs create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandlingStrategy.cs create mode 100644 src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs index 3923f96036c62..885b26aaa2755 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -559,6 +559,7 @@ public abstract partial class ReferenceHandler { protected ReferenceHandler() { } public static System.Text.Json.Serialization.ReferenceHandler Preserve { get { throw null; } } + public static System.Text.Json.Serialization.ReferenceHandler IgnoreCycles { get { throw null; } } public abstract System.Text.Json.Serialization.ReferenceResolver CreateResolver(); } public sealed partial class ReferenceHandler : System.Text.Json.Serialization.ReferenceHandler where T : System.Text.Json.Serialization.ReferenceResolver, new() diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 889b79a4e1d83..ed010eff3ce5a 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -64,6 +64,8 @@ + + @@ -128,6 +130,7 @@ + @@ -174,6 +177,7 @@ + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs index 7ada24a994116..56ffdb52e62d2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs @@ -133,7 +133,7 @@ internal sealed override bool OnTryRead( } // Handle the metadata properties. - bool preserveReferences = options.ReferenceHandler != null; + bool preserveReferences = options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve; if (preserveReferences && state.Current.ObjectState < StackFrameObjectState.PropertyValue) { if (JsonSerializer.ResolveMetadataForJsonObject(ref reader, ref state, options)) @@ -272,8 +272,7 @@ internal sealed override bool OnTryWrite( { state.Current.ProcessedStartToken = true; writer.WriteStartObject(); - - if (options.ReferenceHandler != null) + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve) { if (JsonSerializer.WriteReferenceForObject(this, dictionary, ref state, writer) == MetadataPropertyName.Ref) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs index e7038ac9ec28b..67613d90973a3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IEnumerableDefaultConverter.cs @@ -90,7 +90,7 @@ internal override bool OnTryRead( { // Slower path that supports continuation and preserved references. - bool preserveReferences = options.ReferenceHandler != null; + bool preserveReferences = options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve; if (state.Current.ObjectState == StackFrameObjectState.None) { if (reader.TokenType == JsonTokenType.StartArray) @@ -236,12 +236,7 @@ internal sealed override bool OnTryWrite( if (!state.Current.ProcessedStartToken) { state.Current.ProcessedStartToken = true; - - if (options.ReferenceHandler == null) - { - writer.WriteStartArray(); - } - else + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve) { MetadataPropertyName metadata = JsonSerializer.WriteReferenceForCollection(this, value, ref state, writer); if (metadata == MetadataPropertyName.Ref) @@ -251,6 +246,10 @@ internal sealed override bool OnTryWrite( state.Current.MetadataPropertyName = metadata; } + else + { + writer.WriteStartArray(); + } state.Current.DeclaredJsonPropertyInfo = state.Current.JsonClassInfo.ElementClassInfo!.PropertyInfoForClassInfo; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs index 9dde207e48c21..636a0037e0620 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs @@ -75,7 +75,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, // Handle the metadata properties. if (state.Current.ObjectState < StackFrameObjectState.PropertyValue) { - if (options.ReferenceHandler != null) + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve) { if (JsonSerializer.ResolveMetadataForJsonObject(ref reader, ref state, options)) { @@ -233,8 +233,7 @@ internal sealed override bool OnTryWrite( if (!state.SupportContinuation) { writer.WriteStartObject(); - - if (options.ReferenceHandler != null) + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve) { if (JsonSerializer.WriteReferenceForObject(this, objectValue, ref state, writer) == MetadataPropertyName.Ref) { @@ -289,8 +288,7 @@ internal sealed override bool OnTryWrite( if (!state.Current.ProcessedStartToken) { writer.WriteStartObject(); - - if (options.ReferenceHandler != null) + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve) { if (JsonSerializer.WriteReferenceForObject(this, objectValue, ref state, writer) == MetadataPropertyName.Ref) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceHandler.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceHandler.cs new file mode 100644 index 0000000000000..23f174c831ee6 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceHandler.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Text.Json.Serialization +{ + internal sealed class IgnoreReferenceHandler : ReferenceHandler + { + public IgnoreReferenceHandler() => HandlingStrategy = ReferenceHandlingStrategy.IgnoreCycles; + + public override ReferenceResolver CreateResolver() => new IgnoreReferenceResolver(); + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceResolver.cs new file mode 100644 index 0000000000000..53a13a2b66c91 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IgnoreReferenceResolver.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; + +namespace System.Text.Json.Serialization +{ + internal sealed class IgnoreReferenceResolver : ReferenceResolver + { + // The stack of references on the branch of the current object graph, used to detect reference cycles. + private Stack? _stackForCycleDetection; + + internal override void PopReferenceForCycleDetection() + { + Debug.Assert(_stackForCycleDetection != null); + _stackForCycleDetection.Pop(); + } + + internal override bool ContainsReferenceForCycleDetection(object value) + => _stackForCycleDetection?.Contains(new ReferenceEqualsWrapper(value)) ?? false; + + internal override void PushReferenceForCycleDetection(object value) + { + var wrappedValue = new ReferenceEqualsWrapper(value); + + if (_stackForCycleDetection is null) + { + _stackForCycleDetection = new Stack(); + } + + Debug.Assert(!_stackForCycleDetection.Contains(wrappedValue)); + _stackForCycleDetection.Push(wrappedValue); + } + + public override void AddReference(string referenceId, object value) => throw new InvalidOperationException(); + + public override string GetReference(object value, out bool alreadyExists) => throw new InvalidOperationException(); + + public override object ResolveReference(string referenceId) => throw new InvalidOperationException(); + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs index 94240f3368829..1d60dbf4e38c3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs @@ -11,10 +11,19 @@ internal sealed override bool WriteCoreAsObject( JsonSerializerOptions options, ref WriteStack state) { - // Value types can never have a null except for Nullable. - if (value == null && IsValueType && Nullable.GetUnderlyingType(TypeToConvert) == null) + if (IsValueType) { - ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypeToConvert); + // Value types can never have a null except for Nullable. + if (value == null && Nullable.GetUnderlyingType(TypeToConvert) == null) + { + ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypeToConvert); + } + + // Root object is a boxed value type, we need to push it to the reference stack before it gets unboxed here. + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && value != null) + { + state.ReferenceResolver.PushReferenceForCycleDetection(value); + } } T actualValue = (T)value!; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs index d9cda3bafd97e..b55bc531c8cd8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs @@ -196,7 +196,8 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali ref reader); } - if (CanBePolymorphic && options.ReferenceHandler != null && value is JsonElement element) + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve && + CanBePolymorphic && value is JsonElement element) { // Edge case where we want to lookup for a reference when parsing into typeof(object) // instead of return `value` as a JsonElement. @@ -303,23 +304,48 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions ThrowHelper.ThrowJsonException_SerializerCycleDetected(options.EffectiveMaxDepth); } + if (CanBeNull && !HandleNullOnWrite && IsNull(value)) + { + // We do not pass null values to converters unless HandleNullOnWrite is true. Null values for properties were + // already handled in GetMemberAndWriteJson() so we don't need to check for IgnoreNullValues here. + writer.WriteNullValue(); + return true; + } + + bool ignoreCyclesPopReference = false; + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && + !IsValueType && !IsNull(value)) + { + Debug.Assert(value != null); + ReferenceResolver resolver = state.ReferenceResolver; + + // Write null to break reference cycles. + if (resolver.ContainsReferenceForCycleDetection(value)) + { + writer.WriteNullValue(); + return true; + } + + // For boxed reference types: do not push when boxed in order to avoid false positives + // when we run the ContainsReferenceForCycleDetection check for the converter of the unboxed value. + if (!CanBePolymorphic) + { + resolver.PushReferenceForCycleDetection(value); + ignoreCyclesPopReference = true; + } + } + if (CanBePolymorphic) { if (value == null) { - if (!HandleNullOnWrite) - { - writer.WriteNullValue(); - } - else - { - Debug.Assert(ClassType == ClassType.Value); - Debug.Assert(!state.IsContinuation); + Debug.Assert(ClassType == ClassType.Value); + Debug.Assert(!state.IsContinuation); + Debug.Assert(HandleNullOnWrite); - int originalPropertyDepth = writer.CurrentDepth; - Write(writer, value, options); - VerifyWrite(originalPropertyDepth, writer); - } + int originalPropertyDepth = writer.CurrentDepth; + Write(writer, value, options); + VerifyWrite(originalPropertyDepth, writer); return true; } @@ -339,18 +365,26 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions JsonConverter jsonConverter = state.Current.InitializeReEntry(type, options); if (jsonConverter != this) { + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && + jsonConverter.IsValueType) + { + // For boxed value types: push the value before it gets unboxed on TryWriteAsObject. + state.ReferenceResolver.PushReferenceForCycleDetection(value); + ignoreCyclesPopReference = true; + } + // We found a different converter; forward to that. - return jsonConverter.TryWriteAsObject(writer, value, options, ref state); + bool success2 = jsonConverter.TryWriteAsObject(writer, value, options, ref state); + + if (ignoreCyclesPopReference) + { + state.ReferenceResolver.PopReferenceForCycleDetection(); + } + + return success2; } } } - else if (CanBeNull && !HandleNullOnWrite && IsNull(value)) - { - // We do not pass null values to converters unless HandleNullOnWrite is true. Null values for properties were - // already handled in GetMemberAndWriteJson() so we don't need to check for IgnoreNullValues here. - writer.WriteNullValue(); - return true; - } if (ClassType == ClassType.Value) { @@ -390,6 +424,11 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions state.Pop(success); + if (ignoreCyclesPopReference) + { + state.ReferenceResolver.PopReferenceForCycleDetection(); + } + return success; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs index 797a96c43ceb0..075832706db1f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs @@ -138,6 +138,15 @@ public override bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf { T value = Get!(obj); + if (Options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.IgnoreCycles && + !Converter.IsValueType && value != null && + state.ReferenceResolver.ContainsReferenceForCycleDetection(value)) + { + // If a reference cycle is detected, treat value as null. + value = default!; + Debug.Assert(value == null); + } + if (IgnoreDefaultValuesOnWrite) { // If value is null, it is a reference type or nullable. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs index 17a0669b2a4ab..f0138ebf9997d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs @@ -79,7 +79,7 @@ internal static ReadOnlySpan GetPropertyName( unescapedPropertyName = propertyName; } - if (options.ReferenceHandler != null) + if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve) { if (propertyName.Length > 0 && propertyName[0] == '$') { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index 8ade08277fae7..e832865f9518a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -89,6 +89,7 @@ public JsonSerializerOptions(JsonSerializerOptions options) Converters = new ConverterList(this, (ConverterList)options.Converters); EffectiveMaxDepth = options.EffectiveMaxDepth; + ReferenceHandlingStrategy = options.ReferenceHandlingStrategy; // _classes is not copied as sharing the JsonClassInfo and JsonPropertyInfo caches can result in // unnecessary references to type metadata, potentially hindering garbage collection on the source options. @@ -487,9 +488,13 @@ public ReferenceHandler? ReferenceHandler { VerifyMutable(); _referenceHandler = value; + ReferenceHandlingStrategy = value?.HandlingStrategy ?? ReferenceHandlingStrategy.None; } } + // The cached value used to determine if ReferenceHandler should use Preserve or IgnoreCycles semanitcs or None of them. + internal ReferenceHandlingStrategy ReferenceHandlingStrategy = ReferenceHandlingStrategy.None; + internal MemberAccessor MemberAccessorStrategy { get diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs index ac03e3b6adba6..c74e84b4ee020 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs @@ -89,7 +89,7 @@ public void Initialize(Type type, JsonSerializerOptions options, bool supportCon Current.NumberHandling = Current.JsonPropertyInfo.NumberHandling; - bool preserveReferences = options.ReferenceHandler != null; + bool preserveReferences = options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve; if (preserveReferences) { ReferenceResolver = options.ReferenceHandler!.CreateResolver(writing: false); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceEqualsWrapper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceEqualsWrapper.cs new file mode 100644 index 0000000000000..dc292e2213096 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceEqualsWrapper.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; + +namespace System.Text.Json.Serialization +{ + internal struct ReferenceEqualsWrapper : IEquatable + { + private object _object; + public ReferenceEqualsWrapper(object obj) => _object = obj; + public override bool Equals(object? obj) => obj is ReferenceEqualsWrapper otherObj && Equals(otherObj); + public bool Equals(ReferenceEqualsWrapper obj) => ReferenceEquals(_object, obj._object); + public override int GetHashCode() => RuntimeHelpers.GetHashCode(_object); + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandler.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandler.cs index 5a951c6fb5a1a..ff88d0adb7c88 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandler.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandler.cs @@ -10,6 +10,12 @@ namespace System.Text.Json.Serialization /// public abstract class ReferenceHandler { + /// + /// Indicates whether this ReferenceHandler implementation should use semantics or semantics. + /// The defualt is Preserve. + /// + internal ReferenceHandlingStrategy HandlingStrategy = ReferenceHandlingStrategy.Preserve; + /// /// Metadata properties will be honored when deserializing JSON objects and arrays into reference types and written when serializing reference types. This is necessary to create round-trippable JSON from objects that contain cycles or duplicate references. /// @@ -39,6 +45,11 @@ public abstract class ReferenceHandler /// public static ReferenceHandler Preserve { get; } = new PreserveReferenceHandler(); + /// + /// Ignores an object when a reference cycle is detected during serialization. + /// + public static ReferenceHandler IgnoreCycles { get; } = new IgnoreReferenceHandler(); + /// /// Returns the used for each serialization call. /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandlingStrategy.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandlingStrategy.cs new file mode 100644 index 0000000000000..a99390da2c7d0 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandlingStrategy.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Text.Json.Serialization +{ + internal enum ReferenceHandlingStrategy + { + None, + Preserve, + IgnoreCycles + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceResolver.cs index a90e7f6c5ae10..4fbd68715426c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceResolver.cs @@ -33,5 +33,13 @@ public abstract class ReferenceResolver /// The reference id related to the returned object. /// The reference type object related to specified reference id. public abstract object ResolveReference(string referenceId); + + // We are breaking single responsibility on this class internally. + // In the future, if this model is required to be exposed, we can add a base class and extend this class and a new class containing below members from that base class. + internal virtual void PopReferenceForCycleDetection() => throw new InvalidOperationException(); + + internal virtual void PushReferenceForCycleDetection(object value) => throw new InvalidOperationException(); + + internal virtual bool ContainsReferenceForCycleDetection(object value) => throw new InvalidOperationException(); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs index e3378711eb1eb..7f231046fc036 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs @@ -73,9 +73,10 @@ public JsonConverter Initialize(Type type, JsonSerializerOptions options, bool s Current.DeclaredJsonPropertyInfo = jsonClassInfo.PropertyInfoForClassInfo; Current.NumberHandling = Current.DeclaredJsonPropertyInfo.NumberHandling; - if (options.ReferenceHandler != null) + if (options.ReferenceHandlingStrategy != ReferenceHandlingStrategy.None) { - ReferenceResolver = options.ReferenceHandler!.CreateResolver(writing: true); + Debug.Assert(options.ReferenceHandler != null); + ReferenceResolver = options.ReferenceHandler.CreateResolver(writing: true); } SupportContinuation = supportContinuation; diff --git a/src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs b/src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs new file mode 100644 index 0000000000000..c7de09f8199f3 --- /dev/null +++ b/src/libraries/System.Text.Json/tests/Serialization/ReferenceHandlerTests.IgnoreCycles.cs @@ -0,0 +1,442 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.IO; +using System.Threading.Tasks; +using Xunit; + +namespace System.Text.Json.Serialization.Tests +{ + public class ReferenceHandlerTests_IgnoreCycles + { + private static readonly JsonSerializerOptions s_optionsIgnoreCycles = + new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.IgnoreCycles }; + + [Fact] + public async Task IgnoreCycles_OnObject() + { + await Verify(); + await Verify(); + + async Task Verify() where T : class, new() + { + T root = new T(); + SetNextProperty(typeof(T), root, root); + + await Test_Serialize_And_SerializeAsync(root, @"{""Next"":null}", s_optionsIgnoreCycles); + + // Verify that object property is not mutated on serialization. + object rootNext = GetNextProperty(typeof(T), root); + Assert.NotNull(rootNext); + Assert.Same(rootNext, root); + } + } + + [Fact] + public async Task IgnoreCycles_OnObject_AsProperty() + { + await Verify(); + await Verify(); + + async Task Verify() where T : class, new() + { + var node = new T(); + SetNextProperty(typeof(T), node, node); + + var root = new ClassWithGenericProperty(); + root.Foo = node; + await Test_Serialize_And_SerializeAsync(root, expected: @"{""Foo"":{""Next"":null}}", s_optionsIgnoreCycles); + + object nodeNext = GetNextProperty(typeof(T), node); + Assert.NotNull(nodeNext); + Assert.Same(nodeNext, node); + + var rootWithObjProperty = new ClassWithGenericProperty(); + rootWithObjProperty.Foo = node; + await Test_Serialize_And_SerializeAsync(rootWithObjProperty, expected: @"{""Foo"":{""Next"":null}}", s_optionsIgnoreCycles); + + nodeNext = GetNextProperty(typeof(T), node); + Assert.NotNull(nodeNext); + Assert.Same(nodeNext, node); + } + } + + [Fact] + public async Task IgnoreCycles_OnBoxedValueType() + { + await Verify(); + await Verify(); + + async Task Verify() where T : new() + { + object root = new T(); + SetNextProperty(typeof(T), root, root); + await Test_Serialize_And_SerializeAsync(root, expected: @"{""Next"":null}", s_optionsIgnoreCycles); + + object rootNext = GetNextProperty(typeof(T), root); + Assert.NotNull(rootNext); + Assert.Same(rootNext, root); + } + } + + [Fact] + public async Task IgnoreCycles_OnBoxedValueType_Interface() + { + IValueNodeWithIValueNodeProperty root = new ValueNodeWithIValueNodeProperty(); + root.Next = root; + await Test_Serialize_And_SerializeAsync(root, expected: @"{""Next"":null}", s_optionsIgnoreCycles); + + IValueNodeWithObjectProperty root2 = new ValueNodeWithObjectProperty(); + root2.Next = root2; + await Test_Serialize_And_SerializeAsync(root2, expected: @"{""Next"":null}", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnBoxedValueType_AsProperty() + { + await Verify(); + await Verify(); + + async Task Verify() where T : new() + { + object node = new T(); + SetNextProperty(typeof(T), node, node); + + var rootWithObjProperty = new ClassWithGenericProperty(); + rootWithObjProperty.Foo = node; + await Test_Serialize_And_SerializeAsync(rootWithObjProperty, expected: @"{""Foo"":{""Next"":null}}", s_optionsIgnoreCycles); + + object nodeNext = GetNextProperty(typeof(T), node); + Assert.NotNull(nodeNext); + Assert.Same(nodeNext, node); + } + } + + [Theory] + [InlineData(typeof(Dictionary))] + [InlineData(typeof(GenericIDictionaryWrapper))] + public async Task IgnoreCycles_OnDictionary(Type typeToSerialize) + { + var root = (IDictionary)Activator.CreateInstance(typeToSerialize); + root.Add("self", root); + + await Test_Serialize_And_SerializeAsync(root, @"{""self"":null}", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnRecursiveDictionary() + { + var root = new RecursiveDictionary(); + root.Add("self", root); + + await Test_Serialize_And_SerializeAsync(root, @"{""self"":null}", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnReadOnlyDictionary() + { + var innerDictionary = new Dictionary(); + var root = new ReadOnlyDictionary(innerDictionary); + innerDictionary.Add("self", root); + + await Test_Serialize_And_SerializeAsync(root, @"{""self"":null}", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnIDictionary() + { + var root = new WrapperForIDictionary(); + root.Add("self", root); + + await Test_Serialize_And_SerializeAsync(root, @"{""self"":null}", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnArray() + { + var root = new object[1]; + root[0] = root; + await Test_Serialize_And_SerializeAsync(root, "[null]", s_optionsIgnoreCycles); + } + + [Theory] + [InlineData(typeof(List))] + [InlineData(typeof(GenericIListWrapper))] + public async Task IgnoreCycles_OnList(Type typeToSerialize) + { + var root = (IList)Activator.CreateInstance(typeToSerialize); + root.Add(root); + await Test_Serialize_And_SerializeAsync(root, "[null]", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnRecursiveList() + { + var root = new RecursiveList(); + root.Add(root); + await Test_Serialize_And_SerializeAsync(root, "[null]", s_optionsIgnoreCycles); + } + + [Theory] + [InlineData(typeof(GenericISetWrapper))] + [InlineData(typeof(GenericICollectionWrapper))] + public async Task IgnoreCycles_OnCollections(Type typeToSerialize) + { + var root = (ICollection)Activator.CreateInstance(typeToSerialize); + root.Add(root); + await Test_Serialize_And_SerializeAsync(root, "[null]", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnCollections_WithoutAddMethod() + { + var root = new Stack(); + root.Push(root); + await Test_Serialize_And_SerializeAsync(root, "[null]", s_optionsIgnoreCycles); + + var root2 = new Queue(); + root2.Enqueue(root2); + await Test_Serialize_And_SerializeAsync(root2, "[null]", s_optionsIgnoreCycles); + + var root3 = new ConcurrentStack(); + root3.Push(root3); + await Test_Serialize_And_SerializeAsync(root3, "[null]", s_optionsIgnoreCycles); + + var root4 = new ConcurrentQueue(); + root4.Enqueue(root4); + await Test_Serialize_And_SerializeAsync(root4, "[null]", s_optionsIgnoreCycles); + + var root5 = new Stack(); + root5.Push(root5); + await Test_Serialize_And_SerializeAsync(root5, "[null]", s_optionsIgnoreCycles); + + var root6 = new Queue(); + root6.Enqueue(root6); + await Test_Serialize_And_SerializeAsync(root6, "[null]", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_OnExtensionData() + { + var root = new EmptyClassWithExtensionProperty(); + root.MyOverflow.Add("root", root); + await Test_Serialize_And_SerializeAsync(root, @"{""root"":null}", s_optionsIgnoreCycles); + } + + [Fact] + public async Task IgnoreCycles_DoesNotSupportPreserveSemantics() + { + // Object + var node = new NodeWithExtensionData(); + node.Next = node; + string json = SerializeWithPreserve(node); + + node = JsonSerializer.Deserialize(json, s_optionsIgnoreCycles); + Assert.True(node.MyOverflow.ContainsKey("$id")); + Assert.True(node.Next.MyOverflow.ContainsKey("$ref")); + + using var ms = new MemoryStream(Encoding.UTF8.GetBytes(json)); + node = await JsonSerializer.DeserializeAsync(ms, s_optionsIgnoreCycles); + Assert.True(node.MyOverflow.ContainsKey("$id")); + Assert.True(node.Next.MyOverflow.ContainsKey("$ref")); + + // Dictionary + var dictionary = new RecursiveDictionary(); + dictionary.Add("self", dictionary); + json = SerializeWithPreserve(dictionary); + + Assert.Throws(() => JsonSerializer.Deserialize(json, s_optionsIgnoreCycles)); + using var ms2 = new MemoryStream(Encoding.UTF8.GetBytes(json)); + await Assert.ThrowsAsync(() => JsonSerializer.DeserializeAsync(ms2, s_optionsIgnoreCycles).AsTask()); + + // List + var list = new RecursiveList(); + list.Add(list); + json = SerializeWithPreserve(list); + + Assert.Throws(() => JsonSerializer.Deserialize(json, s_optionsIgnoreCycles)); + using var ms3 = new MemoryStream(Encoding.UTF8.GetBytes(json)); + await Assert.ThrowsAsync(() => JsonSerializer.DeserializeAsync(ms3, s_optionsIgnoreCycles).AsTask()); + } + + [Fact] + public async Task IgnoreCycles_DoesNotSupportPreserveSemantics_Polymorphic() + { + // Object + var node = new NodeWithObjectProperty(); + node.Next = node; + string json = SerializeWithPreserve(node); + + node = JsonSerializer.Deserialize(json, s_optionsIgnoreCycles); + JsonElement nodeAsJsonElement = Assert.IsType(node.Next); + Assert.True(nodeAsJsonElement.GetProperty("$ref").GetString() == "1"); + + using var ms = new MemoryStream(Encoding.UTF8.GetBytes(json)); + node = await JsonSerializer.DeserializeAsync(ms, s_optionsIgnoreCycles); + nodeAsJsonElement = Assert.IsType(node.Next); + Assert.True(nodeAsJsonElement.GetProperty("$ref").GetString() == "1"); + + // Dictionary + var dictionary = new Dictionary(); + dictionary.Add("self", dictionary); + json = SerializeWithPreserve(dictionary); + + dictionary = JsonSerializer.Deserialize>(json, s_optionsIgnoreCycles); + } + + private string SerializeWithPreserve(T value) + { + var opts = new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.Preserve }; + return JsonSerializer.Serialize(value, opts); + } + + [Fact] + public async Task AlreadySeenInstance_ShouldNotBeIgnoredOnSiblingBranch() + { + await Verify(expectedPayload: "{}"); + await Verify(expectedPayload: "{}"); + await Verify(expectedPayload: "{}"); + await Verify>(expectedPayload: "{}"); + await Verify>(expectedPayload: "[]"); + + async Task Verify(string expectedPayload) where T : new() + { + T value = new(); + var root = new TreeNode { Left = value, Right = value }; + await Test_Serialize_And_SerializeAsync(root, $@"{{""Left"":{expectedPayload},""Right"":{expectedPayload}}}", s_optionsIgnoreCycles); + + var rootWithObjectProperties = new TreeNode { Left = value, Right = value }; + await Test_Serialize_And_SerializeAsync(rootWithObjectProperties, $@"{{""Left"":{expectedPayload},""Right"":{expectedPayload}}}", s_optionsIgnoreCycles); + } + } + + [Fact] + public async Task IgnoreCycles_WhenWritingNull() + { + var opts = new JsonSerializerOptions + { + ReferenceHandler = ReferenceHandler.IgnoreCycles, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + }; + + // Reference cycles are treated as null, hence the JsonIgnoreCondition can be used to actually ignore the property. + var rootObj = new NodeWithObjectProperty(); + rootObj.Next = rootObj; + + await Test_Serialize_And_SerializeAsync(rootObj, "{}", opts); + + // JsonIgnoreCondition does not ignore nulls in collections, hence a reference loop should not omit writing it. + // This also helps us to avoid changing the length of a collection when the loop is detected in one of the elements. + var rootList = new List(); + rootList.Add(rootList); + + await Test_Serialize_And_SerializeAsync(rootList, "[null]", opts); + + var rootDictionary = new Dictionary(); + rootDictionary.Add("self", rootDictionary); + + await Test_Serialize_And_SerializeAsync(rootDictionary, @"{""self"":null}", opts); + } + + private async Task Test_Serialize_And_SerializeAsync(T obj, string expected, JsonSerializerOptions options) + { + string json; + Type objType = typeof(T); + + if (objType != typeof(object)) + { + json = JsonSerializer.Serialize(obj, options); + Assert.Equal(expected, json); + + using var ms1 = new MemoryStream(); + await JsonSerializer.SerializeAsync(ms1, obj, options).ConfigureAwait(false); + json = Encoding.UTF8.GetString(ms1.ToArray()); + Assert.Equal(expected, json); + } + + json = JsonSerializer.Serialize(obj, objType, options); + Assert.Equal(expected, json); + + using var ms2 = new MemoryStream(); + await JsonSerializer.SerializeAsync(ms2, obj, objType, options).ConfigureAwait(false); + json = Encoding.UTF8.GetString(ms2.ToArray()); + Assert.Equal(expected, json); + } + + private const string Next = nameof(Next); + private void SetNextProperty(Type type, object obj, object value) + { + type.GetProperty(Next).SetValue(obj, value); + } + + private object GetNextProperty(Type type, object obj) + { + return type.GetProperty(Next).GetValue(obj); + } + + private class NodeWithObjectProperty + { + public object Next { get; set; } + } + + private class NodeWithNodeProperty + { + public NodeWithNodeProperty Next { get; set; } + } + + private class ClassWithGenericProperty + { + public T Foo { get; set; } + } + + private class TreeNode + { + public T Left { get; set; } + public T Right { get; set; } + } + + interface IValueNodeWithObjectProperty + { + public object Next { get; set; } + } + + private struct ValueNodeWithObjectProperty : IValueNodeWithObjectProperty + { + public object Next { get; set; } + } + + interface IValueNodeWithIValueNodeProperty + { + public IValueNodeWithIValueNodeProperty Next { get; set; } + } + + private struct ValueNodeWithIValueNodeProperty : IValueNodeWithIValueNodeProperty + { + public IValueNodeWithIValueNodeProperty Next { get; set; } + } + + private class EmptyClass { } + private struct EmptyStruct { } + + private class EmptyClassWithExtensionProperty + { + [JsonExtensionData] + public Dictionary MyOverflow { get; set; } = new Dictionary(); + } + + private class NodeWithExtensionData + { + [JsonExtensionData] + public Dictionary MyOverflow { get; set; } = new Dictionary(); + public NodeWithExtensionData Next { get; set; } + } + + private class RecursiveDictionary : Dictionary { } + + private class RecursiveList : List { } + } +} diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj index 7632275629fe7..dbebf9b1814f3 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj @@ -1,4 +1,4 @@ - + $(NetCoreAppCurrent);net461 true @@ -117,6 +117,7 @@ +