Skip to content

Registering the factory hijacks JsonNode and JsonDocument: they serialize as a quoted string, and ordinary object JSON no longer deserializes #98

Description

@matt-edmondson

What's wrong

IsBuiltInType (RoundTripStringJsonConverter.cs:57-89) excludes only types in the CoreLib System namespace and a few collection shapes. System.Text.Json.Nodes.JsonNode has Parse(string, JsonNodeOptions? = null, JsonDocumentOptions = default), and JsonDocument has Parse(string, JsonDocumentOptions = default). Both return their own type, so IsUsableConversionMethod (:153-162) accepts them, and CanConvert returns true for JsonNode and JsonDocument.

Converters in options.Converters take priority over STJ's built-in converters. So registering this factory replaces STJ's own handling of these types. CanConvert already returned true for them before #96. Since #96 accepts optional parameters, the string form also reads back successfully, so the takeover no longer fails loudly.

Repro

class Holder { public JsonNode? Node { get; set; } }
var o = new JsonSerializerOptions { Converters = { new RoundTripStringJsonConverterFactory() } };

JsonSerializer.Serialize(new Holder { Node = JsonNode.Parse("{\"a\":1}") }, o);
// observed: {"Node":"{\n  \"a\": 1\n}"}   (the object is written as a string)
// expected: {"Node":{"a":1}}

JsonSerializer.Deserialize<Holder>("{\"Node\":{\"a\":1}}", o);
// observed: JsonException "Expected string token, got StartObject"
// expected: succeeds, as it does without the factory

Any app that registers the factory globally and also carries free-form JSON (JsonNode, JsonObject, JsonDocument properties) breaks its wire format. It then fails on every existing payload.

Suggested fix

In IsBuiltInType, exclude types whose namespace is System.Text.Json or starts with System.Text.Json.. More generally, exclude any type STJ already has a built-in converter for. A test should assert that CanConvert(typeof(JsonNode)), CanConvert(typeof(JsonObject)) and CanConvert(typeof(JsonDocument)) are all false.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions