Saturday, July 3, 2010

ASP.NET MVC 2: validation and binding issues with rich-text input, DataAnnotations, view-models, and partial model updates

Here I'm going to tackle a series of validation related annoyances with MVC 2 that tend to come up rather frequently.

This will include:
  • Input Validation: Potentially Dangerous Request even when using [ValidateInput(false)] attribute.
      
  • DataAnnotations validation warnings with partial model updates.
      
  • Dealing with View-Model binding AND DataAnnotations validation warnings with partial model updates together.
      
OK... so, you are writing a page on the asp.net MVC 2 framework that creates a record in the DB for you.

You have a view. Nice!

You have a controller. Also nice!

The view needs select lists and stuff too, so you have a view-model. Sure thing!
Your controller relies on a model class to handle business logic like auto-populating values on the entity and similar. You bet!

And the entity itself is an EF 4 generated class. Nothing special there!

And you created a meta-data buddy class for the entity so you can flag fields with attributes and get MVC to automate your validation. Absolutely!

So you type in your values into the page, hit submit, and the page blows up!

Fuck!

Now, since you were using a view-model, your controller looks something like this:

[Authorize]
public virtual ActionResult Create()
{
    Ticket ticket = new Ticket();
    var model = new TicketCreateViewModel(ticket);
    return View(model);
}

[Authorize]
[HttpPost]
[ValidateInput(false)]
public virtual ActionResult Create(FormCollection collection)
{
    try
    {
        Ticket ticket = new Ticket();
        UpdateModel(ticket, collection);
        if(TicketService.CreateTicket(ticket))
        {
            RedirectToAction("Success");
        }
        else
        {
            return View(new TicketCreateViewModel(ticket));
        }
    }
    catch { return View(new TicketCreateViewModel(ticket)); }
}

The first error you are likely to come across will be a potentially dangerous request error. This will happen if your view tries to submit a field that contains HTML characters, like with a rich text editor.
Now, you used to be able to work around this by just flagging the action method with [ValidateInput(false)] and all was well; but not with MVC 2.0 you don't!

In .NET 4.0, the MS dev team decided to overhaul (badly) the input validation mechanism. They wanted to have the same mechanism work for asp.net, web services, and just about any other kind of request too. So they now have a new input validation system that operates so high-up in the request pipeline that it dies long before it actually gets to your controller to see if you've overridden the default behavior. More info in this whitepaper.

Basically, to work around this you need to tell ASP.NET to use the old .NET 2.0 validation mechanism instead of the fancy new one. You do this in web.config by adding this to the system.web section:

<httpRuntime requestValidationMode="2.0"/>
  
  
Basically, this change is so retarded, that any application with a rich text editor ANYWHERE has to turn off the new security feature for the entire application. The good news is that the old mechanism still protects asp.net pages, but you are on your own again for web services and other request types.

Good job guys!

Now, once you've fixed that up the next problem you'll likely run across is that the UpdateModel method has trouble populating the data into the right properties. This is because you are using a view-model class, but when the form is posted back you are trying to use UpdateModel on just an entity.

Generally this is pretty easy. You just tell the UpdateModel method the "prefix" for the values in the form that should match the entity you are updating. In my example here, the property in the view-model that had the ticket fields was called "NewTicket"... we we just alter the controller to supply the prefix "NewTicket" and the UpdateModel method can figure out what properties in the form data should go get shoved into our entity.

[Authorize]
[HttpPost]
[ValidateInput(false)]
public virtual ActionResult Create(FormCollection collection)
{
    try
    {
        Ticket ticket = new Ticket();
        UpdateModel(ticket, "NewTicket", collection);
        if(TicketService.CreateTicket(ticket))
        {
            RedirectToAction("Success");
        }
        else
        {
            return View(new TicketCreateViewModel(ticket));
        }
    }
    catch { return View(new TicketCreateViewModel(ticket)); }
}

This is also how the standard example apps for MVC typically do things. And it works fantastic as long as you are posting back ALL of the properties from the view, or at least all the ones that have validations via DataAnnotations attached to them.

But when you have a [Required] attribute from DataAnnotations on entity properties that are NOT being passed back from the view, then you run into the third major problem... the UpdateModel will blow up with validation errors for the fields that your view didn't post to the controller.

This is also due to a late-breaking, and very unwise, change in behavior that the asp.net team made right before MVC 2.0 was released.

The MVC validation stuff in 1.0 only validated properties that were posted up; a mechanism called input based validation. The problem with it is that a hacker could, in theory, get around triggering validation errors by hacking out required fields from the HTTP post. So, they decided to switch to "model based validation" where the validation would check the validity of ALL properties in the model even if they weren't included in the post.

Again... this is an ok idea, but the asp.net team should not have implemented such a breaking-change without giving you some way to easily override the behavior too... after-all, doing a partial model update from a form is NOT exactly an edge case scenario. It is damned common!

