dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.23k stars 9.95k forks source link

Corrupted form data when using an explicit index and the modelstate is not valid #26217

Open graott opened 4 years ago

graott commented 4 years ago

The Issue

If a razor page posts a collection of elements that use explicit indexing, where the explicit index is non sequential and non numerically ordered and the data they submit won't pass model validation on the server side. Then when the page is returned with the validation error the input fields will have incorrect data in them.

For example this data when posted (including the corresponding Parent.Child.Index fields) with a validation error will be rendered fine when the page returns.

As will this:

This however won't be:

To Reproduce

I have reproduced this by creating a simple model that has a parent class that contains a number of children. I render the parent out on a razor page and have a button that appends new children items dynamically in a random order using javascript. If the user then posts the data back with a non valid field (Note client side validation is turned off for this example.) then the form will show corrupted data.

If you debug the model that is posted back you can see that it is uncorrupted. It only gets corrupted when the page has been rendered out.

Code steps to reproduce the issue:

  1. Create a brand new razor pages web app .netcore 3.1 application

  2. Create a basic Model.

    public class Parent
    {
        public int Id { get; set; }
        [Required]
        public string Name { get; set; }
        public IList<Child> Children { get; set; }
    }
    public class Child
    {
        public int Id { get; set; }
        [Required]
        public string Name { get; set; }
        public int ParentID { get; set; }
    }
  3. Create a new razorpage code-behind file to process the data:

 public class ExplicitIndex : PageModel
    {
        [BindProperty]
        public Parent Parent { get; set; }
        public string JsonObject { get; set; }
        public void OnGet()
        {
            Parent = new Parent { Id = 1, Name = "Parent" };
            Parent.Children = new List<Child> { new Child { Id = 1, Name = "child1", ParentID=1 },
                                                new Child { Id = 2, Name = "child2", ParentID=1 },
                                                new Child { Id = 3, Name = "child3", ParentID=1 },
                                                new Child { Id = 4, Name = "child4", ParentID=1 }};
        }

        public IActionResult OnPost()
        {
            if(!ModelState.IsValid)
            {
                JsonObject = JsonConvert.SerializeObject(Parent);
                return Page();
            }

            return RedirectToPage("./Index");
        }
    }
  1. Create the following razorpage file:
@page
@model WebApplication3.Pages.ExplicitIndex
@{
}

@Model.JsonObject

<form method="post">
    <div asp-validation-summary="ModelOnly" class="text-danger"></div>

    <input type="submit" value="save">
    <input asp-for="Parent.Id" />
    <input asp-for="Parent.Name" />
    <span asp-validation-for="Parent.Name" class="text-danger"></span>

    <table id="table">
        <thead>
            <tr>
                <td>Name</td>
                <td>ID</td>
                <td>Parent ID</td>
            </tr>
        </thead>
        <tbody>
        </tbody>

        @for (int index = 0; index < Model.Parent.Children.Count; index++)
        {

            <tr>
                <td>
                    <input type="hidden" name="Parent.Children.Index" value="@index">
                    <input asp-for="Parent.Children[index].Name" />
                    <span asp-validation-for="Parent.Children[index].Name" class="text-danger"></span>
                </td>
                <td>
                    <input asp-for="Parent.Children[index].Id" />
                </td>
                <td>
                    <input asp-for="Parent.Children[index].ParentID" />
                </td>
            </tr>

        }
    </table>
    <button type="button" onclick="addItem()">Add Row</button>
</form>

@section Scripts{

    @* Prevent client side form validation*@
    @*@{await Html.RenderPartialAsync("_ValidationScriptsPartial");}*@

    <script>

        function addItem() {
            let table = document.getElementById("table");

            let numRows = table.rows.length - 1;
            let rowBefore = table.rows[getRandomInt(1, numRows)];

            let addedRow = rowBefore.parentNode.insertBefore(document.createElement("tr"), rowBefore);

            // Generate unique index value outside of existing index items
            let rowNum = table.rows.length + 10;

            addedRow.innerHTML = `
                    <td>
                        <input type="hidden" name="Parent.Children.Index" value="${rowNum}">
                        <input type="text" data-val="true" data-val-required="The Name field is required." id="Parent_Children_${rowNum}__Name" name="Parent.Children[${rowNum}].Name" value="">
                        <span class="text-danger field-validation-valid" data-valmsg-for="Parent.Children[${rowNum}].Name" data-valmsg-replace="true"></span>
                    </td>
                    <td>
                        <input type="number" data-val="true" data-val-required="The Id field is required." id="Parent_Children_${rowNum}__Id" name="Parent.Children[${rowNum}].Id" value="">
                    </td>
                    <td>
                        <input type="number" data-val="true" data-val-required="The ParentID field is required." id="Parent_Children_${rowNum}__ParentID" name="Parent.Children[${rowNum}].ParentID" value="">
                    </td>`

        }

        function getRandomInt(min, max) {
            min = Math.ceil(min);
            max = Math.floor(max);
            return Math.floor(Math.random() * (max - min + 1)) + min;
        }

    </script>
}
  1. Start the application navigate to the razor page above, click the "Add Row" button and then click save (don't enter any info). The page will then be returned as it has failed validation. The page displays the JSON representation of the object and the form displays the returned form data. These don't match with the form data becoming corrupted.

Further technical details

.NET SDK (reflecting any global.json):
 Version:   5.0.100-preview.8.20417.9
 Commit:    fc62663a35

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17763
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.100-preview.8.20417.9\

Host (useful for support):
  Version: 5.0.0-preview.8.20407.11
  Commit:  bf456654f9

.NET SDKs installed:
  2.2.207 [C:\Program Files\dotnet\sdk]
  3.1.402 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.8.20417.9 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.0-preview.8.20414.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.22 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0-preview.8.20407.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0-preview.8.20411.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
ghost commented 4 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 1 year ago

Thanks for contacting us. We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

MackinnonBuck commented 1 year ago

There seems to be a problem with rendering null model values, but only when returning a PageResult from an action. Rather than rendering an empty value, it renders the previously rendered value. Maybe this is only a symptom of the real root cause but this seems like a legitimate bug to me.

JandosKh commented 1 year ago

This is a quite an issue for dynamically added collection items. The reason data corruption occurs is during model binding non-sequential indices used as they are, including in modelState, but when invalid view or page is re-rendered, collection indices become sequential. I think, default model binding should fix non-sequential indices to sequential before validation.