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

Fix case-insensitive JSON deserialization of enum member names #112028

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

PranavSenthilnathan
Copy link
Member

@PranavSenthilnathan PranavSenthilnathan commented Jan 31, 2025

Fixes #110745

The JSON serializer supports deserializing an enum case-insensitively from the C# enum member name even when a naming policy is used. For example, consider the following enum:

enum MyEnum { EnumValue }

If JsonNamingPolicy.SnakeCaseLower is specified, then "enum_value" will be deserialized to MyEnum.EnumValue (this is case-sensitive, so deserializion of "Enum_value" will throw). In addition, "EnumValue" also will be deserialized to MyEnum.EnumValue because it is the name of the member (this is case-insensitive so "enumValue" will also be deserialized).

However, there is a regression in .NET 9.0 when the enum member name is the same as the name from the naming policy:

var options = new JsonSerializerOptions();
options.Converters.Add(new JsonStringEnumConverter(JsonNamingPolicy.SnakeCaseLower));

// Both work in .NET 8.0 but throw in .NET 9.0
Console.WriteLine(JsonSerializer.Deserialize<MyEnum>("\"Example\"", options));
Console.WriteLine(JsonSerializer.Deserialize<MyEnum>("\"Another_Example\"", options));

enum MyEnum
{
  // C# name: "example"
  // lower snake-case: "example"
  example,

  // C# name: "another_example"
  // lower snake-case: "another_example"
  another_example
}

Note that the enum member name for both members is the same as the lower snake casing.

The regression was introduced in #105032 which added the JsonStringEnumMemberName attribute allowing custom names to be specified for enum members. That change added a new design for the converter which essentially uses the custom name, the naming policy derived name, and/or the C# member name as the key to figure out which C# enum value to deserialize to. This edge case was not covered because the C# member name and the naming-policy derived name both would have the same key and thus the C# member name was not inserted into the lookup.

The change in this PR fixes this by always adding both the naming-policy key and the C# member name key regardless of whether they are the same. There were no tests for this specific code path, so new tests have been added to cover this case now.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.


// Determines whether the first field info matches everything that the second field info matches,
// in which case the second field info is redundant and doesn't need to be added to the list.
static bool MatchesSupersetOf(EnumFieldInfo field1, EnumFieldInfo field2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll admit I find the use of the term "superset" to be somewhat confusing in this context. I would recommend renaming to something like MatchesWith or ConflictsWith. Perhaps also rename field1 and field2 to current and other to better emphasize what field is stored and which one is being added.

Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great apart from a minor naming nit pick. Because it's a regression, this should also be backported to release/9.0.

@PranavSenthilnathan PranavSenthilnathan changed the title Fix case-insensitive JSON deserialization of default enum vals Fix case-insensitive JSON deserialization of enum member names Jan 31, 2025
@PranavSenthilnathan PranavSenthilnathan merged commit 377c43b into dotnet:main Jan 31, 2025
88 checks passed
@eiriktsarpalis
Copy link
Member

/backport to release/9.0

Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/13082434283

grendello added a commit to grendello/runtime that referenced this pull request Feb 3, 2025
* main:
  System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
  Keep test PDB in helix payload for native AOT (dotnet#111949)
  Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
  Always inline number conversions (dotnet#112061)
  Use Contains{Any} in Regex source generator (dotnet#112065)
  Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
  JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
  Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
  Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
  JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
  Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
  JIT: Update type when return temp is freshly created (dotnet#111948)
  Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
  Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
  WasmAppBuilder: Remove double computation of a value (dotnet#112047)
  Disable LTCG for brotli and zlibng. (dotnet#111805)
  JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
  simplify x86 special intrinsic imports (dotnet#111836)
  JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
  Fix explicit offset of ByRefLike fields. (dotnet#111584)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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