Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Limit the maximum number of Model errors to a reasonable value.
Browse files Browse the repository at this point in the history
Fixes #490
  • Loading branch information
pranavkm committed Sep 16, 2014
1 parent 9befa6e commit 646c0d7
Show file tree
Hide file tree
Showing 22 changed files with 680 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ private Task<object> ReadInternal(InputFormatterContext context,
errorHandler = (sender, e) =>
{
var exception = e.ErrorContext.Error;
context.ActionContext.ModelState.AddModelError(e.ErrorContext.Path, e.ErrorContext.Error);
context.ActionContext.ModelState.TryAddModelError(e.ErrorContext.Path, e.ErrorContext.Error);
// Error must always be marked as handled
// Failure to do so can cause the exception to be rethrown at every recursive level and
// overflow the stack for x64 CLR processes
Expand Down
20 changes: 20 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public class MvcOptions
{
private AntiForgeryOptions _antiForgeryOptions = new AntiForgeryOptions();
private RazorViewEngineOptions _viewEngineOptions = new RazorViewEngineOptions();
private int _maxModelStateErrors = 200;

public MvcOptions()
{
Expand Down Expand Up @@ -94,6 +95,25 @@ public RazorViewEngineOptions ViewEngineOptions
}

/// <summary>
/// Gets or sets the maximum number of validation errors that are allowed by this application before further
/// errors are ignored.
/// </summary>
public int MaxModelValidationErrors
{
get { return _maxModelStateErrors; }
set
{
if (value < 0)
{
throw new ArgumentOutOfRangeException(nameof(value));
}

_maxModelStateErrors = value;
}
}

/// <summary>
/// Get a list of the <see cref="ModelBinderDescriptor" /> used by the
/// Gets a list of the <see cref="ModelBinderDescriptor" /> used by the
/// <see cref="ModelBinding.CompositeModelBinder" />.
/// </summary>
Expand Down
18 changes: 11 additions & 7 deletions src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.AspNet.Routing;
using Microsoft.Framework.DependencyInjection;
using Microsoft.Framework.Logging;
using Microsoft.Framework.OptionsModel;

namespace Microsoft.AspNet.Mvc
{
Expand All @@ -32,7 +33,7 @@ public string GetVirtualPath([NotNull] VirtualPathContext context)
public async Task RouteAsync([NotNull] RouteContext context)
{
var services = context.HttpContext.RequestServices;

// Verify if AddMvc was done before calling UseMvc
// We use the MvcMarkerService to make sure if all the services were added.
MvcServicesHelper.ThrowIfMvcNotRegistered(services);
Expand Down Expand Up @@ -64,19 +65,22 @@ public async Task RouteAsync([NotNull] RouteContext context)
return;
}

if (actionDescriptor.RouteValueDefaults != null)
{
foreach (var kvp in actionDescriptor.RouteValueDefaults)
if (actionDescriptor.RouteValueDefaults != null)
{
if (!context.RouteData.Values.ContainsKey(kvp.Key))
foreach (var kvp in actionDescriptor.RouteValueDefaults)
{
context.RouteData.Values.Add(kvp.Key, kvp.Value);
if (!context.RouteData.Values.ContainsKey(kvp.Key))
{
context.RouteData.Values.Add(kvp.Key, kvp.Value);
}
}
}
}

var actionContext = new ActionContext(context.HttpContext, context.RouteData, actionDescriptor);

var optionsAccessor = services.GetService<IOptionsAccessor<MvcOptions>>();
actionContext.ModelState.MaxAllowedErrors = optionsAccessor.Options.MaxModelValidationErrors;

var contextAccessor = services.GetService<IContextAccessor<ActionContext>>();
using (contextAccessor.SetContextSource(() => actionContext, PreventExchange))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ public virtual async Task<bool> BindModelAsync([NotNull] ModelBindingContext bin

// Only perform validation at the root of the object graph. ValidationNode will recursively walk the graph.
// Ignore ComplexModelDto since it essentially wraps the primary object.
if (newBindingContext.ModelMetadata.ContainerType == null &&
newBindingContext.ModelMetadata.ModelType != typeof(ComplexModelDto))
if (IsBindingAtRootOfObjectGraph(newBindingContext))
{
// run validation and return the model
// If we fell back to an empty prefix above and are dealing with simple types,
Expand All @@ -92,7 +91,7 @@ public virtual async Task<bool> BindModelAsync([NotNull] ModelBindingContext bin
return true;
}

private async Task<bool> TryBind([NotNull] ModelBindingContext bindingContext)
private async Task<bool> TryBind(ModelBindingContext bindingContext)
{
// TODO: RuntimeHelpers.EnsureSufficientExecutionStack does not exist in the CoreCLR.
// Protects against stack overflow for deeply nested model binding
Expand All @@ -110,6 +109,16 @@ private async Task<bool> TryBind([NotNull] ModelBindingContext bindingContext)
return false;
}

private static bool IsBindingAtRootOfObjectGraph(ModelBindingContext bindingContext)
{
// We're at the root of the object graph if the model does does not have a container.
// This statement is true for complex types at the root twice over - once with the actual model
// and once when when it is represented by a ComplexModelDto. Ignore the latter case.

return bindingContext.ModelMetadata.ContainerType == null &&
bindingContext.ModelMetadata.ModelType != typeof(ComplexModelDto);
}

private static ModelBindingContext CreateNewBindingContext(ModelBindingContext oldBindingContext,
string modelName,
bool reuseValidationNode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ internal static EventHandler<ModelValidatedEventArgs> CreateNullCheckFailedHandl
// var errorMessage = ModelBinderConfig.ValueRequiredErrorMessageProvider(e.ValidationContext,
// modelMetadata,
// incomingValue);
var errorMessage = "A value is required.";
var errorMessage = Resources.ModelBinderConfig_ValueRequired;
if (errorMessage != null)
{
modelState.AddModelError(validationNode.ModelStateKey, errorMessage);
modelState.TryAddModelError(validationNode.ModelStateKey, errorMessage);
}
}
};
Expand Down Expand Up @@ -232,7 +232,7 @@ internal void ProcessDto(ModelBindingContext bindingContext, ComplexModelDto dto
// (oddly) succeeded.
if (!addedError)
{
bindingContext.ModelState.AddModelError(
bindingContext.ModelState.TryAddModelError(
modelStateKey,
Resources.FormatMissingRequiredMember(missingRequiredProperty));
}
Expand Down Expand Up @@ -285,7 +285,7 @@ protected virtual void SetProperty(ModelBindingContext bindingContext,
var validationContext = new ModelValidationContext(bindingContext, propertyMetadata);
foreach (var validationResult in requiredValidator.Validate(validationContext))
{
bindingContext.ModelState.AddModelError(modelStateKey, validationResult.Message);
bindingContext.ModelState.TryAddModelError(modelStateKey, validationResult.Message);
}
}
}
Expand Down Expand Up @@ -337,7 +337,7 @@ private static bool RunValidator(IModelValidator validator,
var addedError = false;
foreach (var validationResult in validator.Validate(validationContext))
{
bindingContext.ModelState.AddModelError(modelStateKey, validationResult.Message);
bindingContext.ModelState.TryAddModelError(modelStateKey, validationResult.Message);
addedError = true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ internal static void AddModelErrorBasedOnExceptionType(ModelBindingContext bindi
{
if (IsFormatException(ex))
{
bindingContext.ModelState.AddModelError(bindingContext.ModelName, ex.Message);
bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex.Message);
}
else
{
bindingContext.ModelState.AddModelError(bindingContext.ModelName, ex);
bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex);
}
}

Expand Down
Loading

0 comments on commit 646c0d7

Please sign in to comment.