Skip to content

Commit 4a52cfe

Browse files
wtgodbewtgodbe
committed
Merged PR 24735: [6.0] ASP.NET ModelStateDictionary fix
Adding Recursion depth control to ModelStateDictionary
1 parent f87cbf7 commit 4a52cfe

File tree

6 files changed

+308
-11
lines changed

6 files changed

+308
-11
lines changed

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

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

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

@@ -43,8 +46,18 @@ public ModelStateDictionary()
4346
/// Initializes a new instance of the <see cref="ModelStateDictionary"/> class.
4447
/// </summary>
4548
public ModelStateDictionary(int maxAllowedErrors)
49+
: this(maxAllowedErrors, maxValidationDepth: DefaultMaxRecursionDepth, maxStateDepth: DefaultMaxRecursionDepth)
50+
{
51+
}
52+
53+
/// <summary>
54+
/// Initializes a new instance of the <see cref="ModelStateDictionary"/> class.
55+
/// </summary>
56+
private ModelStateDictionary(int maxAllowedErrors, int maxValidationDepth, int maxStateDepth)
4657
{
4758
MaxAllowedErrors = maxAllowedErrors;
59+
MaxValidationDepth = maxValidationDepth;
60+
MaxStateDepth = maxStateDepth;
4861
var emptySegment = new StringSegment(buffer: string.Empty);
4962
_root = new ModelStateNode(subKey: emptySegment)
5063
{
@@ -58,7 +71,9 @@ public ModelStateDictionary(int maxAllowedErrors)
5871
/// </summary>
5972
/// <param name="dictionary">The <see cref="ModelStateDictionary"/> to copy values from.</param>
6073
public ModelStateDictionary(ModelStateDictionary dictionary)
61-
: this(dictionary?.MaxAllowedErrors ?? DefaultMaxAllowedErrors)
74+
: this(dictionary?.MaxAllowedErrors ?? DefaultMaxAllowedErrors,
75+
dictionary?.MaxValidationDepth ?? DefaultMaxRecursionDepth,
76+
dictionary?.MaxStateDepth ?? DefaultMaxRecursionDepth)
6277
{
6378
if (dictionary == null)
6479
{
@@ -154,7 +169,7 @@ public bool IsValid
154169
}
155170

156171
/// <inheritdoc />
157-
public ModelValidationState ValidationState => GetValidity(_root) ?? ModelValidationState.Valid;
172+
public ModelValidationState ValidationState => GetValidity(_root, currentDepth: 0) ?? ModelValidationState.Valid;
158173

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

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

220-
ErrorCount++;
221239
AddModelErrorCore(key, exception);
222240
return true;
223241
}
@@ -327,7 +345,6 @@ public bool TryAddModelError(string key, Exception exception, ModelMetadata meta
327345
return TryAddModelError(key, exception.Message);
328346
}
329347

330-
ErrorCount++;
331348
AddModelErrorCore(key, exception);
332349
return true;
333350
}
@@ -385,13 +402,13 @@ public bool TryAddModelError(string key, string errorMessage)
385402
return false;
386403
}
387404

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

411+
ErrorCount++;
395412
return true;
396413
}
397414

@@ -411,7 +428,7 @@ public ModelValidationState GetFieldValidationState(string key)
411428
}
412429

413430
var item = GetNode(key);
414-
return GetValidity(item) ?? ModelValidationState.Unvalidated;
431+
return GetValidity(item, currentDepth: 0) ?? ModelValidationState.Unvalidated;
415432
}
416433

417434
/// <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: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,7 @@
180180
<data name="RecordTypeHasValidationOnProperties" xml:space="preserve">
181181
<value>Record type '{0}' has validation metadata defined on property '{1}' that will be ignored. '{1}' is a parameter in the record primary constructor and validation metadata must be associated with the constructor parameter.</value>
182182
</data>
183-
</root>
183+
<data name="ModelStateDictionary_MaxModelStateDepth" xml:space="preserve">
184+
<value>The specified key exceeded the maximum ModelState depth: {0}</value>
185+
</data>
186+
</root>

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

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,6 +1601,163 @@ public void GetModelStateForProperty_ReturnsModelStateForIndexedChildren()
16011601
Assert.Equal("value1", property.RawValue);
16021602
}
16031603

