Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

A POCO parameter with an exact name match in a value provider fails to model bind #1865

Closed
dougbu opened this issue Jan 22, 2015 · 3 comments
Assignees
Milestone

Comments

@dougbu
Copy link
Member

dougbu commented Jan 22, 2015

when POCO model contains a property with the same name as the parameter e.g.

public class Vehicle
{
    public string Model { get; set; }
}
...
[HttpPost("/vehicles/{id:int}/edit")]
public IActionResult Edit(int id, Vehicle model)
{
}

then model will not be initialized -- all properties have default values. rename the Model property or model parameter and everything works as expected.

because the problem relates to model binders falling back to the empty prefix, can workaround it: send field names prefixed with the parameter name e.g.

@model Vehicle
@using (Html.BeginForm())
{
  ViewData.TemplateInfo.HtmlFieldPrefix = "model";
...
  <div class="editor-field">
    @Html.EditorFor(m => m)
  </div>
...
  ViewData.TemplateInfo.HtmlFieldPrefix = string.Empty;
}

bug does not appear to be a regression from MVC 5.2, MVC 6 Beta1, or MVC 6 Beta2.

@dougbu dougbu added the bug label Jan 22, 2015
@yishaigalatzer yishaigalatzer changed the title Top-level collections cause model binding to fail Top-level collections cause a parameter named model binding to fail Jan 28, 2015
@yishaigalatzer yishaigalatzer added this to the 6.0.0 milestone Jan 28, 2015
@yishaigalatzer yishaigalatzer changed the title Top-level collections cause a parameter named model binding to fail Top-level collections cause a parameter named model to fail model binding Jan 28, 2015
@danroth27 danroth27 modified the milestones: 6.0.0-rc1, 6.0.0 Mar 16, 2015
dougbu added a commit that referenced this issue Mar 16, 2015
- #1865
- pass indication through to `CompositeModelBinder` in `ModelBindingResult`
- use in `CompositeModelBinder` to detect another fallback case

nit: also clean up `ModelBindingResult` doc comments
dougbu added a commit that referenced this issue Mar 18, 2015
- #1865
- pass indication through to `CompositeModelBinder` in `ModelBindingResult`
- use in `CompositeModelBinder` to detect another fallback case

nit: also clean up `ModelBindingResult` doc comments
dougbu added a commit that referenced this issue Mar 18, 2015
- revert most of previous fix attempt (leaving doc comment updates)
- change `MutableObjectModelBinder` to ignore exact match in value providers
 - had an incorrect assumption: don't want exact model name to match since
   this binder supports only complex objects
 - ignored `BinderModelName`, value provider filtering, et cetera
- reduces over-binding e.g. `[Required]` validation within missing properties
@dougbu
Copy link
Member Author

dougbu commented Mar 18, 2015

original bug is better described in #2129. changing the subject to make it clear the Vehicles model was actually bound incorrectly due to match between property and parameter names. like #2129, the root cause is the model binding system not falling back to an empty prefix. unlike #2129, the problem is specific to top-level POCO models

@dougbu dougbu changed the title Top-level collections cause a parameter named model to fail model binding Top-level property named Model and a parameter named model fails to model bind Mar 18, 2015
@dougbu dougbu changed the title Top-level property named Model and a parameter named model fails to model bind A POCO parameter with an exact name match in a value provider fails to model bind Mar 19, 2015
@dougbu
Copy link
Member Author

dougbu commented Mar 19, 2015

realized the "top-level property" in the subject was a bit misleading and have updated the subject again. problems occur when an action parameter's name is an exact match for something in the value providers. so the previously-mentioned property must have a value in the query, form, etc. to cause problems.

also edited the issue description to remove mention of collections.

dougbu added a commit that referenced this issue Mar 19, 2015
- #1865
- pass indication through to `CompositeModelBinder` in `ModelBindingResult`
- use in `CompositeModelBinder` to detect another fallback case

nit: also clean up `ModelBindingResult` doc comments
dougbu added a commit that referenced this issue Mar 19, 2015
- revert most of previous fix attempt (leaving doc comment updates)
- change `MutableObjectModelBinder` to ignore exact match in value providers
 - had an incorrect assumption: don't want exact model name to match since
   this binder supports only complex objects
 - ignored `BinderModelName`, value provider filtering, et cetera
- reduces over-binding e.g. `[Required]` validation within missing properties
dougbu added a commit that referenced this issue Mar 20, 2015
- #1865
- pass indication through to `CompositeModelBinder` in `ModelBindingResult`
- use in `CompositeModelBinder` to detect another fallback case

nit: also clean up `ModelBindingResult` doc comments
dougbu added a commit that referenced this issue Mar 20, 2015
- revert most of previous fix attempt (leaving doc comment updates)
- change `MutableObjectModelBinder` to ignore exact match in value providers
 - had an incorrect assumption: don't want exact model name to match since
   this binder supports only complex objects
 - ignored `BinderModelName`, value provider filtering, et cetera
- reduces over-binding e.g. `[Required]` validation within missing properties
dougbu added a commit that referenced this issue Mar 20, 2015
…e match

- #1865
- change `MutableObjectModelBinder` to ignore exact match in value providers
 - had an incorrect assumption: don't want exact model name to match since
   this binder supports only complex objects
 - also ignored `BinderModelName`, value provider filtering, et cetera
- reduces over-binding e.g. `[Required]` validation within missing properties

also add more tests of #2129 scenarios
@dougbu
Copy link
Member Author

dougbu commented Mar 20, 2015

commit 533474d

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants