Skip to content

Commit 6a4fa60

Browse files
author
Bruno Oliveira
committed
Merged PR 24336: [3.1] ASP.NET ModelStateDictionary fix
Adding Recursion depth control to `ModelStateDictionary`.
2 parents a4601d1 + ec4420f commit 6a4fa60

File tree

6 files changed

+319
-10
lines changed

6 files changed

+319
-10
lines changed

‎src/Mvc/Mvc.Abstractions/ref/Microsoft.AspNetCore.Mvc.Abstractions.Manual.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,14 @@ internal static partial class ClosedGenericMatcher
88
public static System.Type ExtractGenericInterface(System.Type queryType, System.Type interfaceType) { throw null; }
99
}
1010
}
11+
12+
namespace Microsoft.AspNetCore.Mvc.ModelBinding
13+
{
14+
public partial class ModelStateDictionary
15+
{
16+
internal const int DefaultMaxRecursionDepth = 32;
17+
18+
internal int? MaxValidationDepth { get; set; }
19+
internal int? MaxStateDepth { get; set; }
20+
}
21+
}

‎src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelStateDictionary.cs

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ public class ModelStateDictionary : IReadOnlyDictionary<string, ModelStateEntry>
2424
/// </summary>
2525
public static readonly int DefaultMaxAllowedErrors = 200;
2626

27+
// internal for testing
28+
internal const int DefaultMaxRecursionDepth = 32;
29+
2730
private const char DelimiterDot = '.';
2831
private const char DelimiterOpen = '[';
2932

@@ -42,8 +45,18 @@ public ModelStateDictionary()
4245
/// Initializes a new instance of the <see cref="ModelStateDictionary"/> class.
4346
/// </summary>
4447
public ModelStateDictionary(int maxAllowedErrors)
48+
: this(maxAllowedErrors, maxValidationDepth: DefaultMaxRecursionDepth, maxStateDepth: DefaultMaxRecursionDepth)
49+
{
50+
}
51+
52+
/// <summary>
53+
/// Initializes a new instance of the <see cref="ModelStateDictionary"/> class.
54+
/// </summary>
55+
private ModelStateDictionary(int maxAllowedErrors, int maxValidationDepth, int maxStateDepth)
4556
{
4657
MaxAllowedErrors = maxAllowedErrors;
58+
MaxValidationDepth = maxValidationDepth;
59+
MaxStateDepth = maxStateDepth;
4760
var emptySegment = new StringSegment(buffer: string.Empty);
4861
_root = new ModelStateNode(subKey: emptySegment)
4962
{
@@ -57,7 +70,9 @@ public ModelStateDictionary(int maxAllowedErrors)
5770
/// </summary>
5871
/// <param name="dictionary">The <see cref="ModelStateDictionary"/> to copy values from.</param>
5972
public ModelStateDictionary(ModelStateDictionary dictionary)
60-
: this(dictionary?.MaxAllowedErrors ?? DefaultMaxAllowedErrors)
73+
: this(dictionary?.MaxAllowedErrors ?? DefaultMaxAllowedErrors,
74+
dictionary?.MaxValidationDepth ?? DefaultMaxRecursionDepth,
75+
dictionary?.MaxStateDepth ?? DefaultMaxRecursionDepth)
6176
{
6277
if (dictionary == null)
6378
{
@@ -153,7 +168,7 @@ public bool IsValid
153168
}
154169

155170
/// <inheritdoc />
156-
public ModelValidationState ValidationState => GetValidity(_root) ?? ModelValidationState.Valid;
171+
public ModelValidationState ValidationState => GetValidity(_root, currentDepth: 0) ?? ModelValidationState.Valid;
157172

158173
/// <inheritdoc />
159174
public ModelStateEntry this[string key]
@@ -173,6 +188,10 @@ public ModelStateEntry this[string key]
173188
// Flag that indicates if TooManyModelErrorException has already been added to this dictionary.
174189
private bool HasRecordedMaxModelError { get; set; }
175190

191+
internal int? MaxValidationDepth { get; set; }
192+
193+
internal int? MaxStateDepth { get; set; }
194+
176195
/// <summary>
177196
/// Adds the specified <paramref name="exception"/> to the <see cref="ModelStateEntry.Errors"/> instance
178197
/// that is associated with the specified <paramref name="key"/>. If the maximum number of allowed
@@ -216,7 +235,6 @@ public bool TryAddModelException(string key, Exception exception)
216235
return false;
217236
}
218237

219-
ErrorCount++;
220238
AddModelErrorCore(key, exception);
221239
return true;
222240
}
@@ -325,7 +343,6 @@ public bool TryAddModelError(string key, Exception exception, ModelMetadata meta
325343
return TryAddModelError(key, exception.Message);
326344
}
327345

328-
ErrorCount++;
329346
AddModelErrorCore(key, exception);
330347
return true;
331348
}
@@ -383,13 +400,13 @@ public bool TryAddModelError(string key, string errorMessage)
383400
return false;
384401
}
385402

386-
ErrorCount++;
387403
var modelState = GetOrAddNode(key);
388404
Count += !modelState.IsContainerNode ? 0 : 1;
389405
modelState.ValidationState = ModelValidationState.Invalid;
390406
modelState.MarkNonContainerNode();
391407
modelState.Errors.Add(errorMessage);
392408

409+
ErrorCount++;
393410
return true;
394411
}
395412

@@ -409,7 +426,7 @@ public ModelValidationState GetFieldValidationState(string key)
409426
}
410427

411428
var item = GetNode(key);
412-
return GetValidity(item) ?? ModelValidationState.Unvalidated;
429+
return GetValidity(item, currentDepth: 0) ?? ModelValidationState.Unvalidated;
413430
}
414431

415432
/// <summary>
@@ -611,11 +628,18 @@ private ModelStateNode GetOrAddNode(string key)
611628
var current = _root;
612629
if (key.Length > 0)
613630
{
631+
var currentDepth = 0;
614632
var match = default(MatchResult);
615633
do
616634
{
635+
if (MaxStateDepth != null && currentDepth >= MaxStateDepth)
636+
{
637+
throw new InvalidOperationException(Resources.FormatModelStateDictionary_MaxModelStateDepth(MaxStateDepth));
638+
}
639+
617640
var subKey = FindNext(key, ref match);
618641
current = current.GetOrAddNode(subKey);
642+
currentDepth++;
619643

620644
} while (match.Type != Delimiter.None);
621645

@@ -661,9 +685,10 @@ private static StringSegment FindNext(string key, ref MatchResult currentMatch)
661685
return new StringSegment(key, keyStart, index - keyStart);
662686
}
663687

664-
private static ModelValidationState? GetValidity(ModelStateNode node)
688+
private ModelValidationState? GetValidity(ModelStateNode node, int currentDepth)
665689
{
666-
if (node == null)
690+
if (node == null ||
691+
(MaxValidationDepth != null && currentDepth >= MaxValidationDepth))
667692
{
668693
return null;
669694
}
@@ -686,9 +711,11 @@ private static StringSegment FindNext(string key, ref MatchResult currentMatch)
686711

687712
if (node.ChildNodes != null)
688713
{
714+
currentDepth++;
715+
689716
for (var i = 0; i < node.ChildNodes.Count; i++)
690717
{
691-
var entryState = GetValidity(node.ChildNodes[i]);
718+
var entryState = GetValidity(node.ChildNodes[i], currentDepth);
692719

693720
if (entryState == ModelValidationState.Unvalidated)
694721
{
@@ -712,7 +739,6 @@ private void EnsureMaxErrorsReachedRecorded()
712739
var exception = new TooManyModelErrorsException(Resources.ModelStateDictionary_MaxModelStateErrors);
713740
AddModelErrorCore(string.Empty, exception);
714741
HasRecordedMaxModelError = true;
715-
ErrorCount++;
716742
}
717743
}
718744

@@ -723,6 +749,8 @@ private void AddModelErrorCore(string key, Exception exception)
723749
modelState.ValidationState = ModelValidationState.Invalid;
724750
modelState.MarkNonContainerNode();
725751
modelState.Errors.Add(exception);
752+
753+
ErrorCount++;
726754
}
727755

728756
/// <summary>

‎src/Mvc/Mvc.Abstractions/src/Resources.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,4 +177,7 @@
177177
<data name="BinderType_MustBeIModelBinder" xml:space="preserve">
178178
<value>The type '{0}' must implement '{1}' to be used as a model binder.</value>
179179
</data>
180+
<data name="ModelStateDictionary_MaxModelStateDepth" xml:space="preserve">
181+
<value>The specified key exceeded the maximum ModelState depth: {0}</value>
182+
</data>
180183
</root>

‎src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelStateDictionaryTest.cs

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Linq;
56
using Microsoft.AspNetCore.Mvc.Formatters;
67
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
78
using Microsoft.Extensions.Options;
@@ -1601,6 +1602,162 @@ public void GetModelStateForProperty_ReturnsModelStateForIndexedChildren()
16011602
Assert.Equal("value1", property.RawValue);
16021603
}
16031604

