-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Description
While reviewing the nullability analysis PR: #32474, found this issue.
This issue got introduced recently in #2259 and regresses the 3.1 behavior.
[Fact]
public static void WritePocoArray()
{
var input = new MyPoco[] { null, new MyPoco { Foo = "foo" } };
// On 5.0/master, this fails with InvalidOperationException since the MyPocoConverter.Write gets called with null.
string json = JsonSerializer.Serialize(input, new JsonSerializerOptions { Converters = { new MyPocoConverter() } });
// On 3.1, correctly outputs the expected value:
Assert.Equal("[null,{\"Foo\":\"foo\"}]", json);
}
public class MyPoco
{
public string Foo { get; set; }
}
public class MyPocoConverter : JsonConverter<MyPoco>
{
public override MyPoco Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
throw new NotImplementedException();
}
public override void Write(Utf8JsonWriter writer, [DisallowNull] MyPoco value, JsonSerializerOptions options)
{
// Debug.Assert(value != null);
if (value == null)
{
throw new InvalidOperationException("The custom converter should never get called with null value.");
}
writer.WriteStartObject();
writer.WriteString(nameof(value.Foo), value.Foo);
writer.WriteEndObject();
}
}System.Text.Json.Serialization.Tests.ArrayTests.WritePocoArray [FAIL]
System.InvalidOperationException : The custom converter should never get called with null value.
Stack Trace:
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\tests\Serialization\Array.WriteTests.cs(50,0): at System.Text.Json.Serialization.Tests.ArrayTests.MyPocoConverter.Write(Utf8JsonWriter writer, MyPoco value, JsonSerializerOptions options)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonConverterOfT.cs(251,0): at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, WriteStack& state)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\Converters\Collection\ArrayConverter.cs(59,0): at System.Text.Json.Serialization.Converters.ArrayConverter`2.OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, WriteStack& state)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\Converters\Collection\IEnumerableDefaultConverter.cs(274,0): at System.Text.Json.Serialization.Converters.IEnumerableDefaultConverter`2.OnTryWrite(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, WriteStack& state)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonConverterOfT.cs(267,0): at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, WriteStack& state)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonConverterOfT.cs(60,0): at System.Text.Json.Serialization.JsonConverter`1.TryWriteAsObject(Utf8JsonWriter writer, Object value, JsonSerializerOptions options, WriteStack& state)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Write.cs(24,0): at System.Text.Json.JsonSerializer.WriteCore(Utf8JsonWriter writer, Object value, JsonSerializerOptions options, WriteStack& state, JsonConverter jsonConverter)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Write.Helpers.cs(105,0): at System.Text.Json.JsonSerializer.WriteCore(Utf8JsonWriter writer, Object value, Type type, JsonSerializerOptions options)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Write.Helpers.cs(82,0): at System.Text.Json.JsonSerializer.WriteCore(PooledByteBufferWriter output, Object value, Type type, JsonSerializerOptions options)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Write.Helpers.cs(56,0): at System.Text.Json.JsonSerializer.WriteCoreString(Object value, Type type, JsonSerializerOptions options)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Write.String.cs(21,0): at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
E:\GitHub\Fork\runtime\src\libraries\System.Text.Json\tests\Serialization\Array.WriteTests.cs(27,0): at System.Text.Json.Serialization.Tests.ArrayTests.WritePocoArray()
Current master (5.0):
Lines 58 to 59 in 5d8f924
| TElement element = array[index]; | |
| if (!elementConverter.TryWrite(writer, element, options, ref state)) |
I believe this needs to special-case/handle null:
runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
Lines 63 to 68 in 5d8f924
| // Provide a default implementation for value converters. | |
| internal virtual bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, ref WriteStack state) | |
| { | |
| Write(writer, value, options); | |
| return true; | |
| } |
Also, we have a test gap here for null elements in collections and enumerables (both with custom converter and default converter).
Reactions are currently unavailable