Devlico.Us
CodeBetter.Com
RSS 2.0 via Feedburner
           Do you Twitter? Follow us @devlicious

Casey Charlton - Insane World

Hang the code, and hang the rules. They're more like guidelines anyway


ASP.NET MVC Problems with UpdateModel(model, Request.Form.AllKeys);

We just added a new dropdown list to one of our forms, which has a javascript function to change the background colour of a textarea. And the UpdateModel() on our controller class promptly fell over ...

It turns out that UpdateModel() requires your class to have a property it can map for every form control with a Name(presuming you are using the lazy 'Request.Form.AllKeys')... or it throws an exception ... not the most helpful or obviously expected behaviour. Instead, use TryUpdateModel(), or use our quick fix to remove the Name from our dropdown as it wasn't needed.

For binding that uses reflection, I would far rather the default behaviour was just to ignore the property ... this would allow views to be changed more easily without having to change the controller.

 



Comments

ASP.NET MVC Archived Blog Posts, Page 1 said:

Pingback from  ASP.NET MVC Archived Blog Posts, Page 1

# September 4, 2008 9:08 AM

Steve said:

Agreed.

With Monorail you could simply have the model object as a parameter without jumping through all these hoops.

public ActionResult UpdateAddress(Address address)

it would use reflection and fill the address object from the form collection

# September 4, 2008 12:29 PM

Rick said:

This particular change has been biting me hard too. All my updates before used the built in method, and the closest replacement is UpdateModel. Replace I have, but now I either need to list out all the field names that /are/ valid, or do some filtering to remove all the ones that are not.

Why wasn't this method written to mimic the old behaviour and simply ignore submitted values it doesn't map? I don't need exceptions forcing me to filter out "Save.x" and "Save.y" because I am using an image to submit.

I've been updating and using 2 apps in production since P1, and this is the worst update preview so far, in terms of behaviour changes. Yes, I know it's pre-beta etc. I'm glad to be getting it. Even so, this feels like it is not road tested to the standard of the previous previews. I actually considered not updating, except I can't bare the pain I think doing 2 preview updates at once would cause me.

# September 4, 2008 3:30 PM

Colin Jack said:

@Rick

"Why wasn't this method written to mimic the old behaviour and simply ignore submitted values it doesn't map? I don't need exceptions forcing me to filter out "Save.x" and "Save.y" because I am using an image to submit."

Yeah agreed.

# September 5, 2008 9:12 AM

Stephen Edwards said:

You need to pass in the properties of the object you want to bind to into the UpdateModel method rather that the forms keys.

bsr = new BusinessSavingsRequest();

                   string[] properties = typeof(BusinessSavingsRequest).GetProperties().Select(p=> p.Name).ToArray();

                   UpdateModel(bsr, properties);

Hope this helps.

Checkout:

weblogs.asp.net/.../asp-net-mvc-preview-5-and-form-posting-scenarios.aspx

# September 7, 2008 4:13 PM

Casey Charlton said:

@Steohen

Indeed ... but then you are duplicating the magic string names of your fields in every controller method where they are used ... once is plenty, but putting them in the View and in the Controller too is horrible ...  it is just ripe for hard to find bugs and hard to maintain code.

I still assert that you should only have to tell the model to be updated from the form, and have reflection figure it out, and not throw an exception if you have unknown fields included.

# September 8, 2008 6:09 AM

Eilon Lipton said:

Hi there,

We don't recommend that you just update *all* properties of your object because that has the potential to be a major security problem. If a malicious user crafts a request they can set any property of your object. Instead we recommend that you use that whitelist property as it was meant to be used: Specify exactly which properties you *want* to update, and nothing more.

Nevertheless, for the next release we've made some significant improvements to model binders and UpdateModel so some of these scenarios should work much better.

Thanks,

Eilon

# September 30, 2008 11:52 PM

Leave a Comment

(required)  
(optional)
(required)  

Enter the numbers above:
Add

About Casey Charlton

A somewhat passionate and opinionated developer, with occassional sparks of wisdom, and occasional useful information. Check out Devlicio.us!

Our Sponsors

Proudly Partnered With