Skip to content

System.Text.Json deserializing of object[bool] does not produce boolean #29960

Description

@radek7210

I understand that the System.Text.Json is still in development. However, I would like to point out that the new deserializer produces very different results than the previous one.

Scenario:

public class MyRequest
{
  public string ParamName { get; set; }
  public object ParamValue { get; set; }
}

Controller method:

[HttpPost]
public ActionResult<string> Post([FromBody] MyRequest myRequest)
{
  return Content($"Type of '{myRequest.ParamName}' is {myRequest.ParamValue.GetType()}; value is {myRequest.ParamValue}");
}

Posting this json:

{
  "ParamName": "Bool param",
  "ParamValue": false
}

In .net core 2.1, the false value deserializes as boolean:
Type of 'Bool param' is System.Boolean; value is False

However, in .net core 3.0 preview 6, the false value deserializes as System.Text.Json.JsonElement:
Type of 'Bool param' is System.Text.Json.JsonElement; value is False

Will there be any chance to make the new deserializer work the same as in 2.1?

Note: we declare the ParamValue as object, as in the real app the values are of several different types, and so far the deserializer handled all them for us without issues. In 3.0, all this functionality is broken.

Thanks.

Activity

  1. steveharter commented on Jun 20, 2019

    @steveharter
    Contributor

    Current functionality treats any object parameter as JsonElement when deserializing. The reason is that we don't know what CLR type to create, and decided as part of the design that the deserializer shouldn't "guess".

    For example, a JSON string could be a DateTime but the deserializer doesn't attempt to inspect. For a "True" or "False" in JSON that is fairly unambiguous to deserialize to a Boolean, but we don't since we don't want to special case String or Number, we don't want to make an exception for True or False.

    With the upcoming preview 7, it is possible to write a custom converter for object that changes that behavior. Here's a sample converter:

    public class ObjectBoolConverter : JsonConverter<object>
    {
        public override object Read(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options)
        {
            if (reader.TokenType == JsonTokenType.True)
            {
                return true;
            }
    
            if (reader.TokenType == JsonTokenType.False)
            {
                return false;
            }
    
            // Forward to the JsonElement converter
            var converter = options.GetConverter(typeof(JsonElement)) as JsonConverter<JsonElement>;
            if (converter != null)
            {
                return converter.Read(ref reader, type, options);
            }
    
            throw new JsonException();
    
            // or for best performance, copy-paste the code from that converter:
            //using (JsonDocument document = JsonDocument.ParseValue(ref reader))
            //{
            //    return document.RootElement.Clone();
            //}
        }
    
        public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
        {
            throw new InvalidOperationException("Directly writing object not supported");
        }
    }

    Used like

    var options = new JsonSerializerOptions();
    options.Converters.Add(new ObjectConverter());
    
    object boolObj = JsonSerializer.Parse<object>("true", options);
    bool b = (bool)boolObj;
    Debug.Assert(b == true);
    
    object elemObj = JsonSerializer.Parse<object>(@"{}", options);
    Debug.Assert(elemObj is JsonElement);
  2. radek7210 commented on Jun 21, 2019

    @radek7210
    Author

    sounds good, I'll use the custom converter. Thanks.

  3. John0King commented on Jun 21, 2019

    @John0King

    @steveharter Json and Javascript don't have many type, but boolean is known one.
    System.Text.Json should map json known'd type to clr type directly without custom converter

    json clr
    boolean bool
    number double
    string string
    null null
    undefined null
  4. steveharter commented on Jun 21, 2019

    @steveharter
    Contributor

    @John0King I provided the rationale previously. Automatically mapping number to double won't work for large decimals.

    What are the scenarios for using system.object instead of the strong type (double, string, bool)?

  5. Mike-E-angelo commented on Jun 21, 2019

    @Mike-E-angelo

    FWIW, using an object/dynamic property for message deserialization is a security consideration and should be handled with care:

    https://www.alphabot.com/security/blog/2017/net/How-to-configure-Json.NET-to-create-a-vulnerable-web-API.html

  6. John0King commented on Jun 25, 2019

    @John0King

    @steveharter
    you are right, In many scenarios , handle JsonElement is much easier than an unknow/dynamic object type . it's just different than Json.Net that will break many people

  7. GSPP commented on Jul 26, 2019

    @GSPP

    Maybe it's better to throw instead of deserializing as JsonElement. That way the decision can still be made in the future on how to deserialize to type object. Now, we're locked in to JsonElement.

    I have personally wanted to deserialize to object a few times in the past with JSON.NET. I always wanted the "obvious" deserialization to bool etc.

    If I want to deserialize to JsonElement then I can make the property a JsonElement.

  8. nick2893 commented on Aug 8, 2019

    @nick2893

    @steveharter
    So, regarding dictionary<string,object>, what is this supposed to look like? The following throws an exception.

            if (reader.TokenType == System.Text.Json.JsonTokenType.StartObject) 
            { 
                // https://github.com/dotnet/corefx/issues/39953 
                // https://github.com/dotnet/corefx/issues/38713 
                var conv = options.GetConverter(typeof(Dictionary<string, object>)) as System.Text.Json.Serialization.JsonConverter<Dictionary<string, object>>; 
                if (conv != null) 
                { 
                    return conv.Read(ref reader, type, options); 
                } 
                throw new System.Text.Json.JsonException(); 
            } 
    

    Regarding the design choices;
    What can be used to represent an Object other than dictionary?
    What can be used to represent an Array other than array?

    I can appreciate abstraction, but somebody has the make the final implementation decision. The current design completely ignores people who happened to stumble upon a Json file as a job requirement, and most likely need to use Json and System.Text.Json once and never again. For those folks, something that reasonably easily generates dictionarys and arrays and strings (Json files are one big string, right?) with little effort (such as System.Web.Script.Serialization.JavaScriptSerializer), and some (any?) reasonable default deserialization decisions/assumptions would make life so much easier.

  9. gmurray81 commented on Sep 24, 2019

    @gmurray81

    I don't really like this design. You are making the most common use cases for a parser an impenetrable mess to serve the general correctness. To serve the 2% edge cases, you are making this yet another unusable .NET Json API for the 98% use cases. You have an options bucket, use it to OPT into the generally correct, but unhelpful behavior, not have it be the default.

  10. steveharter commented on Sep 24, 2019

    @steveharter
    Contributor

    @gmurray81 can you explain your scenario for using System.Object (instead of just a bool) and what other values it may hold other than a bool?

    Since JsonElement would still be used for non-bool types (e.g. objects, arrays, numbers) how would having a special case for bool make the code easier to consume?

  11. scalablecory commented on Sep 24, 2019

    @scalablecory
    Contributor

    In most cases when I've seen this, it has been an anti-pattern. object is rarely what people want, and have seen far too many model.Foo.ToString() from people who didn't realize they shouldn't be using object.

  12. scalablecory commented on Sep 24, 2019

    @scalablecory
    Contributor

    The JSON->POCO auto-generator in VS creates object members when it sees null, so this behavior breaks anyone migrating from such a thing.

    But in most cases, this is again people just letting the default give them a bad model when they should be fixing what it gives them.

  13. John0King commented on Sep 25, 2019

    @John0King

    suggest to add a new strategy :PreferRawObject:true to use default object mapping

    json clr
    boolean bool
    number double
    string string
    null null
    undefined null
    Date(string with TimezoneInfo) DateTimeOffset
    Date(string without TimezoneInfo ) DateTime
  14. steveharter commented on Sep 25, 2019

    @steveharter
    Contributor

    suggest to add a new strategy :PreferRawObject:true to use default object mapping

    Yes some global option is doable.

    However, it is actually fairly easy to create a custom converter to handle the cases you show above. Earlier I provided the sample for bool and here's a sample for bool, double, string, DateTimeOffset and DateTime. I will get the sample added to

    private class SystemObjectNewtonsoftCompatibleConverter : JsonConverter<object>

    private class SystemObjectNewtonsoftCompatibleConverter : JsonConverter<object>
    {
        public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            if (reader.TokenType == JsonTokenType.True)
            {
                return true;
            }
    
            if (reader.TokenType == JsonTokenType.False)
            {
                return false;
            }
    
            if (reader.TokenType == JsonTokenType.Number)
            {
                if (reader.TryGetInt64(out long l))
                {
                    return l;
                }
    
                return reader.GetDouble();
            }
    
            if (reader.TokenType == JsonTokenType.String)
            {
                if (reader.TryGetDateTime(out DateTime datetime))
                {
                    return datetime;
                }
    
                return reader.GetString();
            }
    
            // Use JsonElement as fallback.
            // Newtonsoft uses JArray or JObject.
            using (JsonDocument document = JsonDocument.ParseValue(ref reader))
            {
                return document.RootElement.Clone();
            }
        }
    
        public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
        {
            throw new InvalidOperationException("Should not get here.");
        }
    }
  15. steveharter commented on Sep 26, 2019

    @steveharter
    Contributor

    See the SystemObjectNewtonsoftCompatibleConverter sample class in

    private class SystemObjectNewtonsoftCompatibleConverter : JsonConverter<object>
    for semantics similar to Json.NET (except for objects and arrays).

  16. 9 remaining items

  17. added this to the Future milestone on Feb 25, 2021
  18. Banyc commented on Mar 7, 2021

    @Banyc
    Contributor

    Is there a concrete solution for the controllers in ASP.NET Core? The object in parameter IDictionary<string, object> request is always JsonElement, which does not support a typecast.

    Update

    The solution is to set this line to startup.cs:

    services.AddControllers().AddJsonOptions(options =>
        options.JsonSerializerOptions.Converters.Add(new SystemObjectNewtonsoftCompatibleConverter())
    );

    SystemObjectNewtonsoftCompatibleConverter is copied from the code in the previous discussions.

    After that, the object in the parameter will be castable:

    [HttpPost("Post")]
    public async Task<IActionResult> PostAsync(IDictionary<string, object> requestModel)
    {
        // ...
    }
    
  19. steveharter commented on Jul 8, 2021

    @steveharter
    Contributor

    As mentioned above, the current design avoids deserializing JSON string and bool to CLR string and bool because this would not be consistent with how JSON numbers are deserialized (since we don't know the CLR number type and don't want to "guess").

    Imagine having a CLR object[] property and if the JSON has mixed values (strings, bools, numbers), some array elements would be are CLR string, some bool and the numbers would be JsonElement -- that would be more confusing IMHO. Also, Newtonsoft deserializes JSON string sometimes as a GUID or DateTime, which can also be confusing and\or unexpected.

    So if we want to add a Newtonsoft custom converter that is possible, but we can't change the default behavior in any case since that would break backwards compat.

  20. layomia commented on Jul 8, 2021

    @layomia
    Contributor

    Per comments above, I'll close this issue. Changing the default behavior would be a breaking change, and there are workarounds described in System.Text.Json documentation: https://docs.microsoft.com/dotnet/standard/serialization/system-text-json-converters-how-to?pivots=dotnet-5-0#deserialize-inferred-types-to-object-properties.

  21. johnwc commented on Jul 8, 2021

    @johnwc

    @steveharter there was a few shared examples in these comments that gave you great examples of how to test if a JavaScript Number type can safely be deserialized as a valid CLR type.

    As for the bool type you keep through into the mix with Number, what other bool type is there in JavaScript and CLR other than Bool? Why are you not at least converting bool types to valid CLR bool type? I can somewhat see your point about Number type; even though there is simple ways to test for valid serialization, but I don't get the reason for not allowing to do one-to-one bool type.

  22. steveharter commented on Jul 9, 2021

    @steveharter
    Contributor

    @johnwc can you share some scenarios where the CLR type is System.Object for numbers and why it can't be strongly typed as System.Decimal or System.Double?

    JavaScript Number type can safely be deserialized as a valid CLR type.

    If the only known JSON producer\consumer is JavaScript, then I recommend using a custom converter that converts to double only.

    The two main issues with using double for JSON numbers:

    • Loss of precision when dealing with multiple monetary values (such as adding several values and doing an Equals() -- example below).
    • Very large or small integers should be converted to decimal or BigNumber or lose precision.

    Neither Utf8JsonReader or JsonElement automatically perform JSON number conversions (it leaves it up to the user to explicitly call GetDouble(), etc), so the serializer follows that same logic and doesn't "guess". However, it does support using a custom converter (in a relatively easy manner) to do this in whatever fashion is appropriate.

    To avoid precision loss when dealing with monetary values, decimal should be used, not double. If JSON numbers are converted to double (when the CLR type is unknown via System.Object) that means the corresponding user logic will need to use double which may cause various precision issues:

    using System;
    
    public class Example
    {
       public static void Main()
       {
          Double[] values = { 10.0, 2.88, 2.88, 2.88, 9.0 };
          Double result = 27.64;
          Double total = 0;
          foreach (var value in values)
             total += value;
    
          if (total.Equals(result))
             Console.WriteLine("The sum of the values equals the total.");
          else
             Console.WriteLine("The sum of the values ({0}) does not equal the total ({1}).",
                               total, result);
       }
    }
    // The example displays the following output:
    //     The sum of the values (27.639999999999997) does not equal the total (27.64).

    Note Newtonsoft behavior; not exactly intutitive IMHO:

        object oDouble = JsonConvert.DeserializeObject("1.0");
        Console.WriteLine(oDouble.GetType()); // "System.Double
        object oLong = JsonConvert.DeserializeObject("1");
        Console.WriteLine(oLong.GetType()); // System.Int64
        object oBigInteger = JsonConvert.DeserializeObject(decimal.MaxValue.ToString());
        Console.WriteLine(oBigInteger.GetType()); // System.Numerics.BigInteger
  23. johnwc commented on Jul 9, 2021

    @johnwc

    @steveharter again, you keep focusing on doubles and Number type. Use decimals... or use doubles... but not to some arbitrary JsonElement object. It really does not matter what it converts it to, when there is a way to have the ability to overwrite it for exactly how you need it to be outside of the default. Truthfully, I think you're getting caught up on something that is not really an issue for 99% of use cases. As most will not be using an object type for when needing to do math or working with monetary values. With the way it is now, you are forcing the 99% to now write more custom functionality, instead of just using OoB functionality.

    Can you please answer the main part of the comment that was about Bool types not being deserialized?

  24. steveharter commented on Jul 9, 2021

    @steveharter
    Contributor

    Can you please answer the main part of the comment that was about Bool types not being deserialized?

    Here's what I mentioned prior:

    Imagine having a CLR object[] property and if the JSON has mixed values (strings, bools, numbers), some array elements would be are CLR string, some bool and the numbers would be JsonElement -- that would be more confusing IMHO

    Thus if we auto-convert JSON bool to System.Boolean the doc and behavior would be somewhat ambiguous: "When the CLR type is System.Object then all JSON values are converted to JsonElement except a JSON bool which is converted to System.Boolean".

    And if we do that one-off behavior, the next question is what about JSON string (System.String), JSON arrays (System.Array), JSON objects (Dictionary<string, object>) and then of course numbers.

    Going forward, since we can't break default behavior, here are options to address:

    1. Add values to the JsonUnknownTypeHandling enum (which is set on JsonSerializerOptions to make this easier such as:
      JsonUnknownTypeHandling.AutoPrimitivesThenJsonElement
      JsonUnknownTypeHandling.AutoPrimitivesThenJsonElement
    2. Provide a "SystemObjectPrimitiveConverter" or "SystemObjectNewtonsoftCompat" custom converter that must be manually added to JsonSerializerOptions.
    3. Do nothing; users add an appropriate custom converter either from samples or hand-written based on requirements.
  25. steveharter commented on Jul 9, 2021

    @steveharter
    Contributor

    FWIW the new JsonNode may help in certain scenarios since it at least allows explicit casts to primitives:

    // If you can use JsonNode\JsonArray in the signature:
    JsonArray nodes = JsonSerializer.Deserialize<JsonArray>("[1,1.1,\"Hello\"]");
    long l = (long)nodes[0];
    double d = (double)nodes[1];
    string s = (string)nodes[2];

    If you must have System.Object, you can still leverage nodes:

    JsonSerializerOptions options = new();
    options.UnknownTypeHandling = JsonUnknownTypeHandling.JsonNode;
    object[] objects = JsonSerializer.Deserialize<object[]>("[1,1.1,\"Hello\"]", options);
    
    long l = (long)(JsonNode)objects[0];
    // or alternatively:
    l = ((JsonValue)objects[0]).GetValue<long>();
    double d = (double)(JsonNode)objects[1];
    string s = (string)(JsonNode)objects[2];
  26. johnwc commented on Jul 9, 2021

    @johnwc

    I was under the assumption that strings were actually being deserialized as CLR strings, and that only bool and Number types were for some reason being left as JsonElement.

  27. Zetanova commented on Jul 10, 2021

    @Zetanova

    Is there a reason for that JsonElement has no explicit cast operators implemented?

  28. John0King commented on Jul 11, 2021

    @John0King

    Per comments above, I'll close this issue. Changing the default behavior would be a breaking change

    In fact , I think you can not break anyone who use .net to build their application, because no one use System.Text.Json ,
    unless, they are writing demos 🤣.

    and for the scenario that use System.Text.Json in Authentication, well , you already break us before , why you so afraid to break us again ? and beside , break change comes anyway , if it's not here, then it will be there .

    so breaking change should not effect what we talk here, we should talk about what does it should be (the best practice)

  29. steveharter commented on Jul 12, 2021

    @steveharter
    Contributor

    so breaking change should not effect what we talk here, we should talk about what does it should be (the best practice)

    Best practice IMO is to be explicit on the CLR type. This mean either:

    • Change the consuming code to use an explicit type and not System.Object. This avoids the ambiguity (and is faster).
    • Use the current explicit behavior of JsonElement \ JsonNode to obtain the appropriate CLR type from the JSON primitive, possibly calling helper methods to parse the string if the Type is not directly supported:
    • JSON bool: Boolean
    • JSON string: String, DateTime, Guid, Uri, etc
    • JSON number: Long, Ulong, Decimal, etc
  30. ghost locked as resolved and limited conversation to collaborators on Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-System.Text.JsonenhancementProduct code improvement that does NOT require public API changes/additionsjson-functionality-docMissing JSON specific functionality that needs documenting

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions