ModelState.IsValid even when it should not be?

Question

I have API where I need to validate my user model. I choose an approach where I create different classes for Create/Edit actions to avoid mass-assignment and divide validation and actual model apart.

I don't know why but ModelState.IsValid returns true even when it should not. Am I doing something wrong?

Controller

public HttpResponseMessage Post(UserCreate user)
{
    if (ModelState.IsValid) // It's valid even when user = null
    {
        var newUser = new User
        {
            Username = user.Username,
            Password = user.Password,
            Name = user.Name
        };
        _db.Users.Add(newUser);
        _db.SaveChanges();
        return Request.CreateResponse(HttpStatusCode.Created, new { newUser.Id, newUser.Username, newUser.Name });
    }
    return Request.CreateErrorResponse(HttpStatusCode.BadRequest, ModelState);
}

Model

public class UserCreate
{
    [Required]
    public string Username { get; set; }
    [Required]
    public string Password { get; set; }
    [Required]
    public string Name { get; set; }
}

Debug proof

Hakenkreuz

Be sure that the Html Input names you have provided in your view should match your model properties. i.e. names of the input element on view should be "Username", "Password", "Name". it could be the reason that your user object is null which resulted into ModelState.IsValid – Coulson

@KD It's not using HTML, it's expecting JSON since it's API, not a website. But yea, I get your point - even if I send empty JSON object it will work. – Hakenkreuz

Hi @Steve : Can you show the View ? – Recurved

@PKKG There is no view, it's API. Anyway the problem occurs when I send nothing. I need to send at-minimum empty json object. – Hakenkreuz

You need to put [Required] before UserCreate – Misdemean

Answer 1

The ModelState.IsValid internally checks the Values.All(modelState => modelState.Errors.Count == 0) expression.

Because there was no input the Values collection will be empty so ModelState.IsValid will be true.

So you need to explicitly handle this case with:

if (user != null && ModelState.IsValid)
{

}

Whether this is a good or bad design decision that if you validate nothing it will true is a different question...

Madson

But how can I avoid it then? I thought cases like these are built-in by default. Indeed, when I send empty JSON object it shows errors. Very strange.. – Hakenkreuz

I don't think that the framework should handle this case for you, because this is valid scenario that a parameter is null. You can create an actionfilter for example that handles that the parameter cannot be null... – Madson

I will just implement some global filter that will check if it's valid and it's null so I won't even have to write that in my controllers. Thanks for answer! – Hakenkreuz

@Steve: By any chance, did you manage to write a filter that could check if the model is null as well as a valid one ? – Allene

@Madson I'm not sure I agree with you. You're asking for a specific model and if you get null in place of that model, it's NOT the model. So, how could the model be valid if it doesn't exist? – Cordon

Throwing badrequest(modelstate) from within this "if" will result in the error: "the modelstate is valid".. which is a very message for an error imho – Opinionated

@Madson where can I get the isValid method imeplementation code. how do you know isValid internally check Values.All(modelState => modelState.Errors.Count == 0) expression. – Merilyn

@MannanBahelim ASP.NET MVC/Web.api is open source, you can find the source code on github: github.com/aspnet/AspNetWebStack to code is in the question can be found here: github.com/aspnet/AspNetWebStack/blob/master/src/… – Madson

Answer 2

Here is an action filter to check for null models or invalid models. (so you dont have to write the check on every action)

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Web.Http.Controllers;
using System.Web.Http.Filters;

namespace Studio.Lms.TrackingServices.Filters
{
    public class ValidateViewModelAttribute : ActionFilterAttribute
    {
        public override void OnActionExecuting(HttpActionContext actionContext)
        {
            if (actionContext.ActionArguments.Any(kv => kv.Value == null)) {
                actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, "Arguments cannot be null");
            }

            if (actionContext.ModelState.IsValid == false) {
                actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, actionContext.ModelState);
            }
        }
    }
}

You can register it globally:

config.Filters.Add(new ValidateViewModelAttribute());

