-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
System.Text.Json deserializing of object[bool] does not produce boolean #29960
Comments
Current functionality treats any 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 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); |
sounds good, I'll use the custom converter. Thanks. |
@steveharter Json and Javascript don't have many type, but
|
@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)? |
FWIW, using an |
@steveharter |
Maybe it's better to throw instead of deserializing as I have personally wanted to deserialize to object a few times in the past with JSON.NET. I always wanted the "obvious" deserialization to If I want to deserialize to |
@steveharter
Regarding the design choices; 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. |
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. |
@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 |
In most cases when I've seen this, it has been an anti-pattern. |
The JSON->POCO auto-generator in VS creates 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. |
suggest to add a new strategy :
|
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 runtime/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Object.cs Line 267 in 3e4a06c
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.");
}
} |
See the runtime/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Object.cs Line 267 in 7eea339
|
Is JsonElement now a new type for POCO? Any news about this issue? I am trying to deserialize a response with a value-list as object. Currently with dotnet 3.1 the System.Text.Json Serializer is putting the values as JsonElement (string)model.Values[0]; //will fail
(DateTimeOffset)model.Values[1]; //will fail
(double)model.Values[2]; //will fail The only option what i see is to change from Object[] to JsonElement[] |
It's a struct that holds the JSON itself and provides ways for you to walk the document object model (DOM), programmatically.
You can cast the So: object model = JsonSerializer.Deserialize<object>("[\"str\", \"2020-02-22T05:50:46.3605858+00:00\", 25.5]");
if (model is JsonElement element)
{
string str = element[0].GetString(); // returns "str"
DateTimeOffset date = element[1].GetDateTimeOffset();
double num = element[2].GetDouble(); //returns 25.5
} |
@ahsonkhan Thx for your answere, but it is not answering it. |
For those looking to convert JSON object to Hashtable, feel free to use my example. It should cover all JSON types. But, I have not tested it thoroughly, only for what we are needing it for. I have not used the write either, as we are only reading. var options = new JsonSerializerOptions();
options.Converters.Add(new JsonHashtableConverter());
var obj = JsonSerializer.Deserialize<Hashtable>(object, options);
public class JsonHashtableConverter : JsonConverterFactory
{
private static JsonConverter<Hashtable> _valueConverter = null;
public override bool CanConvert(Type typeToConvert)
{
return typeToConvert == typeof(Hashtable);
}
public override JsonConverter CreateConverter(Type type, JsonSerializerOptions options)
{
return _valueConverter ?? (_valueConverter = new HashtableConverterInner(options));
}
private class HashtableConverterInner : JsonConverter<Hashtable>
{
private JsonSerializerOptions _options;
private JsonConverter<Hashtable> _valueConverter = null;
JsonConverter<Hashtable> converter
{
get
{
return _valueConverter ?? (_valueConverter = (JsonConverter<Hashtable>)_options.GetConverter(typeof(Hashtable)));
}
}
public HashtableConverterInner(JsonSerializerOptions options)
{
_options = options;
}
public override Hashtable Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.StartObject)
{
throw new JsonException();
}
Hashtable hashtable = new Hashtable();
while (reader.Read())
{
if (reader.TokenType == JsonTokenType.EndObject)
{
return hashtable;
}
// Get the key.
if (reader.TokenType != JsonTokenType.PropertyName)
{
throw new JsonException();
}
string propertyName = reader.GetString();
reader.Read();
hashtable[propertyName] = getValue(ref reader, options);
}
return hashtable;
}
private object getValue(ref Utf8JsonReader reader, JsonSerializerOptions options)
{
switch (reader.TokenType)
{
case JsonTokenType.String:
return reader.GetString();
case JsonTokenType.False:
return false;
case JsonTokenType.True:
return true;
case JsonTokenType.Null:
return null;
case JsonTokenType.Number:
if (reader.TryGetInt64(out long _long))
return _long;
else if (reader.TryGetDecimal(out decimal _dec))
return _dec;
throw new JsonException($"Unhandled Number value");
case JsonTokenType.StartObject:
return JsonSerializer.Deserialize<Hashtable>(ref reader, options);
case JsonTokenType.StartArray:
List<object> array = new List<object>();
while (reader.Read() &&
reader.TokenType != JsonTokenType.EndArray)
{
array.Add(getValue(ref reader, options));
}
return array.ToArray();
}
throw new JsonException($"Unhandled TokenType {reader.TokenType}");
}
public override void Write(Utf8JsonWriter writer, Hashtable hashtable, JsonSerializerOptions options)
{
writer.WriteStartObject();
foreach (KeyValuePair<string, object> kvp in hashtable)
{
writer.WritePropertyName(kvp.Key);
if (converter != null &&
kvp.Value is Hashtable)
{
converter.Write(writer, (Hashtable)kvp.Value, options);
}
else
{
JsonSerializer.Serialize(writer, kvp.Value, options);
}
}
writer.WriteEndObject();
}
}
} |
Edit: Added in write ability for json objects being deserialized (returned) from the controller actions as well. I omitted the extension methods for isIntegerType etc... for brevity. I agree with @gmurray81 that I expected this to be a fairly common use case to serialize/deserialize basic primitives in an object property. It was a 2 hour detour for me today to solve this problem, and I'm not even sure if I solved it the right way (it works, but I'm open to feedback on better ways). Multiply that by all the developers out there who have and will continue to stumble through this problem, coming up with their own clunky workarounds. For others wanting to do something similar... I needed an object property that accepts bool, number, or string for binding through ASP.NET core controller actions: public class MyConverter : JsonConverter<object>
{
public override object Read(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options)
{
long longValue = 0;
Decimal decimalValue = 0.0M;
if (reader.TokenType == JsonTokenType.Number)
{
if (reader.TryGetInt64(out longValue))
{
return longValue;
}
else
{
if (reader.TryGetDecimal(out decimalValue))
{
return decimalValue;
}
}
}
else if (reader.TokenType == JsonTokenType.True)
{
return true;
}
else if (reader.TokenType == JsonTokenType.False)
{
return false;
}
else if (reader.TokenType == JsonTokenType.String)
{
return reader.GetString();
}
throw new JsonException($"Invalid type {reader.TokenType}");
}
public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
{
if (value.IsString())
{
writer.WriteStringValue((string)value);
}
else if (value.IsFloating())
{
writer.WriteNumberValue((decimal)value);
}
else if (value.IsInteger())
{
writer.WriteNumberValue((long)value);
}
else if (value.IsBoolean())
{
writer.WriteBooleanValue((bool)value);
}
else
{
throw new InvalidOperationException("Directly writing object not supported");
}
}
public override bool CanConvert(Type typeToConvert)
{
return typeToConvert.IsObjectType() || typeToConvert.IsIntegerType() || typeToConvert.IsFloatingType() || typeToConvert.IsBooleanType() || typeToConvert.IsStringType();
}
} Which I applied to my model for ASP.NET core binding: public class MyModel
{
...
[JsonConverter(typeof(MyConverter))]
public object Value { get; set; }
...
} And used in my Controller Action: public async Task Create([FromBody]MyModel model)
{
await DoWork(model);
} |
Microsoft docs now has section Deserialize inferred types to object properties w/ an example JSON object converter. When using Bind(IConfiguration, Object) with JSON provider, it will deserialize as a string, i.e. 1 as "1" & true as "True". |
Is there a concrete solution for the controllers in ASP.NET Core? The UpdateThe solution is to set this line to services.AddControllers().AddJsonOptions(options =>
options.JsonSerializerOptions.Converters.Add(new SystemObjectNewtonsoftCompatibleConverter())
);
After that, the
|
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 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. |
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. |
@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. |
@johnwc can you share some scenarios where the CLR type is
If the only known JSON producer\consumer is JavaScript, then I recommend using a custom converter that converts to The two main issues with using
Neither To avoid precision loss when dealing with monetary values, 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 |
@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? |
Here's what I mentioned prior:
Thus if we auto-convert JSON bool to And if we do that one-off behavior, the next question is what about JSON string ( Going forward, since we can't break default behavior, here are options to address:
|
FWIW the new // 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 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]; |
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. |
Is there a reason for that |
In fact , I think you can not break anyone who use .net to build their application, because no one use and for the scenario that use 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:
|
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:
Controller method:
Posting this json:
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.
The text was updated successfully, but these errors were encountered: