[System.Text.Json] Expose a setting disallowing duplicate JSON properties

[eiriktsarpalis]

Background

JsonSerializer today tolerates JSON payloads that contain duplicate properties, following a last-write-wins strategy when binding property values. Duplicate properties can be problematic from a security standpoint since they introduce ambiguity, which can be exploited in the context of JSON interoperability vulnerabilities. We should expose an option that prevents duplicate properties from being accepted.

API Proposal

namespace System.Text.Json.Serialization;

public enum JsonDuplicatePropertyHandling
{
    LastWriteWins = 0, // the current default
    FirstWriteWins = 1,
    Error = 2
}

namespace System.Text.Json;

public partial class JsonSerializerOptions
{
    public JsonDuplicatePropertyHandling DuplicatePropertyHandling { get; set; } = JsonDuplicatePropertyHandling.LastWriteWins;
}

public partial class JsonSourceGenerationsOptionsAttribute
{
    public JsonDuplicatePropertyHandling DuplicatePropertyHandling { get; set; } = JsonDuplicatePropertyHandling.LastWriteWins;
}

API Usage

string json = """{ "Value": 1, "Value": -1 }""";
JsonSerializer.Deserialize<MyPoco>(json).Value; // -1

JsonSerializerOptions options = new () { DuplicatePropertyHandling = JsonDuplicatePropertyHandling.FirstWriteWins }
JsonSerializer.Deserialize<MyPoco>(json).Value; // 1

JsonSerializerOptions options = new () { DuplicatePropertyHandling = JsonDuplicatePropertyHandling.Error }
JsonSerializer.Deserialize<MyPoco>(json).Value; // JsonException

record MyPoco(int Value);

Additional Notes

The option should extend to JsonObject but is not applicable to JsonDocument which stores the full JSON payload. We might still be able to enforce lack of duplication which could be expressed as a boolean property on JsonDocumentOptions:

namespace System.Text.Json;

public partial struct JsonDocumentOptions
{
    public bool AllowDuplicateProperties { get; set; } = true;
}

cc @GrabYourPitchforks @JeffreyRichter

[dotnet-policy-service[bot]]

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

[gregsdennis]

This is surprising to me since both JsonElement and JsonNode explode when trying to access objects that even contain duplicates (even when you're not accessing the duplicate property itself).

Good change. 👍

[eiriktsarpalis]

JsonElement should tolerate duplicates, it's JsonNode that does throw ungracefully and lazily.

[JeffreyRichter]

I don't see a need for LastWriteWins & FirstWriteWins. How would anyone decide which to use as it really depends on the JSON payload being parsed. If the current behavior is LastWriteWins then I would just keep that and not offer FirstWriteWins at all. Also, perhaps LastPropertyWins (or BottommostPropertyWins) is a better name as I'd like to think about the JSON payload and not how the deserializer works internally.

And I like your AllowDuplicateProperties proposal as this is simpler and then you can ignore the naming issue I mention above. However, usually you want the befault of something to be false and for back-compat, you might want DisallowDuplicateProperties instead so that this defaults to current behavior.

[eiriktsarpalis]

I came up with the distiction between LastWriteWins and FirstWriteWins for a few reasons:

1. It makes it explicit what the default built-in behavior is (rather than just vaguely "allowing" duplicates)
2. Adding FirstWriteWins unblocks the scenario where you want to be able to tolerate dupes but at the same time ensure that they are being skipped.