search

billbogaiv / hybrid-model-binding

Provides the ability to bind models using both ModelBinder and ValueProvider in ASP.NET Core.
MIT License
104 stars 19 forks source link

Clearing the ModelState causes loss of initial binding errors and sets default model properties #63

Open ShakiFanatico opened 2 years ago

ShakiFanatico commented 2 years ago

Hi,

What's exactly the reason why the ModelState is being cleared first in src/HybridModelBinding/HybridModelBinder.cs on line 110? Clearing the ModelState there will remove possible initial binding errors (e.g. providing a string for a property that should be a bool). This results in no binding error at all (ModelState is valid) and instead, all properties (including those with valid values) are set their defaults. So, the model binding failed and everything is set to default, but we're not informed of this (ModelState is valid). In the worst case scenario, we'd fully process the request as if it were a valid request (e.g. if all properties are optional).

Anyway, by simply removing the line that clears the ModelState first, I get the expected initial model binding errors and HybridModelBinding still works as expected when the model is valid. So, is there any particular reason why the ModelState is cleared first?

This issue appears to be the same or similar to the initial post of #55 . However, I can't relate my issue to the discussion that follows in that thread.

Misiu commented 2 years ago

@billbogaiv any updates on this? I'm going to create another issue on aspnetcore repo and ask for official support. My last question was closed without any answer, so maybe this time I get a decent response.