From 911751f4ae904f196cdb29b4cfa14b9abf5536f6 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 21 Jul 2015 14:50:03 -0700 Subject: [PATCH] Fix for #2799 - OOM during TryUpdateModelAsync The change here is that when we create the ModelValidationNodes to just return them rather than adding them to the tree. For a very large model, having these extra nodes in the tree would eventually cause an OOM. We're going to be taking a more thorough look at this code separately, hence the quick fix. --- .../Validation/DefaultObjectValidator.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs index 21865ef5cd..c426c1f27c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs @@ -166,7 +166,7 @@ private bool ValidateChildNodes( ValidationContext validationContext) { var isValid = true; - ExpandValidationNode(validationContext, modelExplorer); + var childNodes = GetChildNodes(validationContext, modelExplorer); IList validators = null; var elementMetadata = modelExplorer.Metadata.ElementMetadata; @@ -175,7 +175,7 @@ private bool ValidateChildNodes( validators = GetValidators(validationContext.ModelValidationContext.ValidatorProvider, elementMetadata); } - foreach (var childNode in validationContext.ValidationNode.ChildNodes) + foreach (var childNode in childNodes) { var childModelExplorer = childNode.ModelMetadata.MetadataKind == Metadata.ModelMetadataKind.Type ? _modelMetadataProvider.GetModelExplorerForType(childNode.ModelMetadata.ModelType, childNode.Model) : @@ -271,16 +271,20 @@ private bool IsTypeExcludedFromValidation(IList fi return filters.Any(filter => filter.IsTypeExcluded(type)); } - private void ExpandValidationNode(ValidationContext context, ModelExplorer modelExplorer) + private IList GetChildNodes(ValidationContext context, ModelExplorer modelExplorer) { var validationNode = context.ValidationNode; + + // This is the trivial case where the node-tree that was built-up during binding already has + // all of the nodes we need. if (validationNode.ChildNodes.Count != 0 || !validationNode.ValidateAllProperties || validationNode.Model == null) { - return; + return validationNode.ChildNodes; } + var childNodes = new List(validationNode.ChildNodes); var elementMetadata = modelExplorer.Metadata.ElementMetadata; if (elementMetadata == null) { @@ -293,7 +297,7 @@ private void ExpandValidationNode(ValidationContext context, ModelExplorer model { ValidateAllProperties = true }; - validationNode.ChildNodes.Add(childNode); + childNodes.Add(childNode); } } else @@ -312,10 +316,12 @@ private void ExpandValidationNode(ValidationContext context, ModelExplorer model ValidateAllProperties = true }; - validationNode.ChildNodes.Add(childNode); + childNodes.Add(childNode); index++; } } + + return childNodes; } private class ValidationContext