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

JsonNumberHandling.AllowReadingFromString on class-level fails for double[][], but works in JsonSourceGenerationOptions #110770

Open
petterton opened this issue Dec 17, 2024 · 2 comments

Comments

@petterton
Copy link

petterton commented Dec 17, 2024

Description

The behavior of JsonNumberHandling.AllowReadingFromString is inconsistent when applied at the class level versus when specified in JsonSourceGenerationOptions.

When JsonNumberHandling.AllowReadingFromString is set as an attribute at the class level, deserialization of a double[][] property containing strings fails with a JsonException.

However, when the same option is set in JsonSourceGenerationOptions for a JsonSerializerContext, deserialization works as expected.

This inconsistency violates the principle of least surprise, as developers would expect the attribute and context options to behave similarly.

If applying the attribute on property level, the deserialization also fails, but with a different error message.

Reproduction Steps

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

// Case 1: JsonSourceGenerationOptions with NumberHandling set at context level (Works)
public class TestClassWithContext
{
    public double[][] Numbers { get; set; }
}

// Case 2: Attribute on class level (Fails)
[JsonNumberHandling(JsonNumberHandling.AllowReadingFromString)]
public class TestClassWithAttribute
{
    public double[][] Numbers { get; set; }
}

// JsonSerializerContext for Case 1 (explicitly allows reading from string)
[JsonSourceGenerationOptions(WriteIndented = true, NumberHandling = JsonNumberHandling.AllowReadingFromString)]
[JsonSerializable(typeof(TestClassWithContext))]
internal partial class JsonContextWithNumberHandling : JsonSerializerContext { }

// JsonSerializerContext for Case 2 (default options, does not allow reading from string)
[JsonSourceGenerationOptions(WriteIndented = true)]
[JsonSerializable(typeof(TestClassWithAttribute))]
internal partial class JsonContextDefault : JsonSerializerContext { }

internal class Program
{
    private const string Json = "{\"Numbers\":[[\"1.23\",\"4.56\"],[\"7.89\",\"0.12\"]]}";

    static void Main()
    {
        // Case 1: JsonSourceGenerationOptions (Works)
        Console.WriteLine("--- Case 1: JsonSourceGenerationOptions (Works) ---");
        var result1 = JsonSerializer.Deserialize<TestClassWithContext>(Json, JsonContextWithNumberHandling.Default.TestClassWithContext);
        Console.WriteLine("Result 1: " + JsonSerializer.Serialize(result1, JsonContextWithNumberHandling.Default.TestClassWithContext));

        // Case 2: Attribute on Class (Fails)
        Console.WriteLine("\n--- Case 2: Attribute on Class (Fails) ---");
        try
        {
            var result2 = JsonSerializer.Deserialize<TestClassWithAttribute>(Json, JsonContextDefault.Default.TestClassWithAttribute);
            Console.WriteLine("Result 2: " + JsonSerializer.Serialize(result2));
        }
        catch (Exception ex)
        {
            Console.WriteLine("Failed as expected: " + ex.Message);
        }
    }
}

Expected behavior

Deserialization of a double[][] property containing strings should succeed when JsonNumberHandling.AllowReadingFromString is applied at the class or property level. At the very least, I would expect JsonNumberHandling.AllowReadingFromString to behave consistently when applied as an attribute versus when specified in JsonSourceGenerationOptions

Actual behavior

Class level:
The JSON value could not be converted to System.Double. Path: $.Numbers[0][0] | LineNumber: 0 | BytePositionInLine: 19.
JsonSourceGenerationOptions:
Deserialization succeeds as expected.

Regression?

No response

Known Workarounds

Use JsonSourceGenerationOptions with NumberHandling = JsonNumberHandling.AllowReadingFromString in a JsonSerializerContext

Configuration

OS: Windows 10
.NET Version: .NET 9.0
Architecture: x64

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Dec 17, 2024
Copy link
Contributor

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

@eiriktsarpalis
Copy link
Member

This is because the [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString)] annotation only applies to first-order properties that are declared on the type. When serializing values like double[] or double[][] control passes from the converter for TestClassWithAttribute to the converter for double[][] which is not scoped to the specific declaring type. The global setting works because, well, the global setting also applies to the JsonConverter<double[][]>.

We might consider implementing a dynamic scoping scheme in the future, assuming there is demand for this. It is however going to be complex undertaking that is almost certainly going to count as a breaking change. For now, my recommendation would be to just turn on the global setting.

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Dec 17, 2024
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants