From 86699d3908dec1841d84e1bc9d4d6fc08a029a55 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 30 Jan 2025 17:27:50 -0800 Subject: [PATCH 1/3] Fix case-insensitive deserialization of default enum values --- .../Converters/Value/EnumConverter.cs | 25 +++++++++- .../Serialization/EnumConverterTests.cs | 50 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 901cff255baa91..2e0a9b98a47327 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -601,7 +601,7 @@ public void AppendConflictingField(EnumFieldInfo other) { Debug.Assert(JsonName.Equals(other.JsonName, StringComparison.OrdinalIgnoreCase), "The conflicting entry must be equal up to case insensitivity."); - if (Kind is EnumFieldNameKind.Default || JsonName.Equals(other.JsonName, StringComparison.Ordinal)) + if (MatchesSupersetOf(this, other)) { // Silently discard if the preceding entry is the default or has identical name. return; @@ -612,13 +612,34 @@ public void AppendConflictingField(EnumFieldInfo other) // Walk the existing list to ensure we do not add duplicates. foreach (EnumFieldInfo conflictingField in conflictingFields) { - if (conflictingField.Kind is EnumFieldNameKind.Default || conflictingField.JsonName.Equals(other.JsonName, StringComparison.Ordinal)) + if (MatchesSupersetOf(conflictingField, other)) { return; } } conflictingFields.Add(other); + + // 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) + { + // The default name matches everything case-insensitively. + if (field1.Kind is EnumFieldNameKind.Default) + { + return true; + } + + // field1 matches case-sensitively since it's not the default name. + // field2 matches case-insensitively, so it matches more than field1. + if (field2.Kind is EnumFieldNameKind.Default) + { + return false; + } + + // Both are case-sensitive so they need to be identical. + return field1.JsonName.Equals(field2.JsonName, StringComparison.Ordinal); + } } public EnumFieldInfo? GetMatchingField(ReadOnlySpan input) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/EnumConverterTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/EnumConverterTests.cs index 4668dbc6260f18..aecc33a26760b1 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/EnumConverterTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/EnumConverterTests.cs @@ -1207,5 +1207,55 @@ public enum EnumWithInvalidMemberName6 [JsonStringEnumMemberName("Comma separators not allowed, in flags enums")] Value } + + [Theory] + [InlineData("\"cAmElCaSe\"", EnumWithVaryingNamingPolicies.camelCase, JsonKnownNamingPolicy.SnakeCaseUpper)] + [InlineData("\"cAmElCaSe\"", EnumWithVaryingNamingPolicies.camelCase, JsonKnownNamingPolicy.SnakeCaseLower)] + [InlineData("\"cAmElCaSe\"", EnumWithVaryingNamingPolicies.camelCase, JsonKnownNamingPolicy.KebabCaseUpper)] + [InlineData("\"cAmElCaSe\"", EnumWithVaryingNamingPolicies.camelCase, JsonKnownNamingPolicy.KebabCaseLower)] + [InlineData("\"pAsCaLcAsE\"", EnumWithVaryingNamingPolicies.PascalCase, JsonKnownNamingPolicy.SnakeCaseUpper)] + [InlineData("\"pAsCaLcAsE\"", EnumWithVaryingNamingPolicies.PascalCase, JsonKnownNamingPolicy.SnakeCaseLower)] + [InlineData("\"pAsCaLcAsE\"", EnumWithVaryingNamingPolicies.PascalCase, JsonKnownNamingPolicy.KebabCaseUpper)] + [InlineData("\"pAsCaLcAsE\"", EnumWithVaryingNamingPolicies.PascalCase, JsonKnownNamingPolicy.KebabCaseLower)] + [InlineData("\"sNaKe_CaSe_UpPeR\"", EnumWithVaryingNamingPolicies.SNAKE_CASE_UPPER, JsonKnownNamingPolicy.SnakeCaseUpper)] + [InlineData("\"sNaKe_CaSe_LoWeR\"", EnumWithVaryingNamingPolicies.snake_case_lower, JsonKnownNamingPolicy.SnakeCaseLower)] + [InlineData("\"cAmElCaSe\"", EnumWithVaryingNamingPolicies.camelCase, JsonKnownNamingPolicy.CamelCase)] + [InlineData("\"a\"", EnumWithVaryingNamingPolicies.A, JsonKnownNamingPolicy.CamelCase)] + [InlineData("\"a\"", EnumWithVaryingNamingPolicies.A, JsonKnownNamingPolicy.SnakeCaseUpper)] + [InlineData("\"a\"", EnumWithVaryingNamingPolicies.A, JsonKnownNamingPolicy.SnakeCaseLower)] + [InlineData("\"a\"", EnumWithVaryingNamingPolicies.A, JsonKnownNamingPolicy.KebabCaseUpper)] + [InlineData("\"a\"", EnumWithVaryingNamingPolicies.A, JsonKnownNamingPolicy.KebabCaseLower)] + [InlineData("\"B\"", EnumWithVaryingNamingPolicies.b, JsonKnownNamingPolicy.CamelCase)] + [InlineData("\"B\"", EnumWithVaryingNamingPolicies.b, JsonKnownNamingPolicy.SnakeCaseUpper)] + [InlineData("\"B\"", EnumWithVaryingNamingPolicies.b, JsonKnownNamingPolicy.SnakeCaseLower)] + [InlineData("\"B\"", EnumWithVaryingNamingPolicies.b, JsonKnownNamingPolicy.KebabCaseUpper)] + [InlineData("\"B\"", EnumWithVaryingNamingPolicies.b, JsonKnownNamingPolicy.KebabCaseLower)] + public static void StringConverterWithNamingPolicyIsCaseInsensitive(string json, EnumWithVaryingNamingPolicies expectedValue, JsonKnownNamingPolicy namingPolicy) + { + JsonNamingPolicy policy = namingPolicy switch + { + JsonKnownNamingPolicy.CamelCase => JsonNamingPolicy.CamelCase, + JsonKnownNamingPolicy.SnakeCaseLower => JsonNamingPolicy.SnakeCaseLower, + JsonKnownNamingPolicy.SnakeCaseUpper => JsonNamingPolicy.SnakeCaseUpper, + JsonKnownNamingPolicy.KebabCaseLower => JsonNamingPolicy.KebabCaseLower, + JsonKnownNamingPolicy.KebabCaseUpper => JsonNamingPolicy.KebabCaseUpper, + _ => throw new ArgumentOutOfRangeException(nameof(namingPolicy)), + }; + + JsonSerializerOptions options = new() { Converters = { new JsonStringEnumConverter(policy) } }; + + EnumWithVaryingNamingPolicies value = JsonSerializer.Deserialize(json, options); + Assert.Equal(expectedValue, value); + } + + public enum EnumWithVaryingNamingPolicies + { + SNAKE_CASE_UPPER, + snake_case_lower, + camelCase, + PascalCase, + A, + b, + } } } From cade726f00b97f1ac905dee0cd7e89b73f78325e Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Fri, 31 Jan 2025 10:08:14 -0800 Subject: [PATCH 2/3] renaming --- .../Converters/Value/EnumConverter.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 2e0a9b98a47327..0fdbe56a272858 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -601,7 +601,7 @@ public void AppendConflictingField(EnumFieldInfo other) { Debug.Assert(JsonName.Equals(other.JsonName, StringComparison.OrdinalIgnoreCase), "The conflicting entry must be equal up to case insensitivity."); - if (MatchesSupersetOf(this, other)) + if (ConflictsWith(this, other)) { // Silently discard if the preceding entry is the default or has identical name. return; @@ -612,7 +612,7 @@ public void AppendConflictingField(EnumFieldInfo other) // Walk the existing list to ensure we do not add duplicates. foreach (EnumFieldInfo conflictingField in conflictingFields) { - if (MatchesSupersetOf(conflictingField, other)) + if (ConflictsWith(conflictingField, other)) { return; } @@ -622,23 +622,23 @@ public void AppendConflictingField(EnumFieldInfo other) // 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) + static bool ConflictsWith(EnumFieldInfo current, EnumFieldInfo other) { // The default name matches everything case-insensitively. - if (field1.Kind is EnumFieldNameKind.Default) + if (current.Kind is EnumFieldNameKind.Default) { return true; } - // field1 matches case-sensitively since it's not the default name. - // field2 matches case-insensitively, so it matches more than field1. - if (field2.Kind is EnumFieldNameKind.Default) + // current matches case-sensitively since it's not the default name. + // other matches case-insensitively, so it matches more than current. + if (other.Kind is EnumFieldNameKind.Default) { return false; } // Both are case-sensitive so they need to be identical. - return field1.JsonName.Equals(field2.JsonName, StringComparison.Ordinal); + return current.JsonName.Equals(other.JsonName, StringComparison.Ordinal); } } From b793764d8ff8e6bd0b21aad2f093747a89e9f0ed Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Mon, 3 Feb 2025 10:31:40 -0800 Subject: [PATCH 3/3] Update comment --- .../Text/Json/Serialization/Converters/Value/EnumConverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 0fdbe56a272858..3fd753d7cb8cd3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -603,7 +603,7 @@ public void AppendConflictingField(EnumFieldInfo other) if (ConflictsWith(this, other)) { - // Silently discard if the preceding entry is the default or has identical name. + // Silently discard if the new entry conflicts with the preceding entry return; }