When submitting empty value types, ModelState has a NullRef exception

[sornaks]

The view looks like this:

<form asp-action="Create" asp-controller="Home">
@Html.EditorForModel()
<input type="submit" value="ok" />
</form>

The Model is this:

public class Person
{
    [Required]
    public string Name { get; set; }
    public decimal Height { get; set; }
    public double Weight { get; set; }
    public float BMI { get; set; }  
    [Range(1, 40)]
    public short Age { get; set; }
    public ushort IsWorking { get; set; }
}

NullRef is thrown for - Height, Weight etc. This is the stack trace in mono:

"  at Microsoft.Framework.Internal.PropertyHelper.CallPropertySetter[Person,Decimal] (System.Action`2 setter, System.Object target, System.Object value) [0x00000] in <filename unknown>:0 \n  
at Microsoft.AspNet.Mvc.ModelBinding.MutableObjectModelBinder.SetProperty (Microsoft.AspNet.Mvc.ModelBinding.ModelBindingContext bindingContext, Microsoft.AspNet.Mvc.ModelBinding.ModelExplorer modelExplorer, Microsoft.AspNet.Mvc.ModelBinding.ModelMetadata propertyMetadata, Microsoft.AspNet.Mvc.ModelBinding.ModelBindingResult dtoResult) [0x00000] in <filename unknown>:0 "

[sornaks]

This is the exception in CLR (Windows):

at Microsoft.Framework.Internal.PropertyHelper.CallPropertySetter[TDeclaringType,TValue](Action`2 setter, Object target, Object value)\r\n   
at Microsoft.AspNet.Mvc.ModelBinding.MutableObjectModelBinder.SetProperty(ModelBindingContext bindingContext, ModelExplorer modelExplorer, ModelMetadata propertyMetadata, ModelBindingResult dtoResult)

[dougbu]

Thanks very much for filing this bug @sornaks! This bug uncovers at least 3 intertwined issues, most of which are regressions from MVC 5:

1. If the submission contains an explicit null or empty value for a ValueType property, TypeConverterModelBinder will return a ValueProviderResult with IsModelSet == true && Model == null. This combination does not match MutableObjectModelBinder's expectations and leads to the NRE. Should at least special-case the TypeConverterModelBinder to leave IsModelSet == false when the converted value is null.
2. The properties in the Person model are automatically treated as if they had [Required] attributes i.e. modelMetadata.IsRequired is true for them all. That metadata property causes additional client-side validation but does not change server-side behaviour. If (1) is fixed, the result would be no ModelState errors for ValueType properties bound to null when client-side validation is disabled. Should change the current metadata special case to set IsBindingRequired instead of IsRequired. That would restore the ModelState errors seen in MVC 5 e.g. "The Height field is required." (More generally, the description of [Mvc 5.0 Compat] Null values returned by value providers/model binders for value type properties should not create a model state error. #2423 was incorrect at least for ValueType properties not using a greedy model binder and Consider adding a server validator for value types #2601 was important.)
3. We have few tests with query strings such as ?weight=&height= or equivalent body content; likely none involving ValueType properties. Need to add these and confirm the behaviour matches MVC 5 after (1) and (2) are fixed.

[dougbu]

/cc @Eilon @rynowak we made some incorrect assumptions in recent round of model binding changes