Fix safe recursive traversal crashes (#54/#73)

.github/workflows/_build.yml

@@ -19,7 +19,10 @@ jobs:
     - name: Install .NET SDK
       uses: actions/setup-dotnet@v4
       with:
-        dotnet-version: 6.0.x
+        dotnet-version: |
+          8.0.x
+          9.0.x
+          10.0.x
         global-json-file: "./global.json"
 
     - name: Restore dependencies

global.json

@@ -1,6 +1,6 @@
 {
   "sdk": {
-    "version": "8.0.100",
+    "version": "10.0.100",
     "rollForward": "latestFeature"
   }
-}
+}

samples/Samples.Console/Samples.Console.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>net6.0</TargetFramework>
+    <TargetFramework>net8.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
   </PropertyGroup>
 

samples/Samples.Web/Samples.Web.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk.Web">
 
   <PropertyGroup>
-    <TargetFramework>net7.0</TargetFramework>
+    <TargetFramework>net8.0</TargetFramework>
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>
   </PropertyGroup>

src/MiniValidation/MiniValidation.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <Description>A minimalist validation library built atop the existing validation features in .NET's `System.ComponentModel.DataAnnotations` namespace.</Description>
-    <TargetFrameworks>netstandard2.0;net6.0</TargetFrameworks>
+    <TargetFrameworks>netstandard2.0;net8.0</TargetFrameworks>
     <PackageTags>ComponentModel DataAnnotations validation</PackageTags>
     <PackageReadmeFile>README.md</PackageReadmeFile>
     <LangVersion>10.0</LangVersion>

src/MiniValidation/MiniValidator.cs

@@ -371,6 +371,12 @@ private static async Task<bool> TryValidateImpl(
             throw new ArgumentNullException(nameof(target));
         }
 
