OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.43k stars 2.4k forks source link

OrchardCore.Forms throws InvalidOperationException when restoring checkboxes from ModelState #16975

Open rjpowers10 opened 5 days ago

rjpowers10 commented 5 days ago

Describe the bug

OrchardCore.Forms leverages ModelStateHelpers.SerializeModelState and ModelStateHelpers.DeserializeModelState to preserve ModelState across web requests. This is broken for checkboxes since System.Text.Json was introduced in 2.0.0.

InvalidOperationException: The parameter conversion from type 'System.Collections.Generic.List`1[System.Object]' to type 'System.Boolean' failed because no type converter can convert between these types

Orchard Core version

2.0.2

To Reproduce

In my app I'm not actually using the form content item, but I do use OrchardCore.Forms solely for the ModelState preservation "trick". But I imagine you can reproduce this in OC with a simple form content item with a checkbox. You also need the form to be invalid so the ModelState preservation trick gets invoked and the user gets redirected back to the form.

Here's a simple console app that demonstrates the issue by comparing what happens with Newtonsoft.Json to System.Text.Json.

using Microsoft.Extensions.Primitives;

var checkbox = new ModelStateTransferValue
{
    Key = "MyCheckbox",
    AttemptedValue = bool.FalseString,
    RawValue = new StringValues(bool.FalseString) // Checkboxes in .NET set the RawValue to a StringValues object
};

// ModelStateHelpers.SerializeModelState
string newtonsoftJson = Newtonsoft.Json.JsonConvert.SerializeObject(checkbox);
string stjJson = System.Text.Json.JsonSerializer.Serialize(checkbox);

// ModelStateHelpers.DeserializeModelState
var newtonsoftModel = Newtonsoft.Json.JsonConvert.DeserializeObject<ModelStateTransferValue>(newtonsoftJson);
var stjModel = System.Text.Json.JsonSerializer.Deserialize<ModelStateTransferValue>(stjJson);

Console.WriteLine($"Type of Newtonsoft RawValue: {newtonsoftModel.RawValue.GetType()}"); // Outputs: Newtonsoft.Json.Linq.JArray
Console.WriteLine($"Newtonsoft RawValue is JArray: {newtonsoftModel.RawValue is Newtonsoft.Json.Linq.JArray}"); // Outputs: True
Console.WriteLine();
Console.WriteLine($"Type of STJ RawValue: {stjModel.RawValue.GetType()}"); // Outputs: System.Text.Json.JsonElement
Console.WriteLine($"STJ RawValue is JsonArray: {stjModel.RawValue is System.Text.Json.Nodes.JsonArray}"); // Output: False

public sealed class ModelStateTransferValue
{
    public string Key { get; set; }
    public string AttemptedValue { get; set; }
    public object RawValue { get; set; }
}

Running this console app produces the following output:

Type of Newtonsoft RawValue: Newtonsoft.Json.Linq.JArray
Newtonsoft RawValue is JArray: True

Type of STJ RawValue: System.Text.Json.JsonElement
STJ RawValue is JsonArray: False

The key is this line in ModelStateHelpers.

The Newtonsoft.Json version of the key line is

item.RawValue = item.RawValue is JArray jarray ? jarray.ToObject<object[]>() : item.RawValue;

The STJ version of the key line is

item.RawValue = item.RawValue is JsonArray jarray ? jarray.ToObject<object[]>() : item.RawValue;

Over in .NET we look at ModelBindingHelper.UnwrapPossibleArrayType. With STJ we end up passing down List<object>, which it does not know how to unwrap and so it falls into case 4. Then we get an exception because it doesn't know to convert a list to a checkbox.

rjpowers10 commented 2 days ago

Just remembered something like this happened before #7256