11

Closed

Bug in MVC4: System.Web.PrefixContainer

description

It seems that internal System.Web.PrefixContainer class was introduced in MVC4 and this class has the bug, which is reproduced with the following model, view and controller action method:

public class Model
{
public Model()
{
    Value = new List<Item>();
}

public string SomeData { get; set; }

public string ValueIsSequence { get; set; }

public class Item
{
    public string Data { get; set; }
}

public List<Item> Value { get; set; }
}

@using (Html.BeginForm())
{
<input type="text" name="SomeData" value="123" />
<input type="text" name="ValueIsSequence" value="True" />
<input type="text" name="Value[0].Data" value="ABC" />

<button type="submit">OK</button>
}

[HttpPost]
public ActionResult Testing(Model model)
{
var failed = model.Value.Count == 0;
return View();
}

The bug is that Model.Value list is empty and "Value[0].Data" form field is skipped. Prefix matching fails while checking "Value" prefix (for the Model.Value property) and probing "ValueIsSequence" field in binary search: "Value" is not equal to "ValueIsSequence" of course, and then the PrefixContainer supposed (incorrectly) that field can only be "Value." or ""Value[", but it detects "ValueIsSequence" that does not match "Value." nor "Value[", the comparer returns -1 (less) and binary search is moving to less half to probing "SomeData" field. So the binary search never detects "Value[0].Data" and Model.Value stays not filled.

Renaming "ValueIsSequence" to "IsSequence" for example, is a possible workaround.

file attachments

Closed Jun 7, 2013 at 11:25 PM by jacalvar
verified

comments

McSyko wrote Nov 28, 2012 at 6:47 AM

I also had this issue. Also worked around it by renaming the field, but I think it's impact is higher than a low - given that it's not immediately obvious that this is the way to fix the problem.

Attached a project which replicates the error (limited to the binary search code).
Binary search cannot find "Owners" prefix, when the result set contains "Owners[0].Id".

imranbaloch wrote Dec 8, 2012 at 1:13 PM

ErikMarkland wrote Jan 8, 2013 at 3:21 PM

We also ran into this exact problem. This should have an impact much higher than "low".

Maxild wrote Mar 13, 2013 at 3:25 PM

Here is a failing test
[Fact]
public void Bug()
{
    var sut = new PrefixContainer(new[] { "SomeData", "ValueIsSequence", "Value[0].Data" });
    sut.ContainsPrefix("Value").ShouldBeTrue();
}
I hate it that PrefixContainer is internal. Why? If it is good enough for Mvc why isn't it good enough for me? Also PrefixContainer has not tests at all:-(

Maxild wrote Mar 13, 2013 at 3:46 PM

Okay found the tests:-) Sry...for that naive comment

beauXjames wrote May 9, 2013 at 3:33 PM

...the object type we were having dumped was an array of long. This was nuts since it was only happening some times...the custom bindings were overkill and the any solutions we found were workarounds. The data was definitely getting corrupted between the js and the code...our models were right. The fact that it actually starts working when we changed the name of our property is insane. Will it break again later?

vshmidt wrote May 10, 2013 at 6:26 PM

also worked for me and also worked around it by renaming the field

yishaigalatzer wrote May 13, 2013 at 8:02 PM

Thanks for reporting the issue.

This was fixed on 4/22. And should be available in the next preview release, and currently is available in the nightly builds.