Or use it on demand on classes/actions

 [ValidateViewModel]
 public class UsersController : ApiController
 { ...

Boat

This returns Bad Request if the action has optional parameters, for example Get(string id, string something = null). – Nimble

Answer 3

I wrote a custom filter which not only ensures that all non optional object properties are passed, but also checks if model state is valid:

[AttributeUsage (AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false)]
public sealed class ValidateModelAttribute : ActionFilterAttribute
{
    private static readonly ConcurrentDictionary<HttpActionDescriptor, IList<string>> NotNullParameterNames =
        new ConcurrentDictionary<HttpActionDescriptor, IList<string>> ();


    /// <summary>
    /// Occurs before the action method is invoked.
    /// </summary>
    /// <param name="actionContext">The action context.</param>
    public override void OnActionExecuting (HttpActionContext actionContext)
    {
        var not_null_parameter_names = GetNotNullParameterNames (actionContext);
        foreach (var not_null_parameter_name in not_null_parameter_names)
        {
            object value;
            if (!actionContext.ActionArguments.TryGetValue (not_null_parameter_name, out value) || value == null)
                actionContext.ModelState.AddModelError (not_null_parameter_name, "Parameter \"" + not_null_parameter_name + "\" was not specified.");
        }


        if (actionContext.ModelState.IsValid == false)
            actionContext.Response = actionContext.Request.CreateErrorResponse (HttpStatusCode.BadRequest, actionContext.ModelState);
    }


    private static IList<string> GetNotNullParameterNames (HttpActionContext actionContext)
    {
        var result = NotNullParameterNames.GetOrAdd (actionContext.ActionDescriptor,
                                                     descriptor => descriptor.GetParameters ()
                                                                             .Where (p => !p.IsOptional && p.DefaultValue == null &&
                                                                                          !p.ParameterType.IsValueType &&
                                                                                          p.ParameterType != typeof (string))
                                                                             .Select (p => p.ParameterName)
                                                                             .ToList ());

        return result;
    }
}

And I put it in global filter for all Web API actions:

config.Filters.Add (new ValidateModelAttribute ());

Donetsk

This does Not display model error when an object's required property is missing. I debugged the GetNotNullParameterNames and it only returns the model itself and checks if it's null. It doesn't look at individual model members. – Bac

@Bac If the model is null, I don't think it makes sense to return individual model validation errors. If the model has a value, but some of it's members are invalid, the above filter will correctly return the invalid errors. – Crustaceous

Answer 4

Updated slightly for asp.net core...

[AttributeUsage(AttributeTargets.Method)]
public sealed class CheckRequiredModelAttribute : ActionFilterAttribute
{
    public override void OnActionExecuting(ActionExecutingContext context)
    {
        var requiredParameters = context.ActionDescriptor.Parameters.Where(
            p => ((ControllerParameterDescriptor)p).ParameterInfo.GetCustomAttribute<RequiredModelAttribute>() != null).Select(p => p.Name);

        foreach (var argument in context.ActionArguments.Where(a => requiredParameters.Contains(a.Key, StringComparer.Ordinal)))
        {
            if (argument.Value == null)
            {
                context.ModelState.AddModelError(argument.Key, $"The argument '{argument.Key}' cannot be null.");
            }
        }

        if (!context.ModelState.IsValid)
        {
            var errors = context.ModelState.Values.SelectMany(v => v.Errors).Select(e => e.ErrorMessage);
            context.Result = new BadRequestObjectResult(errors);
            return;
        }

        base.OnActionExecuting(context);
    }
}

[AttributeUsage(AttributeTargets.Parameter)]
public sealed class RequiredModelAttribute : Attribute
{
}

services.AddMvc(options =>
{
    options.Filters.Add(typeof(CheckRequiredModelAttribute));
});

public async Task<IActionResult> CreateAsync([FromBody][RequiredModel]RequestModel request, CancellationToken cancellationToken)
{
    //...
}

Paganize

Answer 5

This happened to me, and in my case, I had to change using Microsoft.Build.Framework; to using System.ComponentModel.DataAnnotations; (and add the reference).

Gamboa

Answer 6

I was looking for a solution to this problem and came out here first. After some further research I have realized the following solution:

How do you use my solution? You can register it globally:

config.Filters.Add(new ValidateModelStateAttribute());

Or use it on demand for a class

[ValidateModelState]
public class UsersController : ApiController
{...

or for a methode

[ValidateModelState]
public IHttpActionResult Create([Required] UserModel data)
{...

As you can see, a [System.ComponentModel.DataAnnotations.Required] atribute has been placed in the method parameter. This indicates that the model is required and can not be null.

You can also use with a custom message:

[ValidateModelState]
public IHttpActionResult Create([Required(ErrorMessage = "Custom message")] UserModel data)
{...

Here is my code:

using System;
using System.Collections.Concurrent;
using System.ComponentModel.DataAnnotations;
using System.Net;
using System.Net.Http;
using System.Reflection;
using System.Web.Http.Controllers;
using System.Web.Http.Filters;

namespace your_base_namespace.Web.Http.Filters
{
    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, Inherited = true)]
    public class ValidateModelStateAttribute : ActionFilterAttribute
    {
        private delegate void ValidateHandler(HttpActionContext actionContext);

        private static readonly ConcurrentDictionary<HttpActionBinding, ValidateHandler> _validateActionByActionBinding;

        static ValidateModelStateAttribute()
        {
            _validateActionByActionBinding = new ConcurrentDictionary<HttpActionBinding, ValidateHandler>();
        }

        public override void OnActionExecuting(HttpActionContext actionContext)
        {
            GetValidateHandler(actionContext.ActionDescriptor.ActionBinding)(actionContext);

            if (actionContext.ModelState.IsValid)
                return;

            actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, actionContext.ModelState);
        }

        private ValidateHandler GetValidateHandler(HttpActionBinding actionBinding)
        {
            ValidateHandler validateAction;

            if (!_validateActionByActionBinding.TryGetValue(actionBinding, out validateAction))
                _validateActionByActionBinding.TryAdd(actionBinding, validateAction = CreateValidateHandler(actionBinding));

            return validateAction;
        }

        private ValidateHandler CreateValidateHandler(HttpActionBinding actionBinding)
        {
            ValidateHandler handler = new ValidateHandler(c => { });

            var parameters = actionBinding.ParameterBindings;

            for (int i = 0; i < parameters.Length; i++)
            {
                var parameterDescriptor = (ReflectedHttpParameterDescriptor)parameters[i].Descriptor;
                var attribute = parameterDescriptor.ParameterInfo.GetCustomAttribute<RequiredAttribute>(true);

                if (attribute != null)
                    handler += CreateValidateHandler(attribute, parameterDescriptor.ParameterName);
            }

            return handler;
        }

        private static ValidateHandler CreateValidateHandler(ValidationAttribute attribute, string name)
        {            
            return CreateValidateHandler(attribute, new ValidationContext(new object()) { MemberName = name });
        }

        private static ValidateHandler CreateValidateHandler(ValidationAttribute attribute, ValidationContext context)
        {
            return new ValidateHandler(actionContext =>
            {
                object value;
                actionContext.ActionArguments.TryGetValue(context.MemberName, out value);

                var validationResult = attribute.GetValidationResult(value, context);
                if (validationResult != null)
                    actionContext.ModelState.AddModelError(context.MemberName, validationResult.ErrorMessage);
            });
        }
    }
}

Corsiglia

For my case which is, avoiding null payload as valid modelstate, I changed a bit. RequiredAttribute turned into FromBodyAttribute, then in your last method, I'm just checking if 'value' is null. works great. Thanks! – Kerrin

Answer 7

There is a simple Solution for your problem

public class UserCreate
{
    [Required(AllowEmptyStrings = false)]
    public string Username { get; set; }
}

Here AllowEmptyStrings = false can be used for your validation

Diploma

Can also add minimum string length, and other validations. – Pean

Yes, that can be done, also we can create a custom filters if required, like for INT to have values greater than 0 – Diploma

The default behavior for the Required attribute is that empty strings are considered invalid. See API docs. Using AllowEmptyString = false is redundant and unnecessary. – Transsonic

Answer 8

Try

services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_2);

in the startup.cs file's ConfigureServices()

Farmhouse

Answer 9

What I did was to create an Attribute along with an ActionFilter and a Extension Method to avoid null models.

The extension method looks for parameters with the NotNull attribute and check if they are null, if true, they are instantiated and set in the ActionArguments property.

This solution can be found here: https://gist.github.com/arielmoraes/63a39a758026b47483c405b77c3e96b9

Anew

Answer 10

This "ModelState.IsValid returns true even when it should not" problem can also appear if you forgot to add getters and setters on your model (OP didn't forget, but I did which led me to this question). I hope it's ok to provide solutions that have the same symptoms but are slightly different than OP's code:

Wrong:

    public class UserRegisterModel
    {
        [Required]
        public string Login; // WRONG
        [Required]
        public string Password; // WRONG
    }

Good:

    public class UserRegisterModel
    {
        [Required]
        public string Login { get; set; }
        [Required]
        public string Password { get; set; }
    }

Cyclic

Answer 11

this issue happened to me .i do not know why but take it easy just change your action Object name(UserCreate User) by some other like (UserCreate User_create)

Infante