Mvc: ModelBinding error with boolean values

[Eilon]

_From @leecow on June 21, 2018 2:20_

Issue moved from https://github.com/dotnet/core/issues/1711, opened by @WinthorpeCross

I have a potential problem with the ModelBinder in my ASP.NET Core 2.0 project. I have the following 'Index' controller action:

[HttpGet]
public async Task<IActionResult> Index(SortFilterIndexOptions options) { ... }

The SortFilterIndexOptions are four parameters defined in the following class:

public class SortFilterIndexOptions
{
    public int SelectedBirdId { get; set; }
    public bool ShowAll { get; set; }
    public bool ShowInTable { get; set; }
    public int page { get; set; }
}

These enable the user to filter a paged index page using checkbox toggle buttons.

Everything works until I change 'ShowAll' bool member to TRUE and then try to navigate to a different page. The 'ShowAll' parameter part of the url then contains two bool values ('ShowAll=true, false'):

which of course results in a parse error ('FormatException: String was not recognised as a valid Boolean').

My understanding that the ModelBinder in ordinary MVC automatically parses the TRUE from the true,false.

It only happens when the 'ShowAll' parameter is toggled to TRUE.

_Copied from original issue: aspnet/Home#3246_

[mkArtakMSFT]

Thanks for contacting us, @WinthorpeCross.
Can you please provide a small project so we can reproduce the issue?

[WinthorpeCross]

Hello there @mkArtakMSFT, I'll prepare something for you now. What's the best way to share the project with you? By uploading a repo?

[WinthorpeCross]

I have added a sample as requested. It is the only public repo on my GitHub and here is the link.

To replicate the fault:

• Run the application, then choose 'Birds' from the Navigation Bar. This will open the Birds Index page;

• On the index page, toggle either or both of the check boxes to true (checked). Then use the pager control to choose a different page (the database is seeded with 10 records with two records per page. Therefore the list should be divided into five pages).

You will see the error page with the details of the parse error.

I am now wondering if the fault is being caused by my 'pager' (you can see my paging setup in the sample). However, notwithanding that, my understanding is that the ModelBinder will automatically take care of extracting the 'true' from the 'true,false'" (see this).

Let me know if you have any questions.

[mkArtakMSFT]

Thanks @WinthorpeCross.
@dougbu, can you please look into this? Thanks!

[dougbu]

I narrowed the problem down to the following line in your ModelBindFaultSample\ModelBindFaultSample\Views\Shared\Components\Pager\Default.cshtml file:
``` c#
urlTemplate += "&" + key + "=" + request.Query[key];

Since `request.Query[key]` is of type `StringValues`, converting it to a `string` here leads to problems.

The following fixes the issue:
``` c#
foreach (var keyValue in request.Query[key])
{
    urlTemplate += "&" + key + "=" + keyValue;
}

More generally, your actions should check ModelState.IsValid before generating a view based on submitted data e.g.
c# if (!ModelState.IsValid) { return BadRequest(ModelState); }

However, notwithanding that, my understanding is that the ModelBinder will automatically take care of extracting the 'true' from the 'true,false'" (see this).

The page you mentioned (https://forums.asp.net/t/1314753.aspx) doesn't include any similar examples. (Displaying Request.Form["paid"] as "true,false" is a shortcut for "The form contained at least ?paid=true&paid=false.")

I've tested with ASP.NET MVC and am sure it also supported ?paid=true&paid=false but not paid=true,false.

[WinthorpeCross]

Thank you very much @dougbu for investigating this and for getting to the bottom of the problem.

It was only yesterday, when I was preparing the sample, that I realised that my Pager code could be the source of the problem. Even then I could not quite put my finger on the faulty code. Thank you for identifying the issue, and for going one step further and suggesting the fix.

I have implemented the changes you suggested which have resolved the issue.

The design I replicated in the sample, which is a bad design, was the first iteration. In my project I have replaced the bool members / controls with enumerated drop down options. My refactored design did not encounter the binding problem and that led me to think there could be a possible problem with the ModelBinder with bool values. Of course I was wrong to think that.

And you are quite right to remind me that I should have been checking the ModelState.IsValid in the action.

Thank you again for investigating this. I really hope I haven't wasted too much of anyone's time. I've certainly learned a thing or two! Please let me know if you need any more information from my end.
Thank you. :)

[mkArtakMSFT]

Thanks for contacting us. We believe that the question you've raised have been answered. If you still feel a need to continue the discussion, feel free to reopen it and add your comments.