From 650b303ee14ff14ceb09f8a55de1e5443cee26f1 Mon Sep 17 00:00:00 2001 From: Antoine Aubry Date: Tue, 12 Feb 2019 13:05:20 +0000 Subject: [PATCH] Produce a detailed error message when too much recursion is detected during serialization --- README.md | 5 + .../Serialization/SerializationTests.cs | 20 ++ .../FullObjectGraphTraversalStrategy.cs | 177 +++++++++++------- .../RoundtripObjectGraphTraversalStrategy.cs | 4 +- 4 files changed, 140 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 8c68b67ed..c312364ab 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,11 @@ Please read [CONTRIBUTING.md](CONTRIBUTING.md) for guidelines. # Changelog +## Version 5.3.1 + +New features: +* Produce a detailed error message when too much recursion is detected during serialization. + ## Version 5.3.0 New features: diff --git a/YamlDotNet.Test/Serialization/SerializationTests.cs b/YamlDotNet.Test/Serialization/SerializationTests.cs index 55a6ed9d1..aeabc475a 100644 --- a/YamlDotNet.Test/Serialization/SerializationTests.cs +++ b/YamlDotNet.Test/Serialization/SerializationTests.cs @@ -1625,6 +1625,26 @@ public void TypesAreConvertedWhenNeededInsideDictionary() Assert.Equal(3, result[5]); } + [Fact] + public void InfiniteRecursionIsDetected() + { + var sut = new SerializerBuilder() + .DisableAliases() + .Build(); + + var recursionRoot = new + { + Nested = new[] + { + new Dictionary() + } + }; + + recursionRoot.Nested[0].Add("loop", recursionRoot); + + var exception = Assert.Throws(() => sut.Serialize(recursionRoot)); + } + [TypeConverter(typeof(DoublyConvertedTypeConverter))] public class DoublyConverted { diff --git a/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/FullObjectGraphTraversalStrategy.cs b/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/FullObjectGraphTraversalStrategy.cs index 7717144da..f0cbcbe03 100644 --- a/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/FullObjectGraphTraversalStrategy.cs +++ b/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/FullObjectGraphTraversalStrategy.cs @@ -23,6 +23,9 @@ using System.Collections; using System.Collections.Generic; using System.Globalization; +using System.Linq; +using System.Text; +using YamlDotNet.Core; using YamlDotNet.Helpers; using YamlDotNet.Serialization.Utilities; @@ -66,14 +69,49 @@ public FullObjectGraphTraversalStrategy(ITypeInspector typeDescriptor, ITypeReso void IObjectGraphTraversalStrategy.Traverse(IObjectDescriptor graph, IObjectGraphVisitor visitor, TContext context) { - Traverse(graph, visitor, 0, context); + Traverse("", graph, visitor, context, new Stack(maxRecursion)); } - protected virtual void Traverse(IObjectDescriptor value, IObjectGraphVisitor visitor, int currentDepth, TContext context) + protected struct ObjectPathSegment { - if (++currentDepth > maxRecursion) + public object name; + public IObjectDescriptor value; + + public ObjectPathSegment(object name, IObjectDescriptor value) { - throw new InvalidOperationException("Too much recursion when traversing the object graph"); + this.name = name; + this.value = value; + } + } + + protected virtual void Traverse(object name, IObjectDescriptor value, IObjectGraphVisitor visitor, TContext context, Stack path) + { + if (path.Count >= maxRecursion) + { + var message = new StringBuilder(); + message.AppendLine("Too much recursion when traversing the object graph."); + message.AppendLine("The path to reach this recursion was:"); + + var lines = new Stack>(path.Count); + var maxNameLength = 0; + foreach (var segment in path) + { + var segmentName = TypeConverter.ChangeType(segment.name); + maxNameLength = Math.Max(maxNameLength, segmentName.Length); + lines.Push(new KeyValuePair(segmentName, segment.value.Type.FullName)); + } + + foreach (var line in lines) + { + message + .Append(" -> ") + .Append(line.Key.PadRight(maxNameLength)) + .Append(" [") + .Append(line.Value) + .AppendLine("]"); + } + + throw new MaximumRecursionLevelReachedException(message.ToString()); } if (!visitor.Enter(value, context)) @@ -81,62 +119,70 @@ protected virtual void Traverse(IObjectDescriptor value, IObjectGraphV return; } - var typeCode = value.Type.GetTypeCode(); - switch (typeCode) + path.Push(new ObjectPathSegment(name, value)); + try { - case TypeCode.Boolean: - case TypeCode.Byte: - case TypeCode.Int16: - case TypeCode.Int32: - case TypeCode.Int64: - case TypeCode.SByte: - case TypeCode.UInt16: - case TypeCode.UInt32: - case TypeCode.UInt64: - case TypeCode.Single: - case TypeCode.Double: - case TypeCode.Decimal: - case TypeCode.String: - case TypeCode.Char: - case TypeCode.DateTime: - visitor.VisitScalar(value, context); - break; - - case TypeCode.Empty: - throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, "TypeCode.{0} is not supported.", typeCode)); - - default: - if (value.IsDbNull()) - { - visitor.VisitScalar(new ObjectDescriptor(null, typeof(object), typeof(object)), context); - } - - if (value.Value == null || value.Type == typeof(TimeSpan)) - { + var typeCode = value.Type.GetTypeCode(); + switch (typeCode) + { + case TypeCode.Boolean: + case TypeCode.Byte: + case TypeCode.Int16: + case TypeCode.Int32: + case TypeCode.Int64: + case TypeCode.SByte: + case TypeCode.UInt16: + case TypeCode.UInt32: + case TypeCode.UInt64: + case TypeCode.Single: + case TypeCode.Double: + case TypeCode.Decimal: + case TypeCode.String: + case TypeCode.Char: + case TypeCode.DateTime: visitor.VisitScalar(value, context); break; - } - - var underlyingType = Nullable.GetUnderlyingType(value.Type); - if (underlyingType != null) - { - // This is a nullable type, recursively handle it with its underlying type. - // Note that if it contains null, the condition above already took care of it - Traverse(new ObjectDescriptor(value.Value, underlyingType, value.Type, value.ScalarStyle), visitor, currentDepth, context); - } - else - { - TraverseObject(value, visitor, currentDepth, context); - } - break; + + case TypeCode.Empty: + throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, "TypeCode.{0} is not supported.", typeCode)); + + default: + if (value.IsDbNull()) + { + visitor.VisitScalar(new ObjectDescriptor(null, typeof(object), typeof(object)), context); + } + + if (value.Value == null || value.Type == typeof(TimeSpan)) + { + visitor.VisitScalar(value, context); + break; + } + + var underlyingType = Nullable.GetUnderlyingType(value.Type); + if (underlyingType != null) + { + // This is a nullable type, recursively handle it with its underlying type. + // Note that if it contains null, the condition above already took care of it + Traverse("Value", new ObjectDescriptor(value.Value, underlyingType, value.Type, value.ScalarStyle), visitor, context, path); + } + else + { + TraverseObject(value, visitor, context, path); + } + break; + } + } + finally + { + path.Pop(); } } - protected virtual void TraverseObject(IObjectDescriptor value, IObjectGraphVisitor visitor, int currentDepth, TContext context) + protected virtual void TraverseObject(IObjectDescriptor value, IObjectGraphVisitor visitor, TContext context, Stack path) { if (typeof(IDictionary).IsAssignableFrom(value.Type)) { - TraverseDictionary(value, visitor, currentDepth, typeof(object), typeof(object), context); + TraverseDictionary(value, visitor, typeof(object), typeof(object), context, path); return; } @@ -145,56 +191,59 @@ protected virtual void TraverseObject(IObjectDescriptor value, IObject { var adaptedDictionary = new GenericDictionaryToNonGenericAdapter(value.Value, genericDictionaryType); var genericArguments = genericDictionaryType.GetGenericArguments(); - TraverseDictionary(new ObjectDescriptor(adaptedDictionary, value.Type, value.StaticType, value.ScalarStyle), visitor, currentDepth, genericArguments[0], genericArguments[1], context); + TraverseDictionary(new ObjectDescriptor(adaptedDictionary, value.Type, value.StaticType, value.ScalarStyle), visitor, genericArguments[0], genericArguments[1], context, path); return; } if (typeof(IEnumerable).IsAssignableFrom(value.Type)) { - TraverseList(value, visitor, currentDepth, context); + TraverseList(value, visitor, context, path); return; } - TraverseProperties(value, visitor, currentDepth, context); + TraverseProperties(value, visitor, context, path); } - protected virtual void TraverseDictionary(IObjectDescriptor dictionary, IObjectGraphVisitor visitor, int currentDepth, Type keyType, Type valueType, TContext context) + protected virtual void TraverseDictionary(IObjectDescriptor dictionary, IObjectGraphVisitor visitor, Type keyType, Type valueType, TContext context, Stack path) { visitor.VisitMappingStart(dictionary, keyType, valueType, context); var isDynamic = dictionary.Type.FullName.Equals("System.Dynamic.ExpandoObject"); foreach (DictionaryEntry entry in (IDictionary)dictionary.Value) { - var keyString = isDynamic ? namingConvention.Apply(entry.Key.ToString()) : entry.Key; - var key = GetObjectDescriptor(keyString, keyType); + var keyValue = isDynamic ? namingConvention.Apply(entry.Key.ToString()) : entry.Key; + var key = GetObjectDescriptor(keyValue, keyType); var value = GetObjectDescriptor(entry.Value, valueType); if (visitor.EnterMapping(key, value, context)) { - Traverse(key, visitor, currentDepth, context); - Traverse(value, visitor, currentDepth, context); + var keyAsString = TypeConverter.ChangeType(key); + Traverse(keyValue, key, visitor, context, path); + Traverse(keyValue, value, visitor, context, path); } } visitor.VisitMappingEnd(dictionary, context); } - private void TraverseList(IObjectDescriptor value, IObjectGraphVisitor visitor, int currentDepth, TContext context) + private void TraverseList(IObjectDescriptor value, IObjectGraphVisitor visitor, TContext context, Stack path) { var enumerableType = ReflectionUtility.GetImplementedGenericInterface(value.Type, typeof(IEnumerable<>)); var itemType = enumerableType != null ? enumerableType.GetGenericArguments()[0] : typeof(object); visitor.VisitSequenceStart(value, itemType, context); + var index = 0; foreach (var item in (IEnumerable)value.Value) { - Traverse(GetObjectDescriptor(item, itemType), visitor, currentDepth, context); + Traverse(index, GetObjectDescriptor(item, itemType), visitor, context, path); + ++index; } visitor.VisitSequenceEnd(value, context); } - protected virtual void TraverseProperties(IObjectDescriptor value, IObjectGraphVisitor visitor, int currentDepth, TContext context) + protected virtual void TraverseProperties(IObjectDescriptor value, IObjectGraphVisitor visitor, TContext context, Stack path) { visitor.VisitMappingStart(value, typeof(string), typeof(object), context); @@ -204,8 +253,8 @@ protected virtual void TraverseProperties(IObjectDescriptor value, IOb if (visitor.EnterMapping(propertyDescriptor, propertyValue, context)) { - Traverse(new ObjectDescriptor(propertyDescriptor.Name, typeof(string), typeof(string)), visitor, currentDepth, context); - Traverse(propertyValue, visitor, currentDepth, context); + Traverse(propertyDescriptor.Name, new ObjectDescriptor(propertyDescriptor.Name, typeof(string), typeof(string)), visitor, context, path); + Traverse(propertyDescriptor.Name, propertyValue, visitor, context, path); } } diff --git a/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/RoundtripObjectGraphTraversalStrategy.cs b/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/RoundtripObjectGraphTraversalStrategy.cs index 8a2e1320b..526b5d23f 100644 --- a/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/RoundtripObjectGraphTraversalStrategy.cs +++ b/YamlDotNet/Serialization/ObjectGraphTraversalStrategies/RoundtripObjectGraphTraversalStrategy.cs @@ -41,14 +41,14 @@ public RoundtripObjectGraphTraversalStrategy(IEnumerable con this.converters = converters; } - protected override void TraverseProperties(IObjectDescriptor value, IObjectGraphVisitor visitor, int currentDepth, TContext context) + protected override void TraverseProperties(IObjectDescriptor value, IObjectGraphVisitor visitor, TContext context, Stack path) { if (!value.Type.HasDefaultConstructor() && !converters.Any(c => c.Accepts(value.Type))) { throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "Type '{0}' cannot be deserialized because it does not have a default constructor or a type converter.", value.Type)); } - base.TraverseProperties(value, visitor, currentDepth, context); + base.TraverseProperties(value, visitor, context, path); } } } \ No newline at end of file