+        var targetType = target.GetType();
+        if (TypeDetailsCache.IsNonValidatableType(targetType))
+        {
+            return true;
+        }
+
         // Once we get to this point we have to box the target in order to track whether we've validated it or not
         if (validatedObjects.ContainsKey(target))
         {
@@ -384,7 +390,6 @@ private static async Task<bool> TryValidateImpl(
         // Add current target to tracking dictionary in null (validating) state
         validatedObjects.Add(target, null);
 
-        var targetType = target.GetType();
         var (typeProperties, _) = _typeDetailsCache.Get(targetType);
 
         var isValid = true;
@@ -418,6 +423,7 @@ private static async Task<bool> TryValidateImpl(
             }
 
             if (recurse && propertyValue is not null &&
+                !TypeDetailsCache.IsNonValidatableType(propertyValueType!) &&
                 (property.Recurse
                  || typeof(IValidatableObject).IsAssignableFrom(propertyValueType)
                  || typeof(IAsyncValidatableObject).IsAssignableFrom(propertyValueType)

src/MiniValidation/TypeDetailsCache.cs

@@ -14,19 +14,35 @@ internal class TypeDetailsCache
     private static readonly PropertyDetails[] _emptyPropertyDetails = Array.Empty<PropertyDetails>();
     private readonly ConcurrentDictionary<Type, (PropertyDetails[] Properties, bool RequiresAsync)> _cache = new();
 
+    public TypeDetailsCache()
+    {
+        TypeDescriptor.Refreshed += args =>
+        {
+            if (args.TypeChanged is { } type)
+            {
+                _cache.TryRemove(type, out _);
+            }
+            else
+            {
+                _cache.Clear();
+            }
+        };
+    }
+
     public (PropertyDetails[] Properties, bool RequiresAsync) Get(Type? type)
     {
         if (type is null)
         {
             return (_emptyPropertyDetails, false);
         }
 
-        if (!_cache.ContainsKey(type))
+        (PropertyDetails[] Properties, bool RequiresAsync) details;
+        while (!_cache.TryGetValue(type, out details))
         {
             Visit(type);
         }
 
-        return _cache[type];
+        return details;
     }
 
     private void Visit(Type type)
@@ -48,7 +64,7 @@ private void Visit(Type type, HashSet<Type> visited, ref bool requiresAsync)
             return;
         }
 
-        if (DoNotRecurseIntoPropertiesOf(type))
+        if (DoNotRecurseIntoPropertiesOf(type) || IsNonValidatableType(type))
         {
             _cache[type] = (_emptyPropertyDetails, false);
             return;
@@ -89,6 +105,7 @@ private void Visit(Type type, HashSet<Type> visited, ref bool requiresAsync)
             validationAttributes ??= Array.Empty<ValidationAttribute>();
             var hasValidationOnProperty = validationAttributes.Length > 0;
             var hasSkipRecursionOnProperty = skipRecursionAttribute is not null;
+            var propertyTypeIsNonValidatable = IsNonValidatableType(property.PropertyType);
             var enumerableType = GetEnumerableType(property.PropertyType);
             if (enumerableType != null)
             {
@@ -109,11 +126,12 @@ private void Visit(Type type, HashSet<Type> visited, ref bool requiresAsync)
             var propertyTypeHasProperties = _cache.TryGetValue(property.PropertyType, out var typeCache) && typeCache.Properties.Length > 0;
             var propertyTypeIsValidatableObject = typeof(IValidatableObject).IsAssignableFrom(property.PropertyType)
                                                   || typeof(IAsyncValidatableObject).IsAssignableFrom(property.PropertyType);
-            var propertyTypeSupportsPolymorphism = !property.PropertyType.IsSealed;
+            var propertyTypeSupportsPolymorphism = !propertyTypeIsNonValidatable && !property.PropertyType.IsSealed;
             var enumerableTypeHasProperties = enumerableType != null
                 && _cache.TryGetValue(enumerableType, out var enumProperties)
                 && enumProperties.Properties.Length > 0;
-            var recurse = (enumerableTypeHasProperties || propertyTypeHasProperties
+            var recurse = !propertyTypeIsNonValidatable
+                && (enumerableTypeHasProperties || propertyTypeHasProperties
                 || propertyTypeIsValidatableObject
                 || propertyTypeSupportsPolymorphism)
                 && !hasSkipRecursionOnProperty;
@@ -162,6 +180,28 @@ private static bool DoNotRecurseIntoPropertiesOf(Type type) =>
 #endif
         ;
 
+    internal static bool IsNonValidatableType(Type type) =>
+        typeof(Delegate).IsAssignableFrom(type)
+        || typeof(MemberInfo).IsAssignableFrom(type)
+        || typeof(ParameterInfo).IsAssignableFrom(type)
+        || typeof(Module).IsAssignableFrom(type)
+        || typeof(Assembly).IsAssignableFrom(type)
+        || IsKnownNonValidatableFrameworkType(type);
+
+    private static bool IsKnownNonValidatableFrameworkType(Type type)
+    {
+        var @namespace = type.Namespace;
+        return @namespace is not null
+            && (@namespace == "System.Text.Json"
+                || @namespace.StartsWith("System.Text.Json.", StringComparison.Ordinal)
+                || @namespace == "Newtonsoft.Json.Linq"
+                || @namespace.StartsWith("Newtonsoft.Json.Linq.", StringComparison.Ordinal)
+                || @namespace == "Microsoft.AspNetCore.JsonPatch"
+                || @namespace.StartsWith("Microsoft.AspNetCore.JsonPatch.", StringComparison.Ordinal)
+                || @namespace == "Microsoft.AspNetCore.OData.Deltas"
+                || @namespace.StartsWith("Microsoft.AspNetCore.OData.Deltas.", StringComparison.Ordinal));
+    }
+
     private static (ValidationAttribute[]?, DisplayAttribute?, SkipRecursionAttribute?) GetPropertyAttributes(ParameterInfo[]? primaryCtorParameters, PropertyInfo property)
     {
         List<ValidationAttribute>? validationAttributes = null;

tests/MiniValidation.Benchmarks/MiniValidation.Benchmarks.csproj

@@ -2,8 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFrameworks>net6.0;net7.0</TargetFrameworks>
-    <TargetFrameworks Condition=" $([MSBuild]::IsOsPlatform('Windows')) ">net471;net6.0;net7.0</TargetFrameworks>
+    <TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>
     <LangVersion>10</LangVersion>

tests/MiniValidation.UnitTests/FakeJValue.cs

@@ -0,0 +1,6 @@
+namespace Newtonsoft.Json.Linq;
+
+public sealed class FakeJValue
+{
+    public object First => throw new InvalidOperationException("Cannot access child value on Newtonsoft.Json.Linq.JValue.");
+}

tests/MiniValidation.UnitTests/MiniValidation.UnitTests.csproj

@@ -1,8 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFrameworks>net6.0;net7.0;net8.0</TargetFrameworks>
-    <TargetFrameworks Condition=" $([MSBuild]::IsOsPlatform('Windows')) ">net471;net6.0;net7.0;net8.0</TargetFrameworks>
+    <TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks>
     <LangVersion>10.0</LangVersion>
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>

tests/MiniValidation.UnitTests/Recursion.cs

@@ -492,4 +492,38 @@ public async Task DoesntThrow_When_Validates_Without_Recurse_And_Object_Has_Not_
 
         Assert.True(isValid);
     }
-}
+
+    [Fact]
+    public void Valid_When_Model_Has_Func_Property()
+    {
+        var result = MiniValidator.TryValidate(new TestTypeWithFuncProperty(), out var errors);
+
+        Assert.True(result);
+        Assert.Empty(errors);
+    }
+
+#if NET6_0_OR_GREATER
+    [Fact]
+    public void Valid_When_Model_Has_JsonSerializerOptions_Property()
+    {
+        var result = MiniValidator.TryValidate(new TestTypeWithJsonSerializerOptions(), out var errors);
+
+        Assert.True(result);
+        Assert.Empty(errors);
+    }
+#endif
+
+    [Fact]
+    public void Valid_When_Object_Property_Has_JTokenLike_Value_With_Throwing_Getter()
+    {
+        var thingToValidate = new TestTypeWithObjectPayload
+        {
+            Payload = new Newtonsoft.Json.Linq.FakeJValue()
+        };
+
+        var result = MiniValidator.TryValidate(thingToValidate, out var errors);
+
+        Assert.True(result);
+        Assert.Empty(errors);
+    }
+}

tests/MiniValidation.UnitTests/TestTypes.cs

@@ -267,3 +267,20 @@ class TestTypeWithNotImplementedProperty
 
     public TestTypeForTypeDescriptor NotImplementedProperty => throw new Exception();
 }
+
+class TestTypeWithFuncProperty
+{
+    public Func<object?> SomeFunc { get; set; } = () => null;
+}
+
+#if NET6_0_OR_GREATER
+class TestTypeWithJsonSerializerOptions
+{
+    public System.Text.Json.JsonSerializerOptions Options { get; set; } = new(System.Text.Json.JsonSerializerDefaults.Web);
+}
+#endif
+
+class TestTypeWithObjectPayload
+{
+    public object? Payload { get; set; }
+}