diff --git a/CHANGELOG.md b/CHANGELOG.md index fe5fa49..5ef03d0 100644 --- a/CHANGELOG.md +++ b/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` 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>`, a `List>`, an `object[]` that holds a list, and a list of a sealed or struct collection such as `ImmutableList` 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`: `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` 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` 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` 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`. 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. diff --git a/README.md b/README.md index 9523f84..49b9e13 100644 --- a/README.md +++ b/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` 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`. diff --git a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs index 06b71dd..dc01c62 100644 --- a/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs +++ b/src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs @@ -11,6 +11,13 @@ namespace RecursiveDataAnnotationsValidation /// Recursive validator for DataAnnotation attribute-based validation. 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; + /// Runs validation on an object. /// The object being validated. /// Validation context. @@ -120,7 +127,8 @@ IDictionary validationContextItems new HashSet(ObjectReferenceComparer.Instance), new List(), serviceProvider, - validationContextItems + validationContextItems, + 0 ); } @@ -172,6 +180,7 @@ private static bool EqualsAnAncestor(object obj, Type type, List 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 equalityPath, IServiceProvider serviceProvider, IDictionary 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,6 +286,7 @@ 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, @@ -273,7 +294,8 @@ private bool TryValidateObjectRecursive( 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,6 +337,8 @@ 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, @@ -320,7 +346,8 @@ private bool TryValidateItems( ISet validatedObjects, List equalityPath, IServiceProvider serviceProvider, - IDictionary validationContextItems + IDictionary validationContextItems, + int collectionDepth ) { var result = true; @@ -340,6 +367,7 @@ IDictionary validationContextItems equalityPath, serviceProvider, validationContextItems, + collectionDepth + 1, enumerateItems: true )) { @@ -349,9 +377,12 @@ IDictionary 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 { 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 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 diff --git a/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs index 8d719df..8e44291 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/GraphSizeTests.cs @@ -11,7 +11,7 @@ namespace RecursiveDataAnnotationsValidation.Tests { /// /// 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. /// 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(); 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); } diff --git a/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs new file mode 100644 index 0000000..3b0188a --- /dev/null +++ b/test/RecursiveDataAnnotationsValidation.Tests/MaxDepthTests.cs @@ -0,0 +1,375 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; +using System.Runtime.ExceptionServices; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace RecursiveDataAnnotationsValidation.Tests +{ + /// + /// The maximum depth of the object graph is 128. + /// + /// The walk calls itself once for each level, so a deep graph is a deep call stack. .NET cannot + /// catch a StackOverflowException (https://learn.microsoft.com/dotnet/api/system.stackoverflowexception), + /// so the process ends. A graph this deep is not a real model. It points to a cycle that the + /// validator cannot see, such as a computed property that returns a new object on each read, + /// or to a graph that was built from untrusted input. The validator therefore fails the + /// validation and does not walk any further. + /// + /// The depth of an object is the number of segments in its path. In "Value[0][0].Name", + /// Value is level 1, the first [0] is level 2, the second [0] is level 3 and Name is level 4. + /// Each property step and each collection index counts as one level. System.Text.Json counts + /// nearly the same way, where each object and each array is one level, and its limit is 64 + /// (https://learn.microsoft.com/dotnet/api/system.text.json.jsonserializeroptions.maxdepth). + /// A document within that limit stays within about 63 levels here. The root object is level 0. + /// Our limit is twice the JSON limit. + /// An object at level 128 is validated. An object at level 129 is not validated, and its path + /// gets one error that says so. A collection of leaf types is not walked, so it never counts. + /// + /// A tree of objects that holds its children in a List has two levels for each tree level, + /// one for the property and one for the index, so the tree can be 64 levels deep. + /// + /// A graph that is too deep fails the validation. It does not throw, and it does not stop + /// silently, because a graph that nobody validated must not pass. + /// + /// The limit is internal and fixed. It is not a setting. + /// + public class MaxDepthTests + { + private const int OneMegabyte = 1024 * 1024; + + private const string TooDeep = "The object is nested more than 128 levels deep and was not validated."; + + public class Node + { + public Node Next { get; set; } + + [Required(ErrorMessage = "Name is required")] + public string Name { get; set; } + } + + public class TreeNode + { + public List Children { get; set; } = new List(); + + [Required(ErrorMessage = "Name is required")] + public string Name { get; set; } + } + + public class Holder + { + public List Value { get; set; } + } + + /// + /// Builds a new object on each read and does not override Equals, so no object ever + /// matches an object on its path and the reference check never sees a repeat. + /// + public class Vector + { + public Vector Zero => new Vector(); + } + + // A chain of `levels` objects below the root. The object at the bottom has the given name. + private static Node Chain(int levels, string bottomName) + { + var root = new Node { Name = levels == 0 ? bottomName : "ok" }; + var current = root; + for (var i = 0; i < levels; i++) + { + current.Next = new Node { Name = i == levels - 1 ? bottomName : "ok" }; + current = current.Next; + } + + return root; + } + + // The path of the object at the bottom of a chain: "Next" repeated. + private static string NextPath(int levels) => + string.Join(".", Enumerable.Repeat("Next", levels)); + + // A tree with one child on each node, `levels` below the root. + private static TreeNode Tree(int levels, string bottomName) + { + var root = new TreeNode { Name = levels == 0 ? bottomName : "ok" }; + var current = root; + for (var i = 0; i < levels; i++) + { + var child = new TreeNode { Name = i == levels - 1 ? bottomName : "ok" }; + current.Children.Add(child); + current = child; + } + + return root; + } + + private static string ChildrenPath(int levels) => + string.Join(".", Enumerable.Repeat("Children[0]", levels)); + + // A Node that is invalid, inside `lists` lists that each hold the next one: Value[0][0]...Name. + private static Holder NestedLists(int lists) + { + object inner = new Node { Name = null }; + for (var i = 0; i < lists; i++) + { + inner = new List { inner }; + } + + return new Holder { Value = (List)inner }; + } + + private static string ValueIndexes(int count) => + "Value" + string.Concat(Enumerable.Repeat("[0]", count)); + + private static (bool Valid, List Errors) Run(object model) + { + var results = new List(); + var valid = new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(model, results); + return (valid, ResultText.Describe(results)); + } + + public class ObjectsInAChain + { + [Fact] + public void Object_at_level_128_is_validated() + { + var (valid, errors) = Run(Chain(128, null)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(128)}.Name | Name is required"), errors); + } + + /// + /// The string at level 129 is a leaf: it can never produce a result, so nothing is + /// walked there and the limit does not apply. + /// + [Fact] + public void Valid_chain_with_its_last_object_at_level_128_passes() + { + var (valid, errors) = Run(Chain(128, "ok")); + + Assert.True(valid); + Assert.Empty(errors); + } + + [Fact] + public void Valid_object_at_level_129_fails_the_validation() + { + var (valid, errors) = Run(Chain(129, "ok")); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(129)} | {TooDeep}"), errors); + } + + /// + /// The object at level 129 is not validated, so its own errors are not reported. + /// The one error says why. + /// + [Fact] + public void Invalid_object_at_level_129_gets_only_the_depth_error() + { + var (valid, errors) = Run(Chain(129, null)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(129)} | {TooDeep}"), errors); + } + + [Fact] + public void Walk_stops_at_the_first_object_that_is_too_deep() + { + var (valid, errors) = Run(Chain(500, null)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(129)} | {TooDeep}"), errors); + } + + [Fact] + public void Errors_above_the_limit_are_still_reported() + { + var root = Chain(129, "ok"); + root.Name = null; + + var (valid, errors) = Run(root); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect( + "Name | Name is required", + $"{NextPath(129)} | {TooDeep}"), + errors); + } + + [Fact] + public void A_null_results_list_still_gets_false() + { + var valid = new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(Chain(129, "ok"), null); + + Assert.False(valid); + } + + [Fact] + public async Task The_async_method_fails_the_same_way() + { + var results = new List(); + + var valid = await new RecursiveDataAnnotationValidator() + .TryValidateObjectRecursiveAsync(Chain(129, "ok"), results); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(129)} | {TooDeep}"), ResultText.Describe(results)); + } + + /// + /// Before the limit, a chain of about 1,300 objects was the most that a 1 MB stack, the + /// default on Windows, could walk. A chain of 600 is well below that, so this test cannot + /// end the test run when the limit is missing, and it still shows that the validator now + /// fails the validation at level 129. A chain of 100,000 would end the process. + /// + [Fact] + public void A_chain_of_600_objects_fails_on_a_small_stack() + { + var results = new List(); + var valid = true; + Exception error = null; + + var thread = new Thread(() => + { + try + { + valid = new RecursiveDataAnnotationValidator().TryValidateObjectRecursive(Chain(600, "ok"), results); + } + catch (Exception e) + { + error = e; + } + }, OneMegabyte); + + thread.Start(); + thread.Join(); + + if (error != null) ExceptionDispatchInfo.Capture(error).Throw(); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{NextPath(129)} | {TooDeep}"), ResultText.Describe(results)); + } + } + + public class ComputedProperties + { + /// + /// Before the limit, this walk never ended and overflowed the stack, which ends the + /// process. A type that overrides Equals is stopped earlier (see EqualsAnAncestor), and + /// a type that does not override it is stopped by the limit. + /// + [Fact] + public void Property_that_returns_a_new_object_on_each_read_fails_at_the_limit() + { + var (valid, errors) = Run(new Vector()); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{string.Join(".", Enumerable.Repeat("Zero", 129))} | {TooDeep}"), errors); + } + } + + public class ObjectsInCollections + { + /// + /// Each tree level is two path levels: the Children property and the index. + /// The 64th child is at level 128. + /// + [Fact] + public void Tree_64_levels_deep_is_validated_to_the_bottom() + { + var (valid, errors) = Run(Tree(64, null)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{ChildrenPath(64)}.Name | Name is required"), errors); + } + + [Fact] + public void Valid_tree_64_levels_deep_passes() + { + var (valid, errors) = Run(Tree(64, "ok")); + + Assert.True(valid); + Assert.Empty(errors); + } + + /// + /// The 65th child is at level 130. The path of the error ends with the index, not with + /// a member, because the object at that path was not validated. + /// + [Fact] + public void Tree_65_levels_deep_fails_at_the_65th_child() + { + var (valid, errors) = Run(Tree(65, null)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{ChildrenPath(65)} | {TooDeep}"), errors); + } + + [Fact] + public void Every_item_that_is_too_deep_gets_its_own_error() + { + var node = Tree(64, "ok"); + var bottom = node; + while (bottom.Children.Count > 0) bottom = bottom.Children[0]; + bottom.Children.Add(new TreeNode { Name = "ok" }); + bottom.Children.Add(new TreeNode { Name = "ok" }); + + var (valid, errors) = Run(node); + + Assert.False(valid); + Assert.Equal( + ResultText.Expect( + $"{ChildrenPath(64)}.Children[0] | {TooDeep}", + $"{ChildrenPath(64)}.Children[1] | {TooDeep}"), + errors); + } + } + + /// + /// An item that is a collection counts one level for its own index, like any other item. + /// The path of the object inside the lists is the path of the lists, as in "Value[0][0].Name". + /// + public class ObjectsInCollectionsOfCollections + { + [Fact] + public void Object_at_level_128_is_validated() + { + var (valid, errors) = Run(NestedLists(127)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{ValueIndexes(127)}.Name | Name is required"), errors); + } + + [Fact] + public void Object_at_level_129_is_not_validated_and_the_path_ends_with_its_index() + { + var (valid, errors) = Run(NestedLists(128)); + + Assert.False(valid); + Assert.Equal(ResultText.Expect($"{ValueIndexes(128)} | {TooDeep}"), errors); + } + + /// + /// A list nested more than 128 levels deep is not a stack risk any more. Before the limit, + /// 1,200 nested lists overflowed a 1 MB stack, and in a Debug build, which uses more stack + /// for each level, 1,000 lists overflowed the test host. A depth of 300 is safe in both + /// cases, and it is more than twice the limit. + /// + [Fact] + public void Three_hundred_nested_lists_fail_the_validation() + { + var (valid, errors) = Run(NestedLists(300)); + + Assert.False(valid); + Assert.Single(errors); + Assert.EndsWith($" | {TooDeep}", errors[0]); + } + } + } +} diff --git a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs index 51d3e44..b89e334 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/UpgradeCompatibilityTests.cs @@ -536,16 +536,18 @@ public void Deep_chain_reports_the_full_dotted_path() } /// - /// A chain a few hundred objects deep is validated without running out of stack. + /// A chain a hundred objects deep is validated without running out of stack. /// Both versions recurse once per level, so very deep graphs (thousands of levels) - /// can still overflow the stack in both. + /// can still overflow the stack in the old version. Since 3.0 the validator stops at + /// 128 levels and fails the validation (see MaxDepthTests), so a chain of 200, which + /// this test used before, is no longer the same in both versions. /// [Fact] - public void Chain_of_two_hundred_objects_is_validated() + public void Chain_of_one_hundred_objects_is_validated() { var root = new Node { Name = "n" }; var node = root; - for (var i = 0; i < 200; i++) + for (var i = 0; i < 100; i++) { node.Next = new Node { Name = "n" }; node = node.Next; @@ -554,7 +556,7 @@ public void Chain_of_two_hundred_objects_is_validated() Assert.False(Validate(root, out var results)); var only = Assert.Single(results); - Assert.StartsWith(string.Concat(Enumerable.Repeat("Next.", 200)) + "Name |", only); + Assert.StartsWith(string.Concat(Enumerable.Repeat("Next.", 100)) + "Name |", only); } public class Node diff --git a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs index 61ae1a0..14baf3e 100644 --- a/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs +++ b/test/RecursiveDataAnnotationsValidation.Tests/ValidatorHardeningTests.cs @@ -16,8 +16,7 @@ namespace RecursiveDataAnnotationsValidation.Tests /// Each nested class covers one item. Its summary gives the problem and the proposed fix. /// Some tests guard behavior that a fix must not break. Their comments say why. /// Not covered: - /// - A max-depth limit. A deep acyclic graph causes an uncatchable StackOverflowException - /// that would kill the test host, so it needs a design decision first. + /// - A max-depth limit. It is covered in MaxDepthTests. /// - Lazy or infinite sequences of objects, and user getters that throw. The desired /// behavior (skip, report or propagate) is not decided yet. Lazy sequences of leaf types /// are no longer run (see PrimitiveCollections). @@ -154,7 +153,8 @@ public async Task Null_validation_context_throws_ArgumentNullException_async() /// overflowed (see Records_that_reference_each_other_are_validated). On .NET Framework /// they still always overflow, inside the framework's Validator (see that test). /// - A property that returns a new object on each read, on a type that does not override - /// Equals, such as `Vector Zero => new Vector()`, overflows the stack. + /// Equals, such as `Vector Zero => new Vector()`, overflowed the stack. Since 3.0 the + /// maximum depth stops it and fails the validation (see MaxDepthTests.ComputedProperties). /// public class ReferenceEquality {