diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs index 090c624162..6c8bcabf60 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs @@ -42,7 +42,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } catch (Exception ex) { - ModelBindingHelper.AddModelErrorBasedOnExceptionType(bindingContext, ex); + bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex); } return true; diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs index 800f14567f..8a02d22683 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs @@ -35,7 +35,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } catch (Exception ex) { - ModelBindingHelper.AddModelErrorBasedOnExceptionType(bindingContext, ex); + bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex); } return true; diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs index 8ef8510508..37ea739747 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs @@ -96,30 +96,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Internal } } - internal static void AddModelErrorBasedOnExceptionType(ModelBindingContext bindingContext, Exception ex) - { - if (IsFormatException(ex)) - { - bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex.Message); - } - else - { - bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex); - } - } - - internal static bool IsFormatException(Exception ex) - { - for (; ex != null; ex = ex.InnerException) - { - if (ex is FormatException) - { - return true; - } - } - return false; - } - public static object ConvertValuesToCollectionType(Type modelType, IList values) { // There's a limited set of collection types we can support here. diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs index 07caea2a4f..90530144ab 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs @@ -202,6 +202,26 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return false; } + if (exception is FormatException) + { + // Convert FormatExceptions to Invalid value messages. + ModelState modelState; + TryGetValue(key, out modelState); + string errorMessage; + if (modelState == null) + { + errorMessage = Resources.FormatModelBinderUtil_ValueInvalidGeneric(key); + } + else + { + errorMessage = Resources.FormatModelBinderUtil_ValueInvalid( + modelState.Value.AttemptedValue, + key); + } + + return TryAddModelError(key, errorMessage); + } + ErrorCount++; AddModelErrorCore(key, exception); return true; diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs index 5ba29cced0..7a071820bf 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs @@ -362,22 +362,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return string.Format(CultureInfo.CurrentCulture, GetString("ValueProviderResult_CannotConvertEnum"), p0, p1); } - /// - /// The parameter conversion from type '{0}' to type '{1}' failed. See the inner exception for more information. - /// - internal static string ValueProviderResult_ConversionThrew - { - get { return GetString("ValueProviderResult_ConversionThrew"); } - } - - /// - /// The parameter conversion from type '{0}' to type '{1}' failed. See the inner exception for more information. - /// - internal static string FormatValueProviderResult_ConversionThrew(object p0, object p1) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ValueProviderResult_ConversionThrew"), p0, p1); - } - /// /// The parameter conversion from type '{0}' to type '{1}' failed because no type converter can convert between these types. /// @@ -458,6 +442,38 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return string.Format(CultureInfo.CurrentCulture, GetString("BinderType_MustBeIModelBinderOrIModelBinderProvider"), p0, p1, p2); } + /// + /// The value '{0}' is not valid for {1}. + /// + internal static string ModelBinderUtil_ValueInvalid + { + get { return GetString("ModelBinderUtil_ValueInvalid"); } + } + + /// + /// The value '{0}' is not valid for {1}. + /// + internal static string FormatModelBinderUtil_ValueInvalid(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ValueInvalid"), p0, p1); + } + + /// + /// The supplied value is invalid for {0}. + /// + internal static string ModelBinderUtil_ValueInvalidGeneric + { + get { return GetString("ModelBinderUtil_ValueInvalidGeneric"); } + } + + /// + /// The supplied value is invalid for {0}. + /// + internal static string FormatModelBinderUtil_ValueInvalidGeneric(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ValueInvalidGeneric"), p0); + } + 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 347d0fe052..dbb8698d8f 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx @@ -183,9 +183,6 @@ Cannot convert value '{0}' to enum type '{1}'. - - The parameter conversion from type '{0}' to type '{1}' failed. See the inner exception for more information. - The parameter conversion from type '{0}' to type '{1}' failed because no type converter can convert between these types. @@ -201,4 +198,10 @@ The type '{0}' must implement either '{1}' or '{2}' to be used as a model binder. + + The value '{0}' is not valid for {1}. + + + The supplied value is invalid for {0}. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs index b2d25afe12..38cd7e6b16 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs @@ -6,6 +6,7 @@ using System.Collections; using System.ComponentModel; using System.Globalization; using System.Reflection; +using System.Runtime.ExceptionServices; namespace Microsoft.AspNet.Mvc.ModelBinding { @@ -162,8 +163,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } catch (Exception ex) { - throw new InvalidOperationException( - Resources.FormatValueProviderResult_ConversionThrew(value.GetType(), destinationType), ex); + if (ex is FormatException) + { + throw ex; + } + else + { + // TypeConverter throws System.Exception wrapping the FormatException, + // so we throw the inner exception. + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + + // this code is never reached because the previous line is throwing; + throw; + } } } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/Compiler/Resources/TagHelpersWebSite.Employee.Create.Invalid.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/Compiler/Resources/TagHelpersWebSite.Employee.Create.Invalid.html index 11b09a7ed1..ad08955122 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/Compiler/Resources/TagHelpersWebSite.Employee.Create.Invalid.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/Compiler/Resources/TagHelpersWebSite.Employee.Create.Invalid.html @@ -19,7 +19,7 @@
  • The field Age must be between 10 and 100.
    • -
  • The parameter conversion from type 'System.String' to type 'System.Int32' failed. See the inner exception for more information.
    • +
  • The value 'z' is not valid for Salary.
  • The JoinDate field is required.
@@ -63,7 +63,7 @@
- The parameter conversion from type 'System.String' to type 'System.Int32' failed. See the inner exception for more information. + The value 'z' is not valid for Salary.
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index 67107569b4..af986d4cf4 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -1529,5 +1529,27 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var body = await response.Content.ReadAsStringAsync(); Assert.Equal(expectedContent, body); } + + public async Task ModelBinder_FormatsDontMatch_ThrowsUserFriendlyException() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + var url = "http://localhost/Home/GetErrorMessage"; + + var nameValueCollection = new List> + { + new KeyValuePair("birthdate", "random string"), + }; + var formData = new FormUrlEncodedContent(nameValueCollection); + + // Act + var response = await client.PostAsync(url, formData); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var result = await response.Content.ReadAsStringAsync(); + Assert.Equal("The value 'random string' is not valid for birthdate.", result); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs index 14cd56fbfe..c13af2c32b 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs @@ -61,8 +61,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Arrange var expected = TestPlatformHelper.IsMono ? "Invalid length." : - "The input is not a valid Base-64 string as it contains a non-base 64 character," + - " more than two padding characters, or an illegal character among the padding characters. "; + "The supplied value is invalid for foo."; var valueProvider = new SimpleHttpValueProvider() { diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs index d8910b277c..d69ac40bb6 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs @@ -71,8 +71,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public async Task BindModel_Error_FormatExceptionsTurnedIntoStringsInModelState() { // Arrange - var message = "The parameter conversion from type 'System.String' to type 'System.Int32' failed." + - " See the inner exception for more information."; + var message = "The value 'not an integer' is not valid for theModelName."; var bindingContext = GetBindingContext(typeof(int)); bindingContext.ValueProvider = new SimpleHttpValueProvider { diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs index c82d8d98ae..cb7d4897cb 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs @@ -538,6 +538,51 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Equal(expected, canAdd); } + [Fact] + public void ModelStateDictionary_ReturnGenericErrorMessage_WhenModelStateNotSet() + { + // Arrange + var expected = "The supplied value is invalid for key."; + var dictionary = new ModelStateDictionary(); + + // Act + dictionary.TryAddModelError("key", new FormatException()); + + // Assert + var error = Assert.Single(dictionary["key"].Errors); + Assert.Equal(expected, error.ErrorMessage); + } + + [Fact] + public void ModelStateDictionary_ReturnSpecificErrorMessage_WhenModelStateSet() + { + // Arrange + var expected = "The value 'some value' is not valid for key."; + var dictionary = new ModelStateDictionary(); + dictionary.SetModelValue("key", GetValueProviderResult()); + + // Act + dictionary.TryAddModelError("key", new FormatException()); + + // Assert + var error = Assert.Single(dictionary["key"].Errors); + Assert.Equal(expected, error.ErrorMessage); + } + + [Fact] + public void ModelStateDictionary_NoErrorMessage_ForNonFormatException() + { + // Arrange + var dictionary = new ModelStateDictionary(); + dictionary.SetModelValue("key", GetValueProviderResult()); + + // Act + dictionary.TryAddModelError("key", new InvalidOperationException()); + + // Assert + var error = Assert.Single(dictionary["key"].Errors); + Assert.Empty(error.ErrorMessage); + } private static ValueProviderResult GetValueProviderResult(object rawValue = null, string attemptedValue = null) { diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs index dc850da4ec..f878f82d2f 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs @@ -339,17 +339,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Theory] - [InlineData(typeof(int), typeof(InvalidOperationException), typeof(Exception))] - [InlineData(typeof(double?), typeof(InvalidOperationException), typeof(Exception))] - [InlineData(typeof(MyEnum?), typeof(InvalidOperationException), typeof(FormatException))] - public void ConvertToThrowsIfConverterThrows(Type destinationType, Type exceptionType, Type innerExceptionType) + [InlineData(typeof(int))] + [InlineData(typeof(double?))] + [InlineData(typeof(MyEnum?))] + public void ConvertToThrowsIfConverterThrows(Type destinationType) { // Arrange var vpr = new ValueProviderResult("this-is-not-a-valid-value", null, CultureInfo.InvariantCulture); // Act & Assert - var ex = Assert.Throws(exceptionType, () => vpr.ConvertTo(destinationType)); - Assert.IsType(innerExceptionType, ex.InnerException); + var ex = Assert.Throws(typeof(FormatException), () => vpr.ConvertTo(destinationType)); } [Fact] @@ -380,7 +379,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Assert Assert.Equal(12.5M, cultureResult); - Assert.Throws(() => vpr.ConvertTo(typeof(decimal))); + Assert.Throws(() => vpr.ConvertTo(typeof(decimal))); } [Fact] diff --git a/test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs b/test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs index 2ff33600fa..37f8b13bbf 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/HomeController.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 System; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -59,6 +60,11 @@ namespace ModelBindingWebSite.Controllers return customer; } + public IActionResult GetErrorMessage(DateTime birthdate) + { + return Content(ModelState[nameof(birthdate)].Errors[0].ErrorMessage); + } + private Customer CreateCustomer(int id) { return new Customer()