Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TimeSpan is (de)serialized incorrectly using the JSON source generator #62082

Closed
martincostello opened this issue Nov 26, 2021 · 3 comments · Fixed by #62130
Closed

TimeSpan is (de)serialized incorrectly using the JSON source generator #62082

martincostello opened this issue Nov 26, 2021 · 3 comments · Fixed by #62130
Assignees
Milestone

Comments

@martincostello
Copy link
Member

Description

If a TimeSpan is serialized using the JSON source generator it is output as an object:

{"duration":{"days":0,"hours":0,"milliseconds":0,"minutes":0,"seconds":1,"ticks":10000000,"totalDays":1.1574074074074073E-05,"totalHours":0.0002777777777777778,"totalMilliseconds":1000,"totalMinutes":0.016666666666666666,"totalSeconds":1}}

If a TimeSpan is serialized without using the source generator, it is serialized as a string:

{"duration":"00:00:01"}

Similarly, the source generator will fail to deserialize strings to TimeSpan values as it is expecting an object.

Curiously, if the instance of JsonSerializerOptions used to serialize as a string is passed to a new instance of the custom JSON serializer context but then not used, then same operation will then change to serializing as an object from a string.

Reproduction Steps

To reproduce run dotnet build using the .NET 6.0.100 SDK for the below application.

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

var value = new MyObject {  Duration = TimeSpan.FromSeconds(1) };

var context = new MyJsonContext(new(JsonSerializerDefaults.Web));
var options = new JsonSerializerOptions(JsonSerializerDefaults.Web);

// Outputs the TimeSpan as an object
var bytes = JsonSerializer.SerializeToUtf8Bytes(value, typeof(MyObject), context);
var json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);
Console.WriteLine();

// Outputs the TimeSpan as a string
bytes = JsonSerializer.SerializeToUtf8Bytes<MyObject>(value, options);
json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);
Console.WriteLine();

options = new JsonSerializerOptions(JsonSerializerDefaults.Web);

// Create a new context that uses the same options.
// This causes the behavior of SerializeToUtf8Bytes<T>() to change.
context = new MyJsonContext(options);

// Outputs the TimeSpan as an object again
bytes = JsonSerializer.SerializeToUtf8Bytes<MyObject>(value, options);
json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);
Console.WriteLine();

try
{
    JsonSerializer.Deserialize<MyObject>("{\"duration\":\"00:00:01\"}", options);
}
catch (JsonException ex)
{
    Console.Error.WriteLine("Failed to deserialize TimeSpan from string: {0}", ex);
    Console.Error.WriteLine();
}

[JsonSerializable(typeof(MyObject))]
public partial class MyJsonContext : JsonSerializerContext
{
}

public class MyObject
{
    public TimeSpan Duration { get; set; }
}
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <ImplicitUsings>enable</ImplicitUsings>
    <NoWarn>$(NoWarn);CA1050</NoWarn>
    <Nullable>enable</Nullable>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
  </PropertyGroup>
</Project>

Expected behavior

The application outputs the following:

{"duration":"00:00:01"}
{"duration":"00:00:01"}
{"duration":"00:00:01"}

Actual behavior

The application outputs the following:

{"duration":{"days":0,"hours":0,"milliseconds":0,"minutes":0,"seconds":1,"ticks":10000000,"totalDays":1.1574074074074073E-05,"totalHours":0.0002777777777777778,"totalMilliseconds":1000,"totalMinutes":0.016666666666666666,"totalSeconds":1}}

{"duration":"00:00:01"}

{"duration":{"days":0,"hours":0,"milliseconds":0,"minutes":0,"seconds":1,"ticks":10000000,"totalDays":1.1574074074074073E-05,"totalHours":0.0002777777777777778,"totalMilliseconds":1000,"totalMinutes":0.016666666666666666,"totalSeconds":1}}

Failed to deserialize TimeSpan from string: System.Text.Json.JsonException: The JSON value could not be converted to System.TimeSpan. Path: $.duration | LineNumber: 0 | BytePositionInLine: 22.
   at System.Text.Json.ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(Type propertyType)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at Program.<Main>$(String[] args) in C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\Program.cs:line 39

Regression?

No.

Known Workarounds

No response

Configuration

Partial output from dotnet --info:

