Add a maximum depth of 128 levels

CHANGELOG.md

@@ -8,6 +8,10 @@ Breaking changes are marked **BREAKING**.
 
 ## Unreleased
 
+### Added
+
+- **BREAKING** A maximum depth of 128 levels. An object at a path of more than 128 segments is not validated, and the validation fails with one error at that path: `The object is nested more than 128 levels deep and was not validated.` Each property step and each collection index counts as one level, so a tree that holds its children in a `List<T>` can be 64 levels deep. Before, the validator had no limit, and a graph of about 1,500 objects, or a property that builds a new object on each read, overflowed the stack, which ends the process. A graph that is deeper than 128 levels passed before and fails now. Change any model that is that deep, or mark the property that leads into it with `[SkipRecursiveValidation]`. The limit is not a setting.
+
 ### Fixed
 
 - Objects inside an item that is itself a collection are now validated. Before, a `List<List<T>>`, a `List<HashSet<T>>`, an `object[]` that holds a list, and a list of a sealed or struct collection such as `ImmutableList<T>` passed validation even when an object inside was invalid. The error is reported with the index of each level, for example `Value[0][0].Name`. A model that passed before can fail now.
@@ -23,7 +27,7 @@ Breaking changes are marked **BREAKING**.
   - `SortedSet<T>`: `Value[0].Min.Name` is now `Value[0][0].Name`.
   - A collection of your own with a property that returns its items, such as `View`: `Value[0].View[0].Name` is now `Value[0][0].Name`.
 - **BREAKING** An item that is a collection is now enumerated, so a lazy sequence in an item runs, as it does when a property holds it. An item that throws when enumerated now throws from validation, and an item that never ends makes validation hang. To avoid it, mark the property that holds the collection with `[SkipRecursiveValidation]`.
-- A deeply nested collection uses more of the stack. On a 1 MB stack, a chain of `List<object>` that holds the next list passes at 1,100 levels and overflows at 1,200, which ends the process (net8.0, macOS, Release). Before, such a list passed without being enumerated. The validator still has no maximum depth.
+- A deeply nested collection uses more of the stack. On a 1 MB stack, a chain of `List<object>` that holds the next list passes at 1,100 levels and overflows at 1,200, which ends the process (net8.0, macOS, Release). Before, such a list passed without being enumerated. The maximum depth of 128 now stops the walk long before the stack is full.
 - The NuGet package title now reads "Recursive DataAnnotations Validation". It was misspelled "Recurisive".
 - Known limit: a struct collection such as `ImmutableArray<T>` is validated when it is an item, but not yet when a property holds it.
 - Known limit: an invalid struct that an item collection returns by enumeration and also through one of its properties is reported twice, with two member names, for example `Value[0][0].Text` and `Value[0].Array[0].Text` for an `ArraySegment<T>`. The validator cannot match two copies of a struct by reference. A struct that two properties hold was already reported twice. Objects of a class are reported once.

README.md