1604+
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
@@ -20,6 +20,8 @@ internal class ControllerActionInvokerProvider : IActionInvokerProvider
2020
private readonly ControllerActionInvokerCache _controllerActionInvokerCache;
2121
private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories;
2222
private readonly int _maxModelValidationErrors;
23+
private readonly int? _maxValidationDepth;
24+
private readonly int _maxModelBindingRecursionDepth;
2325
private readonly ILogger _logger;
2426
private readonly DiagnosticListener _diagnosticListener;
2527
private readonly IActionResultTypeMapper _mapper;
@@ -46,6 +48,8 @@ public ControllerActionInvokerProvider(
4648
_controllerActionInvokerCache = controllerActionInvokerCache;
4749
_valueProviderFactories = optionsAccessor.Value.ValueProviderFactories.ToArray();
4850
_maxModelValidationErrors = optionsAccessor.Value.MaxModelValidationErrors;
51+
_maxValidationDepth = optionsAccessor.Value.MaxValidationDepth;
52+
_maxModelBindingRecursionDepth = optionsAccessor.Value.MaxModelBindingRecursionDepth;
4953
_logger = loggerFactory.CreateLogger<ControllerActionInvoker>();
5054
_diagnosticListener = diagnosticListener;
5155
_mapper = mapper;
@@ -70,6 +74,8 @@ public void OnProvidersExecuting(ActionInvokerProviderContext context)
7074
ValueProviderFactories = new CopyOnWriteList<IValueProviderFactory>(_valueProviderFactories)
7175
};
7276
controllerContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
77+
controllerContext.ModelState.MaxValidationDepth = _maxValidationDepth;
78+
controllerContext.ModelState.MaxStateDepth = _maxModelBindingRecursionDepth;
7379

7480
var (cacheEntry, filters) = _controllerActionInvokerCache.GetCachedResult(controllerContext);
7581

‎src/Mvc/Mvc.Core/src/Routing/ControllerRequestDelegateFactory.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ internal class ControllerRequestDelegateFactory : IRequestDelegateFactory
2121
private readonly ControllerActionInvokerCache _controllerActionInvokerCache;
2222
private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories;
2323
private readonly int _maxModelValidationErrors;
24+
private readonly int? _maxValidationDepth;
25+
private readonly int _maxModelBindingRecursionDepth;
2426
private readonly ILogger _logger;
2527
private readonly DiagnosticListener _diagnosticListener;
2628
private readonly IActionResultTypeMapper _mapper;
@@ -48,6 +50,8 @@ public ControllerRequestDelegateFactory(
4850
_controllerActionInvokerCache = controllerActionInvokerCache;
4951
_valueProviderFactories = optionsAccessor.Value.ValueProviderFactories.ToArray();
5052
_maxModelValidationErrors = optionsAccessor.Value.MaxModelValidationErrors;
53+
_maxValidationDepth = optionsAccessor.Value.MaxValidationDepth;
54+
_maxModelBindingRecursionDepth = optionsAccessor.Value.MaxModelBindingRecursionDepth;
5155
_enableActionInvokers = optionsAccessor.Value.EnableActionInvokers;
5256
_logger = loggerFactory.CreateLogger<ControllerActionInvoker>();
5357
_diagnosticListener = diagnosticListener;
@@ -86,6 +90,8 @@ public ControllerRequestDelegateFactory(
8690
};
8791

8892
controllerContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
93+
controllerContext.ModelState.MaxValidationDepth = _maxValidationDepth;
94+
controllerContext.ModelState.MaxStateDepth = _maxModelBindingRecursionDepth;
8995

9096
var (cacheEntry, filters) = _controllerActionInvokerCache.GetCachedResult(controllerContext);
9197

0 commit comments

Comments
 (0)