> dotnet --info
.NET SDK (reflecting any global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.100\

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Nov 26, 2021
@ghost
Copy link

ghost commented Nov 26, 2021

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

Issue Details

Description

If a TimeSpan is serialized using the JSON source generator it is output as an object:

{"duration":{"days":0,"hours":0,"milliseconds":0,"minutes":0,"seconds":1,"ticks":10000000,"totalDays":1.1574074074074073E-05,"totalHours":0.0002777777777777778,"totalMilliseconds":1000,"totalMinutes":0.016666666666666666,"totalSeconds":1}}

If a TimeSpan is serialized without using the source generator, it is serialized as a string:

{"duration":"00:00:01"}

Similarly, the source generator will fail to deserialize strings to TimeSpan values as it is expecting an object.

Curiously, if the instance of JsonSerializerOptions used to serialize as a string is passed to a new instance of the custom JSON serializer context but then not used, then same operation will then change to serializing as an object from a string.

Reproduction Steps

To reproduce run dotnet build using the .NET 6.0.100 SDK for the below application.

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

var value = new MyObject {  Duration = TimeSpan.FromSeconds(1) };

var context = new MyJsonContext(new(JsonSerializerDefaults.Web));
var options = new JsonSerializerOptions(JsonSerializerDefaults.Web);

// Outputs the TimeSpan as an object
var bytes = JsonSerializer.SerializeToUtf8Bytes(value, typeof(MyObject), context);
var json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);
Console.WriteLine();

// Outputs the TimeSpan as a string
bytes = JsonSerializer.SerializeToUtf8Bytes<MyObject>(value, options);
json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);
Console.WriteLine();

options = new JsonSerializerOptions(JsonSerializerDefaults.Web);

// Create a new context that uses the same options.
// This causes the behavior of SerializeToUtf8Bytes<T>() to change.
context = new MyJsonContext(options);

// Outputs the TimeSpan as an object again
bytes = JsonSerializer.SerializeToUtf8Bytes<MyObject>(value, options);
json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);
Console.WriteLine();

try
{
    JsonSerializer.Deserialize<MyObject>("{\"duration\":\"00:00:01\"}", options);
}
catch (JsonException ex)
{
    Console.Error.WriteLine("Failed to deserialize TimeSpan from string: {0}", ex);
    Console.Error.WriteLine();
}

[JsonSerializable(typeof(MyObject))]
public partial class MyJsonContext : JsonSerializerContext
{
}

public class MyObject
{
    public TimeSpan Duration { get; set; }
}
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <ImplicitUsings>enable</ImplicitUsings>
    <NoWarn>$(NoWarn);CA1050</NoWarn>
    <Nullable>enable</Nullable>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
  </PropertyGroup>
</Project>

Expected behavior

The application outputs the following:

{"duration":"00:00:01"}
{"duration":"00:00:01"}
{"duration":"00:00:01"}

Actual behavior

The application outputs the following:

{"duration":{"days":0,"hours":0,"milliseconds":0,"minutes":0,"seconds":1,"ticks":10000000,"totalDays":1.1574074074074073E-05,"totalHours":0.0002777777777777778,"totalMilliseconds":1000,"totalMinutes":0.016666666666666666,"totalSeconds":1}}

{"duration":"00:00:01"}

{"duration":{"days":0,"hours":0,"milliseconds":0,"minutes":0,"seconds":1,"ticks":10000000,"totalDays":1.1574074074074073E-05,"totalHours":0.0002777777777777778,"totalMilliseconds":1000,"totalMinutes":0.016666666666666666,"totalSeconds":1}}

Failed to deserialize TimeSpan from string: System.Text.Json.JsonException: The JSON value could not be converted to System.TimeSpan. Path: $.duration | LineNumber: 0 | BytePositionInLine: 22.
   at System.Text.Json.ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(Type propertyType)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Converters.JsonMetadataServicesConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at Program.<Main>$(String[] args) in C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\Program.cs:line 39

Regression?

No.

Known Workarounds

No response

Configuration

Partial output from dotnet --info:

> dotnet --info
.NET SDK (reflecting any global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.100\

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa

Other information

No response

Author: martincostello
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 29, 2021
@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Nov 29, 2021

Good catch, thanks. Looks like #54186 didn't add sourcegen support. I've created #62130 that fixes the issue. I don't believe this meets the bar for servicing but it's low risk enough to consider. cc @ericstj

@eiriktsarpalis eiriktsarpalis added bug and removed untriaged New issue has not been triaged by the area owner labels Nov 29, 2021
@eiriktsarpalis eiriktsarpalis self-assigned this Nov 29, 2021
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Nov 29, 2021
@martincostello
Copy link
Member Author

My (clearly biased) opinion is that this would be a good candidate for servicing as the built-in TimeSpan converter is more performant than a hand-rolled implementation that we would have used in .NET 5. Responses using TimeSpan already are then less compelled to move to the JSON source-generator as the workaround for this issue eats into the perf gains it would bring.

I'm still testing out the JSON source generator in an internal app (which is where these issues have been surfaced), and for the app using TimeSpan I'm not seeing particularly noticeable gains in performance on our benchmarks with the generator plugged in, so I wondered if restoring the custom TimeSpan converter might explain that.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 30, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants