JsonSerializer doesn't support valid TimeSpan formats

[eerhardt]

Description

When trying to deserialize TimeSpan strings from JSON into an object using JsonSerializer.Deserialize, not all valid TimeSpan formats are supported.

This is inconsistent with our other APIs that deserialize TimeSpan values from strings (for example Microsoft.Extensions.Configuration.Binder). It can cause problems when people are reading TimeSpan information from other sources and can't control the format being written.

Reproduction Steps

using Newtonsoft.Json;
using System.Buffers.Text;
using System.Text;

string[] timeSpanStrings = ["00:01", "10"];

foreach (string timeSpanString in timeSpanStrings)
{
    TimeSpan timeSpan = TimeSpan.Parse(timeSpanString);
    Console.WriteLine(timeSpan);

    var timeSpanStringUtf8 = Encoding.UTF8.GetBytes(timeSpanString);
    Utf8Parser.TryParse(timeSpanStringUtf8, out timeSpan, out _, standardFormat: 'c');
    Console.WriteLine(timeSpan);

    string json = $$"""
    {
      "Delay": "{{timeSpanString}}"
    }
    """;

    var options = JsonConvert.DeserializeObject<MyOptions>(json);
    Console.WriteLine(options?.Delay);

    try
    {
        options = System.Text.Json.JsonSerializer.Deserialize<MyOptions>(json);
        Console.WriteLine(options?.Delay);
    }
    catch (Exception e)
    {
        Console.WriteLine(e);
    }

    Console.WriteLine("----");
}
public class MyOptions
{
    public TimeSpan Delay { get; set; }
}

Expected behavior

The above code should print each TimeSpan value in timeSpanStrings 4 times:

• TimeSpan.Parse
• Utf8Parser.TryParse
• The result of Newtonsoft.Json's JsonConvert.DeserializeObject
• The result from JsonSerializer.Deserialize

Actual behavior

00:01:00
00:01:00
00:01:00
System.Text.Json.JsonException: The JSON value could not be converted to System.TimeSpan. Path: $.Delay | LineNumber: 1 | BytePositionInLine: 18.
 ---> System.FormatException: The JSON value is not in a supported TimeSpan format.
   at System.Text.Json.ThrowHelper.ThrowFormatException(DataType dataType)
   at System.Text.Json.Serialization.Converters.TimeSpanConverter.ReadCore(Utf8JsonReader& reader)
   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.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   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`1 jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo`1 jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at Program.<Main>$(String[] args) in C:\Users\eerhardt\source\repos\ConsoleApp115\ConsoleApp115\Program.cs:line 27
----
10.00:00:00
10.00:00:00
10.00:00:00
System.Text.Json.JsonException: The JSON value could not be converted to System.TimeSpan. Path: $.Delay | LineNumber: 1 | BytePositionInLine: 15.
 ---> System.FormatException: The JSON value is not in a supported TimeSpan format.
   at System.Text.Json.ThrowHelper.ThrowFormatException(DataType dataType)
   at System.Text.Json.Serialization.Converters.TimeSpanConverter.ReadCore(Utf8JsonReader& reader)
   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.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   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`1 jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo`1 jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at Program.<Main>$(String[] args) in C:\Users\eerhardt\source\repos\ConsoleApp115\ConsoleApp115\Program.cs:line 27
----

Regression?

No

Known Workarounds

No response

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]

The built-in converter for TimeSpan is intentionally blocking out non-canonical representations. This discussion in the implementation PR should provide more context as to why we went for that route. TL;DR it was found that the TimeSpan parser was too permissive sometimes resulting in surprising results. Because the converter is capable of round tripping formats generated by TimeSpan.ToString() we decided to go ahead with this more restrictive version, and potentially relaxing it in the future provided there is demand.

[eerhardt]

TL;DR it was found that the TimeSpan parser was too permissive sometimes resulting in surprising results.

I can totally understand restricting to the 'c' format to eliminate the surprising results, that makes sense. The part that I think should be fixed is adding further restrictions from the 'c' format. Basically if Utf8Parser.TryParse(timeSpanStringUtf8, out timeSpan, out _, standardFormat: 'c'); supports the format, so should JsonSerializer.

[danielmarbach]

We ran into a similar issue. Newtonsoft.Json supports both the 'c' format and the 'g' format (short version) but the built-in TimeSpanConverter restricts the minimum length to be '8' even though Utf8Parser.TryParse(timeSpanStringUtf8, out timeSpan, out _, standardFormat: 'c') would accept with pleasure and parse also the shorter 7 digits version.