@@ -33,9 +33,17 @@ The [`[SkipRecursiveValidation]`](https://github.com/tgharold/RecursiveDataAnnot
 - Objects are compared by reference. Two separate objects that are `Equals` to each other, such as records with the same values or entities with the same `Id`, are each validated.
 - Public static properties are walked, as well as instance properties.
 - A property that builds a new object on each read, such as `public Money Zero => new Money(0)`, could make the walk go on forever. So when an object's type overrides `Equals`, and the object equals an object of the same type, a base type or a derived type on its own path from the root, the validator checks that object's own attributes but does not walk into its properties. An invalid object below it is not reached.
-- For a type that does not override `Equals`, mark such a property with `[SkipRecursiveValidation]`. Otherwise the walk overflows the stack, which ends the process.
+- For a type that does not override `Equals`, mark such a property with `[SkipRecursiveValidation]`. Otherwise the walk goes on until it reaches the maximum depth (see below), and the validation fails.
 - Two records that reference each other can also overflow the stack. A record's generated `Equals` compares properties in declaration order, so it follows the reference forever if it reaches it before a property that differs. This happens when the records have equal values, or when the reference is declared first. On .NET Framework it always happens, because the framework's `Validator` calls the record's generated `GetHashCode`, which follows the reference too. Marking the reference with `[SkipRecursiveValidation]` avoids the walk, but not the `GetHashCode` call on .NET Framework. There, override `GetHashCode` so it does not include the reference.
 
+### Maximum depth
+
+The validator walks at most 128 levels. The depth of an object is the number of segments in its path: each property and each collection index is one level, and the root object is level 0. In `Items[1].Name`, `Items` is level 1, `[1]` is level 2 and `Name` is level 3. System.Text.Json counts nearly the same way, with each object and each array as one level, and its default limit is 64. A JSON document within that limit stays within about 63 levels here.
+
+An object at level 128 is validated. An object at level 129 is not validated, and the validation fails with one error at its path: `The object is nested more than 128 levels deep and was not validated.` The walk does not go deeper below that object. A graph that is too deep means that something has gone wrong, such as a property that builds a new object on each read, so the validator fails the validation instead of letting the graph pass. It does not throw.
+
+A tree that holds its children in a `List<T>` uses two levels for each tree level, so it can be 64 levels deep. The limit is fixed.
+
 ### Collections
 
 - The items of a collection that a property holds are validated. The error names the property and the index of the item: `Items[1].Name`.

src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs

@@ -11,6 +11,13 @@ namespace RecursiveDataAnnotationsValidation
     /// <summary>Recursive validator for DataAnnotation attribute-based validation.</summary>
     public class RecursiveDataAnnotationValidator : IRecursiveDataAnnotationValidator, IAsyncRecursiveDataAnnotationValidator
     {
+        //The deepest level that is validated. The depth of an object is the number of segments in its
+        //path: each property step and each collection index is one level, and the root is level 0.
+        //An object at a deeper level is not validated and fails the validation (see DepthExceededResult).
+        //The walk calls itself once for each level, and a StackOverflowException cannot be caught.
+        //128 is twice the depth that System.Text.Json allows by default, which counts nearly the same way.
+        internal const int MaxDepth = 128;
+
         /// <summary>Runs validation on an object.</summary>
         /// <param name="obj">The object being validated.</param>
         /// <param name="validationContext">Validation context.</param>
@@ -120,7 +127,8 @@ IDictionary<object, object> validationContextItems
                 new HashSet<object>(ObjectReferenceComparer.Instance),
                 new List<object>(),
                 serviceProvider,
-                validationContextItems
+                validationContextItems,
+                0
                 );
         }
 
