50

I have a Model object with a required attribute

public class ApiPing
{
    [Required]
    public DateTime ClientTime { get; set; }

    public DateTime ServerTime { get; set; }
}

I have a Controller method that checks model state.

public IHttpActionResult Ping(ApiPing model)
{    
    if (!ModelState.IsValid)
        return BadRequest(ModelState);

    model.ServerTime = DateTime.UtcNow;

    return Ok(model);
}

If I submit a submit a proper request (with a model) to the action method I get an correct value from ModeState.IsValid (true). However, when I submit an invalid request (without a model, so the model is null) I get an erroneous ModelState.IsValid (also true).

I could simply check if the model is null in my code, but that smells. Is this an intended 'feature' or a bug in ModelState validation? Am I doing something wrong ? Am I expecting too much ?

spottedmahn
  • 14,823
  • 13
  • 108
  • 178
nVentimiglia
  • 1,888
  • 4
  • 21
  • 37
  • http://cstruter.com/blog/415 – cstruter Sep 08 '16 at 20:05
  • Possible duplicate of [ModelState.IsValid even when it should not be?](https://stackoverflow.com/questions/17923622/modelstate-isvalid-even-when-it-should-not-be) – Zze Aug 27 '19 at 01:08

4 Answers4

24

I had the same problem before and the answer is already available in a few forums and even here at SO: ModelState.IsValid even when it should not be?

You can also add a custom filter to validate (invalidate) missing fields and/or null values http://www.asp.net/web-api/overview/formats-and-model-binding/model-validation-in-aspnet-web-api

http://www.strathweb.com/2012/10/clean-up-your-web-api-controllers-with-model-validation-and-null-check-filters/

Community
  • 1
  • 1
julianox
  • 758
  • 1
  • 7
  • 9
21

Here is an action filter to check for null models or invalid models.

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
 { ...
tanguy_k
  • 11,307
  • 6
  • 54
  • 58
Jose Ch.
  • 3,856
  • 1
  • 20
  • 34
  • 1
    First: Thank you for your solution, I like it. Second: We are currently experiencing an issue with it. If the maximum request length is reached, the entries value is null and gives back the "Arguments cannot be null" message which is misleading in that case. – timmkrause Jun 23 '17 at 12:47
4
  1. Declare your model as a struct
  2. Change type of all your required properties to be nullable
Oleg Sakharov
  • 1,127
  • 2
  • 20
  • 19
  • 1
    Thank you very much! All i needed is change type from class to struct! Dude, I don't know why this answer have such a small rating! – RustemMazitov Dec 15 '15 at 09:59
  • 12
    @RMazitov: Probably because it's generally not a good idea to use struct. Here are Microsoft's guidelines: https://msdn.microsoft.com/en-us/library/ms229017(v=vs.110).aspx – Nelson Rothermel Jan 08 '16 at 15:38
4

I have found the hint for this problem here. So i will give my solution here.

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);
            });
        }
    }
}
Roberto B
  • 542
  • 5
  • 13