Place limits on model binding collection size and recursion depth (#7214)

- #7052
- add MvcOptions.MaxModelBindingCollectionSize` and `MvcOptions.MaxModelBindingRecursionDepth`

nits:
- update syntax of a few `Resources.Designer.cs` files (I ran `/t:resx` on Mvc.sln)
- take VS suggestions in a few test classes

src/Hosting/Hosting/src/Properties/Resources.Designer.cs

@@ -15,64 +15,56 @@ namespace Microsoft.AspNetCore.Hosting
         /// </summary>
         internal static string ErrorPageHtml_Title
         {
-            get { return GetString("ErrorPageHtml_Title"); }
+            get => GetString("ErrorPageHtml_Title");
         }
 
         /// <summary>
         /// Internal Server Error
         /// </summary>
         internal static string FormatErrorPageHtml_Title()
-        {
+            => GetString("ErrorPageHtml_Title");
-            return GetString("ErrorPageHtml_Title");
-        }
 
         /// <summary>
         /// An error occurred while starting the application.
         /// </summary>
         internal static string ErrorPageHtml_UnhandledException
         {
-            get { return GetString("ErrorPageHtml_UnhandledException"); }
+            get => GetString("ErrorPageHtml_UnhandledException");
         }
 
         /// <summary>
         /// An error occurred while starting the application.
         /// </summary>
         internal static string FormatErrorPageHtml_UnhandledException()
-        {
+            => GetString("ErrorPageHtml_UnhandledException");
-            return GetString("ErrorPageHtml_UnhandledException");
-        }
 
         /// <summary>
         /// Unknown location
         /// </summary>
         internal static string ErrorPageHtml_UnknownLocation
         {
-            get { return GetString("ErrorPageHtml_UnknownLocation"); }
+            get => GetString("ErrorPageHtml_UnknownLocation");
         }
 
         /// <summary>
         /// Unknown location
         /// </summary>
         internal static string FormatErrorPageHtml_UnknownLocation()
-        {
+            => GetString("ErrorPageHtml_UnknownLocation");
-            return GetString("ErrorPageHtml_UnknownLocation");
-        }
 
         /// <summary>
         /// WebHostBuilder allows creation only of a single instance of WebHost
         /// </summary>
         internal static string WebHostBuilder_SingleInstance
         {
-            get { return GetString("WebHostBuilder_SingleInstance"); }
+            get => GetString("WebHostBuilder_SingleInstance");
         }
 
         /// <summary>
         /// WebHostBuilder allows creation only of a single instance of WebHost
         /// </summary>
         internal static string FormatWebHostBuilder_SingleInstance()
-        {
+            => GetString("WebHostBuilder_SingleInstance");
-            return GetString("WebHostBuilder_SingleInstance");
-        }
 
         private static string GetString(string name, params string[] formatterNames)
         {

src/Middleware/Localization/src/Properties/Resources.Designer.cs

@@ -1,6 +1,7 @@
 // <auto-generated />
 namespace Microsoft.AspNetCore.Localization
 {
+    using System.Globalization;
     using System.Reflection;
     using System.Resources;
 
@@ -14,9 +15,15 @@ namespace Microsoft.AspNetCore.Localization
         /// </summary>
         internal static string Exception_CulturesShouldNotBeEmpty
         {
-            get { return GetString("Exception_CulturesShouldNotBeEmpty"); }
+            get => GetString("Exception_CulturesShouldNotBeEmpty");
         }
 
+        /// <summary>
+        /// Please provide at least one culture.
+        /// </summary>
+        internal static string FormatException_CulturesShouldNotBeEmpty()
+            => GetString("Exception_CulturesShouldNotBeEmpty");
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);

src/Middleware/Session/src/Properties/Resources.Designer.cs

@@ -15,96 +15,84 @@ namespace Microsoft.AspNetCore.Session
         /// </summary>
         internal static string Exception_KeyLengthIsExceeded
         {
-            get { return GetString("Exception_KeyLengthIsExceeded"); }
+            get => GetString("Exception_KeyLengthIsExceeded");
         }
 
         /// <summary>
         /// The key cannot be longer than '{0}' when encoded with UTF-8.
         /// </summary>
         internal static string FormatException_KeyLengthIsExceeded(object p0)
-        {
+            => string.Format(CultureInfo.CurrentCulture, GetString("Exception_KeyLengthIsExceeded"), p0);
-            return string.Format(CultureInfo.CurrentCulture, GetString("Exception_KeyLengthIsExceeded"), p0);
-        }
 
         /// <summary>
         /// The session cannot be established after the response has started.
         /// </summary>
         internal static string Exception_InvalidSessionEstablishment
         {
-            get { return GetString("Exception_InvalidSessionEstablishment"); }
+            get => GetString("Exception_InvalidSessionEstablishment");
         }
 
         /// <summary>
         /// The session cannot be established after the response has started.
         /// </summary>
         internal static string FormatException_InvalidSessionEstablishment()
-        {
+            => GetString("Exception_InvalidSessionEstablishment");
-            return GetString("Exception_InvalidSessionEstablishment");
-        }
 
         /// <summary>
         /// The value cannot be serialized in two bytes.
         /// </summary>
         internal static string Exception_InvalidToSerializeIn2Bytes
         {
-            get { return GetString("Exception_InvalidToSerializeIn2Bytes"); }
+            get => GetString("Exception_InvalidToSerializeIn2Bytes");
         }
 
         /// <summary>
         /// The value cannot be serialized in two bytes.
         /// </summary>
         internal static string FormatException_InvalidToSerializeIn2Bytes()
-        {
+            => GetString("Exception_InvalidToSerializeIn2Bytes");
-            return GetString("Exception_InvalidToSerializeIn2Bytes");
-        }
 
         /// <summary>
         /// The value cannot be serialized in three bytes.
         /// </summary>
         internal static string Exception_InvalidToSerializeIn3Bytes
         {
-            get { return GetString("Exception_InvalidToSerializeIn3Bytes"); }
+            get => GetString("Exception_InvalidToSerializeIn3Bytes");
         }
 
         /// <summary>
         /// The value cannot be serialized in three bytes.
         /// </summary>
         internal static string FormatException_InvalidToSerializeIn3Bytes()
-        {
+            => GetString("Exception_InvalidToSerializeIn3Bytes");
-            return GetString("Exception_InvalidToSerializeIn3Bytes");
-        }
 
         /// <summary>
         /// The value cannot be negative.
         /// </summary>
         internal static string Exception_NumberShouldNotBeNegative
         {
-            get { return GetString("Exception_NumberShouldNotBeNegative"); }
+            get => GetString("Exception_NumberShouldNotBeNegative");
         }
 
         /// <summary>
         /// The value cannot be negative.
         /// </summary>
         internal static string FormatException_NumberShouldNotBeNegative()
-        {
+            => GetString("Exception_NumberShouldNotBeNegative");
-            return GetString("Exception_NumberShouldNotBeNegative");
-        }
 
         /// <summary>
         /// Argument cannot be null or empty string.
         /// </summary>
         internal static string ArgumentCannotBeNullOrEmpty
         {
-            get { return GetString("ArgumentCannotBeNullOrEmpty"); }
+            get => GetString("ArgumentCannotBeNullOrEmpty");
         }
 
         /// <summary>
         /// Argument cannot be null or empty string.
         /// </summary>
         internal static string FormatArgumentCannotBeNullOrEmpty()
-        {
+            => GetString("ArgumentCannotBeNullOrEmpty");
-            return GetString("ArgumentCannotBeNullOrEmpty");
-        }
 
         private static string GetString(string name, params string[] formatterNames)
         {

src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs

@@ -191,8 +191,6 @@ namespace Microsoft.AspNetCore.Mvc.Description
             var action = CreateActionDescriptor(nameof(FromRouting));
             action.AttributeRouteInfo = new AttributeRouteInfo { Template = template };
 
-            var parameterDescriptor = action.Parameters[0];
-
             // Act
             var descriptions = GetApiDescriptions(action);
 
@@ -1511,7 +1509,6 @@ namespace Microsoft.AspNetCore.Mvc.Description
         {
             // Arrange
             var action = CreateActionDescriptor(nameof(AcceptsHasCollection));
-            var parameterDescriptor = action.Parameters.Single();
 
             // Act
             var descriptions = GetApiDescriptions(action);
@@ -1531,7 +1528,6 @@ namespace Microsoft.AspNetCore.Mvc.Description
         {
             // Arrange
             var action = CreateActionDescriptor(nameof(AcceptsHasCollection_Complex));
-            var parameterDescriptor = action.Parameters.Single();
 
             // Act
             var descriptions = GetApiDescriptions(action);

src/Mvc/Mvc.Core/src/ModelBinding/Binders/ArrayModelBinder.cs

@@ -52,6 +52,36 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         {
         }
 
+        /// <summary>
+        /// Creates a new <see cref="ArrayModelBinder{TElement}"/>.
+        /// </summary>
+        /// <param name="elementBinder">
+        /// The <see cref="IModelBinder"/> for binding <typeparamref name="TElement"/>.
+        /// </param>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        /// <param name="allowValidatingTopLevelNodes">
+        /// Indication that validation of top-level models is enabled. If <see langword="true"/> and
+        /// <see cref="ModelMetadata.IsBindingRequired"/> is <see langword="true"/> for a top-level model, the binder
+        /// adds a <see cref="ModelStateDictionary"/> error when the model is not bound.
+        /// </param>
+        /// <param name="mvcOptions">The <see cref="MvcOptions"/>.</param>
+        /// <remarks>
+        /// <para>This is the preferred <see cref="ArrayModelBinder{TElement}"/> constructor.</para>
+        /// <para>
+        /// The <paramref name="allowValidatingTopLevelNodes"/> parameter is currently ignored.
+        /// <see cref="CollectionModelBinder{TElement}.AllowValidatingTopLevelNodes"/> is always <see langword="true"/>
+        /// in <see cref="ArrayModelBinder{TElement}"/>.
+        /// </para>
+        /// </remarks>
+        public ArrayModelBinder(
+            IModelBinder elementBinder,
+            ILoggerFactory loggerFactory,
+            bool allowValidatingTopLevelNodes,
+            MvcOptions mvcOptions)
+            : base(elementBinder, loggerFactory, allowValidatingTopLevelNodes: true, mvcOptions)
+        {
+        }
+
         /// <inheritdoc />
         public override bool CanCreateInstance(Type targetType)
         {

src/Mvc/Mvc.Core/src/ModelBinding/Binders/ArrayModelBinderProvider.cs

@@ -4,6 +4,7 @@
 using System;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
 {
@@ -23,15 +24,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             if (context.Metadata.ModelType.IsArray)
             {
                 var elementType = context.Metadata.ElementMetadata.ModelType;
+                var binderType = typeof(ArrayModelBinder<>).MakeGenericType(elementType);
                 var elementBinder = context.CreateBinder(context.Metadata.ElementMetadata);
 
-                var binderType = typeof(ArrayModelBinder<>).MakeGenericType(elementType);
                 var loggerFactory = context.Services.GetRequiredService<ILoggerFactory>();
+                var mvcOptions = context.Services.GetRequiredService<IOptions<MvcOptions>>().Value;
                 return (IModelBinder)Activator.CreateInstance(
                     binderType,
                     elementBinder,
                     loggerFactory,
-                    true /* allowValidatingTopLevelNodes */);
+                    true /* allowValidatingTopLevelNodes */,
+                    mvcOptions);
             }
 
             return null;

src/Mvc/Mvc.Core/src/ModelBinding/Binders/CollectionModelBinder.cs

@@ -9,6 +9,8 @@ using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Mvc.Core;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
 using Microsoft.Extensions.Logging;
 
@@ -21,12 +23,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
     public class CollectionModelBinder<TElement> : ICollectionModelBinder
     {
         private static readonly IValueProvider EmptyValueProvider = new CompositeValueProvider();
+        private readonly int _maxModelBindingCollectionSize = MvcOptions.DefaultMaxModelBindingCollectionSize;
         private Func<object> _modelCreator;
 
         /// <summary>
         /// Creates a new <see cref="CollectionModelBinder{TElement}"/>.
         /// </summary>
-        /// <param name="elementBinder">The <see cref="IModelBinder"/> for binding elements.</param>
+        /// <param name="elementBinder">
+        /// The <see cref="IModelBinder"/> for binding <typeparamref name="TElement"/>.
+        /// </param>
         /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
         public CollectionModelBinder(IModelBinder elementBinder, ILoggerFactory loggerFactory)
             : this(elementBinder, loggerFactory, allowValidatingTopLevelNodes: true)
@@ -36,7 +41,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         /// <summary>
         /// Creates a new <see cref="CollectionModelBinder{TElement}"/>.
         /// </summary>
-        /// <param name="elementBinder">The <see cref="IModelBinder"/> for binding elements.</param>
+        /// <param name="elementBinder">
+        /// The <see cref="IModelBinder"/> for binding <typeparamref name="TElement"/>.
+        /// </param>
         /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
         /// <param name="allowValidatingTopLevelNodes">
         /// Indication that validation of top-level models is enabled. If <see langword="true"/> and
@@ -63,6 +70,35 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             AllowValidatingTopLevelNodes = allowValidatingTopLevelNodes;
         }
 
+        /// <summary>
+        /// Creates a new <see cref="CollectionModelBinder{TElement}"/>.
+        /// </summary>
+        /// <param name="elementBinder">
+        /// The <see cref="IModelBinder"/> for binding <typeparamref name="TElement"/>.
+        /// </param>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        /// <param name="allowValidatingTopLevelNodes">
+        /// Indication that validation of top-level models is enabled. If <see langword="true"/> and
+        /// <see cref="ModelMetadata.IsBindingRequired"/> is <see langword="true"/> for a top-level model, the binder
+        /// adds a <see cref="ModelStateDictionary"/> error when the model is not bound.
+        /// </param>
+        /// <param name="mvcOptions">The <see cref="MvcOptions"/>.</param>
+        /// <remarks>This is the preferred <see cref="CollectionModelBinder{TElement}"/> constructor.</remarks>
+        public CollectionModelBinder(
+            IModelBinder elementBinder,
+            ILoggerFactory loggerFactory,
+            bool allowValidatingTopLevelNodes,
+            MvcOptions mvcOptions)
+            : this(elementBinder, loggerFactory, allowValidatingTopLevelNodes)
+        {
+            if (mvcOptions == null)
+            {
+                throw new ArgumentNullException(nameof(mvcOptions));
+            }
+
+            _maxModelBindingCollectionSize = mvcOptions.MaxModelBindingCollectionSize;
+        }
+
         // Internal for testing.
         internal bool AllowValidatingTopLevelNodes { get; }
 
@@ -305,8 +341,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             else
             {
                 indexNamesIsFinite = false;
-                indexNames = Enumerable.Range(0, int.MaxValue)
+                var limit = _maxModelBindingCollectionSize == int.MaxValue ?
-                                       .Select(i => i.ToString(CultureInfo.InvariantCulture));
+                    int.MaxValue :
+                    _maxModelBindingCollectionSize + 1;
+                indexNames = Enumerable
+                    .Range(0, limit)
+                    .Select(i => i.ToString(CultureInfo.InvariantCulture));
             }
 
             var elementMetadata = bindingContext.ModelMetadata.ElementMetadata;
@@ -345,6 +385,25 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 boundCollection.Add(ModelBindingHelper.CastOrDefault<TElement>(boundValue));
             }
 
+            // Did the collection grow larger than the limit?
+            if (boundCollection.Count > _maxModelBindingCollectionSize)
+            {
+                // Look for a non-empty name. Both ModelName and OriginalModelName may be empty at the top level.
+                var name = string.IsNullOrEmpty(bindingContext.ModelName) ?
+                    (string.IsNullOrEmpty(bindingContext.OriginalModelName) &&
+                        bindingContext.ModelMetadata.MetadataKind != ModelMetadataKind.Type ?
+                        bindingContext.ModelMetadata.Name :
+                        bindingContext.OriginalModelName) : // This name may unfortunately be empty.
+                    bindingContext.ModelName;
+
+                throw new InvalidOperationException(Resources.FormatModelBinding_ExceededMaxModelBindingCollectionSize(
+                    name,
+                    nameof(MvcOptions),
+                    nameof(MvcOptions.MaxModelBindingCollectionSize),
+                    _maxModelBindingCollectionSize,
+                    bindingContext.ModelMetadata.ElementType));
+            }
+
             return new CollectionResult
             {
                 Model = boundCollection,

src/Mvc/Mvc.Core/src/ModelBinding/Binders/CollectionModelBinderProvider.cs

@@ -32,22 +32,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 return null;
             }
 
-            var loggerFactory = context.Services.GetRequiredService<ILoggerFactory>();
-            var mvcOptions = context.Services.GetRequiredService<IOptions<MvcOptions>>().Value;
-
             // If the model type is ICollection<> then we can call its Add method, so we can always support it.
             var collectionType = ClosedGenericMatcher.ExtractGenericInterface(modelType, typeof(ICollection<>));
             if (collectionType != null)
             {
-                var elementType = collectionType.GenericTypeArguments[0];
+                return CreateInstance(context, collectionType);
-                var elementBinder = context.CreateBinder(context.MetadataProvider.GetMetadataForType(elementType));
-
-                var binderType = typeof(CollectionModelBinder<>).MakeGenericType(collectionType.GenericTypeArguments);
-                return (IModelBinder)Activator.CreateInstance(
-                    binderType,
-                    elementBinder,
-                    loggerFactory,
-                    true /* allowValidatingTopLevelNodes */);
             }
 
             // If the model type is IEnumerable<> then we need to know if we can assign a List<> to it, since
@@ -59,19 +48,29 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 var listType = typeof(List<>).MakeGenericType(enumerableType.GenericTypeArguments);
                 if (modelType.GetTypeInfo().IsAssignableFrom(listType.GetTypeInfo()))
                 {
-                    var elementType = enumerableType.GenericTypeArguments[0];
+                    return CreateInstance(context, listType);
-                    var elementBinder = context.CreateBinder(context.MetadataProvider.GetMetadataForType(elementType));
-
-                    var binderType = typeof(CollectionModelBinder<>).MakeGenericType(enumerableType.GenericTypeArguments);
-                    return (IModelBinder)Activator.CreateInstance(
-                        binderType,
-                        elementBinder,
-                        loggerFactory,
-                        true /* allowValidatingTopLevelNodes */);
                 }
             }
 
             return null;
         }
+
+        private static IModelBinder CreateInstance(ModelBinderProviderContext context, Type collectionType)
+        {
+            var binderType = typeof(CollectionModelBinder<>).MakeGenericType(collectionType.GenericTypeArguments);
+            var elementType = collectionType.GenericTypeArguments[0];
+            var elementBinder = context.CreateBinder(context.MetadataProvider.GetMetadataForType(elementType));
+
+            var loggerFactory = context.Services.GetRequiredService<ILoggerFactory>();
+            var mvcOptions = context.Services.GetRequiredService<IOptions<MvcOptions>>().Value;
+            var binder = (IModelBinder)Activator.CreateInstance(
+                binderType,
+                elementBinder,
+                loggerFactory,
+                true /* allowValidatingTopLevelNodes */,
+                mvcOptions);
+
+            return binder;
+        }
     }
 }

src/Mvc/Mvc.Core/src/ModelBinding/Binders/DictionaryModelBinder.cs

@@ -74,6 +74,48 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             _valueBinder = valueBinder;
         }
 
+        /// <summary>
+        /// Creates a new <see cref="DictionaryModelBinder{TKey, TValue}"/>.
+        /// </summary>
+        /// <param name="keyBinder">The <see cref="IModelBinder"/> for <typeparamref name="TKey"/>.</param>
+        /// <param name="valueBinder">The <see cref="IModelBinder"/> for <typeparamref name="TValue"/>.</param>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        /// <param name="allowValidatingTopLevelNodes">
+        /// Indication that validation of top-level models is enabled. If <see langword="true"/> and
+        /// <see cref="ModelMetadata.IsBindingRequired"/> is <see langword="true"/> for a top-level model, the binder
+        /// adds a <see cref="ModelStateDictionary"/> error when the model is not bound.
+        /// </param>
+        /// <param name="mvcOptions">The <see cref="MvcOptions"/>.</param>
+        /// <remarks>
+        /// <para>This is the preferred <see cref="DictionaryModelBinder{TKey, TValue}"/> constructor.</para>
+        /// <para>
+        /// The <paramref name="allowValidatingTopLevelNodes"/> parameter is currently ignored.
+        /// <see cref="CollectionModelBinder{TElement}.AllowValidatingTopLevelNodes"/> is always
+        /// <see langword="false"/> in <see cref="DictionaryModelBinder{TKey, TValue}"/>. This class ignores that
+        /// property and unconditionally checks for unbound top-level models with
+        /// <see cref="ModelMetadata.IsBindingRequired"/>.
+        /// </para>
+        /// </remarks>
+        public DictionaryModelBinder(
+            IModelBinder keyBinder,
+            IModelBinder valueBinder,
+            ILoggerFactory loggerFactory,
+            bool allowValidatingTopLevelNodes,
+            MvcOptions mvcOptions)
+            : base(
+                  new KeyValuePairModelBinder<TKey, TValue>(keyBinder, valueBinder, loggerFactory),
+                  loggerFactory,
+                  allowValidatingTopLevelNodes: false,
+                  mvcOptions)
+        {
+            if (valueBinder == null)
+            {
+                throw new ArgumentNullException(nameof(valueBinder));
+            }
+
+            _valueBinder = valueBinder;
+        }
+
         /// <inheritdoc />
         public override async Task BindModelAsync(ModelBindingContext bindingContext)
         {

src/Mvc/Mvc.Core/src/ModelBinding/Binders/DictionaryModelBinderProvider.cs

@@ -27,13 +27,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             var dictionaryType = ClosedGenericMatcher.ExtractGenericInterface(modelType, typeof(IDictionary<,>));
             if (dictionaryType != null)
             {
+                var binderType = typeof(DictionaryModelBinder<,>).MakeGenericType(dictionaryType.GenericTypeArguments);
+
                 var keyType = dictionaryType.GenericTypeArguments[0];
                 var keyBinder = context.CreateBinder(context.MetadataProvider.GetMetadataForType(keyType));
 
                 var valueType = dictionaryType.GenericTypeArguments[1];
                 var valueBinder = context.CreateBinder(context.MetadataProvider.GetMetadataForType(valueType));
 
-                var binderType = typeof(DictionaryModelBinder<,>).MakeGenericType(dictionaryType.GenericTypeArguments);
                 var loggerFactory = context.Services.GetRequiredService<ILoggerFactory>();
                 var mvcOptions = context.Services.GetRequiredService<IOptions<MvcOptions>>().Value;
                 return (IModelBinder)Activator.CreateInstance(
@@ -41,7 +42,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                     keyBinder,
                     valueBinder,
                     loggerFactory,
-                    true /* allowValidatingTopLevelNodes */);
+                    true /* allowValidatingTopLevelNodes */,
+                    mvcOptions);
             }
 
             return null;

src/Mvc/Mvc.Core/src/ModelBinding/DefaultModelBindingContext.cs

@@ -3,7 +3,10 @@
 
 using System;
 using System.Collections.Generic;
+using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Mvc.ModelBinding
 {
@@ -18,6 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         private ActionContext _actionContext;
         private ModelStateDictionary _modelState;
         private ValidationStateDictionary _validationState;
+        private int? _maxModelBindingRecursionDepth;
 
         private State _state;
         private readonly Stack<State> _stack = new Stack<State>();
@@ -184,6 +188,25 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
         }
 
+        private int MaxModelBindingRecursionDepth
+        {
+            get
+            {
+                if (!_maxModelBindingRecursionDepth.HasValue)
+                {
+                    // Ignore incomplete initialization. This must be a test scenario because CreateBindingContext(...)
+                    // has not been called or was called without MvcOptions in the service provider.
+                    _maxModelBindingRecursionDepth = MvcOptions.DefaultMaxModelBindingRecursionDepth;
+                }
+
+                return _maxModelBindingRecursionDepth.Value;
+            }
+            set
+            {
+                _maxModelBindingRecursionDepth = value;
+            }
+        }
+
         /// <summary>
         /// Creates a new <see cref="DefaultModelBindingContext"/> for top-level model binding operation.
         /// </summary>
@@ -223,16 +246,16 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
 
             var binderModelName = bindingInfo?.BinderModelName ?? metadata.BinderModelName;
+            var bindingSource = bindingInfo?.BindingSource ?? metadata.BindingSource;
             var propertyFilterProvider = bindingInfo?.PropertyFilterProvider ?? metadata.PropertyFilterProvider;
 
-            var bindingSource = bindingInfo?.BindingSource ?? metadata.BindingSource;
+            var bindingContext = new DefaultModelBindingContext()
-
-            return new DefaultModelBindingContext()
             {
                 ActionContext = actionContext,
                 BinderModelName = binderModelName,
                 BindingSource = bindingSource,
                 PropertyFilter = propertyFilterProvider?.PropertyFilter,
+                ValidationState = new ValidationStateDictionary(),
 
                 // Because this is the top-level context, FieldName and ModelName should be the same.
                 FieldName = binderModelName ?? modelName,
@@ -245,9 +268,16 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
 
                 OriginalValueProvider = valueProvider,
                 ValueProvider = FilterValueProvider(valueProvider, bindingSource),
-
-                ValidationState = new ValidationStateDictionary(),
             };
+
+            // mvcOptions may be null when this method is called in test scenarios.
+            var mvcOptions = actionContext.HttpContext.RequestServices?.GetService<IOptions<MvcOptions>>();
+            if (mvcOptions != null)
+            {
+                bindingContext.MaxModelBindingRecursionDepth = mvcOptions.Value.MaxModelBindingRecursionDepth;
+            }
+
+            return bindingContext;
         }
 
         /// <inheritdoc />
@@ -299,6 +329,21 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         {
             _stack.Push(_state);
 
+            // Would this new scope (which isn't in _stack) exceed the allowed recursion depth? That is, has the model
+            // binding system already nested MaxModelBindingRecursionDepth binders?
+            if (_stack.Count >= MaxModelBindingRecursionDepth)
+            {
+                // Find the root of this deeply-nested model.
+                var states = _stack.ToArray();
+                var rootModelType = states[states.Length - 1].ModelMetadata.ModelType;
+
+                throw new InvalidOperationException(Resources.FormatModelBinding_ExceededMaxModelBindingRecursionDepth(
+                    nameof(MvcOptions),
+                    nameof(MvcOptions.MaxModelBindingRecursionDepth),
+                    MaxModelBindingRecursionDepth,
+                    rootModelType));
+            }
+
             Result = default;
 
             return new NestedScope(this);

src/Mvc/Mvc.Core/src/MvcOptions.cs

@@ -4,15 +4,16 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
 using Microsoft.AspNetCore.Mvc.Controllers;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Formatters;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
+using Microsoft.AspNetCore.WebUtilities;
 
 namespace Microsoft.AspNetCore.Mvc
 {
@@ -21,8 +22,14 @@ namespace Microsoft.AspNetCore.Mvc
     /// </summary>
     public class MvcOptions : IEnumerable<ICompatibilitySwitch>
     {
+        internal const int DefaultMaxModelBindingCollectionSize = FormReader.DefaultValueCountLimit;
+        internal const int DefaultMaxModelBindingRecursionDepth = 32;
+
         private readonly IReadOnlyList<ICompatibilitySwitch> _switches = Array.Empty<ICompatibilitySwitch>();
+
         private int _maxModelStateErrors = ModelStateDictionary.DefaultMaxAllowedErrors;
+        private int _maxModelBindingCollectionSize = DefaultMaxModelBindingCollectionSize;
+        private int _maxModelBindingRecursionDepth = DefaultMaxModelBindingRecursionDepth;
         private int? _maxValidationDepth = 32;
 
         /// <summary>
@@ -231,6 +238,85 @@ namespace Microsoft.AspNetCore.Mvc
         /// </value>
         public bool SuppressAsyncSuffixInActionNames { get; set; } = true;
 
+        /// <summary>
+        /// Gets or sets the maximum size of a complex collection to model bind. When this limit is reached, the model
+        /// binding system will throw an <see cref="InvalidOperationException"/>.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// When binding a collection, some element binders may succeed unconditionally and model binding may run out
+        /// of memory. This limit constrains such unbounded collection growth; it is a safeguard against incorrect
+        /// model binders and models.
+        /// </para>
+        /// <para>
+        /// This limit does not <em>correct</em> the bound model. The <see cref="InvalidOperationException"/> instead
+        /// informs the developer of an issue in their model or model binder. The developer must correct that issue.
+        /// </para>
+        /// <para>
+        /// This limit does not apply to collections of simple types. When
+        /// <see cref="CollectionModelBinder{TElement}"/> relies entirely on <see cref="IValueProvider"/>s, it cannot
+        /// create collections larger than the available data.
+        /// </para>
+        /// <para>
+        /// A very high value for this option (<c>int.MaxValue</c> for example) effectively removes the limit and is
+        /// not recommended.
+        /// </para>
+        /// </remarks>
+        /// <value>The default value is <c>1024</c>, matching <see cref="FormReader.DefaultValueCountLimit"/>.</value>
+        public int MaxModelBindingCollectionSize
+        {
+            get => _maxModelBindingCollectionSize;
+            set
+            {
+                // Disallowing an empty collection would cause the CollectionModelBinder to throw unconditionally.
+                if (value <= 0)
+                {
+                    throw new ArgumentOutOfRangeException(nameof(value));
+                }
+
+                _maxModelBindingCollectionSize = value;
+            }
+        }
+
+        /// <summary>
+        /// Gets or sets the maximum recursion depth of the model binding system. The
+        /// <see cref="DefaultModelBindingContext"/> will throw an <see cref="InvalidOperationException"/> if more than
+        /// this number of <see cref="IModelBinder"/>s are on the stack. That is, an attempt to recurse beyond this
+        /// level will fail.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// For some self-referential models, some binders may succeed unconditionally and model binding may result in
+        /// stack overflow. This limit constrains such unbounded recursion; it is a safeguard against incorrect model
+        /// binders and models. This limit also protects against very deep model type hierarchies lacking
+        /// self-references.
+        /// </para>
+        /// <para>
+        /// This limit does not <em>correct</em> the bound model. The <see cref="InvalidOperationException"/> instead
+        /// informs the developer of an issue in their model. The developer must correct that issue.
+        /// </para>
+        /// <para>
+        /// A very high value for this option (<c>int.MaxValue</c> for example) effectively removes the limit and is
+        /// not recommended.
+        /// </para>
+        /// </remarks>
+        /// <value>The default value is <c>32</c>, matching the default <see cref="MaxValidationDepth"/> value.</value>
+        public int MaxModelBindingRecursionDepth
+        {
+            get => _maxModelBindingRecursionDepth;
+            set
+            {
+                // Disallowing one model binder (if supported) would cause the model binding system to throw
+                // unconditionally. DefaultModelBindingContext always allows a top-level binder i.e. its own creation.
+                if (value <= 1)
+                {
+                    throw new ArgumentOutOfRangeException(nameof(value));
+                }
+
+                _maxModelBindingRecursionDepth = value;
+            }
+        }
+
         IEnumerator<ICompatibilitySwitch> IEnumerable<ICompatibilitySwitch>.GetEnumerator() => _switches.GetEnumerator();
 
         IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator();

src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs

@@ -1704,6 +1704,34 @@ namespace Microsoft.AspNetCore.Mvc.Core
         internal static string FormatReferenceToNewtonsoftJsonRequired(object p0, object p1, object p2, object p3, object p4)
             => string.Format(CultureInfo.CurrentCulture, GetString("ReferenceToNewtonsoftJsonRequired"), p0, p1, p2, p3, p4);
 
+        /// <summary>
+        /// Collection bound to '{0}' exceeded {1}.{2} ({3}). This limit is a safeguard against incorrect model binders and models. Address issues in '{4}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information.
+        /// </summary>
+        internal static string ModelBinding_ExceededMaxModelBindingCollectionSize
+        {
+            get => GetString("ModelBinding_ExceededMaxModelBindingCollectionSize");
+        }
+
+        /// <summary>
+        /// Collection bound to '{0}' exceeded {1}.{2} ({3}). This limit is a safeguard against incorrect model binders and models. Address issues in '{4}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information.
+        /// </summary>
+        internal static string FormatModelBinding_ExceededMaxModelBindingCollectionSize(object p0, object p1, object p2, object p3, object p4)
+            => string.Format(CultureInfo.CurrentCulture, GetString("ModelBinding_ExceededMaxModelBindingCollectionSize"), p0, p1, p2, p3, p4);
+
+        /// <summary>
+        /// Model binding system exceeded {0}.{1} ({2}). Reduce the potential nesting of '{3}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information.
+        /// </summary>
+        internal static string ModelBinding_ExceededMaxModelBindingRecursionDepth
+        {
+            get => GetString("ModelBinding_ExceededMaxModelBindingRecursionDepth");
+        }
+
+        /// <summary>
+        /// Model binding system exceeded {0}.{1} ({2}). Reduce the potential nesting of '{3}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information.
+        /// </summary>
+        internal static string FormatModelBinding_ExceededMaxModelBindingRecursionDepth(object p0, object p1, object p2, object p3)
+            => string.Format(CultureInfo.CurrentCulture, GetString("ModelBinding_ExceededMaxModelBindingRecursionDepth"), p0, p1, p2, p3);
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);

src/Mvc/Mvc.Core/src/Resources.resx

@@ -493,4 +493,12 @@
   <data name="ReferenceToNewtonsoftJsonRequired" xml:space="preserve">
     <value>'{0}' requires a reference to '{1}'. Configure your application by adding a reference to the '{1}' package and calling '{2}.{3}' inside the call to '{4}' in the application startup code.</value>
   </data>
+  <data name="ModelBinding_ExceededMaxModelBindingCollectionSize" xml:space="preserve">
+    <value>Collection bound to '{0}' exceeded {1}.{2} ({3}). This limit is a safeguard against incorrect model binders and models. Address issues in '{4}'. For example, this type may have a property with a model binder that always succeeds. See the {1}.{2} documentation for more information.</value>
+    <comment>{0} is model name, {1} is MvcOptions, {2} is MaxModelBindingCollectionSize, {3} is option value, {4} is affected type.</comment>
+  </data>
+  <data name="ModelBinding_ExceededMaxModelBindingRecursionDepth" xml:space="preserve">
+    <value>Model binding system exceeded {0}.{1} ({2}). Reduce the potential nesting of '{3}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information.</value>
+    <comment>{0} is MvcOptions, {1} is MaxModelBindingRecursionDepth, {2} is option value, {3} is (loopy or deeply nested) top-level model type.</comment>
+  </data>
 </root>

src/Mvc/test/Mvc.IntegrationTests/ArrayModelBinderIntegrationTest.cs

@@ -1,6 +1,7 @@
 // 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.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Abstractions;
@@ -369,5 +370,38 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
                 (e) => Assert.Equal("Alias1", e),
                 (e) => Assert.Equal("Alias2", e));
         }
+
+        [Fact]
+        public async Task ArrayModelBinder_ThrowsOn1025Items_AtTopLevel()
+        {
+            // Arrange
+            var expectedMessage = $"Collection bound to 'parameter' exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} (1024). This limit is a " +
+                $"safeguard against incorrect model binders and models. Address issues in " +
+                $"'{typeof(SuccessfulModel)}'. For example, this type may have a property with a model binder that " +
+                $"always succeeds. See the {nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} " +
+                $"documentation for more information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(SuccessfulModel[]),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(request =>
+            {
+                // CollectionModelBinder binds an empty collection when value providers are all empty.
+                request.QueryString = new QueryString("?a=b");
+            });
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
     }
 }

src/Mvc/test/Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs

@@ -968,6 +968,164 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
             Assert.Equal(0, modelState.ErrorCount);
         }
 
+        // Regression test for #7052
+        [Fact]
+        public async Task CollectionModelBinder_ThrowsOn1025Items_AtTopLevel()
+        {
+            // Arrange
+            var expectedMessage = $"Collection bound to 'parameter' exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} (1024). This limit is a " +
+                $"safeguard against incorrect model binders and models. Address issues in " +
+                $"'{typeof(SuccessfulModel)}'. For example, this type may have a property with a model binder that " +
+                $"always succeeds. See the {nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} " +
+                $"documentation for more information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(IList<SuccessfulModel>),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(request =>
+            {
+                // CollectionModelBinder binds an empty collection when value providers are all empty.
+                request.QueryString = new QueryString("?a=b");
+            });
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
+        // Ensure CollectionModelBinder allows MaxModelBindingCollectionSize items.
+        [Fact]
+        public async Task CollectionModelBinder_Binds3Items_WithIndices()
+        {
+            // Arrange
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(IList<SuccessfulModel>),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(
+                request =>
+                {
+                    request.QueryString = new QueryString("?Index=0&Index=1&Index=2");
+                },
+                options => options.MaxModelBindingCollectionSize = 3);
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act
+            var result = await parameterBinder.BindModelAsync(parameter, testContext);
+
+            // Assert
+            Assert.True(modelState.IsValid);
+            Assert.Equal(0, modelState.ErrorCount);
+
+            Assert.True(result.IsModelSet);
+            var locations = Assert.IsType<List<SuccessfulModel>>(result.Model);
+            Assert.Collection(
+                locations,
+                item =>
+                {
+                    Assert.True(item.IsBound);
+                    Assert.Null(item.Name);
+                },
+                item =>
+                {
+                    Assert.True(item.IsBound);
+                    Assert.Null(item.Name);
+                },
+                item =>
+                {
+                    Assert.True(item.IsBound);
+                    Assert.Null(item.Name);
+                });
+        }
+
+        // Ensure CollectionModelBinder disallows one more than MaxModelBindingCollectionSize items.
+        [Fact]
+        public async Task CollectionModelBinder_ThrowsOn4Items_WithIndices()
+        {
+            // Arrange
+            var expectedMessage = $"Collection bound to 'parameter' exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} (3). This limit is a " +
+                $"safeguard against incorrect model binders and models. Address issues in " +
+                $"'{typeof(SuccessfulModel)}'. For example, this type may have a property with a model binder that " +
+                $"always succeeds. See the {nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} " +
+                $"documentation for more information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(IList<SuccessfulModel>),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(
+                request =>
+                {
+                    request.QueryString = new QueryString("?Index=0&Index=1&Index=2&Index=3");
+                },
+                options => options.MaxModelBindingCollectionSize = 3);
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
+        private class SuccessfulContainer
+        {
+            public IList<SuccessfulModel> Successes { get; set; }
+        }
+
+        [Fact]
+        public async Task CollectionModelBinder_ThrowsOn1025Items()
+        {
+            // Arrange
+            var expectedMessage = $"Collection bound to 'Successes' exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} (1024). This limit is a " +
+                $"safeguard against incorrect model binders and models. Address issues in " +
+                $"'{typeof(SuccessfulModel)}'. For example, this type may have a property with a model binder that " +
+                $"always succeeds. See the {nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} " +
+                $"documentation for more information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(SuccessfulContainer),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(request =>
+            {
+                // CollectionModelBinder binds an empty collection when value providers lack matching data.
+                request.QueryString = new QueryString("?Successes[0]=b");
+            });
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
         private class ClosedGenericCollection : Collection<string>
         {
         }
@@ -978,7 +1136,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
 
         private class ExplicitClosedGenericCollection : ICollection<string>
         {
-            private List<string> _data = new List<string>();
+            private readonly List<string> _data = new List<string>();
 
             int ICollection<string>.Count
             {
@@ -1034,7 +1192,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
 
         private class ExplicitClosedGenericList : IList<string>
         {
-            private List<string> _data = new List<string>();
+            private readonly List<string> _data = new List<string>();
 
             string IList<string>.this[int index]
             {
@@ -1118,7 +1276,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
 
         private class ExplicitCollection<T> : ICollection<T>
         {
-            private List<T> _data = new List<T>();
+            private readonly List<T> _data = new List<T>();
 
             int ICollection<T>.Count
             {
@@ -1174,7 +1332,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
 
         private class ExplicitList<T> : IList<T>
         {
-            private List<T> _data = new List<T>();
+            private readonly List<T> _data = new List<T>();
 
             T IList<T>.this[int index]
             {

src/Mvc/test/Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs

@@ -3411,6 +3411,181 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
             Assert.True(modelState.IsValid);
         }
 
+        private class LoopyModel
+        {
+            [ModelBinder(typeof(SuccessfulModelBinder))]
+            public bool IsBound { get; set; }
+
+            public LoopyModel SelfReference { get; set; }
+        }
+
+        // Regression test for #7052
+        [Fact]
+        public async Task ModelBindingSystem_ThrowsOn33Binders()
+        {
+            // Arrange
+            var expectedMessage = $"Model binding system exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingRecursionDepth)} (32). Reduce the " +
+                $"potential nesting of '{typeof(LoopyModel)}'. For example, this type may have a property with a " +
+                $"model binder that always succeeds. See the " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingRecursionDepth)} documentation for more " +
+                $"information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(LoopyModel),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext();
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
+        private class TwoDeepModel
+        {
+            [ModelBinder(typeof(SuccessfulModelBinder))]
+            public bool IsBound { get; set; }
+        }
+
+        private class ThreeDeepModel
+        {
+            [ModelBinder(typeof(SuccessfulModelBinder))]
+            public bool IsBound { get; set; }
+
+            public TwoDeepModel Inner { get; set; }
+        }
+
+        // Ensure model binding system allows MaxModelBindingRecursionDepth binders on the stack.
+        [Fact]
+        public async Task ModelBindingSystem_BindsWith3Binders()
+        {
+            // Arrange
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(ThreeDeepModel),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(
+                updateOptions: options => options.MaxModelBindingRecursionDepth = 3);
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act
+            var result = await parameterBinder.BindModelAsync(parameter, testContext);
+
+            // Assert
+            Assert.True(modelState.IsValid);
+            Assert.Equal(0, modelState.ErrorCount);
+
+            Assert.True(result.IsModelSet);
+            var model = Assert.IsType<ThreeDeepModel>(result.Model);
+            Assert.True(model.IsBound);
+            Assert.NotNull(model.Inner);
+            Assert.True(model.Inner.IsBound);
+        }
+
+        private class FourDeepModel
+        {
+            [ModelBinder(typeof(SuccessfulModelBinder))]
+            public bool IsBound { get; set; }
+
+            public ThreeDeepModel Inner { get; set; }
+        }
+
+        // Ensure model binding system disallows one more than MaxModelBindingRecursionDepth binders on the stack.
+        [Fact]
+        public async Task ModelBindingSystem_ThrowsOn4Binders()
+        {
+            // Arrange
+            var expectedMessage = $"Model binding system exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingRecursionDepth)} (3). Reduce the " +
+                $"potential nesting of '{typeof(FourDeepModel)}'. For example, this type may have a property with a " +
+                $"model binder that always succeeds. See the " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingRecursionDepth)} documentation for more " +
+                $"information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(FourDeepModel),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(
+                updateOptions: options => options.MaxModelBindingRecursionDepth = 3);
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
+        private class LoopyModel1
+        {
+            [ModelBinder(typeof(SuccessfulModelBinder))]
+            public bool IsBound { get; set; }
+
+            public LoopyModel2 Inner { get; set; }
+        }
+
+        private class LoopyModel2
+        {
+            [ModelBinder(typeof(SuccessfulModelBinder))]
+            public bool IsBound { get; set; }
+
+            public LoopyModel3 Inner { get; set; }
+        }
+
+        private class LoopyModel3
+        {
+            [ModelBinder(typeof(SuccessfulModelBinder))]
+            public bool IsBound { get; set; }
+
+            public LoopyModel1 Inner { get; set; }
+        }
+
+        [Fact]
+        public async Task ModelBindingSystem_ThrowsOn33Binders_WithIndirectModelTypeLoop()
+        {
+            // Arrange
+            var expectedMessage = $"Model binding system exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingRecursionDepth)} (32). Reduce the " +
+                $"potential nesting of '{typeof(LoopyModel1)}'. For example, this type may have a property with a " +
+                $"model binder that always succeeds. See the " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingRecursionDepth)} documentation for more " +
+                $"information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(LoopyModel1),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext();
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
         private static void SetJsonBodyContent(HttpRequest request, string content)
         {
             var stream = new MemoryStream(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetBytes(content));

src/Mvc/test/Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs

@@ -1128,6 +1128,40 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
             Assert.Equal(0, modelState.ErrorCount);
         }
 
+        [Fact]
+        public async Task DictionaryModelBinder_ThrowsOn1025Items_AtTopLevel()
+        {
+            // Arrange
+            var expectedMessage = $"Collection bound to 'parameter' exceeded " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} (1024). This limit is a " +
+                $"safeguard against incorrect model binders and models. Address issues in " +
+                $"'{typeof(KeyValuePair<SuccessfulModel, SuccessfulModel>)}'. For example, this type may have a " +
+                $"property with a model binder that always succeeds. See the " +
+                $"{nameof(MvcOptions)}.{nameof(MvcOptions.MaxModelBindingCollectionSize)} documentation for more " +
+                $"information.";
+            var parameter = new ParameterDescriptor()
+            {
+                Name = "parameter",
+                ParameterType = typeof(Dictionary<SuccessfulModel, SuccessfulModel>),
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(request =>
+            {
+                // CollectionModelBinder binds an empty collection when value providers are all empty.
+                request.QueryString = new QueryString("?a=b");
+            });
+
+            var modelState = testContext.ModelState;
+            var metadata = testContext.MetadataProvider.GetMetadataForType(parameter.ParameterType);
+            var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+            // Act & Assert
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => parameterBinder.BindModelAsync(parameter, testContext));
+            Assert.Equal(expectedMessage, exception.Message);
+        }
+
         private class ClosedGenericDictionary : Dictionary<string, string>
         {
         }
@@ -1138,7 +1172,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
 
         private class ExplicitClosedGenericDictionary : IDictionary<string, string>
         {
-            private IDictionary<string, string> _data = new Dictionary<string, string>();
+            private readonly IDictionary<string, string> _data = new Dictionary<string, string>();
 
             string IDictionary<string, string>.this[string key]
             {
@@ -1243,7 +1277,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
 
         private class ExplicitDictionary<TKey, TValue> : IDictionary<TKey, TValue>
         {
-            private IDictionary<TKey, TValue> _data = new Dictionary<TKey, TValue>();
+            private readonly IDictionary<TKey, TValue> _data = new Dictionary<TKey, TValue>();
 
             TValue IDictionary<TKey, TValue>.this[TKey key]
             {

src/Mvc/test/Mvc.IntegrationTests/Models/SuccessfulModel.cs

@@ -0,0 +1,13 @@
+// Copyright (c) Microsoft Open Technologies, Inc. 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.IntegrationTests
+{
+    public class SuccessfulModel
+    {
+        [ModelBinder(typeof(SuccessfulModelBinder))]
+        public bool IsBound { get; set; }
+
+        public string Name { get; set; }
+    }
+}

src/Mvc/test/Mvc.IntegrationTests/SuccessfulModelBinder.cs

@@ -0,0 +1,22 @@
+// 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.Threading.Tasks;
+using Microsoft.AspNetCore.Mvc.ModelBinding;
+
+namespace Microsoft.AspNetCore.Mvc.IntegrationTests
+{
+    /// <summary>
+    /// An unconditionally-successful model binder.
+    /// </summary>
+    public class SuccessfulModelBinder : IModelBinder
+    {
+        public Task BindModelAsync(ModelBindingContext bindingContext)
+        {
+            var model = bindingContext.ModelType == typeof(bool) ? (object)true : null;
+            bindingContext.Result = ModelBindingResult.Success(model);
+
+            return Task.CompletedTask;
+        }
+    }
+}

src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs

@@ -2239,6 +2239,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
         /// </summary>
         internal static string FormatServerShutdownDuringConnectionInitialization()
             => GetString("ServerShutdownDuringConnectionInitialization");
+
         /// <summary>
         /// Cannot call GetMemory() until response has started. Call HttpResponse.StartAsync() before calling GetMemory().
         /// </summary>