Example

Newtonsoft.Json accepts 0:00:02 and 00:00:02 while the System.Text.Json converter only accepts 00:00:02

To not break wire compatibility, we had to implement a custom TimeSpanConverter that is an almost 1:1 copy of the .NET version but allows starting with 7 digits (shortest format).

[eiriktsarpalis]

Basically if Utf8Parser.TryParse(timeSpanStringUtf8, out timeSpan, out _, standardFormat: 'c'); supports the format, so should JsonSerializer.

The built-in converter does delegate to that routine when parsing:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TimeSpanConverter.cs

Line 60 in 9be6287

bool result = Utf8Parser.TryParse(source, out TimeSpan tmpValue, out int bytesConsumed, 'c');

Modulo rejecting leading whitespace the two should be identical.

[danielmarbach]

Except this check

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TimeSpanConverter.cs

Line 35 in 9be6287

if (!JsonHelpers.IsInRangeInclusive(reader.ValueLength, MinimumTimeSpanFormatLength, MaximumEscapedTimeSpanFormatLength))

prevents things

[eiriktsarpalis]

Can you share examples of valid inputs that are caught by this particular check? This is there simply to avoid allocating an intermediate buffer (and multiplies the maximum expected size by a factor of 6 to account for potential escaped values)

[danielmarbach]

Example

Newtonsoft.Json accepts 0:00:02 and 00:00:02 while the System.Text.Json converter only accepts 00:00:02

See #100538 (comment)

[danielmarbach]

The following program demonstrates things

using System;
using JsonSerializer = System.Text.Json.JsonSerializer;
using Newtonsoft.Json;
					
public class Program
{
	public static void Main()
	{
		var timespan = TimeSpan.FromSeconds(2);
		var toSerialize = new ToSerialize() { SomeTimeSpan = timespan };
		
		Console.WriteLine(JsonConvert.DeserializeObject<ToSerialize>(@"{ ""SomeTimeSpan"": ""0:00:02"" }"));
		Console.WriteLine(JsonConvert.DeserializeObject<ToSerialize>(@"{ ""SomeTimeSpan"": ""00:00:02"" }"));
		
		Console.WriteLine(JsonSerializer.Deserialize<ToSerialize>(@"{ ""SomeTimeSpan"": ""0:00:02"" }"));
	}
	
	class ToSerialize {
	  public TimeSpan SomeTimeSpan { get; set; }	
	}
}

Console.WriteLine(JsonSerializer.Deserialize<ToSerialize>(@"{ ""SomeTimeSpan"": ""0:00:02"" }"));

fails with

Unhandled exception. System.Text.Json.JsonException: The JSON value could not be converted to System.TimeSpan. Path: $.SomeTimeSpan | LineNumber: 0 | BytePositionInLine: 27.
 ---> System.FormatException: The JSON value is not in a supported TimeSpan format.
   at System.Text.Json.ThrowHelper.ThrowFormatException(DataType dataType)
   at System.Text.Json.Serialization.Converters.TimeSpanConverter.ReadCore(Utf8JsonReader& reader)
   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.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   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`1 jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo`1 jsonTypeInfo)

while

Utf8Parser.TryParse(Encoding.UTF8.GetBytes("0:00:02"), out TimeSpan parsedTimeSpan, out _, standardFormat: 'c')

would return true

[eiriktsarpalis]

Example

Newtonsoft.Json accepts 0:00:02 and 00:00:02 while the System.Text.Json converter only accepts 00:00:02

Right, but that input would be rejected on account of it not satisfying the 'c' format, not because the input is too long.

[danielmarbach]

Utf8Parser.TryParse(Encoding.UTF8.GetBytes("0:00:02"), out TimeSpan parsedTimeSpan, out _, standardFormat: 'c')

returns true

[eiriktsarpalis]

I see, so presumably it gets rejected because the input is too short? I think we should address that.

[eiriktsarpalis]

Happy to accept a PR that addresses this by the way, it's unlikely we would be able to prioritize this any time soon.

[danielmarbach]

I see, so presumably it gets rejected because the input is too short? I think we should address that.

That's my understanding but I haven't spent enough time to be 100% sure

[danielmarbach]

Happy to accept a PR that addresses this by the way, it's unlikely we would be able to prioritize this any time soon.

I see what I can do

[danielmarbach]

@eiriktsarpalis Here we go #102091

// cc @eerhardt

[danielmarbach]

Will the fix be backported?

[eiriktsarpalis]

Unfortunately, no. Only high-impact bugs and security fixes get backported.