Now... Steve Sanderson (smart guy!) posted about a really slick action filter attribute way to handle partial model updates. In short, his mechanism allows you to flag an action method with an attribute, and an action filter will automatically loop in after the model binding is done and remove any model state validation errors for fields that weren't included in the post.

Now... suppose you were doing this instead of what we were doing above:

[Authorize]
[HttpPost]
[ValidateInput(false)]
public virtual ActionResult Create(Ticket newTicket)
{
 // stuff
}

With Steven's simple attribute, you could get the desired partial model update to work, without barfing on the data annotations by just flagging the action with one more attribute; like this:

[Authorize]
[HttpPost]
[ValidateInput(false)]
[ValidateOnlyIncomingValuesAttribute]
public virtual ActionResult Create(Ticket newTicket)
{
 // stuff
}

Fantastic!

Except that it only works if you are using a model binder, but we weren't. Remember, in our case we're using a view-model. We don't want to bind to the view-model on the postback... we just want to bind to an instance of the entity we are trying to create.

The UpdateModel method is NOT a model binder though largely it does the same thing as one. So the action filter attribute mechanism Steven describes doesn't work when using the UpdateModel method.

sigh...

OK, now I could just put similar code to what Steven's attribute was using in the controller itself. All his attribute does is loop through the model state and remove errors for fields that weren't in the posted form collection. But honestly, that's an ugly beast and I HATE having my controllers marshaling values around like that.

Now... this example is actually simplistic, and the easiest way to work around this is to use the default binder and bind to the model automatically. This can be done simply with the Bind attribute, which allows you to supply a "prefix" to use for the binding. It looks something like this:

[Authorize]
[HttpPost]
[ValidateInput(false)]
[ValidateOnlyIncomingValuesAttribute]
public virtual ActionResult Create
(
 [Bind(Prefix = "NewTicket")] Ticket ticket
)
{
 if(TicketService.CreateTicket(ticket))
 {
  RedirectToAction("Success");
 }
 else
 {
     return View(new TicketCreateViewModel(ticket));
 }
}

This works fantastic for simpler cases like this one. We just supply the prefix to use and the binder takes care of it.

But, for reasons I don't want to delve into here, my code was a tad more complex and this technique didn't quite work out for me...

So... what I did to work around this when the Bind attribute wasn't enough, was to create a custom model binder that could do the same thing we're doing with UpdateModel. Anyway, once we have a custom binder we can then modify the controller action to use the custom binder AND Steven's attribute both.

So... here was my custom model binder:

public class NewTicketModelBinder : DefaultModelBinder
{
    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        bindingContext.ModelName = "NewTicket";
        return base.BindModel(controllerContext, bindingContext);
    }
}

Simple enough. All it does is supply the "prefix" to the binding context to achieve pretty much the same results as our UpdateModel method used to. Otherwise it uses the DefaultModelBinder's behavior as-is. Again though, this example is omitting some details that just aren't necessary to describe here, but my custom binder has a few other overloads besides what's shown here.

Now the controller looks like this:

[Authorize]
[HttpPost]
[ValidateInput(false)]
[ValidateOnlyIncomingValuesAttribute]
public virtual ActionResult Create
(
 [ModelBinder(typeof(NewTicketModelBinder))] Ticket ticket
)
{
 if(TicketService.CreateTicket(ticket))
 {
  RedirectToAction("Success");
 }
 else
 {
     return View(new TicketCreateViewModel(ticket));
 }
}

All I had to do here was declare what binder to use and specify Steven's [ValidateOnlyIncomingValuesAttribute]. I was also able to clean up the controller a bit, because now I don't need the try/catch for binding failures either.

Now... one thing I still didn't like was that the custom model binder here is specific to each view model. There isn't a clean way to supply the "prefix" to the binder without some ugly reflection or such. But I didn't like the idea that every time I needed to bind this way I'd have to make a new custom model binder.

So I settled on a "convention" for this and created a single custom model binder that could be used with any view-model, as long as it followed the convention:

public class ViewModelBinder : DefaultModelBinder
{
    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        bindingContext.ModelName = bindingContext.ModelType.Name;
        return base.BindModel(controllerContext, bindingContext);
    }
} 

Here, the binder requires that the property on the view-model be named the same as the entity's type. So if we want to expose a Ticket entity, we need to name the property in the view-model "Ticket" too.  Previously in my view-model I was using the property named "NewTicket", so I just change that to use property name "Ticket" instead. This way the ViewModelBinder can always find the right prefix name to supply for the default binder.

This still seems like an awful lot of work for a common scenario. I sure hope the MVC team gets all this worked out in the next release of the MVC framework to support this kind of situation more elegantly.

No comments:

Post a Comment