@@ -172,6 +180,7 @@ private static bool EqualsAnAncestor(object obj, Type type, List<object> equalit
 
         //validatedObjects holds every object visited so far, compared by reference.
         //equalityPath holds the objects on the path from the root to this object whose type overrides Equals.
+        //depth is the number of segments in the path of obj, which is 0 for the root object.
         //enumerateItems is true for an item of a collection. If the item is itself a collection, its
         //items are validated too. A collection that a property holds is enumerated by the caller, and
         //the root object is never enumerated.
@@ -182,6 +191,7 @@ private bool TryValidateObjectRecursive(
             List<object> equalityPath,
             IServiceProvider serviceProvider,
             IDictionary<object, object> validationContextItems,
+            int depth,
             bool enumerateItems = false
             )
         {
@@ -210,6 +220,15 @@ private bool TryValidateObjectRecursive(
                 return TryValidateObject(obj, validationResults, serviceProvider, validationContextItems);
             }
 
+            //an object this deep is not validated and not walked, and the validation fails: nobody
+            //has checked it, so it must not pass. A leaf, an object seen before and an object that
+            //Equals an ancestor all returned above, because none of them walks any further.
+            if (depth > MaxDepth)
+            {
+                validationResults.Add(new DepthExceededResult());
+                return false;
+            }
+
             validatedObjects.Add(obj);
             if (overridesEquals) equalityPath.Add(obj);
 
@@ -235,7 +254,8 @@ private bool TryValidateObjectRecursive(
                     validatedObjects,
                     equalityPath,
                     serviceProvider,
-                    validationContextItems
+                    validationContextItems,
+                    depth
                     ))
                 {
                     result = false;
@@ -266,14 +286,16 @@ private bool TryValidateObjectRecursive(
                         //Array.SyncRoot, and enumerating it a second time would find nothing new
                         if (enumeratedItems && ReferenceEquals(value, obj)) continue;
 
+                        //the property is one level and the index of each item is another
                         if (!TryValidateItems(
                             asEnumerable,
                             property.Name,
                             validationResults,
                             validatedObjects,
                             equalityPath,
                             serviceProvider,
-                            validationContextItems
+                            validationContextItems,
+                            depth + 1
                             ))
                         {
                             result = false;
@@ -288,18 +310,20 @@ private bool TryValidateObjectRecursive(
                             validatedObjects, 
                             equalityPath,
                             serviceProvider,
-                            validationContextItems
+                            validationContextItems,
+                            depth + 1
                             ))
                         {
                             result = false;
                             foreach (var validationResult in nestedResults)
                             {
                                 var property1 = property;
-                                validationResults.Add(
-                                new ValidationResult(
-                                    validationResult.ErrorMessage, 
-                                    validationResult.MemberNames.Select(x => property1.Name + '.' + x)
-                                    ));
+
+                                //an object that is too deep has no member names, and its path is the property
+                                var memberNames = validationResult is DepthExceededResult
+                                    ? new[] { property1.Name }
+                                    : validationResult.MemberNames.Select(x => property1.Name + '.' + x);
+                                validationResults.Add(new ValidationResult(validationResult.ErrorMessage, memberNames));
                             }
                         }
                         break;
@@ -313,14 +337,17 @@ private bool TryValidateObjectRecursive(
 
         //Validates each item of a collection. The result of an item is added to validationResults with
         //its member names prefixed by the property name, if there is one, and the index of the item.
+        //collectionDepth is the depth of the collection, or of the property that holds it, so each
+        //item is one level deeper.
         private bool TryValidateItems(
             IEnumerable items,
             string propertyName,
             ICollection<ValidationResult> validationResults,
             ISet<object> validatedObjects,
             List<object> equalityPath,
             IServiceProvider serviceProvider,
-            IDictionary<object, object> validationContextItems
+            IDictionary<object, object> validationContextItems,
+            int collectionDepth
             )
         {
             var result = true;
@@ -340,6 +367,7 @@ IDictionary<object, object> validationContextItems
                     equalityPath,
                     serviceProvider,
                     validationContextItems,
+                    collectionDepth + 1,
                     enumerateItems: true
                     ))
                 {
@@ -349,9 +377,12 @@ IDictionary<object, object> validationContextItems
                     {
                         //the member names of an item that is a collection start with the index of its own items
                         var startsWithIndex = validationResult is ItemOfCollectionResult;
-                        var memberNames = validationResult.MemberNames
-                            .Select(x => propertyName + "[" + index + "]" + (startsWithIndex ? "" : ".") + x)
-                            .ToList();
+                        //an item that is too deep has no member names, and its path is the index
+                        var memberNames = validationResult is DepthExceededResult
+                            ? new List<string> { propertyName + "[" + index + "]" }
+                            : validationResult.MemberNames
+                                .Select(x => propertyName + "[" + index + "]" + (startsWithIndex ? "" : ".") + x)
+                                .ToList();
 
                         //With no property name, the result is for an item that is a collection: the
                         //caller puts its own index in front of these names, with no dot.
@@ -365,6 +396,17 @@ IDictionary<object, object> validationContextItems
             return result;
         }
 
+        //The result for an object that is deeper than MaxDepth. It has no member names at first. The
+        //caller that knows how it reached the object gives it that path as its only member name, and
+        //every caller above it prefixes the name like any other, so the final name is the full path.
+        private sealed class DepthExceededResult : ValidationResult
+        {
+            public DepthExceededResult()
+                : base("The object is nested more than " + MaxDepth + " levels deep and was not validated.")
+            {
+            }
+        }
+
         //A result whose member names start with the index of an item, such as "[0].Name", and not
         //with a property name. It marks the names that the caller must join without a dot.
         //The names the items choose are not looked at, so a name that starts with "[", or a null

test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs

@@ -11,7 +11,7 @@ namespace RecursiveDataAnnotationsValidation.Tests
 {
     /// <summary>
     /// Large graphs: deep chains and wide lists. Real models are shallow, but a linked list, a
-    /// comment thread or a category tree can be hundreds of levels deep, and an import can hold
+    /// comment thread or a category tree can be dozens of levels deep, and an import can hold
     /// thousands of items.
     ///
     /// The walk calls itself once for each level, so the depth of the graph is the depth of the
@@ -20,20 +20,22 @@ namespace RecursiveDataAnnotationsValidation.Tests
     /// stack would pass on those systems and still overflow on Windows.
     /// See: https://learn.microsoft.com/dotnet/api/system.threading.thread.-ctor
     ///
-    /// Known limit: the validator has no maximum depth. On a 1 MB stack, a chain of about 1,300
-    /// objects passes and a chain of about 1,600 overflows the stack, in both Debug and Release
-    /// builds on .NET 8 (measured on macOS). The numbers change with the runtime and the size of
-    /// the objects. .NET cannot catch a StackOverflowException, so the process ends. These tests
-    /// stay at a depth of 250, five times below the lowest depth that failed, because a test
-    /// that overflows would end the whole test run. A depth limit is a deferred item from the
-    /// reference-equality work (PR #64).
+    /// The validator has a maximum depth of 128 (see MaxDepthTests), which these tests stay below.
+    /// Before the limit, a chain of about 1,300 objects passed on a 1 MB stack and a chain of about
+    /// 1,600 overflowed it, in both Debug and Release builds on .NET 8 (measured on macOS). The
+    /// numbers change with the runtime and the size of the objects. .NET cannot catch a
+    /// StackOverflowException, so the process ends.
     /// See: https://learn.microsoft.com/dotnet/api/system.stackoverflowexception
     ///
     /// The tests only use the public API, so they also pass against the validator of v2.2.0.
+    /// A chain of 120 objects is deeper than a real model and below the limit. A tree that holds
+    /// its children in a List has two levels for each tree level, so it is 60 levels deep.
     /// </summary>
     public class GraphSizeTests
     {
-        private const int Depth = 250;
+        private const int Depth = 120;
+
+        private const int TreeDepth = 60;
 
         private const int OneMegabyte = 1024 * 1024;
 
@@ -145,7 +147,7 @@ public void Error_at_the_bottom_has_the_full_path()
             public async Task Error_at_the_bottom_has_the_full_path_in_the_async_method()
             {
                 // Task.Run uses a thread-pool thread, whose stack size the caller cannot choose.
-                // A depth of 250 is far below the limit measured for a 1 MB stack.
+                // A depth of 120 is far below the limit measured for a 1 MB stack.
                 var results = new List<ValidationResult>();
 
                 var valid = await new RecursiveDataAnnotationValidator()
@@ -160,16 +162,16 @@ public void Chain_through_collections_has_the_full_path()
             {
                 var root = new TreeNode { Name = "ok" };
                 var current = root;
-                for (var i = 0; i < Depth; i++)
+                for (var i = 0; i < TreeDepth; i++)
                 {
-                    var child = new TreeNode { Name = i == Depth - 1 ? null : "ok" };
+                    var child = new TreeNode { Name = i == TreeDepth - 1 ? null : "ok" };
                     current.Children.Add(child);
                     current = child;
                 }
 
                 var (valid, errors) = Run(root);
 
-                var path = string.Join(".", Enumerable.Repeat("Children[0]", Depth).Concat(new[] { "Name" }));
+                var path = string.Join(".", Enumerable.Repeat("Children[0]", TreeDepth).Concat(new[] { "Name" }));
                 Assert.False(valid);
                 Assert.Equal(ResultText.Expect($"{path} | Name is required"), errors);
             }