From 0a57a9b20905b1e14993dc4604bad3bdf0b57fa2 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 10 May 2023 21:18:38 +0300 Subject: [PATCH] Fix null handling of built-in converters. (#86056) --- .../Converters/Node/JsonArrayConverter.cs | 7 ++- .../Converters/Node/JsonObjectConverter.cs | 7 ++- .../Converters/Node/JsonValueConverter.cs | 15 ++++-- .../Converters/Object/ObjectConverter.cs | 7 ++- .../Converters/Value/JsonDocumentConverter.cs | 6 +++ .../Serialization/Converters/Value/UriConverter.cs | 32 ++++++++----- .../Converters/Value/VersionConverter.cs | 13 ++++- .../Serialization/Null.ReadTests.cs | 39 +++++++++++++++ .../Serialization/Null.WriteTests.cs | 56 ++++++++++++++++++++++ 9 files changed, 164 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonArrayConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonArrayConverter.cs index bd44ed1..d0521e6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonArrayConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonArrayConverter.cs @@ -10,7 +10,12 @@ namespace System.Text.Json.Serialization.Converters { public override void Write(Utf8JsonWriter writer, JsonArray value, JsonSerializerOptions options) { - Debug.Assert(value != null); + if (value is null) + { + writer.WriteNullValue(); + return; + } + value.WriteTo(writer, options); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs index 860009c..6c55059 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs @@ -35,7 +35,12 @@ namespace System.Text.Json.Serialization.Converters public override void Write(Utf8JsonWriter writer, JsonObject value, JsonSerializerOptions options) { - Debug.Assert(value != null); + if (value is null) + { + writer.WriteNullValue(); + return; + } + value.WriteTo(writer, options); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs index 68ae58b..af01541 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics; using System.Text.Json.Nodes; using System.Text.Json.Serialization.Metadata; @@ -11,12 +10,22 @@ namespace System.Text.Json.Serialization.Converters { public override void Write(Utf8JsonWriter writer, JsonValue value, JsonSerializerOptions options) { - Debug.Assert(value != null); + if (value is null) + { + writer.WriteNullValue(); + return; + } + value.WriteTo(writer, options); } - public override JsonValue Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + public override JsonValue? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { + if (reader.TokenType is JsonTokenType.Null) + { + return null; + } + JsonElement element = JsonElement.ParseValue(ref reader); JsonValue value = new JsonValueTrimmable(element, JsonMetadataServices.JsonElementConverter, options.GetNodeOptions()); return value; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs index 97526b2..34c4cb2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs @@ -30,7 +30,12 @@ namespace System.Text.Json.Serialization.Converters public override void Write(Utf8JsonWriter writer, object? value, JsonSerializerOptions options) { - Debug.Assert(value?.GetType() == typeof(object)); + if (value is null) + { + writer.WriteNullValue(); + return; + } + writer.WriteStartObject(); writer.WriteEndObject(); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonDocumentConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonDocumentConverter.cs index de505fe..22c67ce 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonDocumentConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonDocumentConverter.cs @@ -12,6 +12,12 @@ namespace System.Text.Json.Serialization.Converters public override void Write(Utf8JsonWriter writer, JsonDocument value, JsonSerializerOptions options) { + if (value is null) + { + writer.WriteNullValue(); + return; + } + value.WriteTo(writer); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UriConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UriConverter.cs index e50a652..9d582cc 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UriConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UriConverter.cs @@ -2,33 +2,43 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Runtime.CompilerServices; namespace System.Text.Json.Serialization.Converters { internal sealed class UriConverter : JsonPrimitiveConverter { - public override Uri Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + public override Uri? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - string? uriString = reader.GetString(); - if (Uri.TryCreate(uriString, UriKind.RelativeOrAbsolute, out Uri? value)) - { - return value; - } - - ThrowHelper.ThrowJsonException(); - return null; + return reader.TokenType is JsonTokenType.Null ? null : ReadCore(ref reader); } public override void Write(Utf8JsonWriter writer, Uri value, JsonSerializerOptions options) { + if (value is null) + { + writer.WriteNullValue(); + return; + } + writer.WriteStringValue(value.OriginalString); } internal override Uri ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { Debug.Assert(reader.TokenType is JsonTokenType.PropertyName); - return Read(ref reader, typeToConvert, options); + return ReadCore(ref reader); + } + + private static Uri ReadCore(ref Utf8JsonReader reader) + { + string? uriString = reader.GetString(); + + if (!Uri.TryCreate(uriString, UriKind.RelativeOrAbsolute, out Uri? value)) + { + ThrowHelper.ThrowJsonException(); + } + + return value; } internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, Uri value, JsonSerializerOptions options, bool isWritingExtensionDataProperty) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs index fa4d0a0..052d928 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs @@ -15,8 +15,13 @@ namespace System.Text.Json.Serialization.Converters private const int MaximumEscapedVersionLength = JsonConstants.MaxExpansionFactorWhileEscaping * MaximumVersionLength; #endif - public override Version Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + public override Version? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { + if (reader.TokenType is JsonTokenType.Null) + { + return null; + } + if (reader.TokenType != JsonTokenType.String) { ThrowHelper.ThrowInvalidOperationException_ExpectedString(reader.TokenType); @@ -73,6 +78,12 @@ namespace System.Text.Json.Serialization.Converters public override void Write(Utf8JsonWriter writer, Version value, JsonSerializerOptions options) { + if (value is null) + { + writer.WriteNullValue(); + return; + } + #if NETCOREAPP Span span = stackalloc char[MaximumVersionLength]; bool formattedSuccessfully = value.TryFormat(span, out int charsWritten); diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.ReadTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.ReadTests.cs index 9b5ecf8..41de7e6 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.ReadTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.ReadTests.cs @@ -256,5 +256,44 @@ namespace System.Text.Json.Serialization.Tests Assert.Null(JsonSerializer.Deserialize("null", type, options)); } } + + [Theory] + [MemberData(nameof(GetBuiltInConvertersForNullableTypes))] + public static void ReadNullValue_BuiltInConverter(JsonConverter converter) + { + var reader = new Utf8JsonReader("null"u8); + Assert.True(reader.Read()); + Assert.Equal(JsonTokenType.Null, reader.TokenType); + + T? result = converter.Read(ref reader, typeof(T), JsonSerializerOptions.Default); + Assert.True(result is null or JsonDocument { RootElement.ValueKind: JsonValueKind.Null } or JsonElement { ValueKind: JsonValueKind.Null }); + } + + [Theory] + [MemberData(nameof(GetBuiltInConvertersForNullableTypes))] + public static void DeserializeNullValue_BuiltInConverter(JsonConverter converter) + { + _ = converter; // Not needed here. + + T? value = JsonSerializer.Deserialize("null"); + AssertNull(value); + + T[]? array = JsonSerializer.Deserialize("[null]"); + AssertNull(array[0]); + + List? list = JsonSerializer.Deserialize>("[null]"); + AssertNull(list[0]); + + GenericRecord? record = JsonSerializer.Deserialize>("""{"Value":null}"""); + AssertNull(record.Value); + + Dictionary? dictionary = JsonSerializer.Deserialize>("""{"Key":null}"""); + AssertNull(dictionary["Key"]); + + static void AssertNull(T? result) => Assert.True( + result is null + or JsonDocument { RootElement.ValueKind: JsonValueKind.Null } + or JsonElement { ValueKind: JsonValueKind.Null }); + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.WriteTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.WriteTests.cs index dec9dee..7de51c9 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.WriteTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Null.WriteTests.cs @@ -2,6 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Text.Json.Serialization.Metadata; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -273,5 +276,58 @@ namespace System.Text.Json.Serialization.Tests json = JsonSerializer.Serialize(arr, options); Assert.Equal("null", json); } + + [Theory] + [MemberData(nameof(GetBuiltInConvertersForNullableTypes))] + public static void WriteNullValue_BuiltInConverter(JsonConverter converter) + { + T @null = default; + Assert.Null(@null); + + using var stream = new Utf8MemoryStream(); + using var writer = new Utf8JsonWriter(stream); + + converter.Write(writer, @null, JsonSerializerOptions.Default); + writer.Flush(); + + Assert.Equal("null", stream.AsString()); + } + + [Theory] + [MemberData(nameof(GetBuiltInConvertersForNullableTypes))] + public static void SerializeNullValue_BuiltInConverter(JsonConverter converter) + { + _ = converter; // Not needed here. + + T @null = default; + Assert.Null(@null); + + string json = JsonSerializer.Serialize(@null); + Assert.Equal("null", json); + + json = JsonSerializer.Serialize(new T[] { @null }); + Assert.Equal("[null]", json); + + json = JsonSerializer.Serialize(new List { @null }); + Assert.Equal("[null]", json); + + json = JsonSerializer.Serialize(new GenericRecord(@null)); + Assert.Equal("""{"Value":null}""", json); + + json = JsonSerializer.Serialize(new Dictionary { ["Key"] = @null }); + Assert.Equal("""{"Key":null}""", json); + } + + public static IEnumerable GetBuiltInConvertersForNullableTypes() + { + return typeof(JsonMetadataServices) + .GetProperties(BindingFlags.Public | BindingFlags.Static) + .Where(prop => + prop.PropertyType.IsGenericType && prop.PropertyType.GetGenericTypeDefinition() == typeof(JsonConverter<>) && + !prop.PropertyType.GetGenericArguments()[0].IsValueType) + .Select(prop => new object?[] { prop.GetValue(null) }); + } + + public record GenericRecord(T Value); } } -- 2.7.4