microsoft / referencesource

Source from the Microsoft .NET Reference Source that represent a subset of the .NET Framework
https://referencesource.microsoft.com/
MIT License
3.16k stars 1.27k forks source link

Should ValidationContext constructor check that instance is null? #6

Closed sergey-kolodiy closed 8 years ago

sergey-kolodiy commented 8 years ago

I am using CustomValidationAttribute to add my own validation logic for ASP.NET Web API models. It works fine in most cases, except when I send an empty request body. In that case, I get the following exception:

System.ArgumentNullException: Value cannot be null.
Parameter name: instance
   at System.ComponentModel.DataAnnotations.ValidationContext..ctor(Object instance, IServiceProvider serviceProvider, IDictionary`2 items)
   at System.Web.Http.Validation.Validators.DataAnnotationsModelValidator.Validate(ModelMetadata metadata, Object container)
   at System.Web.Http.Validation.DefaultBodyModelValidator.ShallowValidate(ModelMetadata metadata, ValidationContext validationContext, Object container, IEnumerable`1 validators)
   at System.Web.Http.Validation.DefaultBodyModelValidator.ValidateNodeAndChildren(ModelMetadata metadata, ValidationContext validationContext, Object container, IEnumerable`1 validators)
   at System.Web.Http.Validation.DefaultBodyModelValidator.Validate(Object model, Type type, ModelMetadataProvider metadataProvider, HttpActionContext actionContext, String keyPrefix)
   at System.Web.Http.ModelBinding.FormatterParameterBinding.<ExecuteBindingAsyncCore>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.HttpActionBinding.<ExecuteBindingAsyncCore>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ActionFilterResult.<ExecuteAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ExceptionFilterResult.<ExecuteAsync>d__0.MoveNext()

As you can see, the problem is that ValidationContext throws an ArgumentNullException in the constructor if instance is null.

I am wondering, is it correct behavior or not? Posting a request with an empty body can generate an exception in my server code. In turn, server returns 500 status code instead of 400.

terrajobst commented 8 years ago

Thanks for doing this, but this repo is read-only. Please see this blog post with more details around the rationale for that.

For bugs in System.ComponentModel.DataAnnotations.ValidationContext, please file an issue in the http://github.com/dotnet/CoreFX repository.