First things first – oh no they don’t J
But it can look like a bug if you’re not used to MVC, so I thought it worth calling out.
Imagine we have a pair of controller actions like this;
[HttpGet]
public ActionResult Index()
{
var model = new MyModel
Count = 1
};
return View(model);
}
[HttpPost]
public ActionResult Index(MyModel model)
if (!model.Name.StartsWith("Mr"))
model.Name = "Mr " + model.Name;
model.Count++;
What this is doing should be obvious – we want to increment a counter every time a POST to the Index action occurs, and if they didn’t specify a prefix of “Mr” for their name we add it for them. Easy. The “Index” View to render this looks like this;
<% using (Html.BeginForm())
{ %>
<%= Html.HiddenFor(m => m.Count)%>
Name: <%= Html.TextBoxFor(m => m.Name)%>
<input type="submit" value="Submit" />
<% } %>
This is pretty simple – we’re outputting the Counter as a hidden form field, and letting them type a Name into a Text Box.
If you paste these into a solution and give it a try you’ll notice a problem – Name never gets the “Mr” prefix, and our Counter never increments! Instead, they just display exactly as they were POST-ed to the server. Set a breakpoint in the action and inspect the model – you’ll see it is being updated as expected within the action, yet still the changes are not rendered. So what’s going on?
ASP.NET MVC assumes that if you’re rendering a View in response to an HTTP POST, and you’re using the Html Helpers, then you are most likely to be redisplaying a form that has failed validation. Therefore, the Html Helpers actually check in ModelState for the value to display in a field before they look in the Model. This enables them to redisplay erroneous data that was entered by the user, and a matching error message if needed.
Since our [HttpPost] overload of Index relies on Model Binding to parse the POST data, ModelState has automatically been populated with the values of the fields. In our action we change the Model data (not the ModelState), but the Html Helpers (i.e. Html.Hidden and Html.TextBox) check ModelState first… and so display the values that were received by the action, not those we modified.
To prove this, make a tiny temporary change to the second action;
ModelState.Clear();
This call to clear the ModelState will mean that the View now works as we expected. However, I wouldn’t recommend this as a long term solution. In reality, there are a few possible solutions;
1. If we want to display a confirmation page, we should be using the Post-Redirect-Get pattern, and not displaying this content in response to a POST. This means a view render during a POST is always a validation failure, which is what the MVC framework expects.
2. If we don’t want to do that, but we don’t want to perform any validation of the fields, we shouldn’t be using the Html Helpers. They only exist to assist with MVC framework functionality – if all you want to do is render simple HTML, guess what you should use? HTML! Just render data using <%= Model.Count %> etc.
3. If you don’t like that, you could consider avoiding Model Binding, and therefore avoiding ModelState from being populated. If we removed MyModel from the Index parameter list and instead accessed POST data using Request.Form this would work… but this fails to take advantage of MVC programming constructs, and complicates testability, so I would avoid it again.
4. If you have a more complex scenario (perhaps validation on some POSTs, and manual manipulation of posted data on others) you may need to consider calling ModelState.Remove for a specific field – but exercise caution; I do not recommend this approach. Complex action interactions that don’t quite fit with the framework are much more likely to cause you problems later.
There may be other solutions – such as writing your own Model Binder, but fundamentally they’re likely to change how MVC works, so I’d recommend avoiding them for this particular problem. If you’ve come up with something that fits well do let me know though.
So to go with my recommendation of taking control of our HTML, the revised Index view looks like this;
<input type="hidden" name="Count" value="<%: Model.Count %>" />
Name: <input type="text" name="Name" value="<%: Model.Name %>" />
Give it a whirl and you’ll see that it works this time!
* if you’re not using Visual Studio 2010, note that you must use <%= Html.Encode(field) %> instead of <%: field %>
I think I raised a similar issue last March, and Phil Haack commented on it - http://stackoverflow.com/questions/594600/possible-bug-in-asp-net-mvc-with-form-values-being-replaced
In the end, I created my own Html.Hidden() helper, although this was all for MVC v1.
Good link - thanks Dan!
Simon
Thanks - saved (at least part of) my day! I'm just using a manual Html.Hidden now. I'll just have to get my fellow devs not to convert it back to HiddenFor (which looks much better)...
Thanks DAN!!!! yOu are the best!!!!
You rock Dan!!!! That is very very interesting.........You saved my friend Herman a bunch loads of time.
Sorry Simon, I read the comments and thought your name was Dan. Very cool article Simon, yours looks cool too Dan!!
@ Herman,
no problem - pleased you like it :-)
Hi Simon, sorry I also looked quickly at the comments and thought your name was Dan!! Very nice article Simon!!!
Hello.
"ASP.NET MVC assumes that if you’re rendering a View in response to an HTTP POST, and you’re using the Html Helpers, then you are most likely to be redisplaying a form that has failed validation. Therefore, the Html Helpers actually check in ModelState for the value to display in a field before they look in the Model. ".
It's illogically! At first, this assertion based on particular case. At second, If ModelState and Model values aren't modified, they are similar so if programmer want to redisplay values he shouldn't modify Model and if Model is displayed than behavior is correct. And at last, EditorFor(model => model.Value) It MUST Editor for model.value but not for value from ModelState with same name.
Side effects are in Microsoft style. Hgrrrr, I spent about 4 hour on this issue.
@ Dmitry,
That's awful - so sorry you lost time to this. But having said that (and remembering I'm not on the product team so my perspective was similar to yours when I first started using this) I've come to find that this is a great assumption to have made. Once you realise what is happening it saves huge amounts of time - it's convention over configuration I guess.
The reason you can't just redisplay Model values (and hence it must check ModelState) is because some values entered by a user may be impossible to store in the Model.
For example, if I have a Model with an Int32 property that the user edits in a <input type="text" ... />, if they entered a non-numeric value (perhaps their name by mistake) the Model Binder cannot set the Int32 property to the string value. Therefore, in order to redisplay the invalid data the user entered the HtmlHelpers *must* check ModelState.
I hope that makes sense!
This happens on HTTPGet also not just HTTPPost. That is strange and seems like a bug. MS assumes it is a post even when you explicity use a get method?
@ Randall,
interesting point; I didn't intentionally call out "POST" specifically as a part of this... I guess it's because in 99% of cases I use a POST to pass data to the server. In reality you can set the <form /> tag's method to GET or POST and it will have the same effect. In other words, I think it is entirely logical that it happens for a GET as well, as MVC shouldn't force you to use one verb or the other when both are supported in HTTP for passing data.
I'm curious about whether there is a use case that involves this GET that is worth documenting? e.g. if it's a common task (e.g. ordering a list on a grid) then writing up the preferred approach could be useful.
// for each hidden field updated on controller posted to the same page
// Html.HiddenFor cancels modifications if it finds a value on ModelState
if (ModelState.HasKey("hiddenFieldName") ModelState.Remove("hiddenFieldName");
@Jorge;
that's exactly what I refer to in point (4) in my post. I've also seen a good blog post that describes this approach... but can't find it now.
I don't like it though - it's usually a sign you're misusing the HTML helper.
Thank you so much! I just wasted 3 hours and lost significant amounts of hair trying to figure this out. What is the "correct" way then to return a view? My app takes input from a user, and based on what they entered displays different controls for further input, and does this for 5 levels, so I want to return a view. And, since the input is in the form of droplist's, I don't need to validate.