Default Read method of DecimalConverter ignores NumberHandling

[cytoph]

Yeah, kind of what I wrote in the title.

Even when the NumberHandling of the passed JsonSerializerOptions is set to AllowReadingFromString the default Read method will ignore it and throws an exception stating "Cannot get the value of a token type 'String' as a number."

https://github.com/dotnet/runtime/blob/fff4ff0913a2396cfae61c701f2b59ffe5e27b49/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/DecimalConverter.cs#L13-L16

ReadNumberWithCustomHandling on the other hand will indeed follow the set NumberHandling, but the method is internal and can therefore not be used outside of the library.

https://github.com/dotnet/runtime/blob/fff4ff0913a2396cfae61c701f2b59ffe5e27b49/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/DecimalConverter.cs#L33-L42

The use-case I have is a custom JsonConverter that contains a method that tries to get a JsonConverter for a specific generic type from the JsonSerializerOptions like this (which in case of decimal will get the DecimalConverter and throw said exception:

private static T GetValue<T>(ref Utf8JsonReader reader, JsonSerializerOptions options)
{
    var converter = (JsonConverter<T>)options.GetConverter(typeof(T));

    if (converter != null)
    {
        reader.Read();
        return converter.Read(ref reader, typeof(T), options);
    }
    else
        return JsonSerializer.Deserialize<T>(ref reader, options);
}

If in any case relevant, this sample is referring to how it's done in the docs for how to implement a custom JSON converter.

[ghost]

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

Issue Details

---

Yeah, kind of what I wrote in the title. Even when the `NumberHandling` of the passed `JsonSerializerOptions` is set to `AllowReadingFromString` the default `Read` method will ignore it and throws an exception stating "Cannot get the value of a token type 'String' as a number." https://github.com/dotnet/runtime/blob/fff4ff0913a2396cfae61c701f2b59ffe5e27b49/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/DecimalConverter.cs#L13-L16 `ReadNumberWithCustomHandling` on the other hand **will** indeed follow the set `NumberHandling`, but the method is internal and can therefore not be used outside of the library. https://github.com/dotnet/runtime/blob/fff4ff0913a2396cfae61c701f2b59ffe5e27b49/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/DecimalConverter.cs#L33-L42 The use-case I have is a custom `JsonConverter` that contains a method that tries to get a JsonConverter for a specific generic type from the `JsonSerializerOptions` like this (which in case of `decimal` will get the `DecimalConverter` and throw said exception: ``` private static T GetValue(ref Utf8JsonReader reader, JsonSerializerOptions options) { var converter = (JsonConverter)options.GetConverter(typeof(T)); if (converter != null) { reader.Read(); return converter.Read(ref reader, typeof(T), options); } else return JsonSerializer.Deserialize(ref reader, options); ``` If in any case relevant, this sample is referring to how it's done in the [docs for how to implement a custom JSON converter](https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-converters-how-to?pivots=dotnet-5-0#sample-factory-pattern-converter).

Author:	cytoph
Assignees:	-
Labels:	`area-System.Text.Json`, `untriaged`
Milestone:	-

[eiriktsarpalis]

Note that this behavior isn't specific to DecimalConverter -- all numeric converters follow the same pattern, e.g. Int32Converter works the same way. The converter uses separate internal methods for number handling because its value is informed by a few different sources: JsonSerializerOptions, JsonNumberHandlingAttribute as well as the value inherited from the parent type. The exposed public method signatures are simply insufficient when it comes to accommodating such scenaria.

That being said, I'm not sure why the public methods ignore the number handling setting of JsonSerializerOptions. It clearly is intentional, I suspect it might be related to performance considerations but I'm not sure what these are given that the hot path never calls into them. In any case, I'm afraid changing this now would be a breaking change specifically for the use case that you are citing.