A converter that inherits JsonConverter<object> doesn't work on properties of nullable value type

[thomaslevesque]

If I create a converter that inherits from JsonConverter<object>, it can be applied to a property of type Int32, but not to a property of type Nullable<Int32>. It throws:

InvalidCastException: Unable to cast object of type 'HexIntegerConverter' to type
'System.Text.Json.Serialization.JsonConverter`1[System.Nullable`1[System.Int32]]'.

Repro (in LinqPad)

void Main()
{
    var foo = new Foo { IntProperty = 123, NullableIntProperty = 456 };
    var fooJson = JsonSerializer.Serialize(foo).Dump();
    JsonSerializer.Deserialize<Foo>(fooJson).Dump();
}

class Foo
{
    [JsonConverter(typeof(HexIntegerConverter))]
    public int IntProperty { get; set; }
    [JsonConverter(typeof(HexIntegerConverter))]
    public int? NullableIntProperty { get; set; }
}

// Serializes an int to an hex string
class HexIntegerConverter : JsonConverter<object>
{
    public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        var s = reader.GetString();
        if (string.IsNullOrEmpty(s))
        {
            if (typeToConvert == typeof(int))
                return 0;
            else
                return null;
        }
        
        return int.Parse(s, System.Globalization.NumberStyles.HexNumber);
    }

    public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
    {
        if (value is null)
        {
            writer.WriteNullValue();
        }
        else
        {
            writer.WriteStringValue(((int)value).ToString("x"));
        }
    }

    public override bool CanConvert(Type typeToConvert)
    {
        return typeToConvert == typeof(int) || typeToConvert == typeof(int?);
    }
}

This code throws the exception above. If I remove the converter from the nullable property, it works as expected.

This looks like a bug to me. Why is it trying to cast to a JsonSerializer<int?>? It doesn't do that for the non-nullable property...

[thomaslevesque]

I observed the problem in .NET Core 3.1, but apparently JsonConverter<T> has changed significantly since then. I haven't tried it yet, but it looks like it might already work in 5.0 (if not out of the box, maybe by overriding HandleNull to return true)

[thomaslevesque]

Looks like HandleNull doesn't exist yet in preview3... I'll try it later then.

BTW, I'm getting this error when I try to run the above code in .NET 5 preview3:

Fatal error. Internal CLR error. (0x80131506)

Is this expected, or should I report it?

[steveharter]

Please try overriding HandleNull.

[thomaslevesque]

@steveharter just tried (with .NET 5 preview7), same result:

Fatal error. Internal CLR error. (0x80131506)

[thomaslevesque]

Here's a Gist with the full code if you need it to repro
https://gist.github.com/thomaslevesque/e191e438521ad86cd0f99c315508c838

[steveharter]

@thomaslevesque I've pasted an updated version of your converter below. The converter model is not polymorphic, so using "object" doesn't work as you have it. The <T> in JsonConverter<T> is intended to match the property type.

Null is handled automatically, and for converters based on value types the Read method is called when the JSON property value is null (just in case it has special treatment), otherwise you need to override HandlesNull for reference types.

Some takeaways:

• We should add some better examples for nullability.
• We should doc those examples.
• We could add a check to [private JsonConverter GetConverterFromAttribute] (

  runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs

  Lines 312 to 313 in 650f28f

  	// Allow nullable handling to forward to the underlying type's converter.
  	return NullableConverterFactory.CreateValueConverter(underlyingType, converter);

  ) to ensure we won't get a reflection exception if the types don't match up.

    class HexIntegerConverter : JsonConverter<int>
    {
        public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            string s = reader.GetString();
            if (string.IsNullOrEmpty(s))
            {
                return 0;
            }

            return int.Parse(s, System.Globalization.NumberStyles.HexNumber);
        }

        public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options)
        {
            writer.WriteStringValue(value.ToString("x"));
        }
    }

[thomaslevesque]

@steveharter thanks! Indeed, with your modified version it works correctly.

But I guess it only works in .NET 5... in .NET Core 3.1 I get this error:

System.InvalidOperationException: The converter specified on 'TestSystemTextJsonConverter.Program+Foo.NullableIntProperty' is not compatible with the type 'System.Nullable`1[System.Int32]'.

[devsko]

@steveharter @layomia After #40914 is merged the original version of the converter works as expected. Are there any related issues left?

[layomia]

@devsko after #40914 is merged, there is nothing left to do for this issue.

But I guess it only works in .NET 5... in .NET Core 3.1 I get this error:

@thomaslevesque - the fix for the issue does not meet the bar to be back-ported to .NET Core 3.1.

[layomia]

Re-opening until the fix is back-ported to .NET 5 - #41366.

[layomia]

Fixed for 5.0 in #41366.