Aspnetcore: ModelState cannot be added to TempData (cannot be serialized)

[Bartmax]

I found this when dealing with ModelState and transfering using TempData to follow PRG pattern.

``` c#
TempData.Add("ModelStateTransfer", ModelState);

InvalidOperationException:
The 'Microsoft.AspNet.Mvc.ViewFeatures.SessionStateTempDataProvider' cannot serialize an object
of type 'Microsoft.AspNet.Mvc.ModelBinding.ModelStateDictionary' to session state.

Shouldn't be `ModelState` serializable?

I was pointed out to this tests:
https://github.com/aspnet/Mvc/blob/25eb50120eceb62fd24ab5404210428fcdf0c400/test/Microsoft.AspNetCore.Mvc.Core.Test/SerializableErrorTests.cs

so I tried using `SerializableError` with no success

``` C#
TempData.Add("ModelStateTransfer", new SerializableError(ModelState));

InvalidOperationException: 
The 'Microsoft.AspNet.Mvc.ViewFeatures.SessionStateTempDataProvider' cannot serialize an object
of type 'Microsoft.AspNet.Mvc.SerializableError' to session state.

Help? Ideas?

[dougbu]

The ModelStateDictionary may contain Exceptions and those are not necessarily serializable. For many scenarios, the BadRequest(ModelStateDictionary) method does the right thing. It substitutes a default error message and ignores the Exceptions.

But your scenario might not be a good fit for returning the above information to the client. Please let us know more about your scenario if that's the case.

[Bartmax]

Basically, I want to save ModelState on a POST action so I can redirect and have the ModelState on next (get) request. Basic PRG pattern.

[HttpGet]
public IActionResult Create(){
    if (TempData.ContainsKey("ModelStateTransfer") {
        ModelState.Merge(TempData["ModelStateTransfer"] as ModelStateDictionary);
    }
    return View();
}

[HttpPost]
public IActionResult CreateSpecificData(CreateBidningModel binding) {
    if (!ModelState.IsValid) {
        TempData.Add("ModelStateTransfer", ModelState);
        return RedirectToAction(nameof(Create));
    }
    // ....
}

[Eilon]

In my apps when I've used PRG what I end up doing is that for _failed_ requests it's still just a POST, but upon ultimate success (all fields correctly validated) I then do a redirect to a GET. The PRG pattern was intended to prevent re-submitting a form, which when the form data is invalid, is of course not an issue. And once the form data is valid, ModelState generally has nothing interesting in it (no errors, no exceptions, no validation messages, no distinct entered data, etc.).

I'm not familiar with the particular pattern used here. I actually don't think it works super well anyway because if there were ModelState errors and the user is redirected back to the initial GET page and then refresh their browser the TempData will be gone and they won't see any of the model state at all.

[danroth27]

No plans to change anything here.

[thiagomajesk]

@Eilon I don't think I quite understood your explanation. The reason to preserve ModelState here is to provide form usability. Imagine a huge form, if you treat failed requests just as a normal POST, then the user will have to fill every single form field again.

The way I see it, we have two options when ModelState.IsValid = false:

• Rebuilds the model and return view (duplicates code on GET and POST actions)
• (PRG) Redirects to GET that has the responsability to build the view

Of course the second options is preferable because of DRY. What is your approach right now? Anyone else could bring some thoughts on this matter?

[dougbu]

@thiagomajesk the usual choice is close to your first bullet. There's no need to "rebuild the model" because the view uses the (invalid) information from ModelState. And, and there's minimal duplication because the view can be reused.

E.g. from the MusicStore sample:
``` c#
//
// GET: /Account/AddPhoneNumber
public IActionResult AddPhoneNumber()
{
return View();
}

    //
    // POST: /Account/AddPhoneNumber
    [HttpPost]
    [ValidateAntiForgeryToken]
    public async Task<IActionResult> AddPhoneNumber(AddPhoneNumberViewModel model)
    {
        if (!ModelState.IsValid)
        {
            return View(model);
        }

        // ...
    }

```

[thiagomajesk]

@dougbu For simple types like int and string yes, I'm aware.
But if I have a list of an complex type, when post happens, it will always be null (pretty obvious). In those cases we have to rebuild the view model to display the correct data again.

That's when PRG shines, because only the GET action needs to know how to build your views.
If I'm always checking model state on POST, I'll have to duplicate the code that builds the view every single time (both on GET and POST).

All that beeing said, what are your thoughts on reusing that code? I know that a view model builder or something like that is an option, but I'm wondering what other approaches could fit that case instead of PRG.

Update: Beware though that are some complex cases/approaches where view data is populated through TempData/ViewBag. In those specific cases we must have access to either of them to re-insert the values, and this kinda breaks the use of an _out-of-controller_ builder abstraction.

[Bartmax]

@dougbu post model is not always same as get model. This should also be encouraged more.

@thiagomajesk I feel like not supporting this scenario is not understanding the real issue but in done fighting it :)

Maybe a blog post will help explain all get/post scenarios and why PRG is not about simple crud stuff.

Also some views can have multiple forms to multiple actions with different models. This particular simple scenario is kinda wonky right now.

[thiagomajesk]

@Bartmax That's true. PRG by far seems the best way of dealing with this kinda scenarios.

Recently I came across this article that demonstrates how to push the new ModelState into TempData (as we used to do in previous versions of asp.net).

One thing that got me thinking is that a guy in the comment section alleges that this functionality is supported on aspnet core 1.1 through cookie tempdata provider. Would love someone providing any thoughts on that matter.

[Bartmax]

That's similar of what I ended up doing. But that isn't a supported scenario on 1.1. That's just read the data serialize and create the model again in a very manual fashion. While it works, I still think it should be baked in the framework and not responsibility for the developer or external Nuget.

[ctolkien]

I'm going to chime in here as well with a 👍 - this is one of those death by a thousand cuts that makes MVC painful to use. IMO, it's fine when you can post to the same URL that you're currently on, it falls apart when you need to start posting to different URL's and handling error conditions.

[rynowak]

I'm reactivating this for consideration. We might not do exactly this but we should do something for this scenario

[Eilon]

I still don't believe that TempData is a solution for this. One hit of F5 and your TempData goes away.

[Bartmax]

I still don't believe that TempData is a solution for this. One hit of F5 and your TempData goes away.

🤦‍♂️

[thiagomajesk]

@Eilon Same thing for every other form... If the user refreshes the page, it is expected that everything goes away (errors, messages, values he's typed). Normally browsers will warn you about unsaved form changes. I think TempData is the perfect place for it, exactly because of what you just said. TempData's lifecyle is short and we don't want to keep those values hanging on the server any longer than that.

[Eilon]

The problem I describe doesn't happen with the Post-Redirect-Get (PRG) pattern because error cases are still a POST, so if you refresh, first of all the browser warns you, and even if you do post, you just re-post the same (bad) data, in which case you see the same errors again. If you do an initial redirect w/ TempData then the browser might not warn you, and if you proceed, you lose your data.

Is the problem I'm describing not a concern? We can certainly implement what's described here, it just doesn't seem to me like it would promote a good UI for end-users.

[Eilon]

We'll consider opening up the TempData default serializer to support whatever Json.NET supports, though there are no plans to do that at this time.

We can also consider making the TempData serializer itself pluggable (which I think it was in MVC 5.x).

[cgountanis]

Since this is the source thread, I figured I would post what helped me and it works great on any kind of model you create.

https://stackoverflow.com/questions/34638823/store-complex-object-in-tempdata-in-mvc-6

[Bartmax]

Side effects of this and not really understanding the value of proper PRG : https://github.com/aspnet/Announcements/issues/285

[ryanelian]

Laravel (PHP) has redirect function in controller action which can be fluently chained to include errors and form inputs.

https://laravel.com/docs/5.1/validation#other-validation-approaches

class ProductController extends Controller {
    public function post(Request $request) {
        // make $validator here...

        if ($validator->fails()) {
            return redirect('post/create')
                    ->withErrors($validator)
                    ->withInput();
        }

        // else: do something...
    }
}

Related to issue topic, it'd be nice if we can do this, instead of manually playing with TempData / manually serializing ModelState

return Redirect("~/tosomewhere").WithModelState();
return RedirectToPage(...).WithModelState();
return RedirectToAction(...).WithModelState();
// etc.

[thiagomajesk]

@ryanelian Love the idea! Definitely a lot cleaner than using attributes 👍.

[valeriob]

It's astonishing that MVC up to this day does not have a Post Redirect Get story...

[Bartmax]

The recommendations to expose over-posting vulnerability by using the full ViewModel on post and return a View on this thread is alarming.

Specially since this does not solve anything but 1 usecase.

[cccuscott]

Plus one for needing this functionality. Having the user refill out fields because of one error is not ideal.

[bsimser]

I just hit this issues trying to use what I felt was a pretty good implementation of PRG with Toastr. The original article can be found here. It doesn't work with core because a) there's no ControllerExtensions class that will let you do a generation RedirectToAction and b) if you manually do the PRG pattern and stuff the toast message (using the alert class) directly into TempData you get this error. The workaround on StackOverflow works but is that the answer or is there going to be something better built-in?

