Stop ASP.NET MVC from calling all getters in a model class

Question

If you define the following two properties on your model class this will crash with a NullReferenceException during model binding:

        public Customer Customer { get; private set; } //set in the action method
        public bool Name => Customer.Name;

This is because Customer is still null during model binding and ASP.NET MVC calls the getter for Name.

The stack is:

   System.ComponentModel.ReflectPropertyDescriptor.GetValue(Object component) +525
   System.Web.Mvc.ModelMetadata.get_Model() +34
   System.Web.Mvc.DataAnnotationsModelValidator.Validate(Object container) +151
   System.Web.Mvc.<Validate>d__1.MoveNext() +387
   System.Web.Mvc.DefaultModelBinder.OnModelUpdated(ControllerContext controllerContext, ModelBindingContext bindingContext) +163
   System.Web.Mvc.DefaultModelBinder.BindComplexElementalModel(ControllerContext controllerContext, ModelBindingContext bindingContext, Object model) +83
   System.Web.Mvc.DefaultModelBinder.BindComplexModel(ControllerContext controllerContext, ModelBindingContext bindingContext) +1754
   System.Web.Mvc.ControllerActionInvoker.GetParameterValue(ControllerContext controllerContext, ParameterDescriptor parameterDescriptor) +460
   System.Web.Mvc.ControllerActionInvoker.GetParameterValues(ControllerContext controllerContext, ActionDescriptor actionDescriptor) +137
   System.Web.Mvc.ControllerActionInvoker.InvokeAction(ControllerContext controllerContext, String actionName) +982
   System.Web.Mvc.<>c__DisplayClass22.<BeginExecuteCore>b__1e() +39
   System.Web.Mvc.Async.AsyncResultWrapper.<.cctor>b__0(IAsyncResult asyncResult, Action action) +21
   System.Web.Mvc.Controller.EndExecuteCore(IAsyncResult asyncResult) +53
   System.Web.Mvc.Async.WrappedAsyncVoid`1.CallEndDelegate(IAsyncResult asyncResult) +36
   System.Web.Mvc.Controller.EndExecute(IAsyncResult asyncResult) +38
   System.Web.Mvc.MvcHandler.<BeginProcessRequest>b__5(IAsyncResult asyncResult, ProcessRequestState innerState) +44
   System.Web.Mvc.Async.WrappedAsyncVoid`1.CallEndDelegate(IAsyncResult asyncResult) +65
   System.Web.Mvc.MvcHandler.EndProcessRequest(IAsyncResult asyncResult) +38
   System.Web.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +399
   System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +137

From the stack it looks like model validation is querying all getters. I'm not using model validation.

How can I deal with this situation? Can I make ASP.NET MVC not call all getters without any (apparent) reason?

Apart from "why are you doing this", can you show how you use this model? When exactly does this exception occur? — CodeCaster, Jun 15 '16 at 16:10
Using it like `ActionResult M(MyModel model) { m.Customer = ...; return View(model); }`. I find this to be a convenient pattern on form pages. — boot4life, Jun 15 '16 at 16:13
The exception occurs during model binding, e.g. before my code runs. See the stack. — boot4life, Jun 15 '16 at 16:18

score 2 · Answer 1 · answered Jun 15 '16 at 17:29

2

So the Model Binder new's up an instance of your Model, then is probably doing reflection over the model's properties to look for named matches with the values in the FormCollection. What's happening is that the Customer prop is null when that dangerous Name prop is called, thus the NullRef.

The order by which .NET is checking those properties might not be actually random, but your code will be much improved by just treating it as such. Calling a method/prop directly on a Class that's nullable by default is a terrible idea, unless you check it for null. You have two options here, either (1) redesign your Model class so that the Constructor initializes the "Customer" property, or (b) add a null-check in that "Name" method.

Here's the easiest approach to just null-checking it when you grab it:

public bool Name => Customer?.Name ?? false;

This does not solve the underlying issue, which is that you have a Model that has nullable props chained together. Don't worry about your Model's constructor messing up your model binding. The Model Binder will (1) initialize your model class, then (2) try to hydrate it. So initializing the Customer class/prop in your Model's constructor won't impact any mapping of UI fields to say that Customer's fields.

answered Jun 15 '16 at 17:29

Graham

3,217
1
27
29

The thing with the `Customer` value is that it is not model bound. It comes from the DB. Also, it might legitimately be null. In that case it's invalid to access Name and the view does not do that. Is it not a legitimate way of using ASP.NET to write data into the model class? It's for passing data to the view after all. – boot4life Jun 15 '16 at 23:07
I'd have identified the root issue as MVC snooping into data that it has no business to snoop into. Can I make MVC not call my getters? – boot4life Jun 16 '16 at 09:53
"It [Customer] might legitimately be null. " There's your problem. On your model, if the Customer is null, then that getter will throw an error when called. Getters are NOT methods, they are properties of an object which should never trigger an exception just from being read. Perhaps make it a method if you only ever intend to call it conditionally. ASP.NET MVC is not the issue here, your Model class needs to be tweaked to not allow that NullRef. I could actually get that NullRef just from running your code in a debugging session is VS, if I inspect the Name prop of a nulled-Customer model. – Graham Jun 16 '16 at 12:58
I would not object to the debugger showing this exception as the property's value. Properties are very well allowed to throw in many contexts. For example if `!Stream.CanSeek` then `Length` will throw. Alright, although I do not agree with your interpretation of the situation I must work around it as it's apparently impossible to stop MVC from reading out all my properties. – boot4life Jun 20 '16 at 17:22

score 1 · Accepted Answer · answered Jan 30 '19 at 01:38

The DefaultModelBinder in MVC version 5.2.3 does validation in addition to binding, and there is no way to shut it off completely. Other SO posts mention turning off the implicit required attribute for value types with the following line of code in your Global.asax.cs, Application_Start() method...

DataAnnotationsModelValidatorProvider.AddImplicitRequiredAttributeForValueTypes = false;

See: https://stackoverflow.com/a/2224651 (which references a forum with an answer directly from the asp.net team).

Given your stack trace, that might fix your immediate problem. However, that probably won't be enough due to the DefaultModelBinder calling the getters even outside of its validation code for no stated reason (the source code makes no comment as to why it does that).

To solve the problem on my projects, where I use calculated properties all the time similar to your example, I implemented a custom model binder based on the original DefaultModelBinder source code that does not call any getters.

See a more detailed explanation and my full solution here: https://stackoverflow.com/a/54431404/10987278.

Thanks for sharing in all these places on Stack Overflow! MVC data binding and validation seem a bit messed up. It's a great framework overall but they made some consequential design mistakes. It feels like the designers didn't fully understand the problem space and added many hacks to make common scenarios "just work" thereby polluting the overall design. — boot4life, Feb 23 '19 at 14:05

Stop ASP.NET MVC from calling all getters in a model class

2 Answers2