Deserialising null to JsonNode in nested structures fails in System.Text.Json

[adj123]

JsonSerializer.Deserialize<JsonNode>("null") returns null
JsonSerializer.Deserialize<List<JsonNode>>("[null]") throws
JsonSerializer.Deserialize<Dictionary<string, JsonNode>>("{ \"Foo\": null }") throws

Assuming this isn't intentional? Reproducible in .Net 6 & 7.

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

JsonSerializer.Deserialize<JsonNode>("null") returns null
JsonSerializer.Deserialize<List<JsonNode>>("[null]") throws
JsonSerializer.Deserialize<Dictionary<string, JsonNode>>("{ \"Foo\": null }") throws

Assuming this isn't intentional? Reproducible in .Net 6 & 7.

Author:	adj123
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

I can reproduce, looks like the JsonNode converters do not handle JsonTokenType.Null on deserialization, hitting this case:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs

Lines 64 to 66 in b1497d3

	default:
	Debug.Assert(false);
	throw new JsonException();

and the same seems to be true for the derived node converters, e.g.

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonArrayConverter.cs

Lines 19 to 26 in b1497d3

	switch (reader.TokenType)
	{
	case JsonTokenType.StartArray:
	return ReadList(ref reader, options.GetNodeOptions());
	default:
	Debug.Assert(false);
	throw ThrowHelper.GetInvalidOperationException_ExpectedArray(reader.TokenType);
	}

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs

Lines 44 to 51 in b1497d3

	switch (reader.TokenType)
	{
	case JsonTokenType.StartObject:
	return ReadObject(ref reader, options.GetNodeOptions());
	default:
	Debug.Assert(false);
	throw ThrowHelper.GetInvalidOperationException_ExpectedObject(reader.TokenType);
	}

@adj123 would you be interested in contributing a fix?

[eiriktsarpalis]

Tagging @steveharter in case the current behavior baked into the converters is intentional.

[adj123]

Sure I can do. I'll wait to hear confirmation the behaviour should be changed first.

[eiriktsarpalis]

@adj123 I would say go ahead and open the PR. Generally speaking converters should tolerate null tokens if they also serialize null values.