Runtime: System.Text.Json JsonConverterFactory .NET 5 Regression

Created on 16 Jun 2020 · 4Comments · Source: dotnet/runtime

There seems to be a change in .NET 5 with regard to JsonConverters and nullable types. Previously converter would not be called for null values.

Description

This test fails on .NET 5, works fine previously.

        public class SalesOrder
        {
            [JsonConverter(typeof(JsonMicrosoftDateTimeConverter))]
            public DateTime? PaymentDueDate { get; set; }
        }

        [Fact]
        public static void DeserialiseSalesOrderWithNullPaymentDueDateTest()
        {
            string jsonSalesOrder = @"{
                ""PaymentDueDate"": null
            }";

            // Exception thrown here on .NET 5.
            var order = JsonSerializer.Deserialize<SalesOrder>(jsonSalesOrder);

            Assert.NotNull(order);
            Assert.Null(order.PaymentDueDate);

            // Quick check to re-serialize process, to make sure we can go back and forth without issue

            string reSerialized = JsonSerializer.Serialize(order);

            Assert.NotNull(reSerialized);
        }

        public class JsonMicrosoftDateTimeConverter : JsonConverterFactory
        {
            /// <inheritdoc/>
            public override bool CanConvert(Type typeToConvert)
            {
                // Don't perform a typeToConvert == null check for performance. Trust our callers will be nice.
                return typeToConvert == typeof(DateTime)
                    || (typeToConvert.IsGenericType && IsNullableDateTime(typeToConvert));
            }

            /// <inheritdoc/>
            public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
            {
                // Don't perform a typeToConvert == null check for performance. Trust our callers will be nice.
                return typeToConvert.IsGenericType
                    ? (JsonConverter)new JsonNullableDateTimeConverter()
                    : new JsonStandardDateTimeConverter();
            }

            private static bool IsNullableDateTime(Type typeToConvert)
            {
                Type UnderlyingType = Nullable.GetUnderlyingType(typeToConvert);

                return UnderlyingType != null && UnderlyingType == typeof(DateTime);
            }

            internal class JsonStandardDateTimeConverter : JsonDateTimeConverter<DateTime>
            {
                /// <inheritdoc/>
                public override DateTime Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
                    => ReadDateTime(ref reader);

                /// <inheritdoc/>
                public override void Write(Utf8JsonWriter writer, DateTime value, JsonSerializerOptions options)
                    => WriteDateTime(writer, value);
            }

            internal class JsonNullableDateTimeConverter : JsonDateTimeConverter<DateTime?>
            {
                /// <inheritdoc/>
                public override DateTime? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
                    => ReadDateTime(ref reader);

                /// <inheritdoc/>
                public override void Write(Utf8JsonWriter writer, DateTime? value, JsonSerializerOptions options)
                    => WriteDateTime(writer, value!.Value);
            }

            internal abstract class JsonDateTimeConverter<T> : JsonConverter<T>
            {
                private static readonly DateTime s_Epoch = DateTime.SpecifyKind(new DateTime(1970, 1, 1, 0, 0, 0), DateTimeKind.Utc);
                private static readonly Regex s_Regex = new Regex("^/Date\\(([^+-]+)\\)/$", RegexOptions.CultureInvariant | RegexOptions.Compiled);

                public static DateTime ReadDateTime(ref Utf8JsonReader reader)
                {
                    if (reader.TokenType != JsonTokenType.String)
                        throw new JsonException();

                    string formatted = reader.GetString();
                    Match match = s_Regex.Match(formatted);

                    return !match.Success
                        || !long.TryParse(match.Groups[1].Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out long unixTime)
                        ? throw new JsonException("Unexpected value format, unable to parse DateTime.")
                        : s_Epoch.AddMilliseconds(unixTime);
                }

                public static void WriteDateTime(Utf8JsonWriter writer, DateTime value)
                {
                    long unixTime = Convert.ToInt64((value.ToUniversalTime() - s_Epoch).TotalMilliseconds);

                    string formatted = FormattableString.Invariant($"/Date({unixTime})/");
                    writer.WriteStringValue(formatted);
                }
            }
        }

Read method could detect Null token and handle it. But up until now it hasn't needed to. .NET 5 could potentially break a lot of JsonConverters reliant on this behavior.

Configuration

Which version of .NET is the code running on? .NET 5 Preview 3 & Preview 5.
What OS and version, and what distro if applicable? Windows 10
What is the architecture (x64, x86, ARM, ARM64)? x64
Do you know whether it is specific to that configuration? Not configuration specific.