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

ModelState Errors for FormatException are not converted into user friendly (invalid) messages. #1728

Closed
TalalTayyab opened this issue Dec 17, 2014 · 3 comments
Assignees
Milestone

Comments

@TalalTayyab
Copy link

This happen when using model binding in Asp.Net MVC 6

If you have a datetime property (e.g. Birthday) and you bind it to an input element. when you enter an invalid value for the date time (e.g. abc), the binding process adds a model state error with the message

The parameter conversion from type 'System.String' to type 'System.DateTime' failed. See the inner exception for more information

This message gets displayed to the user as well (using the HtmlHelper ValidationMessageFor)

The error is thrown in class ValueProviderResult, function ConvertSimpleType - where a FormatException is wrapped and rethrown as a InvalidOperationException. This gets added to the ModelState error in the class TypeConverterModelBinder.

In MVC 4, the same Exception is thrown by ValueProviderResult but it is processed down in the chain and changed to

The value 'abc' is not valid for Birthday

This happens in class DefaultModelBinder.cs , function BindProperty. The code to transform the errors is:

// Convert FormatExceptions (type conversion failures) into InvalidValue messages
                foreach (ModelError error in modelState.Errors.Where(err => String.IsNullOrEmpty(err.ErrorMessage) && err.Exception != null).ToList())
                {
                    for (Exception exception = error.Exception; exception != null; exception = exception.InnerException)
                    {
                        // We only consider "known" type of exception and do not make too aggressive changes here
                        if (exception is FormatException || exception is OverflowException)
                        {
                            string displayName = propertyMetadata.GetDisplayName();
                            string errorMessageTemplate = GetValueInvalidResource(controllerContext);
                            string errorMessage = String.Format(CultureInfo.CurrentCulture, errorMessageTemplate, modelState.Value.AttemptedValue, displayName);
                            modelState.Errors.Remove(error);
                            modelState.Errors.Add(errorMessage);
                            break;
                        }
                    }
                }

Something similar should also be added for vNext - to make it more user friendly.

@rynowak
Copy link
Member

rynowak commented Dec 29, 2014

@TalalTayyab thanks for the detailed report, we'll take a look at this.

/cc @pranavkm

@rynowak rynowak added the bug label Dec 29, 2014
@rynowak
Copy link
Member

rynowak commented Dec 31, 2014

A little more data here, as the model binding system in MVC 6 is based on WebAPI 2 and not MVC 4/5:

In legacy WebAPI this code when through the 'user-error-message-provider' extensibility point, ultimately falling back to a generic error message.

We got rid of that in MVC 6, but instead we're just taking the message from the exception, which is the cause of the regression. https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs#L97

@ajaybhargavb
Copy link
Contributor

5e704cd

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants