DateTime fails Serialization/Deserialization roundtrip when JsonCompatible is on

[mishun]

Hi!

The following test case appears to fail:

using System.Diagnostics;
using YamlDotNet.Serialization;

public class Program
{
    public static void Main(string[] argv)
    {
        var ser = new SerializerBuilder().JsonCompatible().Build();
        var des = new DeserializerBuilder().Build();

        var src = new DateTime(656788608000000000L, DateTimeKind.Unspecified);
        var yaml = ser.Serialize(src);
        var dst = des.Deserialize<DateTime>(yaml);

        Debug.Assert(src == dst, $"{src} vs {dst}");
    }
}

Values seem to differ by my local timezone offset, so if your timezone is UTC then it may work, I guess? It also seems to work correctly with DateTimeKind.Utc or when JsonCompatible() is not present.

Tested with:

<PackageReference Include="YamlDotNet" Version="13.3.1" />

[EdwardCooke]

It’s due to the datetime kind you have set.

If the kind is local it will use it as is. Otherwise it converts it to utc.

https://github.com/aaubry/YamlDotNet/blob/1a73db760dc440569cfbd86d1d7e11d59cfcdb8a/YamlDotNet/Serialization/Converters/DateTimeConverter.cs#L94C25-L94C25

since time zones offsets are not in the datetime object we can’t deserialize it back to the exact object that it was. That’s what the datetimeoffset type is for.

With the type being set to unspecified there’s no way for the code to know exactly what it is, utc or local.

It should probably have not cared and just wrote it out as is from the beginning. However, changing that now has the potential to cause big issues with current consumers expecting the current results so I’m very hesitant to do that. I would be fine with making an option with defaulting to the current behavior.

[mishun]

It seems that converter is only used when JsonCompatible:

YamlDotNet/YamlDotNet/Serialization/SerializerBuilder.cs

Lines 329 to 339 in 1a73db7

	public SerializerBuilder JsonCompatible()
	{
	this.emitterSettings = this.emitterSettings
	.WithMaxSimpleKeyLength(int.MaxValue)
	.WithoutAnchorName();
	return this
	.WithTypeConverter(new GuidConverter(true), w => w.InsteadOf<GuidConverter>())
	.WithTypeConverter(new DateTimeConverter(doubleQuotes: true))
	.WithEventEmitter(inner => new JsonEventEmitter(inner, yamlFormatter), loc => loc.InsteadOf<TypeAssigningEventEmitter>());
	}

which is a bit weird.

I suppose it's ok if you say that it's an expected behavior. On the other hand, that deserialize of serialized value yields identical result seems like a very natural invariant. For example, System.Text.Json deals with DateTime using suffixes:

Console.WriteLine(JsonSerializer.Serialize(new DateTime(656788608000000000L, DateTimeKind.Unspecified)));
Console.WriteLine(JsonSerializer.Serialize(new DateTime(656788608000000000L, DateTimeKind.Local)));
Console.WriteLine(JsonSerializer.Serialize(new DateTime(656788608000000000L, DateTimeKind.Utc)));

outputs:

"2082-04-13T00:00:00"
"2082-04-13T00:00:00+03:00"
"2082-04-13T00:00:00Z"

And that's basically what YamlDotNet serializer without JsonCompatible currently does:

var ser = new SerializerBuilder().Build();
Console.WriteLine(ser.Serialize(new DateTime(656788608000000000L, DateTimeKind.Unspecified)));
Console.WriteLine(ser.Serialize(new DateTime(656788608000000000L, DateTimeKind.Local)));
Console.WriteLine(ser.Serialize(new DateTime(656788608000000000L, DateTimeKind.Utc)));

--->

2082-04-13T00:00:00.0000000

2082-04-13T00:00:00.0000000+03:00

2082-04-13T00:00:00.0000000Z

[EdwardCooke]

We can add that in as an option. It would be good to be able to round trip deserialize/serialize. It needs to be an option defaulted to current behavior though.

[mishun]

I admit I don't understand motivation here. Is JsonCompatible() supposed to emulate what JavaScript does? As far as I can see, JavaScript uses toISOString method which outputs something more like 2082-04-13T00:00:00.0000000Z from above (those suffixes are actually backed up by ISO standard, Z is because Date in JavaScript operates in UTC). Currently, serializer with JsonCompatible() seems to just use current locale for DateTime formatting. Honestly, it seems like just a bug.

[EdwardCooke]

I see where you're coming from. JsonCompatible is to make sure that the output is valid Json. I'm fine with putting the time zone offset in there, but it'll need to be something that people can turn off to go back to the current behavior. I can try to work on this in the next little bit, but it may be a couple of weeks.

[EdwardCooke]

#891