Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

ASP.NET MVC Beta 1: DefaultModelBinder wrongly persists parameter and validation state between unrelated requests

When I use the default model binding to bind form parameters to a complex object which is a parameter to an action, the framework remembers the values passed to the first request, meaning that any subsequent request to that action gets the same data as the first. The parameter values and validation state are persisted between unrelated web requests.

Here is my controller code (service represents access to the back end of the app):

    [AcceptVerbs(HttpVerbs.Get)]
    public ActionResult Create()
    {
        return View(RunTime.Default);
    }

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Create(RunTime newRunTime)
    {
        if (ModelState.IsValid)
        {
            service.CreateNewRun(newRunTime);
            TempData["Message"] = "New run created";
            return RedirectToAction("index");
        }
        return View(newRunTime);
    }

My .aspx view (strongly typed as ViewPage<RunTime>) contains directives like:

<%= Html.TextBox("newRunTime.Time", ViewData.Model.Time) %>

This uses the DefaultModelBinder class, which is meant to autobind my model's properties.

I hit the page, enter valid data (e.g. time = 1). The app correctly saves the new object with time = 1. I then hit it again, enter different valid data (e.g. time = 2). However the data that gets saved is the original (e.g. time = 1). This also affects validation, so if my original data was invalid, then all data I enter in the future is considered invalid. Restarting IIS or rebuilding my code flushes the persisted state.

I can fix the problem by writing my own hard-coded model binder, a basic naive example of which is shown below.

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Create([ModelBinder(typeof (RunTimeBinder))] RunTime newRunTime)
    {
        if (ModelState.IsValid)
        {
            service.CreateNewRun(newRunTime);
            TempData["Message"] = "New run created";
            return RedirectToAction("index");
        }
        return View(newRunTime);
    }


internal class RunTimeBinder : DefaultModelBinder
{
    public override ModelBinderResult BindModel(ModelBindingContext bindingContext)
    {
        // Without this line, failed validation state persists between requests
        bindingContext.ModelState.Clear();


        double time = 0;
        try
        {
            time = Convert.ToDouble(bindingContext.HttpContext.Request[bindingContext.ModelName + ".Time"]);
        }
        catch (FormatException)
        {
            bindingContext.ModelState.AddModelError(bindingContext.ModelName + ".Time", bindingContext.HttpContext.Request[bindingContext.ModelName + ".Time"] + "is not a valid number");
        }

        var model = new RunTime(time);
        return new ModelBinderResult(model);
    }
}

Am I missing something? I don't think it's a browser session problem as I can reproduce the problem if the first data is entered in one browser and the second in another.

like image 235
Alex Scordellis Avatar asked Oct 26 '08 19:10

Alex Scordellis


People also ask

How to validate ModelState in mvc?

In ASP.NET 5 MVC, the ModelState property of a controller represents the submitted values, and validation errors in those values if such errors exist, during a POST action. During the POST, the values submitted can be validated, and the validation process uses attributes defined by . NET like [Required] and [Range] .

What is ModelState?

The ModelState represents a collection of name and value pairs that were submitted to the server during a POST. It also contains a collection of error messages for each value submitted. Despite its name, it doesn't actually know anything about any model classes, it only has names, values, and errors.


2 Answers

It turns out that the problem was that my controllers were being reused between calls. One of the details I chose to omit from my original post is that I am using the Castle.Windsor container to create my controllers. I had failed to mark my controller with the Transient lifestyle, so I was getting the same instance back on each request. Thus the context being used by the binder was being re-used and of course it contained stale data.

I discovered the problem while carefully analysing the difference between Eilon's code and mine, eliminating all other possibilities. As the Castle documentation says, this is a "terrible mistake"! Let this be a warning to others!

Thanks for your response Eilon - sorry to take up your time.

like image 160
Alex Scordellis Avatar answered Oct 29 '22 21:10

Alex Scordellis


I tried to reproduce this problem but I'm not seeing that same behavior. I created almost exactly the same controller and views that you have (with some assumptions) and every time I created a new "RunTime" I put its value in TempData and sent it off through the Redirect. Then on the target page I grabbed the value and it was always the value I typed in on that request - never a stale value.

Here's my Controller:

public class HomeController : Controller { public ActionResult Index() { ViewData["Title"] = "Home Page"; string message = "Welcome: " + TempData["Message"]; if (TempData.ContainsKey("value")) { int theValue = (int)TempData["value"]; message += " " + theValue.ToString(); } ViewData["Message"] = message; return View(); }

[AcceptVerbs(HttpVerbs.Get)]
public ActionResult Create() {
    return View(RunTime.Default);
}

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Create(RunTime newRunTime) {
    if (ModelState.IsValid) {
        //service.CreateNewRun(newRunTime);
        TempData["Message"] = "New run created";
        TempData["value"] = newRunTime.TheValue;
        return RedirectToAction("index");
    }
    return View(newRunTime);
}

}

And here's my View (Create.aspx):

<% using (Html.BeginForm()) { %>
<%= Html.TextBox("newRunTime.TheValue", ViewData.Model.TheValue) %>
<input type="submit" value="Save" />
<% } %>

Also, I wasn't sure what the "RunTime" type looked like, so I made this one:

   public class RunTime {
        public static readonly RunTime Default = new RunTime(-1);

        public RunTime() {
        }

        public RunTime(int theValue) {
            TheValue = theValue;
        }

        public int TheValue {
            get;
            set;
        }
    }

Is it possible that your implementation of RunTime includes some static values or something?

Thanks,

Eilon

like image 44
Eilon Avatar answered Oct 29 '22 21:10

Eilon