diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonInputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonInputFormatter.cs index 6cdff8e15d..f5802887fd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonInputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonInputFormatter.cs @@ -148,7 +148,7 @@ namespace Microsoft.AspNet.Mvc 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 diff --git a/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs index 895b0941f5..e47e149eca 100644 --- a/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs @@ -17,6 +17,7 @@ namespace Microsoft.AspNet.Mvc { private AntiForgeryOptions _antiForgeryOptions = new AntiForgeryOptions(); private RazorViewEngineOptions _viewEngineOptions = new RazorViewEngineOptions(); + private int _maxModelStateErrors = 200; public MvcOptions() { @@ -94,6 +95,25 @@ namespace Microsoft.AspNet.Mvc } /// + /// Gets or sets the maximum number of validation errors that are allowed by this application before further + /// errors are ignored. + /// + public int MaxModelValidationErrors + { + get { return _maxModelStateErrors; } + set + { + if (value < 0) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + + _maxModelStateErrors = value; + } + } + + /// + /// Get a list of the used by the /// Gets a list of the used by the /// . /// diff --git a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs index 58e2a02e94..47a2dca02d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs +++ b/src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs @@ -11,6 +11,7 @@ using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Logging; +using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Mvc { @@ -32,7 +33,7 @@ namespace Microsoft.AspNet.Mvc 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); @@ -64,19 +65,22 @@ namespace Microsoft.AspNet.Mvc 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>(); + actionContext.ModelState.MaxAllowedErrors = optionsAccessor.Options.MaxModelValidationErrors; + var contextAccessor = services.GetService>(); using (contextAccessor.SetContextSource(() => actionContext, PreventExchange)) { diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs index 0b63a73c7d..8bd9d52328 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs @@ -65,8 +65,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // 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, @@ -92,7 +91,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return true; } - private async Task TryBind([NotNull] ModelBindingContext bindingContext) + private async Task TryBind(ModelBindingContext bindingContext) { // TODO: RuntimeHelpers.EnsureSufficientExecutionStack does not exist in the CoreCLR. // Protects against stack overflow for deeply nested model binding @@ -110,6 +109,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding 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) diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs index 98599dc5de..1aeea1c7a7 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs @@ -127,10 +127,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // 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); } } }; @@ -232,7 +232,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // (oddly) succeeded. if (!addedError) { - bindingContext.ModelState.AddModelError( + bindingContext.ModelState.TryAddModelError( modelStateKey, Resources.FormatMissingRequiredMember(missingRequiredProperty)); } @@ -285,7 +285,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding 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); } } } @@ -337,7 +337,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var addedError = false; foreach (var validationResult in validator.Validate(validationContext)) { - bindingContext.ModelState.AddModelError(modelStateKey, validationResult.Message); + bindingContext.ModelState.TryAddModelError(modelStateKey, validationResult.Message); addedError = true; } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs index 6f34b42c59..4f62884548 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs @@ -98,11 +98,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Internal { 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); } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs index 57d248bfe4..733bd7f16d 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs @@ -9,53 +9,101 @@ using Microsoft.AspNet.Mvc.ModelBinding.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding { + /// + /// Represents the state of an attempt to bind values from an HTTP Request to an action method, which includes + /// validation information. + /// public class ModelStateDictionary : IDictionary { private readonly IDictionary _innerDictionary; + /// + /// Initializes a new instance of the class. + /// public ModelStateDictionary() { _innerDictionary = new Dictionary(StringComparer.OrdinalIgnoreCase); } + /// + /// Initializes a new instance of the class by using values that are copied + /// from the specified . + /// + /// The to copy values from. public ModelStateDictionary([NotNull] ModelStateDictionary dictionary) { _innerDictionary = new CopyOnWriteDictionary(dictionary, StringComparer.OrdinalIgnoreCase); + + MaxAllowedErrors = dictionary.MaxAllowedErrors; + ErrorCount = dictionary.ErrorCount; + HasRecordedMaxModelError = dictionary.HasRecordedMaxModelError; } - #region IDictionary properties + /// + /// Gets or sets the maximum allowed errors in this instance of . + /// Defaults to . + /// + /// + /// The value of this property is used to track the total number of calls to + /// and after which + /// an error is thrown for further invocations. Errors added via modifying do not + /// count towards this limit. + /// + public int MaxAllowedErrors { get; set; } = int.MaxValue; + + /// + /// Gets a flag that determines if the total number of added errors (given by ) is + /// fewer than . + /// + public bool CanAddErrors + { + get { return ErrorCount < MaxAllowedErrors; } + } + + /// + /// Gets the number of errors added to this instance of via + /// and . + /// + public int ErrorCount { get; private set; } + + /// public int Count { get { return _innerDictionary.Count; } } + /// public bool IsReadOnly { get { return _innerDictionary.IsReadOnly; } } + /// public ICollection Keys { get { return _innerDictionary.Keys; } } + /// public ICollection Values { get { return _innerDictionary.Values; } } - #endregion + /// public bool IsValid { get { return ValidationState == ModelValidationState.Valid; } } + /// public ModelValidationState ValidationState { get { return GetValidity(_innerDictionary); } } + /// public ModelState this[[NotNull] string key] { get @@ -80,20 +128,94 @@ namespace Microsoft.AspNet.Mvc.ModelBinding get { return _innerDictionary; } } + // Flag that indiciates if TooManyModelErrorException has already been added to this dictionary. + private bool HasRecordedMaxModelError { get; set; } + + /// + /// Adds the specified to the instance + /// that is associated with the specified . + /// + /// The key of the to add errors to. + /// The to add. public void AddModelError([NotNull] string key, [NotNull] Exception exception) { - var modelState = GetModelStateForKey(key); - modelState.ValidationState = ModelValidationState.Invalid; - modelState.Errors.Add(exception); + TryAddModelError(key, exception); } + /// + /// Attempts to add the specified to the + /// instance that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, records a exception instead. + /// + /// The key of the to add errors to. + /// The to add. + /// True if the error was added, false if the dictionary has already recorded + /// at least number of errors. + /// + /// This method only allows adding up to - 1. nt + /// invocation would result in adding a to the dictionary. + /// + public bool TryAddModelError([NotNull] string key, [NotNull] Exception exception) + { + if (ErrorCount >= MaxAllowedErrors - 1) + { + EnsureMaxErrorsReachedRecorded(); + return false; + } + + ErrorCount++; + AddModelErrorCore(key, exception); + return true; + } + + /// + /// Adds the specified to the instance + /// that is associated with the specified . + /// + /// The key of the to add errors to. + /// The error message to add. public void AddModelError([NotNull] string key, [NotNull] string errorMessage) { + TryAddModelError(key, errorMessage); + } + + /// + /// Attempts to add the specified to the + /// instance that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, records a exception instead. + /// + /// The key of the to add errors to. + /// The error message to add. + /// True if the error was added, false if the dictionary has already recorded + /// at least number of errors. + /// + /// This method only allows adding up to - 1. nt + /// invocation would result in adding a to the dictionary. + /// + public bool TryAddModelError([NotNull] string key, [NotNull] string errorMessage) + { + if (ErrorCount >= MaxAllowedErrors - 1) + { + EnsureMaxErrorsReachedRecorded(); + return false; + } + + ErrorCount++; var modelState = GetModelStateForKey(key); modelState.ValidationState = ModelValidationState.Invalid; modelState.Errors.Add(errorMessage); + + return true; } + /// + /// Returns the aggregate for items starting with the + /// specified . + /// + /// The key to look up model state errors for. + /// Returns if no entries are found for the specified + /// key, if at least one instance is found with one or more model + /// state errors; otherwise. public ModelValidationState GetFieldValidationState([NotNull] string key) { var entries = DictionaryHelper.FindKeysWithPrefix(this, key); @@ -105,6 +227,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return GetValidity(entries); } + /// + /// Marks the for the entry with the specified + /// as . + /// + /// The key of the to mark as valid. public void MarkFieldValid([NotNull] string key) { var modelState = GetModelStateForKey(key); @@ -116,6 +243,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding modelState.ValidationState = ModelValidationState.Valid; } + /// + /// Copies the values from the specified into this instance, overwriting + /// existing values if keys are the same. + /// + /// The to copy values from. public void Merge(ModelStateDictionary dictionary) { if (dictionary == null) @@ -129,6 +261,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } + /// + /// Sets the value for the with the specified to the + /// specified . + /// + /// The key for the entry. + /// The value to assign. public void SetModelValue([NotNull] string key, [NotNull] ValueProviderResult value) { GetModelStateForKey(key).Value = value; @@ -165,61 +303,88 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return validationState; } - #region IDictionary members + private void EnsureMaxErrorsReachedRecorded() + { + if (!HasRecordedMaxModelError) + { + var exception = new TooManyModelErrorsException(Resources.ModelStateDictionary_MaxModelStateErrors); + AddModelErrorCore(string.Empty, exception); + HasRecordedMaxModelError = true; + ErrorCount++; + } + } + + private void AddModelErrorCore(string key, Exception exception) + { + var modelState = GetModelStateForKey(key); + modelState.ValidationState = ModelValidationState.Invalid; + modelState.Errors.Add(exception); + } + + /// public void Add(KeyValuePair item) { Add(item.Key, item.Value); } + /// public void Add([NotNull] string key, [NotNull] ModelState value) { _innerDictionary.Add(key, value); } + /// public void Clear() { _innerDictionary.Clear(); } + /// public bool Contains(KeyValuePair item) { return _innerDictionary.Contains(item); } + /// public bool ContainsKey([NotNull] string key) { return _innerDictionary.ContainsKey(key); } + /// public void CopyTo([NotNull] KeyValuePair[] array, int arrayIndex) { _innerDictionary.CopyTo(array, arrayIndex); } + /// public bool Remove(KeyValuePair item) { return _innerDictionary.Remove(item); } + /// public bool Remove([NotNull] string key) { return _innerDictionary.Remove(key); } + /// public bool TryGetValue([NotNull] string key, out ModelState value) { return _innerDictionary.TryGetValue(key, out value); } + /// public IEnumerator> GetEnumerator() { return _innerDictionary.GetEnumerator(); } + /// IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); } - #endregion } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs index 8f6da0b7ee..feb331ddc9 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs @@ -394,6 +394,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return string.Format(CultureInfo.CurrentCulture, GetString("DataAnnotationsModelMetadataProvider_UnreadableProperty"), p0, p1); } + /// + /// The maximum number of allowed model errors has been reached. + /// + internal static string ModelStateDictionary_MaxModelStateErrors + { + get { return GetString("ModelStateDictionary_MaxModelStateErrors"); } + } + + /// + /// The maximum number of allowed model errors has been reached. + /// + internal static string FormatModelStateDictionary_MaxModelStateErrors() + { + return GetString("ModelStateDictionary_MaxModelStateErrors"); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx b/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx index 5f322e7d89..7d53d5b543 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx @@ -189,4 +189,7 @@ {0} has a DisplayColumn attribute for {1}, but property {1} does not have a public 'get' method. + + The maximum number of allowed model errors has been reached. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/TooManyModelErrorsException.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/TooManyModelErrorsException.cs new file mode 100644 index 0000000000..db88243b18 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/TooManyModelErrorsException.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; + +namespace Microsoft.AspNet.Mvc.ModelBinding +{ + /// + /// The that is thrown when too many model errors are encountered. + /// + public class TooManyModelErrorsException : Exception + { + /// + /// Creates a new instance of with the specified + /// exception . + /// + /// The message that describes the error. + public TooManyModelErrorsException([NotNull] string message) + : base(message) + { + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs index f49c2dad23..d81e209b20 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs @@ -102,9 +102,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public void Validate([NotNull] ModelValidationContext validationContext, ModelValidationNode parentNode) { - if (SuppressValidation) + if (SuppressValidation || !validationContext.ModelState.CanAddErrors) { - // no-op + // Short circuit if validation does not need to be applied or if we've reached the max number of validation errors. return; } @@ -171,7 +171,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { var thisErrorKey = ModelBindingHelper.CreatePropertyModelName(propertyKeyRoot, propertyResult.MemberName); - modelState.AddModelError(thisErrorKey, propertyResult.Message); + modelState.TryAddModelError(thisErrorKey, propertyResult.Message); } } } @@ -194,7 +194,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelMetadata.GetDisplayName()); if (parentNode == null && ModelMetadata.Model == null) { - modelState.AddModelError(modelStateKey, Resources.Validation_ValueNotFound); + modelState.TryAddModelError(modelStateKey, Resources.Validation_ValueNotFound); return; } @@ -207,7 +207,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { var currentModelStateKey = ModelBindingHelper.CreatePropertyModelName(ModelStateKey, validationResult.MemberName); - modelState.AddModelError(currentModelStateKey, validationResult.Message); + modelState.TryAddModelError(currentModelStateKey, validationResult.Message); } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/JsonInputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/JsonInputFormatterTest.cs index d60e520487..f1c5c2933a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/JsonInputFormatterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/JsonInputFormatterTest.cs @@ -144,6 +144,30 @@ namespace Microsoft.AspNet.Mvc actionContext.ModelState["Age"].Errors[0].Exception.Message); } + [Fact] + public async Task ReadAsync_UsesTryAddModelValidationErrorsToModelState_WhenCaptureErrorsIsSet() + { + // Arrange + var content = "{name: 'Person Name', Age: 'not-an-age'}"; + var formatter = new JsonInputFormatter { CaptureDeserilizationErrors = true }; + var contentBytes = Encoding.UTF8.GetBytes(content); + + var actionContext = GetActionContext(contentBytes); + var metadata = new EmptyModelMetadataProvider().GetMetadataForType(null, typeof(User)); + var context = new InputFormatterContext(actionContext, metadata.ModelType); + actionContext.ModelState.MaxAllowedErrors = 3; + actionContext.ModelState.AddModelError("key1", "error1"); + actionContext.ModelState.AddModelError("key2", "error2"); + + // Act + var model = await formatter.ReadAsync(context); + + // Assert + Assert.False(actionContext.ModelState.ContainsKey("age")); + var error = Assert.Single(actionContext.ModelState[""].Errors); + Assert.IsType(error.Exception); + } + private static ActionContext GetActionContext(byte[] contentBytes, string contentType = "application/xml") { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/MvcOptionsTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/MvcOptionsTests.cs index a03dd6b6f6..efcc3f2cbf 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/MvcOptionsTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/MvcOptionsTests.cs @@ -16,8 +16,19 @@ namespace Microsoft.AspNet.Mvc.Core.Test // Act & Assert var ex = Assert.Throws(() => options.AntiForgeryOptions = null); - Assert.Equal("The 'AntiForgeryOptions' property of 'Microsoft.AspNet.Mvc.MvcOptions' must not be null." + + Assert.Equal("The 'AntiForgeryOptions' property of 'Microsoft.AspNet.Mvc.MvcOptions' must not be null." + "\r\nParameter name: value", ex.Message); } + + [Fact] + public void MaxValidationError_ThrowsIfValueIsOutOfRange() + { + // Arrange + var options = new MvcOptions(); + + // Act & Assert + var ex = Assert.Throws(() => options.MaxModelValidationErrors = -1); + Assert.Equal("value", ex.ParamName); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs index 0faf10ca08..7792225df1 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/MvcRouteHandlerTests.cs @@ -10,6 +10,7 @@ using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Logging; +using Microsoft.Framework.OptionsModel; using Moq; using Xunit; @@ -18,7 +19,7 @@ namespace Microsoft.AspNet.Mvc public class MvcRouteHandlerTests { [Fact] - public async void RouteAsync_Success_LogsCorrectValues() + public async Task RouteAsync_Success_LogsCorrectValues() { // Arrange var sink = new TestSink(); @@ -56,7 +57,7 @@ namespace Microsoft.AspNet.Mvc } [Fact] - public async void RouteAsync_FailOnNoAction_LogsCorrectValues() + public async Task RouteAsync_FailOnNoAction_LogsCorrectValues() { // Arrange var sink = new TestSink(); @@ -100,7 +101,7 @@ namespace Microsoft.AspNet.Mvc } [Fact] - public async void RouteAsync_FailOnNoInvoker_LogsCorrectValues() + public async Task RouteAsync_FailOnNoInvoker_LogsCorrectValues() { // Arrange var sink = new TestSink(); @@ -144,10 +145,47 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(false, values.Handled); } + [Fact] + public async Task RouteAsync_SetsMaxErrorCountOnModelStateDictionary() + { + // Arrange + var expected = 199; + var optionsAccessor = new Mock>(); + var options = new MvcOptions + { + MaxModelValidationErrors = expected + }; + optionsAccessor.SetupGet(o => o.Options) + .Returns(options); + + var invoked = false; + var mockInvokerFactory = new Mock(); + mockInvokerFactory.Setup(f => f.CreateInvoker(It.IsAny())) + .Callback(c => + { + Assert.Equal(expected, c.ModelState.MaxAllowedErrors); + invoked = true; + }) + .Returns(Mock.Of()); + + var context = CreateRouteContext( + invokerFactory: mockInvokerFactory.Object, + optionsAccessor: optionsAccessor.Object); + + var handler = new MvcRouteHandler(); + + // Act + await handler.RouteAsync(context); + + // Assert + Assert.True(invoked); + } + private RouteContext CreateRouteContext( IActionSelector actionSelector = null, IActionInvokerFactory invokerFactory = null, - ILoggerFactory loggerFactory = null) + ILoggerFactory loggerFactory = null, + IOptionsAccessor optionsAccessor = null) { var mockContextAccessor = new Mock>(); mockContextAccessor.Setup(c => c.SetContextSource( @@ -185,6 +223,15 @@ namespace Microsoft.AspNet.Mvc loggerFactory = NullLoggerFactory.Instance; } + if (optionsAccessor == null) + { + var mockOptionsAccessor = new Mock>(); + mockOptionsAccessor.SetupGet(o => o.Options) + .Returns(new MvcOptions()); + + optionsAccessor = mockOptionsAccessor.Object; + } + var httpContext = new Mock(); httpContext.Setup(h => h.RequestServices.GetService(typeof(IContextAccessor))) .Returns(mockContextAccessor.Object); @@ -196,6 +243,8 @@ namespace Microsoft.AspNet.Mvc .Returns(loggerFactory); httpContext.Setup(h => h.RequestServices.GetService(typeof(IEnumerable))) .Returns(new List { new MvcMarkerService() }); + httpContext.Setup(h => h.RequestServices.GetService(typeof(IOptionsAccessor))) + .Returns(optionsAccessor); return new RouteContext(httpContext.Object); } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTests.cs index 4e6033df5e..6e8d99f9c5 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTests.cs @@ -2,10 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Linq; using System.Net; using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.TestHost; +using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNet.Mvc.FunctionalTests @@ -44,5 +47,48 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.Equal("\0", await response.Content.ReadAsStringAsync()); } + + [Fact] + public async Task ModelBinding_LimitsErrorsToMaxErrorCount() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + var queryString = string.Join("=&", Enumerable.Range(0, 10).Select(i => "field" + i)); + + // Act + var response = await client.GetStringAsync("http://localhost/Home/ModelWithTooManyValidationErrors?" + queryString); + + //Assert + var json = JsonConvert.DeserializeObject>(response); + // 8 is the value of MaxModelValidationErrors for the application being tested. + Assert.Equal(8, json.Count); + Assert.Equal("The Field1 field is required.", json["Field1.Field1"]); + Assert.Equal("The Field2 field is required.", json["Field1.Field2"]); + Assert.Equal("The Field3 field is required.", json["Field1.Field3"]); + Assert.Equal("The Field1 field is required.", json["Field2.Field1"]); + Assert.Equal("The Field2 field is required.", json["Field2.Field2"]); + Assert.Equal("The Field3 field is required.", json["Field2.Field3"]); + Assert.Equal("The Field1 field is required.", json["Field3.Field1"]); + Assert.Equal("The maximum number of allowed model errors has been reached.", json[""]); + } + + [Fact] + public async Task ModelBinding_ValidatesAllPropertiesInModel() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Act + var response = await client.GetStringAsync("http://localhost/Home/ModelWithFewValidationErrors?model="); + + //Assert + var json = JsonConvert.DeserializeObject>(response); + Assert.Equal(3, json.Count); + Assert.Equal("The Field1 field is required.", json["model.Field1"]); + Assert.Equal("The Field2 field is required.", json["model.Field2"]); + Assert.Equal("The Field3 field is required.", json["model.Field3"]); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs index cfe67de69b..608a799bd8 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs @@ -266,6 +266,37 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.Equal("Password does not meet complexity requirements.", error.ErrorMessage); } + [Fact] + public async Task BindModel_UsesTryAddModelError() + { + // Arrange + var validatorProvider = new DataAnnotationsModelValidatorProvider(); + var binder = CreateBinderWithDefaults(); + var valueProvider = new SimpleHttpValueProvider + { + { "user.password", "password" }, + { "user.confirmpassword", "password2" }, + }; + var bindingContext = CreateBindingContext(binder, valueProvider, typeof(User), validatorProvider); + bindingContext.ModelState.MaxAllowedErrors = 2; + bindingContext.ModelState.AddModelError("key1", "error1"); + bindingContext.ModelName = "user"; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + var modelState = bindingContext.ModelState["user.confirmpassword"]; + Assert.Empty(modelState.Errors); + + modelState = bindingContext.ModelState["user"]; + Assert.Empty(modelState.Errors); + + var error = Assert.Single(bindingContext.ModelState[""].Errors); + Assert.IsType(error.Exception); + } + + private static ModelBindingContext CreateBindingContext(IModelBinder binder, IValueProvider valueProvider, Type type, diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs index 176759bff0..72aca9c32c 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs @@ -26,6 +26,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var target = new ModelStateDictionary(source); // Assert + Assert.Equal(0, target.ErrorCount); Assert.Equal(1, target.Count); Assert.Same(modelState, target["key"]); Assert.IsType>(target.InnerDictionary); @@ -41,6 +42,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding dictionary.AddModelError("some key", "some error"); // Assert + Assert.Equal(1, dictionary.ErrorCount); var kvp = Assert.Single(dictionary); Assert.Equal("some key", kvp.Key); var error = Assert.Single(kvp.Value.Errors); @@ -59,6 +61,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding dictionary.AddModelError("some key", ex); // Assert + Assert.Equal(2, dictionary.ErrorCount); var kvp = Assert.Single(dictionary); Assert.Equal("some key", kvp.Key); @@ -365,6 +368,137 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Equal(ModelValidationState.Valid, validationState); } + [Fact] + public void AddModelError_WithErrorString_AddsTooManyModelErrors_WhenMaxErrorsIsReached() + { + // Arrange + var expected = "The maximum number of allowed model errors has been reached."; + var dictionary = new ModelStateDictionary + { + MaxAllowedErrors = 5 + }; + dictionary.AddModelError("key1", "error1"); + dictionary.AddModelError("key2", new Exception()); + dictionary.AddModelError("key3", new Exception()); + dictionary.AddModelError("key4", "error4"); + dictionary.AddModelError("key5", "error5"); + dictionary.AddModelError("key6", "error6"); + + // Act and Assert + Assert.False(dictionary.CanAddErrors); + Assert.Equal(5, dictionary.ErrorCount); + var error = Assert.Single(dictionary[""].Errors); + Assert.IsType(error.Exception); + Assert.Equal(expected, error.Exception.Message); + } + + [Fact] + public void TryAddModelError_WithErrorString_ReturnsFalse_AndAddsMaxModelErrorMessage() + { + // Arrange + var expected = "The maximum number of allowed model errors has been reached."; + var dictionary = new ModelStateDictionary + { + MaxAllowedErrors = 3 + }; + + // Act and Assert + var result = dictionary.TryAddModelError("key1", "error1"); + Assert.True(result); + + result = dictionary.TryAddModelError("key2", new Exception()); + Assert.True(result); + + result = dictionary.TryAddModelError("key3", "error3"); + Assert.False(result); + + result = dictionary.TryAddModelError("key4", "error4"); + Assert.False(result); + + Assert.False(dictionary.CanAddErrors); + Assert.Equal(3, dictionary.ErrorCount); + Assert.Equal(3, dictionary.Count); + + var error = Assert.Single(dictionary[""].Errors); + Assert.IsType(error.Exception); + Assert.Equal(expected, error.Exception.Message); + } + + [Fact] + public void AddModelError_WithException_AddsTooManyModelError_WhenMaxErrorIsReached() + { + // Arrange + var expected = "The maximum number of allowed model errors has been reached."; + var dictionary = new ModelStateDictionary + { + MaxAllowedErrors = 4 + }; + dictionary.AddModelError("key1", new Exception()); + dictionary.AddModelError("key2", "error2"); + dictionary.AddModelError("key3", "error3"); + dictionary.AddModelError("key3", new Exception()); + dictionary.AddModelError("key4", new InvalidOperationException()); + dictionary.AddModelError("key5", new FormatException()); + + // Act and Assert + Assert.False(dictionary.CanAddErrors); + Assert.Equal(4, dictionary.ErrorCount); + Assert.Equal(4, dictionary.Count); + var error = Assert.Single(dictionary[""].Errors); + Assert.IsType(error.Exception); + Assert.Equal(expected, error.Exception.Message); + } + + [Fact] + public void TryAddModelError_WithException_ReturnsFalse_AndAddsMaxModelErrorMessage() + { + // Arrange + var expected = "The maximum number of allowed model errors has been reached."; + var dictionary = new ModelStateDictionary + { + MaxAllowedErrors = 3 + }; + + // Act and Assert + var result = dictionary.TryAddModelError("key1", "error1"); + Assert.True(result); + + result = dictionary.TryAddModelError("key2", new Exception()); + Assert.True(result); + + result = dictionary.TryAddModelError("key3", new Exception()); + Assert.False(result); + + Assert.Equal(3, dictionary.Count); + var error = Assert.Single(dictionary[""].Errors); + Assert.IsType(error.Exception); + Assert.Equal(expected, error.Exception.Message); + } + + [Fact] + public void ModelStateDictionary_TracksAddedErrorsOverCopyConstructor() + { + // Arrange + var expected = "The maximum number of allowed model errors has been reached."; + var dictionary = new ModelStateDictionary + { + MaxAllowedErrors = 3 + }; + + // Act + dictionary.AddModelError("key1", "error1"); + dictionary.TryAddModelError("key3", new Exception()); + + var copy = new ModelStateDictionary(dictionary); + copy.AddModelError("key2", "error2"); + + // Assert + Assert.Equal(3, copy.Count); + var error = Assert.Single(copy[""].Errors); + Assert.IsType(error.Exception); + Assert.Equal(expected, error.Exception.Message); + } + private static ValueProviderResult GetValueProviderResult(object rawValue = null, string attemptedValue = null) { return new ValueProviderResult(rawValue ?? "some value", diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs index e3b9b06d7c..7c3b1f988e 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs @@ -278,6 +278,39 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.False(context.ModelState.ContainsKey("theKey")); } + [Fact] + [ReplaceCulture] + public void Validate_ShortCircuits_IfModelStateHasReachedMaxNumberOfErrors() + { + // Arrange + var model = new ValidateAllPropertiesModel + { + RequiredString = null /* error */, + RangedInt = 0 /* error */, + ValidString = "cat" /* error */ + }; + + var modelMetadata = GetModelMetadata(model); + var node = new ModelValidationNode(modelMetadata, "theKey") + { + ValidateAllProperties = true + }; + var context = CreateContext(modelMetadata); + context.ModelState.MaxAllowedErrors = 3; + context.ModelState.AddModelError("somekey", "error text"); + + // Act + node.Validate(context); + + // Assert + Assert.Equal(3, context.ModelState.Count); + Assert.IsType(context.ModelState[""].Errors[0].Exception); + Assert.Equal("The RequiredString field is required.", + context.ModelState["theKey.RequiredString"].Errors[0].ErrorMessage); + Assert.False(context.ModelState.ContainsKey("theKey.RangedInt")); + Assert.False(context.ModelState.ContainsKey("theKey.ValidString")); + } + private static ModelMetadata GetModelMetadata() { return new EmptyModelMetadataProvider().GetMetadataForType(null, typeof(object)); diff --git a/test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs b/test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs index b4abf7e023..5284020409 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs @@ -1,7 +1,10 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using Microsoft.AspNet.Mvc; +using System.Linq; +using ModelBindingWebSite.Models; namespace ModelBindingWebSite.Controllers { @@ -12,5 +15,32 @@ namespace ModelBindingWebSite.Controllers { return Content(System.Text.Encoding.UTF8.GetString(byteValues)); } + + public object ModelWithTooManyValidationErrors(LargeModelWithValidation model) + { + return CreateValidationDictionary(); + } + + public object ModelWithFewValidationErrors(ModelWithValidation model) + { + return CreateValidationDictionary(); + } + + private Dictionary CreateValidationDictionary() + { + var result = new Dictionary(); + foreach (var item in ModelState) + { + var error = item.Value.Errors.SingleOrDefault(); + if (error != null) + { + var value = error.Exception != null ? error.Exception.Message : + error.ErrorMessage; + result.Add(item.Key, value); + } + } + + return result; + } } } \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Models/LargeModelWithValidation.cs b/test/WebSites/ModelBindingWebSite/Models/LargeModelWithValidation.cs new file mode 100644 index 0000000000..62d08e2df1 --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Models/LargeModelWithValidation.cs @@ -0,0 +1,22 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.ComponentModel.DataAnnotations; + +namespace ModelBindingWebSite.Models +{ + public class LargeModelWithValidation + { + [Required] + public ModelWithValidation Field1 { get; set; } + + [Required] + public ModelWithValidation Field2 { get; set; } + + [Required] + public ModelWithValidation Field3 { get; set; } + + [Required] + public ModelWithValidation Field4 { get; set; } + } +} \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Models/ModelWithValidation.cs b/test/WebSites/ModelBindingWebSite/Models/ModelWithValidation.cs new file mode 100644 index 0000000000..51c2a4f0e3 --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Models/ModelWithValidation.cs @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.ComponentModel.DataAnnotations; + +namespace ModelBindingWebSite +{ + public class ModelWithValidation + { + [Required] + public string Field1 { get; set; } + + [Required] + public string Field2 { get; set; } + + [Required] + public string Field3 { get; set; } + } +} \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Startup.cs b/test/WebSites/ModelBindingWebSite/Startup.cs index e5436b71c7..52546cf15b 100644 --- a/test/WebSites/ModelBindingWebSite/Startup.cs +++ b/test/WebSites/ModelBindingWebSite/Startup.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNet.Mvc; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; @@ -17,7 +18,11 @@ namespace ModelBindingWebSite app.UseServices(services => { // Add MVC services to the services container - services.AddMvc(configuration); + services.AddMvc(configuration) + .SetupOptions(m => + { + m.MaxModelValidationErrors = 8; + }); }); // Add MVC to the request pipeline