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

proof

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 — K D, Jul 29 '13 at 12:22
@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. — Stan, Jul 29 '13 at 12:23
@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. — Stan, Aug 15 '13 at 09:35

score 83 · Accepted Answer · answered Jul 29 '13 at 12:14

83

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...

answered Jul 29 '13 at 12:14

nemesv

138,284
16
416
359

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.. – Stan Jul 29 '13 at 12:17
4

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`... – nemesv Jul 29 '13 at 12:18
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! – Stan Jul 29 '13 at 12:24
@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 ? – now he who must not be named. Jun 16 '14 at 11:52
14

@nemesv 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? – Josh Aug 04 '14 at 08:43
1

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 – JDC Apr 30 '15 at 13:39
@nemesv where can I get the isValid method imeplementation code. how do you know isValid internally check Values.All(modelState => modelState.Errors.Count == 0) expression. – Mannan Bahelim Oct 11 '18 at 08:57
1

@MannanBahelim ASP.NET MVC/Web.api is open source, you can find the source code on github: https://github.com/aspnet/AspNetWebStack to code is in the question can be found here: https://github.com/aspnet/AspNetWebStack/blob/master/src/System.Web.Http/ModelBinding/ModelStateDictionary.cs – nemesv Oct 11 '18 at 09:01

score 27 · Answer 2 · edited Nov 24 '16 at 09:15

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
 { ...

This returns Bad Request if the action has optional parameters, for example `Get(string id, string something = null)`. — Schileru, Aug 07 '17 at 16:27

score 13 · Answer 3 · answered Jul 02 '14 at 07:58

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 ());

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. — tunafish24, Jan 07 '16 at 12:22
@tunafish24 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. — Timothy Schoonover, Oct 12 '16 at 12:22

score 5 · Answer 4 · answered May 19 '17 at 13:06

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)
{
    //...
}

score 4 · Answer 5 · edited Aug 29 '16 at 03:01

4

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).

edited Aug 29 '16 at 03:01

JanR

6,052
3
23
30

answered Feb 18 '16 at 23:28

Jimmy

91
1
4

score 3 · Answer 6 · answered Jul 02 '18 at 12:47

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);
            });
        }
    }
}

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! — Roni Axelrad, Nov 13 '19 at 20:29

score 1 · Answer 7 · answered Dec 18 '18 at 10:25

1

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

answered Dec 18 '18 at 10:25

Yagnesh Khamar

184
3
8

Can also add minimum string length, and other validations. – Andrew May 21 '19 at 15:13
Yes, that can be done, also we can create a custom filters if required, like for INT to have values greater than 0 – Yagnesh Khamar May 05 '21 at 05:22
The default behavior for the `Required` attribute is that empty strings are considered invalid. See [API docs](https://learn.microsoft.com/en-us/dotnet/api/system.componentmodel.dataannotations.requiredattribute.allowemptystrings?view=net-7.0). Using `AllowEmptyString = false` is redundant and unnecessary. – seebiscuit Jan 12 '23 at 22:12

score 1 · Answer 8 · edited Mar 28 '19 at 06:36

1

Try

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

in the startup.cs file's ConfigureServices()

edited Mar 28 '19 at 06:36

Marcello B.

4,177
11
45
65

answered Mar 28 '19 at 04:12

Iran Canul

221
2
3

score 0 · Answer 9 · answered Nov 22 '17 at 21:49

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

score 0 · Answer 10 · 2021-09-30T11:42:18.750

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; }
    }

score -2 · Answer 11 · answered Sep 28 '15 at 16:06

-2

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)

answered Sep 28 '15 at 16:06

javad ghamari

1

ModelState.IsValid even when it should not be?

Controller

Model

Debug proof

11 Answers11

Linked