1605+
[Fact]
1606+
public void GetFieldValidationState_ReturnsUnvalidated_IfTreeHeightIsGreaterThanLimit()
1607+
{
1608+
// Arrange
1609+
var stackLimit = 5;
1610+
var dictionary = new ModelStateDictionary();
1611+
var key = string.Join(".", Enumerable.Repeat("foo", stackLimit + 1));
1612+
dictionary.MaxValidationDepth = stackLimit;
1613+
dictionary.MaxStateDepth = null;
1614+
dictionary.MarkFieldValid(key);
1615+
1616+
// Act
1617+
var validationState = dictionary.GetFieldValidationState("foo");
1618+
1619+
// Assert
1620+
Assert.Equal(ModelValidationState.Unvalidated, validationState);
1621+
}
1622+
1623+
[Fact]
1624+
public void IsValidProperty_ReturnsTrue_IfTreeHeightIsGreaterThanLimit()
1625+
{
1626+
// Arrange
1627+
var stackLimit = 5;
1628+
var dictionary = new ModelStateDictionary();
1629+
var key = string.Join(".", Enumerable.Repeat("foo", stackLimit + 1));
1630+
dictionary.MaxValidationDepth = stackLimit;
1631+
dictionary.MaxStateDepth = null;
1632+
dictionary.AddModelError(key, "some error");
1633+
1634+
// Act
1635+
var isValid = dictionary.IsValid;
1636+
var validationState = dictionary.ValidationState;
1637+
1638+
// Assert
1639+
Assert.True(isValid);
1640+
Assert.Equal(ModelValidationState.Valid, validationState);
1641+
}
1642+
1643+
[Fact]
1644+
public void TryAddModelException_Throws_IfKeyHasTooManySegments()
1645+
{
1646+
// Arrange
1647+
var exception = new TestException();
1648+
1649+
var stateDepth = 5;
1650+
var dictionary = new ModelStateDictionary();
1651+
var key = string.Join(".", Enumerable.Repeat("foo", stateDepth + 1));
1652+
dictionary.MaxStateDepth = stateDepth;
1653+
1654+
// Act
1655+
var invalidException = Assert.Throws<InvalidOperationException>(() => dictionary.TryAddModelException(key, exception));
1656+
1657+
// Assert
1658+
Assert.Equal(
1659+
$"The specified key exceeded the maximum ModelState depth: {dictionary.MaxStateDepth}",
1660+
invalidException.Message);
1661+
}
1662+
1663+
[Fact]
1664+
public void TryAddModelError_Throws_IfKeyHasTooManySegments()
1665+
{
1666+
// Arrange
1667+
var stateDepth = 5;
1668+
var dictionary = new ModelStateDictionary();
1669+
var key = string.Join(".", Enumerable.Repeat("foo", stateDepth + 1));
1670+
dictionary.MaxStateDepth = stateDepth;
1671+
1672+
// Act
1673+
var invalidException = Assert.Throws<InvalidOperationException>(() => dictionary.TryAddModelError(key, "errorMessage"));
1674+
1675+
// Assert
1676+
Assert.Equal(
1677+
$"The specified key exceeded the maximum ModelState depth: {dictionary.MaxStateDepth}",
1678+
invalidException.Message);
1679+
}
1680+
1681+
[Fact]
1682+
public void SetModelValue_Throws_IfKeyHasTooManySegments()
1683+
{
1684+
var stateDepth = 5;
1685+
var dictionary = new ModelStateDictionary();
1686+
var key = string.Join(".", Enumerable.Repeat("foo", stateDepth + 1));
1687+
dictionary.MaxStateDepth = stateDepth;
1688+
1689+
// Act
1690+
var invalidException = Assert.Throws<InvalidOperationException>(() => dictionary.SetModelValue(key, string.Empty, string.Empty));
1691+
1692+
// Assert
1693+
Assert.Equal(
1694+
$"The specified key exceeded the maximum ModelState depth: {dictionary.MaxStateDepth}",
1695+
invalidException.Message);
1696+
}
1697+
1698+
[Fact]
1699+
public void MarkFieldValid_Throws_IfKeyHasTooManySegments()
1700+
{
1701+
// Arrange
1702+
var stateDepth = 5;
1703+
var source = new ModelStateDictionary();
1704+
var key = string.Join(".", Enumerable.Repeat("foo", stateDepth + 1));
1705+
source.MaxStateDepth = stateDepth;
1706+
1707+
// Act
1708+
var exception = Assert.Throws<InvalidOperationException>(() => source.MarkFieldValid(key));
1709+
1710+
// Assert
1711+
Assert.Equal(
1712+
$"The specified key exceeded the maximum ModelState depth: {source.MaxStateDepth}",
1713+
exception.Message);
1714+
}
1715+
1716+
[Fact]
1717+
public void MarkFieldSkipped_Throws_IfKeyHasTooManySegments()
1718+
{
1719+
// Arrange
1720+
var stateDepth = 5;
1721+
var source = new ModelStateDictionary();
1722+
var key = string.Join(".", Enumerable.Repeat("foo", stateDepth + 1));
1723+
source.MaxStateDepth = stateDepth;
1724+
1725+
// Act
1726+
var exception = Assert.Throws<InvalidOperationException>(() => source.MarkFieldSkipped(key));
1727+
1728+
// Assert
1729+
Assert.Equal(
1730+
$"The specified key exceeded the maximum ModelState depth: {source.MaxStateDepth}",
1731+
exception.Message);
1732+
}
1733+
1734+
[Fact]
1735+
public void Constructor_SetsDefaultRecursionDepth()
1736+
{
1737+
// Arrange && Act
1738+
var dictionary = new ModelStateDictionary();
1739+
1740+
// Assert
1741+
Assert.Equal(ModelStateDictionary.DefaultMaxRecursionDepth, dictionary.MaxValidationDepth);
1742+
Assert.Equal(ModelStateDictionary.DefaultMaxRecursionDepth, dictionary.MaxStateDepth);
1743+
}
1744+
1745+
[Fact]
1746+
public void CopyConstructor_PreservesRecursionDepth()
1747+
{
1748+
// Arrange
1749+
var dictionary = new ModelStateDictionary();
1750+
dictionary.MaxValidationDepth = 5;
1751+
dictionary.MaxStateDepth = 4;
1752+
1753+
// Act
1754+
var newDictionary = new ModelStateDictionary(dictionary);
1755+
1756+
// Assert
1757+
Assert.Equal(dictionary.MaxValidationDepth, newDictionary.MaxValidationDepth);
1758+
Assert.Equal(dictionary.MaxStateDepth, newDictionary.MaxStateDepth);
1759+
}
1760+
16041761
private class OptionsAccessor : IOptions<MvcOptions>
16051762
{
16061763
public MvcOptions Value { get; } = new MvcOptions();

‎src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerProvider.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ internal class ControllerActionInvokerProvider : IActionInvokerProvider
1818
private readonly ControllerActionInvokerCache _controllerActionInvokerCache;
1919
private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories;
2020
private readonly int _maxModelValidationErrors;
21+
private readonly int? _maxValidationDepth;
22+
private readonly int _maxModelBindingRecursionDepth;
2123
private readonly ILogger _logger;
2224
private readonly DiagnosticListener _diagnosticListener;
2325
private readonly IActionResultTypeMapper _mapper;
@@ -44,6 +46,8 @@ public ControllerActionInvokerProvider(
4446
_controllerActionInvokerCache = controllerActionInvokerCache;
4547
_valueProviderFactories = optionsAccessor.Value.ValueProviderFactories.ToArray();
4648
_maxModelValidationErrors = optionsAccessor.Value.MaxModelValidationErrors;
49+
_maxValidationDepth = optionsAccessor.Value.MaxValidationDepth;
50+
_maxModelBindingRecursionDepth = optionsAccessor.Value.MaxModelBindingRecursionDepth;
4751
_logger = loggerFactory.CreateLogger<ControllerActionInvoker>();
4852
_diagnosticListener = diagnosticListener;
4953
_mapper = mapper;
@@ -68,6 +72,8 @@ public void OnProvidersExecuting(ActionInvokerProviderContext context)
6872
ValueProviderFactories = new CopyOnWriteList<IValueProviderFactory>(_valueProviderFactories)
6973
};
7074
controllerContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
75+
controllerContext.ModelState.MaxValidationDepth = _maxValidationDepth;
76+
controllerContext.ModelState.MaxStateDepth = _maxModelBindingRecursionDepth;
7177

7278
var (cacheEntry, filters) = _controllerActionInvokerCache.GetCachedResult(controllerContext);
7379

0 commit comments

Comments
 (0)