Validation occurs against declared type rather than real type

[Materix]

Is this a Bug or Feature request?:

Bug - Probably regression

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

Repo with example project on: https://github.com/Materix/ValidationInterface

Send POST request on api/fruit with {"type":"orange"} as body and Content-Type: application/json

Expected result: ModelState.IsValid = false
Result: ModelState.IsValid = true

CompatibilityVersion is set to Version_2_1.
Without setting all works correctly
On 2.0.0. all works correctly.

Description of the problem:

I have interface of model class (IFruit) from which two class derived (Apple, Orange). I have custom JsonConventer which determine proper type bases on one value from json. All validation attributes were in individual implementation. When I bind to IFruit ModelState.IsValid always returns true.

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.Mvc: 2.1.1

Images:

[pranavkm]

Looks like this is a result of the validation changes in 2.1. Specifically, with a 2.0 compatibility setting, ModelMetadata is calculated over the actual model type -
https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ObjectModelValidator.cs#L59

In 2.1, we pass the ModelMetadata of the declared model type -
https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs#L296-L301

Not entirely sure what the intended behavior needs to be. A previous issue suggested that we are meant to look at the declared model type - #6466 (comment). @dougbu thoughts?

@Materix you should be able to retain the older behavior by turning off validation for top level nodes MvcOptions.AllowValidatingTopLevelNodes. It does mean validation attributes on top level properties and controller parameters will not apply, but it does unblock this specific scenario in your case.

[dougbu]

@pranavkm this looks like an exception to the overall declared type behaviour of the validation system. Unfortunately, removing this exception -- whether intended or not -- introduced a regression.

[Materix]

So, the second option is to write own ModelBinder f.e. (already used to bind from json-formatted form field as [FromForm(Name = "name"), ModelBinder(typeof(JsonModelBinder))])

    public class JsonModelBinder : IModelBinder
    {
        private readonly IOptions<MvcJsonOptions> _jsonOptions;

        public JsonModelBinder(IOptions<MvcJsonOptions> jsonOptions)
        {
            _jsonOptions = jsonOptions;
        }

        public async Task BindModelAsync(ModelBindingContext bindingContext)
        {
            var name = bindingContext.ModelName ?? bindingContext.FieldName;
            var valueProviderResult = bindingContext.ValueProvider.GetValue(name);
            if (valueProviderResult == ValueProviderResult.None)
            {
                return;
            }

            bindingContext.ModelState.SetModelValue(name, valueProviderResult);

            var value = await Task.Factory.StartNew(
                            () => JsonConvert.DeserializeObject(valueProviderResult.FirstValue, bindingContext.ModelType, _jsonOptions.Value.SerializerSettings));

            
            bindingContext.Result = ModelBindingResult.Success(value);
        }
    }

and update field bindingContext.ModelMetadata by creating new metadata for real type?

[pranavkm]

@Materix haven't tried your suggested change, but I don't think it'd work. ParameterBinder does not use ModelBindingContext.ModelMetadata during validation (it uses the original value passed in to the BindAsync method). Seeing this is a regression, I'll see if we can get a fix for this in an upcoming patch.