[Neme12]

It would be nice if TempDataSerializer at least supported tuples.

[Perustaja]

Long time since but huge plus on this.
@bsimser I saw this code all over too. Personally I just find this incredibly confusing and cannot imagine stumbling upon this in someone else's controllers and views. It encourages developers to use TempData in a very odd way and anyone trying to use it normally down the line will have to follow the breadcrumb trail and figure out what's going on. We're left with either bypassing TempData's expected usage or bloating the controller/page with properties.

[Yaevh]

Andrew Lock provides a workaround for this problem in MVC: https://andrewlock.net/post-redirect-get-using-tempdata-in-asp-net-core/
Unfortunately his method doesn't work in Razor Pages, so I've created a IPageFilter for this, maybe it'll be helpful for someone: https://gist.github.com/Yaevh/e87f682a3c3ac35d1504c068c9f5e8ab

I however agree that it should be supported by the framework itself and not require additional work by the developer.

[sipi41]

@Yaevh sorry to bother you and excuse my ignorance, I'm a newbie, would you please explain how to use your IPageFilter? is there any example? I understand we must create a class with what you wrote, then add this to Startup.cs but later? how do implement this? thank you for your patience and help.

I would like to know also if is there a solution out of the box in .net core 3.x ?? or this is not supported in any way... thanks for your info.

[Yaevh]

@sipi41 in your Startup.cs:

public class Startup ...
public void ConfigureServices(IServiceCollection services)
{
    services
        .AddRazorPages()
        .AddMvcOptions(options => options.Filters.Add<SerializeModelStatePageFilter>());
    ...
}

[sipi41]

I guess I haven't read about filters :-( I have done some validation attributes but not a filter... :-) but I'm learning and very excited about learning, sometimes I feel like I will never stop learning... I'm on the 22 (of 40 chapters) of Professional c#, have almost completed a book of Entity Framework and now I did stop reading Microsoft Blazor... too much to learn, and they are producing new things every day, thanks for your answer @Yaevh

I think I found a solution... (maybe not the best approach but at least it works) First I was thinking that if action methods are methods in a class, they share fields and/or properties, so by creating a field of type ModelStateDictionary, we could store the value there then retrieve it and vuala!, unfortunately, I did notice that it does save the data there but after calling the form method, the field/property is reset to null again...

SOLUTION: I was thinking... what if we take another approach? something similar that doesn't depend on an instance of the class but belongs to the class, and then I remember we could use a static field, and it did the work!

`public class AdministrationController : Controller
{
public static ModelStateDictionary ModelStateDic { get; set; } = null;

    private UserManager<Usuario> usersManager { get; set; }
    private AppDbContext db { get; set; }

    public AdministrationController(UserManager<Usuario> userMgr, AppDbContext Db)
    {
        this.usersManager = userMgr;
        this.db = Db;
    }

    public IActionResult Main() => View();


    public async Task<IActionResult> ManageUsers() {            

        ViewBag.usuarios = await db.Users.Include(e => e.UserRoles).ToListAsync();

        ViewBag.roles = await db.Roles.Select(s => new SelectListItem { Text = s.Name, Value = s.Id }).ToListAsync();

        if (AdministrationController.ModelStateDic != null)
        {
            ModelState.Merge(AdministrationController.ModelStateDic);                
        }

        return View(model: new CreateNewUser());

    }

    [HttpPost, ValidateAntiForgeryToken]
    public async Task<IActionResult> CreateNewUser(CreateNewUser NuevoUsuario) {


        AdministrationController.ModelStateDic = null;

        ModelState.AddModelError("ErrorAddingUser", "ERROR: error sample msg.");

       AdministrationController.ModelStateDic = ModelState;

        return RedirectToAction("ManageUsers");

    }

}`

In other words, instead of saving the data in Temp, we use a local static field to save the data.

[leniency]

In other words, instead of saving the data in Temp, we use a local static field to save the data.

@sipi41 - the problem with this though is that static variables is that they remain in memory past the current request - they retain value for the lifetime of the application. It may work in test with a single user, but with multiple users, this is very dangerous. _Every_ user will see the same value for that static variable. Not to mention potential concurrency issues.