Skip to content

Multiple form inputs with the same name render incorrect output. #52150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 task done
mikeedwards-rs opened this issue Nov 17, 2023 · 6 comments
Closed
1 task done
Labels
area-ui-rendering Includes: MVC Views/Pages, Razor Views/Pages ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. question Status: Resolved

Comments

@mikeedwards-rs
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When rendering an array of values as input fields. .NET core does not correctly render the values of those fields after a post back.

Instead .NET Core picks the first value that matches the field name from the POST request and applies it to all inputs with that name. It ignores the value explicitly passed in by the code.

Expected Behavior

The expected behaviour is that all the fields after the post back will have the correct A to D values.

Somewhere some state is overriding the values set in the code.

Steps To Reproduce

Create a controller with the following method:

public async Task<IActionResult> Form(FormView view)
 {

     if(view == null)
     {
         view = new FormView();
     }

     view.Values = new string[] { "A", "B", "C", "D" };

     return View(view);
 }

Which is then render with the following view:

@model FormView

@using (Html.BeginForm("Post"))
{
    @foreach (var value in Model.Values)
    {
        @Html.TextBox("Values", value)
    }
}

On the first request the page renders as expected, each textbox as the correct letter:

<form action="/Home/Form?Length=4" method="post">
  <input class="input-validation-error" data-val="true" data-val-required="The Values field is required." id="Values" name="Values" type="text" value="A">
  <input class="input-validation-error" id="Values" name="Values" type="text" value="B">
   <input class="input-validation-error" id="Values" name="Values" type="text" value="C">
  <input class="input-validation-error" id="Values" name="Values" type="text" value="D">    
<button type="submit"> Submit</button>
</form>

However after clicking the submit button you will get the following result:

<form action="/Home/Form?Length=4" method="post">
  <input class="input-validation-error" data-val="true" data-val-required="The Values field is required." id="Values" name="Values" type="text" value="A">
  <input class="input-validation-error" id="Values" name="Values" type="text" value="A">
   <input class="input-validation-error" id="Values" name="Values" type="text" value="A">
  <input class="input-validation-error" id="Values" name="Values" type="text" value="A">    
<button type="submit"> Submit</button>
</form>

Now all the inputs contain the Value A instead of A to D, despite an explicit value being passed to the helper method.

Exceptions (if any)

No response

.NET Version

7.0.404

Anything else?

ASP.NET 7.0.14

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 17, 2023
@mikeedwards-rs
Copy link
Author

mikeedwards-rs commented Nov 17, 2023

The same behaviour can be exhibited with a simple textbox:

Model:

    public class FormView
    {
        public string Value { get; set; }
    }

Controller:

    public async Task<IActionResult> Form(FormView view)
    {

        view.Value = "A";

        return View(view);
    }

View:

@model FormView

@using (Html.BeginForm("Post"))
{
        @Html.TextBox("Value", Model.Value)
    <button type="submit"> Submit</button>
}

Change the value of the Input field before submitting. This value will override the value set by the controller.

The same behaviour is exhibited if we use a model expression:

@model FormView

@using (Html.BeginForm("Post"))
{
     @Html.TextBoxFor(model=>model.Value )
    <button type="submit"> Submit</button>
}

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@gbourass
Copy link

gbourass commented Apr 11, 2024

I have created a project to demonstrate this bug:
https://github.com/gbourass/dotnet-aspnetcore-bug-52150-text-field-rendering-post-conflict-demo
Just run the project and you will see the fields properly rendered with a GET.
image
Then, click POST and you will see multiple fields rendered with the wrong value using exactly the same model and view.
image
The issue is in Microsoft.AspNetCore.Mvc.ViewFeatures.DefaultHtmlGenerator.GenerateInput

var attributeValue = (string)GetModelStateValue(viewContext, fullName, typeof(string));

image
Although the correct value is passed to the renderer, if the ModelState contains a value with the same name, the model state value is used.
I suspect that we should try to get the value from the model state only if the value was not provided to the method?
Maybe something like this would work:
image

@mkArtakMSFT mkArtakMSFT added area-ui-rendering Includes: MVC Views/Pages, Razor Views/Pages and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Apr 24, 2024
@gbourass
Copy link

Making the change to favor the given value instead of the model state value breaks the test Microsoft.AspNetCore.Mvc.Rendering.HtmlHelperHiddenTest.HiddenFor_UsesModelStateValueOverPropertyValue. So it seems intentionnal to always use the model state value if it is defined.

@javiercn
Copy link
Member

@mikeedwards-rs thanks for contacting us.

This is very much by design. ASP.NET Core uses the value from the form over the value from the property when rendering forms so that they roundtrip in the event of errors and the information is not lost. Otherwise if there are errors that produce an empty model, the form will be cleaned out and the user will have to re-add all the data.

The problem with your model is that you are mixing input and output, and that only work if the input and output are the same. In your case, you have multiple output values that are aggregated into a single input value. You have two ways of resolving this:

  • Bind multiple values to your input model (but I imagine that's not what you want to do, you'll likely want to bind the selected value).
  • Add a new property to your model to hold the values and bind the selected value to a different property. You always use the output property to render all the values.

@javiercn javiercn added question ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. labels Apr 25, 2024
@gbourass
Copy link

It would be convenient to be able to render any field by giving it an explicit value without this value being overriden by what is in the model state. You can craft your models to try to avoid it, but this is a lurking side effect that can strike when you don't expect it as your models grow in complexity.

For the use case of the empty model on error, would it have been possible to override the given value by the model state only if the given value was null?

Otherwise, when I do not need the model state to render a field, it would be convenient to add a parameter when calling the renderer to force using the given value. It is a bit confusing that there is already a parameter called "isExplicitValue", but this doesn't mean to prefer value instead of the model state it seems.

The best workarounds I can think of right now (in term of convenience) to avoid the issue are:

  • In each controller action method, call ModelState.Clear() before calling the View.
  • A more global approach is to make an action filter to be added at the controller level that first check the validation (if desired) and then clear the model state. Here is an example:
    image

Copy link
Contributor

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ui-rendering Includes: MVC Views/Pages, Razor Views/Pages ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. question Status: Resolved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants