JsonSerializer uses different default values for properties between reading and writing, causing data loss in round-trips

[nagya]

Description

Serialization uses the default value of the type (e.g. 0 for int), while deserialization uses the default value of the parameter in the constructor.

Reproduction Steps

using System.Text.Json;
using System.Text.Json.Serialization;

namespace ConsoleApp2;

[JsonSerializable(typeof(Test))]
[JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault)]
partial class JsonContext : JsonSerializerContext { }

sealed class Test
{
    public int x { get; }
    public int y { get; }

    public Test(int x, int y = 42)
    {
        this.x = x;
        this.y = y;
    }

    public override string ToString() => $"{x} {y}";
}

static class Program
{
    static void Main(string[] args)
    {
        Test testOrig = new Test(1, 0);
        Console.WriteLine(testOrig);
        string s = JsonSerializer.Serialize(testOrig, JsonContext.Default.Test);
        Console.WriteLine(s);
        Test? testNew = JsonSerializer.Deserialize(s, JsonContext.Default.Test);
        Console.WriteLine(testNew);
        Console.WriteLine(JsonSerializer.Serialize(testNew, JsonContext.Default.Test));
    }
}

Expected behavior

Either of these two would be acceptable:

1 0
{"x":1,"y":0}
1 0
{"x":1,"y":0}

or:

1 0
{"x":1}
1 0
{"x":1}

Actual behavior

1 0
{"x":1}
1 42
{"x":1,"y":42}

Regression?

No response

Known Workarounds

Add another constructor that does not have default parameter values, and mark that with [JsonConstructor]

Configuration

No response

Other information

No response

[dotnet-policy-service]

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

[eiriktsarpalis]

I would consider this by-design behaviour, and would be a breaking change assuming we tried to change it. You can either remove the optional parameter from the constructor, use a secondary deserialization constructor like you suggest, or avoid skipping default values.

[nagya]

@eiriktsarpalis, a fix can always be opt-in and thus non-breaking (e.g. by adding JsonIgnoreCondition.Roundtrip, which would omit the field iff it has the same value the reader would default to.) So the breaking change concern aside, do you still consider this by design? If the reader and the writer both support default values, wouldn't it be a reasonable expectation that they can at least be configured to use the same default value?

[eiriktsarpalis]

So the breaking change concern aside, do you still consider this by design?

I would think so, yes. It is called an ignore condition which means that it only controls the write side of things. Correspondingly, specifying a deserialization constructor only controls deserialization aspects and has no impact on serialization. In isolation I would regard either setting to be sound even if they can conflict in the way you're describing. The serializer doesn't guarantee that any type can be roundtripped for every possible configuration -- it will make a best effort to ensure this but beyond that it becomes the responsibility of the user to model something that works.