Skip to content

Commit 257334b

Browse files
wtgodbewtgodbe
committed
Merge commit 'cb14812a5c6098bd1091f5b7fd471ce2be45e8dd' into wtgodbe/IntMerge914
2 parents 0bb3d68 + cb14812 commit 257334b

File tree

6 files changed

+306
-11
lines changed

6 files changed

+306
-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
@@ -23,6 +23,9 @@ public class ModelStateDictionary : IReadOnlyDictionary<string, ModelStateEntry?
2323
/// </summary>
2424
public static readonly int DefaultMaxAllowedErrors = 200;
2525

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

@@ -41,8 +44,18 @@ public ModelStateDictionary()
4144
/// Initializes a new instance of the <see cref="ModelStateDictionary"/> class.
4245
/// </summary>
4346
public ModelStateDictionary(int maxAllowedErrors)
47+
: this(maxAllowedErrors, maxValidationDepth: DefaultMaxRecursionDepth, maxStateDepth: DefaultMaxRecursionDepth)
48+
{
49+
}
50+
51+
/// <summary>
52+
/// Initializes a new instance of the <see cref="ModelStateDictionary"/> class.
53+
/// </summary>
54+
private ModelStateDictionary(int maxAllowedErrors, int maxValidationDepth, int maxStateDepth)
4455
{
4556
MaxAllowedErrors = maxAllowedErrors;
57+
MaxValidationDepth = maxValidationDepth;
58+
MaxStateDepth = maxStateDepth;
4659
var emptySegment = new StringSegment(buffer: string.Empty);
4760
_root = new ModelStateNode(subKey: emptySegment)
4861
{
@@ -56,7 +69,9 @@ public ModelStateDictionary(int maxAllowedErrors)
5669
/// </summary>
5770
/// <param name="dictionary">The <see cref="ModelStateDictionary"/> to copy values from.</param>
5871
public ModelStateDictionary(ModelStateDictionary dictionary)
59-
: this(dictionary?.MaxAllowedErrors ?? DefaultMaxAllowedErrors)
72+
: this(dictionary?.MaxAllowedErrors ?? DefaultMaxAllowedErrors,
73+
dictionary?.MaxValidationDepth ?? DefaultMaxRecursionDepth,
74+
dictionary?.MaxStateDepth ?? DefaultMaxRecursionDepth)
6075
{
6176
if (dictionary == null)
6277
{
@@ -152,7 +167,7 @@ public bool IsValid
152167
}
153168

154169
/// <inheritdoc />
155-
public ModelValidationState ValidationState => GetValidity(_root) ?? ModelValidationState.Valid;
170+
public ModelValidationState ValidationState => GetValidity(_root, currentDepth: 0) ?? ModelValidationState.Valid;
156171

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

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

218-
ErrorCount++;
219237
AddModelErrorCore(key, exception);
220238
return true;
221239
}
@@ -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>
@@ -609,11 +626,18 @@ private ModelStateNode GetOrAddNode(string key)
609626
var current = _root;
610627
if (key.Length > 0)
611628
{
629+
var currentDepth = 0;
612630
var match = default(MatchResult);
613631
do
614632
{
633+
if (MaxStateDepth != null && currentDepth >= MaxStateDepth)
634+
{
635+
throw new InvalidOperationException(Resources.FormatModelStateDictionary_MaxModelStateDepth(MaxStateDepth));
636+
}
637+
615638
var subKey = FindNext(key, ref match);
616639
current = current.GetOrAddNode(subKey);
640+
currentDepth++;
617641

618642
} while (match.Type != Delimiter.None);
619643

@@ -659,9 +683,10 @@ private static StringSegment FindNext(string key, ref MatchResult currentMatch)
659683
return new StringSegment(key, keyStart, index - keyStart);
660684
}
661685

662-
private static ModelValidationState? GetValidity(ModelStateNode? node)
686+
private ModelValidationState? GetValidity(ModelStateNode? node, int currentDepth)
663687
{
664-
if (node == null)
688+
if (node == null ||
689+
(MaxValidationDepth != null && currentDepth >= MaxValidationDepth))
665690
{
666691
return null;
667692
}
@@ -684,9 +709,11 @@ private static StringSegment FindNext(string key, ref MatchResult currentMatch)
684709

685710
if (node.ChildNodes != null)
686711
{
712+
currentDepth++;
713+
687714
for (var i = 0; i < node.ChildNodes.Count; i++)
688715
{
689-
var entryState = GetValidity(node.ChildNodes[i]);
716+
var entryState = GetValidity(node.ChildNodes[i], currentDepth);
690717

691718
if (entryState == ModelValidationState.Unvalidated)
692719
{
@@ -710,7 +737,6 @@ private void EnsureMaxErrorsReachedRecorded()
710737
var exception = new TooManyModelErrorsException(Resources.ModelStateDictionary_MaxModelStateErrors);
711738
AddModelErrorCore(string.Empty, exception);
712739
HasRecordedMaxModelError = true;
713-
ErrorCount++;
714740
}
715741
}
716742

@@ -721,6 +747,8 @@ private void AddModelErrorCore(string key, Exception exception)
721747
modelState.ValidationState = ModelValidationState.Invalid;
722748
modelState.MarkNonContainerNode();
723749
modelState.Errors.Add(exception);
750+
751+
ErrorCount++;
724752
}
725753

726754
/// <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: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1599,6 +1599,162 @@ public void GetModelStateForProperty_ReturnsModelStateForIndexedChildren()
15991599
Assert.Equal("value1", property.RawValue);
16001600
}
16011601

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

‎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 sealed 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

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ internal sealed class ControllerRequestDelegateFactory : IRequestDelegateFactory
1919
private readonly ControllerActionInvokerCache _controllerActionInvokerCache;
2020
private readonly IReadOnlyList<IValueProviderFactory> _valueProviderFactories;
2121
private readonly int _maxModelValidationErrors;
22+
private readonly int? _maxValidationDepth;
23+
private readonly int _maxModelBindingRecursionDepth;
2224
private readonly ILogger _logger;
2325
private readonly DiagnosticListener _diagnosticListener;
2426
private readonly IActionResultTypeMapper _mapper;
@@ -46,6 +48,8 @@ public ControllerRequestDelegateFactory(
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
_enableActionInvokers = optionsAccessor.Value.EnableActionInvokers;
5054
_logger = loggerFactory.CreateLogger<ControllerActionInvoker>();
5155
_diagnosticListener = diagnosticListener;
@@ -82,6 +86,8 @@ public ControllerRequestDelegateFactory(
8286
};
8387

8488
controllerContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
89+
controllerContext.ModelState.MaxValidationDepth = _maxValidationDepth;
90+
controllerContext.ModelState.MaxStateDepth = _maxModelBindingRecursionDepth;
8591

8692
var (cacheEntry, filters) = _controllerActionInvokerCache.GetCachedResult(controllerContext);
8793

0 commit comments

Comments
 (0)