diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultObjectValidator.cs b/src/Microsoft.AspNetCore.Mvc.Core/DefaultObjectValidator.cs similarity index 70% rename from src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultObjectValidator.cs rename to src/Microsoft.AspNetCore.Mvc.Core/DefaultObjectValidator.cs index 0afcdb0f5e..8fedb7702b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultObjectValidator.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DefaultObjectValidator.cs @@ -2,26 +2,33 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.Extensions.Options; -namespace Microsoft.AspNetCore.Mvc.Internal +namespace Microsoft.AspNetCore.Mvc { /// /// The default implementation of . /// - public class DefaultObjectValidator : ObjectModelValidator + internal class DefaultObjectValidator : ObjectModelValidator { + private readonly MvcOptions _mvcOptions; + /// /// Initializes a new instance of . /// /// The . /// The list of . + /// Accessor to . public DefaultObjectValidator( IModelMetadataProvider modelMetadataProvider, - IList validatorProviders) + IList validatorProviders, + MvcOptions mvcOptions) : base(modelMetadataProvider, validatorProviders) { + _mvcOptions = mvcOptions; } public override ValidationVisitor GetValidationVisitor( @@ -31,12 +38,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal IModelMetadataProvider metadataProvider, ValidationStateDictionary validationState) { - return new ValidationVisitor( + var visitor = new ValidationVisitor( actionContext, validatorProvider, validatorCache, metadataProvider, validationState); + + visitor.MaxValidationDepth = _mvcOptions.MaxValidationDepth; + + return visitor; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 3ebdc371fa..e7e7d5e0dd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -228,7 +228,7 @@ namespace Microsoft.Extensions.DependencyInjection { var options = s.GetRequiredService>().Value; var metadataProvider = s.GetRequiredService(); - return new DefaultObjectValidator(metadataProvider, options.ModelValidatorProviders); + return new DefaultObjectValidator(metadataProvider, options.ModelValidatorProviders, options); }); services.TryAddSingleton(); services.TryAddSingleton(); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs index 76e3a16916..dcc5243fbb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs @@ -35,6 +35,9 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure if (Version >= CompatibilityVersion.Version_2_2) { values[nameof(MvcOptions.EnableEndpointRouting)] = true; + + // Matches JsonSerializerSettingsProvider.DefaultMaxDepth + values[nameof(MvcOptions.MaxValidationDepth)] = 32; } return values; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/NullableCompatibilitySwitch.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/NullableCompatibilitySwitch.cs new file mode 100644 index 0000000000..53563ba157 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/NullableCompatibilitySwitch.cs @@ -0,0 +1,35 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + internal class NullableCompatibilitySwitch : ICompatibilitySwitch where TValue : struct + { + private TValue? _value; + + public NullableCompatibilitySwitch(string name) + { + Name = name; + } + + public bool IsValueSet { get; private set; } + + public string Name { get; } + + public TValue? Value + { + get => _value; + set + { + IsValueSet = true; + _value = value; + } + } + + object ICompatibilitySwitch.Value + { + get => Value; + set => Value = (TValue?)value; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs index fa467588a8..164928c54a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs @@ -4,8 +4,10 @@ using System; using System.Collections.Generic; using System.Runtime.CompilerServices; +using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Internal; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation { @@ -15,6 +17,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation /// public class ValidationVisitor { + private int? _maxValidationDepth; + /// /// Creates a new . /// @@ -76,6 +80,31 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation protected ModelMetadata Metadata { get; set; } protected IValidationStrategy Strategy { get; set; } + /// + /// Gets or sets the maximum depth to constrain the validation visitor when validating. + /// + /// traverses the object graph of the model being validated. For models + /// that are very deep or are infinitely recursive, validation may result in stack overflow. + /// + /// + /// When not , will throw if + /// current traversal depth exceeds the specified value. + /// + /// + public int? MaxValidationDepth + { + get => _maxValidationDepth; + set + { + if (value != null && value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + + _maxValidationDepth = value; + } + } + /// /// Indicates whether validation of a complex type should be performed if validation fails for any of its children. The default behavior is false. /// @@ -192,6 +221,33 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation return true; } + if (MaxValidationDepth != null && CurrentPath.Count > MaxValidationDepth) + { + // Non cyclic but too deep an object graph. + + // Pop the current model to make ValidationStack.Dispose happy + CurrentPath.Pop(model); + + string message; + switch (metadata.MetadataKind) + { + case ModelMetadataKind.Property: + message = Resources.FormatValidationVisitor_ExceededMaxPropertyDepth(nameof(ValidationVisitor), MaxValidationDepth, metadata.Name, metadata.ContainerType); + break; + + default: + // Since the minimum depth is never 0, MetadataKind can never be Parameter. Consequently we only special case MetadataKind.Property. + message = Resources.FormatValidationVisitor_ExceededMaxDepth(nameof(ValidationVisitor), MaxValidationDepth, metadata.ModelType); + break; + } + + message += " " + Resources.FormatValidationVisitor_ExceededMaxDepthFix(nameof(MvcOptions), nameof(MvcOptions.MaxValidationDepth)); + throw new InvalidOperationException(message) + { + HelpLink = "https://aka.ms/AA21ue1", + }; + } + var entry = GetValidationEntry(model); key = entry?.Key ?? key ?? string.Empty; metadata = entry?.Metadata ?? metadata; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs index cafe4e746e..34a55b4ce4 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs @@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Mvc private readonly CompatibilitySwitch _inputFormatterExceptionPolicy; private readonly CompatibilitySwitch _suppressBindingUndefinedValueToEnumType; private readonly CompatibilitySwitch _enableEndpointRouting; + private readonly NullableCompatibilitySwitch _maxValidationDepth; private readonly ICompatibilitySwitch[] _switches; /// @@ -56,6 +57,7 @@ namespace Microsoft.AspNetCore.Mvc _inputFormatterExceptionPolicy = new CompatibilitySwitch(nameof(InputFormatterExceptionPolicy), InputFormatterExceptionPolicy.AllExceptions); _suppressBindingUndefinedValueToEnumType = new CompatibilitySwitch(nameof(SuppressBindingUndefinedValueToEnumType)); _enableEndpointRouting = new CompatibilitySwitch(nameof(EnableEndpointRouting)); + _maxValidationDepth = new NullableCompatibilitySwitch(nameof(MaxValidationDepth)); _switches = new ICompatibilitySwitch[] { @@ -65,6 +67,7 @@ namespace Microsoft.AspNetCore.Mvc _inputFormatterExceptionPolicy, _suppressBindingUndefinedValueToEnumType, _enableEndpointRouting, + _maxValidationDepth, }; } @@ -396,6 +399,49 @@ namespace Microsoft.AspNetCore.Mvc /// public bool RequireHttpsPermanent { get; set; } + /// + /// Gets or sets the maximum depth to constrain the validation visitor when validating. Set to + /// to disable this feature. + /// + /// traverses the object graph of the model being validated. For models + /// that are very deep or are infinitely recursive, validation may result in stack overflow. + /// + /// + /// When not , will throw if + /// traversing an object exceeds the maximum allowed validation depth. + /// + /// + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for + /// guidance and examples of setting the application's compatibility version. + /// + /// + /// Configuring the desired value of the compatibility switch by calling this property's setter will take precedence + /// over the value implied by the application's . + /// + /// + /// If the application's compatibility version is set to then + /// this setting will have the value 200 unless explicitly configured. + /// + /// + /// If the application's compatibility version is set to or + /// earlier then this setting will have the value unless explicitly configured. + /// + /// + public int? MaxValidationDepth + { + get => _maxValidationDepth.Value; + set + { + if (value != null && value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + + _maxValidationDepth.Value = value; + } + } + IEnumerator IEnumerable.GetEnumerator() { return ((IEnumerable)_switches).GetEnumerator(); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs index 65b63b8d27..354ce64258 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs @@ -1536,6 +1536,48 @@ namespace Microsoft.AspNetCore.Mvc.Core internal static string FormatApiConventionMethod_NoMethodFound(object p0, object p1) => string.Format(CultureInfo.CurrentCulture, GetString("ApiConventionMethod_NoMethodFound"), p0, p1); + /// + /// {0} exceeded the maximum configured validation depth '{1}' when validating type '{2}'. + /// + internal static string ValidationVisitor_ExceededMaxDepth + { + get => GetString("ValidationVisitor_ExceededMaxDepth"); + } + + /// + /// {0} exceeded the maximum configured validation depth '{1}' when validating type '{2}'. + /// + internal static string FormatValidationVisitor_ExceededMaxDepth(object p0, object p1, object p2) + => string.Format(CultureInfo.CurrentCulture, GetString("ValidationVisitor_ExceededMaxDepth"), p0, p1, p2); + + /// + /// This may indicate a very deep or infinitely recursive object graph. Consider modifying '{0}.{1}' or suppressing validation on the model type. + /// + internal static string ValidationVisitor_ExceededMaxDepthFix + { + get => GetString("ValidationVisitor_ExceededMaxDepthFix"); + } + + /// + /// This may indicate a very deep or infinitely recursive object graph. Consider modifying '{0}.{1}' or suppressing validation on the model type. + /// + internal static string FormatValidationVisitor_ExceededMaxDepthFix(object p0, object p1) + => string.Format(CultureInfo.CurrentCulture, GetString("ValidationVisitor_ExceededMaxDepthFix"), p0, p1); + + /// + /// {0} exceeded the maximum configured validation depth '{1}' when validating property '{2}' on type '{3}'. + /// + internal static string ValidationVisitor_ExceededMaxPropertyDepth + { + get => GetString("ValidationVisitor_ExceededMaxPropertyDepth"); + } + + /// + /// {0} exceeded the maximum configured validation depth '{1}' when validating property '{2}' on type '{3}'. + /// + internal static string FormatValidationVisitor_ExceededMaxPropertyDepth(object p0, object p1, object p2, object p3) + => string.Format(CultureInfo.CurrentCulture, GetString("ValidationVisitor_ExceededMaxPropertyDepth"), p0, p1, p2, p3); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx index 4b31a83761..4916783a8c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx @@ -457,4 +457,13 @@ A method named '{0}' was not found on convention type '{1}'. + + {0} exceeded the maximum configured validation depth '{1}' when validating type '{2}'. + + + This may indicate a very deep or infinitely recursive object graph. Consider modifying '{0}.{1}' or suppressing validation on the model type. + + + {0} exceeded the maximum configured validation depth '{1}' when validating property '{2}' on type '{3}'. + \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs index d46f73d642..d26aa506c7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ControllerBaseTest.cs @@ -2711,7 +2711,8 @@ namespace Microsoft.AspNetCore.Mvc.Core.Test var controller = GetController(binder, valueProvider: null); controller.ObjectValidator = new DefaultObjectValidator( controller.MetadataProvider, - new[] { Mock.Of() }); + new[] { Mock.Of() }, + new MvcOptions()); var model = new TryValidateModelModel(); @@ -2747,7 +2748,8 @@ namespace Microsoft.AspNetCore.Mvc.Core.Test var controller = GetController(binder, valueProvider: null); controller.ObjectValidator = new DefaultObjectValidator( controller.MetadataProvider, - new[] { provider.Object }); + new[] { provider.Object }, + new MvcOptions()); // Act var result = controller.TryValidateModel(model, "Prefix"); @@ -2783,7 +2785,8 @@ namespace Microsoft.AspNetCore.Mvc.Core.Test var controller = GetController(binder, valueProvider: null); controller.ObjectValidator = new DefaultObjectValidator( controller.MetadataProvider, - new[] { provider.Object }); + new[] { provider.Object }, + new MvcOptions()); // Act var result = controller.TryValidateModel(model); @@ -2867,7 +2870,7 @@ namespace Microsoft.AspNetCore.Mvc.Core.Test ControllerContext = controllerContext, MetadataProvider = metadataProvider, ModelBinderFactory = binderFactory.Object, - ObjectValidator = new DefaultObjectValidator(metadataProvider, validatorProviders), + ObjectValidator = new DefaultObjectValidator(metadataProvider, validatorProviders, new MvcOptions()), }; return controller; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerActivatorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerActivatorTest.cs index 90305c7a08..2e72325feb 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerActivatorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerActivatorTest.cs @@ -135,7 +135,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers .Returns(metadataProvider); services .Setup(s => s.GetService(typeof(IObjectModelValidator))) - .Returns(new DefaultObjectValidator(metadataProvider, new List())); + .Returns(new DefaultObjectValidator(metadataProvider, new List(), new MvcOptions())); return services.Object; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerFactoryTest.cs index 436693bbb9..3a705e334e 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerFactoryTest.cs @@ -215,7 +215,8 @@ namespace Microsoft.AspNetCore.Mvc.Controllers .Setup(s => s.GetService(typeof(IObjectModelValidator))) .Returns(new DefaultObjectValidator( metadataProvider, - TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders)); + TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders, + new MvcOptions())); return services.Object; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcOptionsConfigureCompatibilityOptionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcOptionsConfigureCompatibilityOptionsTest.cs new file mode 100644 index 0000000000..d661fe0bd5 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcOptionsConfigureCompatibilityOptionsTest.cs @@ -0,0 +1,83 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Core.Infrastructure +{ + public class MvcOptionsConfigureCompatibilityOptionsTest + { + [Fact] + public void PostConfigure_ConfiguresMaxValidationDepth() + { + // Arrange + var mvcOptions = new MvcOptions(); + var mvcCompatibilityOptions = new MvcCompatibilityOptions + { + CompatibilityVersion = CompatibilityVersion.Version_2_2, + }; + + var configureOptions = new MvcOptionsConfigureCompatibilityOptions( + NullLoggerFactory.Instance, + Options.Create(mvcCompatibilityOptions)); + + // Act + configureOptions.PostConfigure(string.Empty, mvcOptions); + + // Assert + Assert.Equal(32, mvcOptions.MaxValidationDepth); + } + + [Fact] + public void PostConfigure_DoesNotConfiguresMaxValidationDepth_WhenSetToNull() + { + // Arrange + var mvcOptions = new MvcOptions + { + MaxValidationDepth = null, + }; + var mvcCompatibilityOptions = new MvcCompatibilityOptions + { + CompatibilityVersion = CompatibilityVersion.Version_2_2, + }; + + var configureOptions = new MvcOptionsConfigureCompatibilityOptions( + NullLoggerFactory.Instance, + Options.Create(mvcCompatibilityOptions)); + + // Act + configureOptions.PostConfigure(string.Empty, mvcOptions); + + // Assert + Assert.Null(mvcOptions.MaxValidationDepth); + } + + [Fact] + public void PostConfigure_DoesNotConfiguresMaxValidationDepth_WhenSetToValue() + { + // Arrange + var expected = 13; + var mvcOptions = new MvcOptions + { + MaxValidationDepth = expected, + }; + var mvcCompatibilityOptions = new MvcCompatibilityOptions + { + CompatibilityVersion = CompatibilityVersion.Version_2_2, + }; + + var configureOptions = new MvcOptionsConfigureCompatibilityOptions( + NullLoggerFactory.Instance, + Options.Create(mvcCompatibilityOptions)); + + // Act + configureOptions.PostConfigure(string.Empty, mvcOptions); + + // Assert + Assert.Equal(expected, mvcOptions.MaxValidationDepth); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/NullableCompatibilitySwitchTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/NullableCompatibilitySwitchTest.cs new file mode 100644 index 0000000000..bb5f1de6ae --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/NullableCompatibilitySwitchTest.cs @@ -0,0 +1,77 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + public class NullableCompatibilitySwitchTest + { + [Fact] + public void Constructor_WithName_IsValueSetIsFalse() + { + // Arrange & Act + var @switch = new NullableCompatibilitySwitch("TestProperty"); + + // Assert + Assert.Null(@switch.Value); + Assert.False(@switch.IsValueSet); + } + + [Fact] + public void ValueNonInterface_SettingValueToNull_SetsIsValueSetToTrue() + { + // Arrange + var @switch = new NullableCompatibilitySwitch("TestProperty"); + + // Act + @switch.Value = null; + + // Assert + Assert.Null(@switch.Value); + Assert.True(@switch.IsValueSet); + } + + [Fact] + public void ValueNonInterface_SettingValue_SetsIsValueSetToTrue() + { + // Arrange + var @switch = new NullableCompatibilitySwitch("TestProperty"); + + // Act + @switch.Value = false; + + // Assert + Assert.False(@switch.Value); + Assert.True(@switch.IsValueSet); + } + + [Fact] + public void ValueInterface_SettingValueToNull_SetsIsValueSetToTrue() + { + // Arrange + var @switch = new NullableCompatibilitySwitch("TestProperty"); + + // Act + ((ICompatibilitySwitch)@switch).Value = null; + + // Assert + Assert.Null(@switch.Value); + Assert.True(@switch.IsValueSet); + } + + [Fact] + public void ValueInterface_SettingValue_SetsIsValueSetToTrue() + { + // Arrange + var @switch = new NullableCompatibilitySwitch("TestProperty"); + + // Act + ((ICompatibilitySwitch)@switch).Value = true; + + // Assert + Assert.True(@switch.Value); + Assert.True(@switch.IsValueSet); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs index 6b9b9041db..a666abd913 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs @@ -62,14 +62,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var controller = new TestController(); - var parameterBinder = new ParameterBinder( + + var parameterBinder = GetParameterBinder( modelMetadataProvider, factory, - new DefaultObjectValidator( - modelMetadataProvider, - new[] { GetModelValidatorProvider(mockValidator.Object) }), - _optionsAccessor, - NullLoggerFactory.Instance); + GetModelValidatorProvider(mockValidator.Object)); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -115,18 +112,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal var mockValidator = new Mock(MockBehavior.Strict); mockValidator.Setup(o => o.Validate(It.IsAny())); + + var validatorProvider = GetModelValidatorProvider(mockValidator.Object); var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var controller = new TestController(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, factory, - new DefaultObjectValidator( - modelMetadataProvider, - new[] { GetModelValidatorProvider(mockValidator.Object) }), - _optionsAccessor, - NullLoggerFactory.Instance); + GetModelValidatorProvider(mockValidator.Object)); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -170,14 +165,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); var controllerContext = GetControllerContext(actionDescriptor); var controller = new TestController(); @@ -217,14 +207,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); var controllerContext = GetControllerContext(actionDescriptor); var controller = new TestController(); @@ -272,14 +257,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); var controllerContext = GetControllerContext(actionDescriptor); var controller = new TestController(); @@ -329,14 +309,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal .Setup(p => p.GetMetadataForParameter(ParameterInfos.NoAttributesParameterInfo)) .Returns(modelMetadata.Object); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( mockMetadataProvider.Object, - factory, - new DefaultObjectValidator( - mockMetadataProvider.Object, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -382,14 +357,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal .Setup(p => p.GetMetadataForType(typeof(Person))) .Returns(modelMetadata.Object); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( mockMetadataProvider.Object, - factory, - new DefaultObjectValidator( - mockMetadataProvider.Object, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -431,14 +401,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal .Returns(new[] { new ModelValidationResult("memberName", "some message") }); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, factory, - new DefaultObjectValidator( - modelMetadataProvider, - new[] { GetModelValidatorProvider(mockValidator.Object) }), - _optionsAccessor, - NullLoggerFactory.Instance); + GetModelValidatorProvider(mockValidator.Object)); + var controller = new TestController(); var arguments = new Dictionary(StringComparer.Ordinal); @@ -489,9 +456,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var parameterBinder = new ParameterBinder( modelMetadataProvider, factory, - new DefaultObjectValidator( - modelMetadataProvider, - new[] { GetModelValidatorProvider(mockValidator.Object) }), + GetObjectValidator(modelMetadataProvider, GetModelValidatorProvider(mockValidator.Object)), Options.Create(mvcOptions), NullLoggerFactory.Instance); var controller = new TestController(); @@ -538,14 +503,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory(binder.Object); var controller = new TestController(); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + GetModelValidatorProvider(mockValidator.Object)); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -590,9 +551,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var parameterBinder = new ParameterBinder( modelMetadataProvider, factory, - new DefaultObjectValidator( - modelMetadataProvider, - new[] { GetModelValidatorProvider(mockValidator.Object) }), + GetObjectValidator(modelMetadataProvider, GetModelValidatorProvider(mockValidator.Object)), _optionsAccessor, NullLoggerFactory.Instance); @@ -689,9 +648,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var parameterBinder = new ParameterBinder( modelMetadataProvider, factory, - new DefaultObjectValidator( - modelMetadataProvider, - new[] { GetModelValidatorProvider(mockValidator.Object) }), + GetObjectValidator(modelMetadataProvider, GetModelValidatorProvider(mockValidator.Object)), _optionsAccessor, NullLoggerFactory.Instance); @@ -731,14 +688,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory("Hello"); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -776,14 +728,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var expected = new List { "Hello", "World", "!!" }; var factory = GetModelBinderFactory(expected); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -821,14 +768,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var binder = new StubModelBinder(ModelBindingResult.Success(model: null)); var factory = GetModelBinderFactory(binder); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Some non default value. controller.NonNullableProperty = -1; @@ -867,14 +809,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var binder = new StubModelBinder(ModelBindingResult.Success(model: null)); var factory = GetModelBinderFactory(binder); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Some non default value. controller.NullableProperty = -1; @@ -934,14 +871,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var binder = new StubModelBinder(ModelBindingResult.Success(model: null)); var factory = GetModelBinderFactory(binder); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Some non default value. controller.NullableProperty = -1; @@ -1002,14 +934,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var binder = new StubModelBinder(ModelBindingResult.Success(model: null)); var factory = GetModelBinderFactory(binder); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Some non default value. controller.NullableProperty = -1; @@ -1094,14 +1021,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory(inputValue); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -1174,14 +1096,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal controllerContext.ValueProviderFactories.Add(new SimpleValueProviderFactory()); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( + var parameterBinder = GetParameterBinder( modelMetadataProvider, - factory, - new DefaultObjectValidator( - modelMetadataProvider, - new IModelValidatorProvider[] { }), - _optionsAccessor, - NullLoggerFactory.Instance); + factory); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( @@ -1278,7 +1195,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var parameterBinder = new Mock( new EmptyModelMetadataProvider(), factory, - new DefaultObjectValidator(modelMetadataProvider, new[] { modelValidatorProvider }), + GetObjectValidator(modelMetadataProvider, modelValidatorProvider), _optionsAccessor, NullLoggerFactory.Instance); parameterBinder.Setup(p => p.BindModelAsync( @@ -1415,16 +1332,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal } private static ParameterBinder GetParameterBinder( - IModelBinderFactory factory = null, - IObjectModelValidator validator = null, IModelMetadataProvider modelMetadataProvider = null, + IModelBinderFactory factory = null, IModelValidatorProvider modelValidatorProvider = null) { - if (validator == null) - { - validator = CreateObjectValidator(); - } - if (factory == null) { factory = TestModelBinderFactory.CreateDefault(); @@ -1436,9 +1347,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } var metadataProvider = modelMetadataProvider ?? TestModelMetadataProvider.CreateDefaultProvider(); - var objectModelValidator = new DefaultObjectValidator( - metadataProvider, - new[] { modelValidatorProvider }); + var objectModelValidator = GetObjectValidator(modelMetadataProvider, modelValidatorProvider); return new ParameterBinder( metadataProvider, @@ -1448,16 +1357,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal NullLoggerFactory.Instance); } - private static IObjectModelValidator CreateObjectValidator() + private static DefaultObjectValidator GetObjectValidator( + IModelMetadataProvider modelMetadataProvider, + IModelValidatorProvider validatorProvider) { - var mockValidator = new Mock(MockBehavior.Strict); - mockValidator - .Setup(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())); - return mockValidator.Object; + return new DefaultObjectValidator( + modelMetadataProvider, + new[] { validatorProvider }, + _options); } // No need for bind-related attributes on properties in this controller class. Properties are added directly diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs index a9ee38cfc6..fad6540560 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.IO; using System.Linq; +using System.Reflection; using System.Text; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Infrastructure; @@ -22,7 +23,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public class DefaultObjectValidatorTests { - private IModelMetadataProvider MetadataProvider { get; } = TestModelMetadataProvider.CreateDefaultProvider(); + private readonly MvcOptions _options = new MvcOptions(); + + private ModelMetadataProvider MetadataProvider { get; } = TestModelMetadataProvider.CreateDefaultProvider(); [Fact] public void Validate_SimpleValueType_Valid_WithPrefix() @@ -1258,6 +1261,70 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); } + [Fact] + public void Validate_Throws_IfValidationDepthExceedsMaxDepth() + { + // Arrange + var maxDepth = 5; + var expected = $"ValidationVisitor exceeded the maximum configured validation depth '{maxDepth}' when validating property '{nameof(DepthObject.Depth)}' on type '{typeof(DepthObject)}'. " + + "This may indicate a very deep or infinitely recursive object graph. Consider modifying 'MvcOptions.MaxValidationDepth' or suppressing validation on the model type."; + _options.MaxValidationDepth = maxDepth; + var actionContext = new ActionContext(); + var validator = CreateValidator(); + var model = new DepthObject(maxDepth); + var validationState = new ValidationStateDictionary + { + { model, new ValidationStateEntry() } + }; + + // Act & Assert + var ex = Assert.Throws(() => validator.Validate(actionContext, validationState, prefix: string.Empty, model)); + Assert.Equal(expected, ex.Message); + } + + [Fact] + public void Validate_WorksIfObjectGraphIsSmallerThanMaxDepth() + { + // Arrange + var maxDepth = 5; + _options.MaxValidationDepth = maxDepth; + var actionContext = new ActionContext(); + var validator = CreateValidator(); + var model = new DepthObject(maxDepth - 1); + var validationState = new ValidationStateDictionary + { + { model, new ValidationStateEntry() } + }; + + // Act & Assert + validator.Validate(actionContext, validationState, prefix: string.Empty, model); + Assert.True(actionContext.ModelState.IsValid); + } + + [Fact] + public void Validate_Throws_WithMaxDepth_1() + { + // Arrange + var maxDepth = 1; + var expected = $"ValidationVisitor exceeded the maximum configured validation depth '{maxDepth}' when validating property '{nameof(DepthObject.Depth)}' on type '{typeof(DepthObject)}'. " + + "This may indicate a very deep or infinitely recursive object graph. Consider modifying 'MvcOptions.MaxValidationDepth' or suppressing validation on the model type."; + _options.MaxValidationDepth = maxDepth; + var actionContext = new ActionContext(); + var validator = CreateValidator(); + var model = new DepthObject(maxDepth + 1); + var validationState = new ValidationStateDictionary + { + { model, new ValidationStateEntry() } + }; + var method = GetType().GetMethod(nameof(Validate_Throws_ForTopLeveleMetadataData), BindingFlags.NonPublic | BindingFlags.Instance); + var metadata = MetadataProvider.GetMetadataForParameter(method.GetParameters()[0]); + + // Act & Assert + var ex = Assert.Throws(() => validator.Validate(actionContext, validationState, prefix: string.Empty, model)); + Assert.Equal(expected, ex.Message); + Assert.NotNull(ex.HelpLink); + } + private static DefaultObjectValidator CreateValidator(Type excludedType) { var excludeFilters = new List(); @@ -1268,14 +1335,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(excludeFilters.ToArray()); var validatorProviders = TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders; - return new DefaultObjectValidator(metadataProvider, validatorProviders); + return new DefaultObjectValidator(metadataProvider, validatorProviders, new MvcOptions()); } - private static DefaultObjectValidator CreateValidator(params IMetadataDetailsProvider[] providers) + private DefaultObjectValidator CreateValidator(params IMetadataDetailsProvider[] providers) { var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(providers); var validatorProviders = TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders; - return new DefaultObjectValidator(metadataProvider, validatorProviders); + return new DefaultObjectValidator(metadataProvider, validatorProviders, _options); } private static void AssertKeysEqual(ModelStateDictionary modelState, params string[] keys) @@ -1398,6 +1465,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal void DoSomething(); } + private void Validate_Throws_ForTopLeveleMetadataData(DepthObject depthObject) { } + // Custom validation attribute that returns multiple entries in ValidationResult.MemberNames and those member // names are indexers. An example scenario is an attribute that confirms all entries in a list are unique. private class InvalidItemsAttribute : ValidationAttribute @@ -1442,5 +1511,30 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public string City { get; set; } } + + private class DepthObject + { + public DepthObject(int maxAllowedDepth, int depth = 0) + { + MaxAllowedDepth = maxAllowedDepth; + Depth = depth; + } + + public int Depth { get; } + public int MaxAllowedDepth { get; } + + public DepthObject Instance + { + get + { + if (Depth == MaxAllowedDepth - 1) + { + return this; + } + + return new DepthObject(MaxAllowedDepth, Depth + 1); + } + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs index 062b5fddce..116224e517 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs @@ -84,7 +84,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding metadataProvider, GetModelBinderFactory(binderProviders), valueProvider, - new DefaultObjectValidator(metadataProvider, new[] { validator })); + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions())); // Assert Assert.False(result); @@ -124,7 +124,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding metadataProvider, GetModelBinderFactory(binderProviders), valueProvider, - new DefaultObjectValidator(metadataProvider, new[] { validator })); + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions())); // Assert Assert.True(result); @@ -202,7 +202,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding metadataProvider, GetModelBinderFactory(binderProviders), valueProvider, - new DefaultObjectValidator(metadataProvider, new[] { validator }), + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions()), propertyFilter); // Assert @@ -278,7 +278,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding TestModelMetadataProvider.CreateDefaultProvider(), GetModelBinderFactory(binderProviders), valueProvider, - new DefaultObjectValidator(metadataProvider, new[] { validator }), + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions()), m => m.IncludedProperty, m => m.MyProperty); @@ -329,7 +329,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding metadataProvider, GetModelBinderFactory(binderProviders), valueProvider, - new DefaultObjectValidator(metadataProvider, new[] { validator })); + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions())); // Assert // Includes everything. @@ -531,7 +531,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding metadataProvider, GetModelBinderFactory(binderProviders), valueProvider, - new DefaultObjectValidator(metadataProvider, new[] { validator }), + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions()), propertyFilter); // Assert @@ -599,7 +599,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding metadataProvider, GetModelBinderFactory(binderProviders), valueProvider, - new DefaultObjectValidator(metadataProvider, new[] { validator })); + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions())); // Assert Assert.True(result); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs index d82f37de92..9464e6def7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs @@ -522,7 +522,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Mock.Of(), new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -576,7 +577,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Mock.Of(), new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -630,7 +632,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Mock.Of(), new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -683,7 +686,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Mock.Of(), new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -741,7 +745,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Mock.Of(), new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -802,7 +807,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Mock.Of(), new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -868,7 +874,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Mock.Of(), new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -947,7 +954,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding mockModelBinderFactory.Object, new DefaultObjectValidator( mockModelMetadataProvider.Object, - new[] { GetModelValidatorProvider(validator) }), + new[] { GetModelValidatorProvider(validator) }, + new MvcOptions()), optionsAccessor, loggerFactory ?? NullLoggerFactory.Instance); } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs index 05e12e9f64..138e70f6c8 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs @@ -1,11 +1,13 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.Net; using System.Net.Http; using System.Text; using System.Threading.Tasks; +using FormatterWebSite; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Testing.xunit; using Newtonsoft.Json; @@ -209,7 +211,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange var invalidRequestData = "{\"FirstName\":\"Test\", \"LastName\": \"Testsson\"}"; var content = new StringContent(invalidRequestData, Encoding.UTF8, "application/json"); - var expectedErrorMessage = + var expectedErrorMessage = "{\"LastName\":[\"The field LastName must be a string with a maximum length of 5.\"]}"; // Act @@ -229,7 +231,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Arrange var invalidRequestData = "{\"FirstName\":\"Testname\", \"LastName\": \"\"}"; var content = new StringContent(invalidRequestData, Encoding.UTF8, "application/json"); - var expectedError = + var expectedError = "{\"LastName\":[\"The LastName field is required.\"]," + "\"FirstName\":[\"The field FirstName must be a string with a maximum length of 5.\"]}"; @@ -243,5 +245,22 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var responseContent = await response.Content.ReadAsStringAsync(); Assert.Equal(expectedError, actual: responseContent); } + + // Test for https://github.com/aspnet/Mvc/issues/7357 + [Fact] + public async Task ValidationThrowsError_WhenValidationExceedsMaxValidationDepth() + { + // Arrange + var expected = $"ValidationVisitor exceeded the maximum configured validation depth '32' when validating property 'Value' on type '{typeof(RecursiveIdentifier)}'. " + + "This may indicate a very deep or infinitely recursive object graph. Consider modifying 'MvcOptions.MaxValidationDepth' or suppressing validation on the model type."; + var requestMessage = new HttpRequestMessage(HttpMethod.Post, "Validation/ValidationThrowsError_WhenValidationExceedsMaxValidationDepth") + { + Content = new StringContent(@"{ ""Id"": ""S-1-5-21-1004336348-1177238915-682003330-512"" }", Encoding.UTF8, "application/json"), + }; + + // Act & Assert + var ex = await Assert.ThrowsAsync(() => Client.SendAsync(requestMessage)); + Assert.Equal(expected, ex.Message); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs index b3686104a8..91a290406a 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -9,7 +9,6 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; -using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Routing; @@ -79,7 +78,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests new ModelBinderFactory(metadataProvider, options, serviceProvider), new DefaultObjectValidator( metadataProvider, - new[] { new CompositeModelValidatorProvider(GetModelValidatorProviders(options)) }), + new[] { new CompositeModelValidatorProvider(GetModelValidatorProviders(options)) }, + options.Value), options, NullLoggerFactory.Instance); } @@ -102,7 +102,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests new ModelBinderFactory(metadataProvider, options, services), new DefaultObjectValidator( metadataProvider, - new[] { new CompositeModelValidatorProvider(GetModelValidatorProviders(options)) }), + new[] { new CompositeModelValidatorProvider(GetModelValidatorProviders(options)) }, + options.Value), options, NullLoggerFactory.Instance); } @@ -127,7 +128,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests { return new DefaultObjectValidator( metadataProvider, - GetModelValidatorProviders(options)); + GetModelValidatorProviders(options), + options?.Value ?? new MvcOptions()); } private static IList GetModelValidatorProviders(IOptions options) diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs index 5b7899ac3e..45078ef126 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs @@ -1555,7 +1555,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal return new ParameterBinder( metadataProvider, factory, - new DefaultObjectValidator(metadataProvider, new[] { validator }), + new DefaultObjectValidator(metadataProvider, new[] { validator }, new MvcOptions()), Options.Create(mvcOptions), NullLoggerFactory.Instance); } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs index eed043db10..6c983e1138 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs @@ -553,7 +553,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelBinderFactory, new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -678,7 +679,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelBinderFactory, new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), _optionsAccessor, NullLoggerFactory.Instance); @@ -732,7 +734,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelBinderFactory, new DefaultObjectValidator( modelMetadataProvider, - new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + new[] { TestModelValidatorProvider.CreateDefaultProvider() }, + new MvcOptions()), Options.Create(mvcOptions), NullLoggerFactory.Instance); diff --git a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs index 858dcdf26d..f5a19379d0 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs @@ -40,6 +40,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.False(jsonOptions.AllowInputFormatterExceptionMessages); Assert.False(razorPagesOptions.AllowAreas); Assert.False(mvcOptions.EnableEndpointRouting); + Assert.Null(mvcOptions.MaxValidationDepth); } [Fact] @@ -65,6 +66,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(jsonOptions.AllowInputFormatterExceptionMessages); Assert.True(razorPagesOptions.AllowAreas); Assert.False(mvcOptions.EnableEndpointRouting); + Assert.Null(mvcOptions.MaxValidationDepth); } [Fact] @@ -90,6 +92,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(jsonOptions.AllowInputFormatterExceptionMessages); Assert.True(razorPagesOptions.AllowAreas); Assert.True(mvcOptions.EnableEndpointRouting); + Assert.Equal(32, mvcOptions.MaxValidationDepth); } [Fact] @@ -115,6 +118,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(jsonOptions.AllowInputFormatterExceptionMessages); Assert.True(razorPagesOptions.AllowAreas); Assert.True(mvcOptions.EnableEndpointRouting); + Assert.Equal(32, mvcOptions.MaxValidationDepth); } // This just does the minimum needed to be able to resolve these options. diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs index d88b338f7c..6abab5d7a5 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ControllerTest.cs @@ -458,7 +458,7 @@ namespace Microsoft.AspNetCore.Mvc.Test { ControllerContext = controllerContext, MetadataProvider = metadataProvider, - ObjectValidator = new DefaultObjectValidator(metadataProvider, valiatorProviders), + ObjectValidator = new DefaultObjectValidator(metadataProvider, valiatorProviders, new MvcOptions()), TempData = tempData, ViewData = viewData, }; diff --git a/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs b/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs index 9f2bd1b536..5aee43c803 100644 --- a/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs +++ b/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs @@ -78,5 +78,11 @@ namespace FormatterWebSite return Ok(); } + + [HttpPost] + public IActionResult ValidationThrowsError_WhenValidationExceedsMaxValidationDepth([FromBody] InfinitelyRecursiveModel model) + { + return Ok(); + } } } \ No newline at end of file diff --git a/test/WebSites/FormatterWebSite/Models/InfinitelyRecursiveModel.cs b/test/WebSites/FormatterWebSite/Models/InfinitelyRecursiveModel.cs new file mode 100644 index 0000000000..370151143e --- /dev/null +++ b/test/WebSites/FormatterWebSite/Models/InfinitelyRecursiveModel.cs @@ -0,0 +1,29 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Newtonsoft.Json; + +namespace FormatterWebSite +{ + public class InfinitelyRecursiveModel + { + [JsonConverter(typeof(StringIdentifierConverter))] + public RecursiveIdentifier Id { get; set; } + + private class StringIdentifierConverter : JsonConverter + { + public override bool CanConvert(Type objectType) => objectType == typeof(RecursiveIdentifier); + + public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) + { + return new RecursiveIdentifier(reader.Value.ToString()); + } + + public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) + { + throw new NotImplementedException(); + } + } + } +} diff --git a/test/WebSites/FormatterWebSite/Models/RecursiveIdentifier.cs b/test/WebSites/FormatterWebSite/Models/RecursiveIdentifier.cs new file mode 100644 index 0000000000..847a01b428 --- /dev/null +++ b/test/WebSites/FormatterWebSite/Models/RecursiveIdentifier.cs @@ -0,0 +1,18 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace FormatterWebSite +{ + // A System.Security.Principal.SecurityIdentifier like type that works on xplat + public class RecursiveIdentifier + { + public RecursiveIdentifier(string identifier) + { + Value = identifier; + } + + public string Value { get; } + + public RecursiveIdentifier AccountIdentifier => new RecursiveIdentifier(Value); + } +} \ No newline at end of file diff --git a/test/WebSites/FormatterWebSite/Startup.cs b/test/WebSites/FormatterWebSite/Startup.cs index ee2744a028..c1b5895cea 100644 --- a/test/WebSites/FormatterWebSite/Startup.cs +++ b/test/WebSites/FormatterWebSite/Startup.cs @@ -19,12 +19,12 @@ namespace FormatterWebSite options.InputFormatters.Add(new StringInputFormatter()); }) - .AddXmlDataContractSerializerFormatters(); + .AddXmlDataContractSerializerFormatters() + .SetCompatibilityVersion(CompatibilityVersion.Latest); services.Configure(options => { options.SerializerSettings.Converters.Insert(0, new IModelConverter()); }); } - public void Configure(IApplicationBuilder app) { app.UseMvc(routes =>