Property names should escape leading "$"s when using ReferenceHandling.Preserve

[Jozkee]

Today is totally acceptable that a JSON property name would be written with a leading "$" by using JsonPropertyNameAttribute as an example, this conflicts with metadata properties when using ReferenceHandling.Preserve an causes the payload to be unable to round-trip.

Repro:

private class Node
{
    [JsonPropertyName("$id")]
    public string Id { get; set; }
    public Node Child { get; set; }
}

[Fact]
public static void WritePropertyWithLeadingDollarSign()
{
    Node root = new Node();
    root.Id = "2";
    root.Child = root;

    JsonSerializerOptions opts = new JsonSerializerOptions
    {
        ReferenceHandling = ReferenceHandling.Preserve
    };

    string json = JsonSerializer.Serialize(root, opts);
    Console.WriteLine(json);
    Node rootCopy = JsonSerializer.Deserialize<Node>(json, opts);
}

Output:

{
    "$id": "1",
    "$id": "2",
    "Child": {
        "$ref": "1"
    }
}

The serialized object now contains two "$id" properties that will collide on deserialization and will throw a JsonException for $id not being the first property in the JSON object.

System.Text.Json.JsonException : The metadata property $id must be the first property in the JSON object.
Stack Trace:
    C:\repos\runtime\src\libraries\System.Text.Json\src\System\Text\Json\ThrowHelper.Serialization.cs(311,0): at System.Text.Json.ThrowHelper.ThrowJsonException_MetadataIdIsNotFirstProperty()
    C:\repos\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Read.HandlePropertyName.cs(216,0): at System.Text.Json.JsonSerializer.ResolveMetadataOnObject(ReadOnlySpan`1 propertyName, MetadataPropertyName meta, ReadStack& state, Utf8JsonReader& reader, JsonSerializerOptions options)
    C:\repos\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Read.cs(59,0): at System.Text.Json.JsonSerializer.ReadCore(JsonSerializerOptions options, Utf8JsonReader& reader, ReadStack& readStack)
    C:\repos\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Read.Helpers.cs(23,0): at System.Text.Json.JsonSerializer.ReadCore(Type returnType, JsonSerializerOptions options, Utf8JsonReader& reader)
    C:\repos\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Read.String.cs(91,0): at System.Text.Json.JsonSerializer.Deserialize(String json, Type returnType, JsonSerializerOptions options)
    C:\repos\runtime\src\libraries\System.Text.Json\src\System\Text\Json\Serialization\JsonSerializer.Read.String.cs(33,0): at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
    c:\repos\runtime\src\libraries\System.Text.Json\tests\Serialization\ReferenceHandlingTests.cs(381,0): at System.Text.Json.Tests.ReferenceHandlingTests.WritePropertyWithLeadingDollarSign()

To avoid this, we must do as suggested in dotnet/apireviews#109 (comment)

On serialization, when a JSON property name, that is either a dictionary key or a CLR class property, starts with a '$' character, we must write the escaped character "\u0024" instead.

There are several means of how a CLR property may end up with a "$" into its name. it can be made through IL generation or by using F# which contrary to C#, it does not constrain that properties start with an alphabetical character (IIRC).

On deserialization, metadata will be digested by using only the raw bytes, so no encoded characters are allowed in metadata; to read JSON properties that start with a '$' you will need to pass it with the escaped '$' (\u0024).

Using only raw bytes to determine if a property is metadata or not is already performed on deserialization, so presumably no changes will be need from that side.

cc @ahsonkhan, @steveharter, @layomia

[ghost]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of the experimental issue cleanup initiative we are currently trialing in a limited number of areas. Please share any feedback you might have in the linked issue.

[ghost]

This issue will now be closed since it had been marked no recent activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.