Validation of required non-nullable model properties

[mpetito]

Is this a Bug or Feature request?:

Bug as it seems to contradict official documentation and is surprising behavior.

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

Create a new project dotnet new webapi and modify a controller as follows:

    [Route("api/[controller]")]
    public class ValuesController : Controller
    {
        [HttpPost]
        public IActionResult Post([FromBody] TestRequest request)
        {
            return Ok(new {ModelState.IsValid, request?.Value});
        }
    }

    public class TestRequest
    {
        [Required, BindRequired]
        public int Value { get; set; }
    }

Description of the problem:

"Required" non-nullable properties will never raise a corresponding model validation error if the property is entirely omitted from the request body. Given the following request body sent to the Post action:

{ }

The controller responds with:

{
    "isValid": true,
    "value": 0
}

Per the docs for binding and similarly the docs for validation it is unexpected that a missing property in the json passes validation.

[BindRequired]: This attribute adds a model state error if binding cannot occur.
[Required]: Makes a property required.

It also appears to be difficult to replace the built-in convention and achieve the desired behavior (which is a validation error if a required or binding required property is omitted).

One of the top search results for this issue is this blog post where the author explains a workaround which no longer works:

public class RequiredBindingMetadataProvider : IBindingMetadataProvider
{
    public void GetBindingMetadata(BindingMetadataProviderContext context)
    {
        if (context.PropertyAttributes.OfType<RequiredAttribute>().Any())
        {
            context.BindingMetadata.IsBindingRequired = true;
        }
    }
}

Code within the default model binders declare that binding is successful if the property is non-nullable and the bound value is null:

Mvc/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs

Lines 141 to 151 in c922b0b

	if (model == null && !bindingContext.ModelMetadata.IsReferenceOrNullableType)
	{
	bindingContext.ModelState.TryAddModelError(
	bindingContext.ModelName,
	bindingContext.ModelMetadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor(
	valueProviderResult.ToString()));
	}
	else
	{
	bindingContext.Result = ModelBindingResult.Success(model);
	}

Similarly, it seems none of the usual hooks for custom validation can tell if the value was bound to a value in the request or if it just assumed the default value for the non-nullable type. I've explored:

• Using IModelValidatorProvider / IModelValidator to check the ModelValidationContext (the context has a Model value of the assumed default 0).
• Using IBindingMetadataProvider to make sure IsBindingRequired is true (as per the above example).
• Using IValidationMetadataProvider to make sure IsRequired is true (no effect).

The only work-around I've found is to change the property to a nullable type (int?). This seems like a poor solution since:

• Usage of the model property is more verbose: Value.Value.
• Static analysis tools will identify this usage as a possible NRE.

This has also been discussed in issues #6825, #5391, and #3971, but none provide a resolution.

Is there any way to achieve this desired behavior by convention, and should this not be the default behavior?

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

I've tried this on both:

• 2.1.0-rc1-final
• 2.0.5

[mkArtakMSFT]

Thanks for contacting us, @mpetito.
@dougbu , can you please look into this. I remember we did a bit of work in this area recently.

[dougbu]

This is not a problem we can solve.

[BindRequired] is a feature inextricably tied to the MVC model binding system and none of the supported input formatters supports it. But for JSON.NET, use [JsonProperty(Required = true)] instead.

[Required] is a .NET attribute with very specific, documented semantics:

A validation exception is raised if the property is null, contains an empty string (""), or contains only white-space characters.

[Tragetaschen]

I've spent the better part of the day running through most of @mpetito steps above (while introducing [ApiController]). I think if anything, the documentation for BindRequiredAttribute should be updated to call out the limitations and workaround @dougbu mentioned. The distinction between model binding and input formatters is far from obvious until you start to really dig into this.

IMHO, the TestRequest model above really should fail validation when MVC handles posted JSON of {}.