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

System.Text.Json 9.0 can break case-insensitive enum deserialization when there is a naming policy #110745

Open
atykhyy opened this issue Dec 16, 2024 · 2 comments · May be fixed by #112028
Open
Assignees
Labels
area-System.Text.Json in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@atykhyy
Copy link

atykhyy commented Dec 16, 2024

Description

System.Text.Json 9.0.0 no longer deserializes strings to enums in a case-insensitive manner when there is a naming policy that does not change the case of the enum's string representation (see example below; a do-nothing naming policy which returns the original name also has this problem).

Reproduction Steps

The following example can be run in .NET Fiddle. It works when compiler is set to .NET 8 (which imports STJ 8.x), and fails when compiler is set to .NET 9.

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
					
public class Program
{
  public static void Main()
  {
    var options = new JsonSerializerOptions () ;
    options.Converters.Add (new JsonStringEnumConverter (JsonNamingPolicy.SnakeCaseUpper)) ; // needs to preserve casing of Foo.A
    // works with System.Text.Json 8.x, throws with System.Text.Json 9.0.0
    Console.WriteLine (JsonSerializer.Deserialize<Foo> ("\"a\"", options)) ;
  }
}

enum Foo
{
  A = 1,
}

Expected behavior

In System.Text.Json 8.x and earlier, strings could always be deserialized to enum in a case-insensitive manner regardless of naming policy.

Actual behavior

Upgrading System.Text.Json to 9.0.0 silently breaks existing code as enums in lower case can no longer be deserialized.

Regression?

This regression was introduced by the rewrite of EnumConverter.cs in #105032 (commit 78db74f). The same user code works with System.Text.Json 8.x and earlier.

Known Workarounds

From my reading of EnumConverter.cs, no workaround with the existing System.Text.Json enum converter is possible. One would have to create a custom converter for enums and use it in place of JsonStringEnumConverter, or replace the enum converter factory and change internal fields of each enum converter with reflection after the converter is created.

Configuration

.NET 8

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 16, 2024
@ll9
Copy link

ll9 commented Jan 7, 2025

I am facing the same Issue. In my case this is problematic in ASP.NET Core since this breaks the code for some consumers of the API.

In the meantime the following workaround seems to work for me.

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


public class Program
{
    public static void Main()
    {
        var options = new JsonSerializerOptions();

        // works with System.Text.Json 8.x, throws with System.Text.Json 9.0.0
        // options.Converters.Add (new JsonStringEnumConverter (JsonNamingPolicy.SnakeCaseUpper)) ; // needs to preserve casing of Foo.A

        // also works for .NET 9
        options.Converters.Add(new CaseInsensitiveEnumConverterFactory(JsonNamingPolicy.SnakeCaseUpper));
        Console.WriteLine(JsonSerializer.Deserialize<Foo>("\"a\"", options));
    }
}

enum Foo
{
    A = 1,
}

public class CaseInsensitiveEnumConverterFactory : JsonConverterFactory
{
    private readonly JsonNamingPolicy? _jsonNamingPolicy;

    public CaseInsensitiveEnumConverterFactory()
    {

    }

    public CaseInsensitiveEnumConverterFactory(JsonNamingPolicy jsonNamingPolicy)
    {
        _jsonNamingPolicy = jsonNamingPolicy;
    }

    public override bool CanConvert(Type typeToConvert)
    {
        return typeToConvert.IsEnum;
    }

    public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
    {
        return (JsonConverter)Activator.CreateInstance(
            typeof(CaseInsensitiveEnumConverter<>).MakeGenericType(typeToConvert),
            BindingFlags.Instance | BindingFlags.Public,
            null,
            [_jsonNamingPolicy],
            null)!;
    }
}

public class CaseInsensitiveEnumConverter<T> : JsonConverter<T> where T : struct, Enum
{
    private readonly JsonNamingPolicy? _jsonNamingPolicy;

    public CaseInsensitiveEnumConverter(JsonNamingPolicy? jsonNamingPolicy)
    {
        _jsonNamingPolicy = jsonNamingPolicy;
    }

    public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType != JsonTokenType.String)
        {
            throw new JsonException();
        }

        var value = reader.GetString();
        if (Enum.TryParse(value, ignoreCase: true, out T result))
        {
            return result;
        }

        throw new JsonException($"Unable to convert \"{value}\" to Enum \"{typeof(T)}\".");
    }

    public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
    {
        var result = _jsonNamingPolicy != null ? _jsonNamingPolicy.ConvertName(value.ToString()) : value.ToString();

        writer.WriteStringValue(result);
    }
}

@jeffhandley
Copy link
Member

Assigned to @PranavSenthilnathan for triage.

@PranavSenthilnathan PranavSenthilnathan removed the untriaged New issue has not been triaged by the area owner label Jan 30, 2025
@PranavSenthilnathan PranavSenthilnathan added this to the 10.0.0 milestone Jan 30, 2025
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants