From d21c3f9638992151878e2fceb7380f7bdbbea76f Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Tue, 19 May 2020 15:32:36 -0700 Subject: [PATCH 1/6] Add logic to properly honor naming policy when serializing flag enums --- .../Converters/Value/EnumConverter.cs | 21 ++++++++++++++++++- .../tests/Serialization/EnumConverterTests.cs | 15 +++++++++++++ 2 files changed, 35 insertions(+), 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 5d0fb439c04f4c..62c28605042f1d 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 @@ -16,6 +16,8 @@ internal class EnumConverter : JsonConverter // Odd type codes are conveniently signed types (for enum backing types). private static readonly string? s_negativeSign = ((int)s_enumTypeCode % 2) == 0 ? null : NumberFormatInfo.CurrentInfo.NegativeSign; + private const string EnumValueSeparator = ", "; + private readonly EnumConverterOptions _converterOptions; private readonly JsonNamingPolicy _namingPolicy; private readonly ConcurrentDictionary? _nameCache; @@ -166,7 +168,7 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions if (IsValidIdentifier(original)) { - transformed = _namingPolicy.ConvertName(original); + transformed = FormatEnumValue(original); writer.WriteStringValue(transformed); if (_nameCache != null) { @@ -212,5 +214,22 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions break; } } + + private string FormatEnumValue(string value) + { + if (value.IndexOf(EnumValueSeparator) == -1) + { + return _namingPolicy.ConvertName(value); + } + + string[] enumValues = value.Split(EnumValueSeparator); + + for (int i = 0; i < enumValues.Length; i++) + { + enumValues[i] = _namingPolicy.ConvertName(enumValues[i]); + } + + return string.Join(EnumValueSeparator, enumValues); + } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs index 3c15d9d0869dd8..76ac0be4f1be1f 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs @@ -107,6 +107,21 @@ public void ConvertFileAttributes() options = new JsonSerializerOptions(); options.Converters.Add(new JsonStringEnumConverter(allowIntegerValues: false)); Assert.Throws(() => JsonSerializer.Serialize((FileAttributes)(-1), options)); + + // Flag values honor naming policy correctly + options = new JsonSerializerOptions(); + options.Converters.Add(new JsonStringEnumConverter(new SimpleSnakeCasePolicy())); + + json = JsonSerializer.Serialize( + FileAttributes.Directory | FileAttributes.Compressed | FileAttributes.IntegrityStream, + options); + Assert.Equal(@"""directory, compressed, integrity_stream""", json); + + json = JsonSerializer.Serialize(FileAttributes.Compressed & FileAttributes.Device, options); + Assert.Equal(@"0", json); + + json = JsonSerializer.Serialize(FileAttributes.Directory & FileAttributes.Compressed | FileAttributes.IntegrityStream, options); + Assert.Equal(@"""integrity_stream""", json); } public class FileState From 060f4782c9d8f026d73bbeba65ff8ecacd4dbb09 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Tue, 19 May 2020 18:20:43 -0700 Subject: [PATCH 2/6] Cache JsonEncodedText, add optimization to-do, and use .Contains --- .../Converters/Value/EnumConverter.cs | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 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 62c28605042f1d..318afdc49edac0 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 @@ -16,11 +16,11 @@ internal class EnumConverter : JsonConverter // Odd type codes are conveniently signed types (for enum backing types). private static readonly string? s_negativeSign = ((int)s_enumTypeCode % 2) == 0 ? null : NumberFormatInfo.CurrentInfo.NegativeSign; - private const string EnumValueSeparator = ", "; + private const string ValueSeparator = ", "; private readonly EnumConverterOptions _converterOptions; private readonly JsonNamingPolicy _namingPolicy; - private readonly ConcurrentDictionary? _nameCache; + private readonly ConcurrentDictionary? _nameCache; public override bool CanConvert(Type type) { @@ -37,7 +37,7 @@ public EnumConverter(EnumConverterOptions options, JsonNamingPolicy? namingPolic _converterOptions = options; if (namingPolicy != null) { - _nameCache = new ConcurrentDictionary(); + _nameCache = new ConcurrentDictionary(); } else { @@ -160,7 +160,7 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions if (_converterOptions.HasFlag(EnumConverterOptions.AllowStrings)) { string original = value.ToString(); - if (_nameCache != null && _nameCache.TryGetValue(original, out string? transformed)) + if (_nameCache != null && _nameCache.TryGetValue(original, out JsonEncodedText transformed)) { writer.WriteStringValue(transformed); return; @@ -215,21 +215,28 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions } } - private string FormatEnumValue(string value) + private JsonEncodedText FormatEnumValue(string value) { - if (value.IndexOf(EnumValueSeparator) == -1) + string converted; + + if (!value.Contains(ValueSeparator)) { - return _namingPolicy.ConvertName(value); + converted = _namingPolicy.ConvertName(value); } + else + { + // todo: optimize implementation here by leveraging https://github.com/dotnet/runtime/issues/934. + string[] enumValues = value.Split(ValueSeparator); - string[] enumValues = value.Split(EnumValueSeparator); + for (int i = 0; i < enumValues.Length; i++) + { + enumValues[i] = _namingPolicy.ConvertName(enumValues[i]); + } - for (int i = 0; i < enumValues.Length; i++) - { - enumValues[i] = _namingPolicy.ConvertName(enumValues[i]); + converted = string.Join(ValueSeparator, enumValues); } - return string.Join(EnumValueSeparator, enumValues); + return JsonEncodedText.Encode(converted); } } } From fc2188540af55d27eb4157e415ef5b9cc786aeae Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Tue, 19 May 2020 21:29:58 -0700 Subject: [PATCH 3/6] Fix CI failure --- .../Json/Serialization/Converters/Value/EnumConverter.cs | 8 +++++++- 1 file changed, 7 insertions(+), 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 318afdc49edac0..245965f3167a46 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 @@ -226,7 +226,13 @@ private JsonEncodedText FormatEnumValue(string value) else { // todo: optimize implementation here by leveraging https://github.com/dotnet/runtime/issues/934. - string[] enumValues = value.Split(ValueSeparator); + string[] enumValues = value.Split( +#if BUILDING_INBOX_LIBRARY + ValueSeparator +#else + new string[] { ValueSeparator }, StringSplitOptions.None +#endif + ); for (int i = 0; i < enumValues.Length; i++) { From 39df64fd6ce74e7c4049cb1c46718cab22b77a71 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Wed, 20 May 2020 15:46:08 -0700 Subject: [PATCH 4/6] Apply feedback, use enum value as lookup key, cache result even when naming policy is not used --- .../Converters/Value/EnumConverter.cs | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 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 245965f3167a46..7f59deea7f97a6 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 @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Concurrent; +using System.Diagnostics; using System.Globalization; using System.Runtime.CompilerServices; @@ -19,8 +20,8 @@ internal class EnumConverter : JsonConverter private const string ValueSeparator = ", "; private readonly EnumConverterOptions _converterOptions; - private readonly JsonNamingPolicy _namingPolicy; - private readonly ConcurrentDictionary? _nameCache; + private readonly JsonNamingPolicy? _namingPolicy; + private readonly ConcurrentDictionary _nameCache = new ConcurrentDictionary(); public override bool CanConvert(Type type) { @@ -35,14 +36,6 @@ public EnumConverter(EnumConverterOptions options) public EnumConverter(EnumConverterOptions options, JsonNamingPolicy? namingPolicy) { _converterOptions = options; - if (namingPolicy != null) - { - _nameCache = new ConcurrentDictionary(); - } - else - { - namingPolicy = JsonNamingPolicy.Default; - } _namingPolicy = namingPolicy; } @@ -159,21 +152,21 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions // If strings are allowed, attempt to write it out as a string value if (_converterOptions.HasFlag(EnumConverterOptions.AllowStrings)) { - string original = value.ToString(); - if (_nameCache != null && _nameCache.TryGetValue(original, out JsonEncodedText transformed)) + if (_nameCache.TryGetValue(value, out JsonEncodedText transformed)) { writer.WriteStringValue(transformed); return; } + string original = value.ToString(); if (IsValidIdentifier(original)) { - transformed = FormatEnumValue(original); + transformed = _namingPolicy == null + ? JsonEncodedText.Encode(original, options.Encoder) + : FormatEnumValue(original, options); + writer.WriteStringValue(transformed); - if (_nameCache != null) - { - _nameCache.TryAdd(original, transformed); - } + _nameCache.TryAdd(value, transformed); return; } } @@ -215,8 +208,9 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions } } - private JsonEncodedText FormatEnumValue(string value) + private JsonEncodedText FormatEnumValue(string value, JsonSerializerOptions options) { + Debug.Assert(_namingPolicy != null); string converted; if (!value.Contains(ValueSeparator)) @@ -242,7 +236,7 @@ private JsonEncodedText FormatEnumValue(string value) converted = string.Join(ValueSeparator, enumValues); } - return JsonEncodedText.Encode(converted); + return JsonEncodedText.Encode(converted, options.Encoder); } } } From 7c93170a32a939b6df1fd4454b4f7f41b6cafdf7 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Thu, 28 May 2020 11:58:32 -0700 Subject: [PATCH 5/6] Use ulong as name cache key and perform caching at warm up --- .../Converters/Value/EnumConverter.cs | 52 +++++++++++++++---- .../Converters/Value/EnumConverterFactory.cs | 4 +- .../Serialization/JsonStringEnumConverter.cs | 4 +- 3 files changed, 45 insertions(+), 15 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 7f59deea7f97a6..ce2a80b4b85734 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 @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Globalization; using System.Runtime.CompilerServices; +using System.Text.Encodings.Web; namespace System.Text.Json.Serialization.Converters { @@ -21,22 +22,43 @@ internal class EnumConverter : JsonConverter private readonly EnumConverterOptions _converterOptions; private readonly JsonNamingPolicy? _namingPolicy; - private readonly ConcurrentDictionary _nameCache = new ConcurrentDictionary(); + private readonly ConcurrentDictionary _nameCache; public override bool CanConvert(Type type) { return type.IsEnum; } - public EnumConverter(EnumConverterOptions options) - : this(options, namingPolicy: null) + public EnumConverter(EnumConverterOptions converterOptions, JsonSerializerOptions serializerOptions) + : this(converterOptions, namingPolicy: null, serializerOptions) { } - public EnumConverter(EnumConverterOptions options, JsonNamingPolicy? namingPolicy) + public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? namingPolicy, JsonSerializerOptions serializerOptions) { - _converterOptions = options; + _converterOptions = converterOptions; _namingPolicy = namingPolicy; + _nameCache = new ConcurrentDictionary(); + + string[] names = Enum.GetNames(TypeToConvert); + Array values = Enum.GetValues(TypeToConvert); + Debug.Assert(names.Length > 0 && names.Length == values.Length); + + JavaScriptEncoder? encoder = serializerOptions.Encoder; + + for (int i = 0; i < names.Length; i++) + { + T value = (T)values.GetValue(i)!; + // Enum values can be represented as ulong. Note that F# supports char as enum backing types. + ulong key = Unsafe.As(ref value); + string name = names[i]; + + _nameCache.TryAdd( + key, + namingPolicy == null + ? JsonEncodedText.Encode(name, encoder) + : FormatEnumValue(name, encoder)); + } } public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) @@ -152,7 +174,9 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions // If strings are allowed, attempt to write it out as a string value if (_converterOptions.HasFlag(EnumConverterOptions.AllowStrings)) { - if (_nameCache.TryGetValue(value, out JsonEncodedText transformed)) + ulong key = Unsafe.As(ref value); + + if (_nameCache.TryGetValue(key, out JsonEncodedText transformed)) { writer.WriteStringValue(transformed); return; @@ -161,12 +185,18 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions string original = value.ToString(); if (IsValidIdentifier(original)) { + JavaScriptEncoder? encoder = options.Encoder; + + // We are dealing with flags since all literal values were cached during warm-up. transformed = _namingPolicy == null - ? JsonEncodedText.Encode(original, options.Encoder) - : FormatEnumValue(original, options); + ? JsonEncodedText.Encode(original, encoder) + : FormatEnumValue(original, encoder); writer.WriteStringValue(transformed); - _nameCache.TryAdd(value, transformed); + + // Since the value represents a valid identifier, malicious user input is not added to the cache. + _nameCache.TryAdd(key, transformed); + return; } } @@ -208,7 +238,7 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions } } - private JsonEncodedText FormatEnumValue(string value, JsonSerializerOptions options) + private JsonEncodedText FormatEnumValue(string value, JavaScriptEncoder? encoder) { Debug.Assert(_namingPolicy != null); string converted; @@ -236,7 +266,7 @@ private JsonEncodedText FormatEnumValue(string value, JsonSerializerOptions opti converted = string.Join(ValueSeparator, enumValues); } - return JsonEncodedText.Encode(converted, options.Encoder); + return JsonEncodedText.Encode(converted, encoder); } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverterFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverterFactory.cs index 38ad9315e41d9d..9faa76caec7371 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverterFactory.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverterFactory.cs @@ -19,7 +19,7 @@ public override bool CanConvert(Type type) } [PreserveDependency( - ".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions)", + ".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions, System.Text.Json.JsonSerializerOptions)", "System.Text.Json.Serialization.Converters.EnumConverter`1")] public override JsonConverter CreateConverter(Type type, JsonSerializerOptions options) { @@ -27,7 +27,7 @@ public override JsonConverter CreateConverter(Type type, JsonSerializerOptions o typeof(EnumConverter<>).MakeGenericType(type), BindingFlags.Instance | BindingFlags.Public, binder: null, - new object[] { EnumConverterOptions.AllowNumbers }, + new object[] { EnumConverterOptions.AllowNumbers, options }, culture: null)!; return converter; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonStringEnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonStringEnumConverter.cs index ca363da7be16bf..a5def112ae9485 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonStringEnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonStringEnumConverter.cs @@ -55,7 +55,7 @@ public override bool CanConvert(Type typeToConvert) /// [PreserveDependency( - ".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions, System.Text.Json.JsonNamingPolicy)", + ".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions, System.Text.Json.JsonNamingPolicy, System.Text.Json.JsonSerializerOptions)", "System.Text.Json.Serialization.Converters.EnumConverter`1")] public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options) { @@ -63,7 +63,7 @@ public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializer typeof(EnumConverter<>).MakeGenericType(typeToConvert), BindingFlags.Instance | BindingFlags.Public, binder: null, - new object?[] { _converterOptions, _namingPolicy }, + new object?[] { _converterOptions, _namingPolicy, options }, culture: null)!; return converter; From e10b87846078696de35072bcb2398908d7c3949c Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 1 Jun 2020 20:26:56 -0700 Subject: [PATCH 6/6] Address feedback - set cap for name cache, compute cache key properly, and add more tests --- .../Converters/Value/EnumConverter.cs | 107 +++++++++++++----- .../tests/Serialization/EnumConverterTests.cs | 86 ++++++++++++++ 2 files changed, 164 insertions(+), 29 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 ce2a80b4b85734..41f7302f7db55a 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 @@ -21,9 +21,15 @@ internal class EnumConverter : JsonConverter private const string ValueSeparator = ", "; private readonly EnumConverterOptions _converterOptions; + private readonly JsonNamingPolicy? _namingPolicy; + private readonly ConcurrentDictionary _nameCache; + // This is used to prevent flooding the cache due to exponential bitwise combinations of flags. + // Since multiple threads can add to the cache, a few more values might be added. + private const int NameCacheSizeSoftLimit = 64; + public override bool CanConvert(Type type) { return type.IsEnum; @@ -42,15 +48,19 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na string[] names = Enum.GetNames(TypeToConvert); Array values = Enum.GetValues(TypeToConvert); - Debug.Assert(names.Length > 0 && names.Length == values.Length); + Debug.Assert(names.Length == values.Length); JavaScriptEncoder? encoder = serializerOptions.Encoder; for (int i = 0; i < names.Length; i++) { + if (_nameCache.Count >= NameCacheSizeSoftLimit) + { + break; + } + T value = (T)values.GetValue(i)!; - // Enum values can be represented as ulong. Note that F# supports char as enum backing types. - ulong key = Unsafe.As(ref value); + ulong key = ConvertToUInt64(value); string name = names[i]; _nameCache.TryAdd( @@ -155,47 +165,45 @@ public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerial return default; } - private static bool IsValidIdentifier(string value) - { - // Trying to do this check efficiently. When an enum is converted to - // string the underlying value is given if it can't find a matching - // identifier (or identifiers in the case of flags). - // - // The underlying value will be given back with a digit (e.g. 0-9) possibly - // preceded by a negative sign. Identifiers have to start with a letter - // so we'll just pick the first valid one and check for a negative sign - // if needed. - return (value[0] >= 'A' && - (s_negativeSign == null || !value.StartsWith(s_negativeSign))); - } - public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options) { // If strings are allowed, attempt to write it out as a string value if (_converterOptions.HasFlag(EnumConverterOptions.AllowStrings)) { - ulong key = Unsafe.As(ref value); + ulong key = ConvertToUInt64(value); - if (_nameCache.TryGetValue(key, out JsonEncodedText transformed)) + if (_nameCache.TryGetValue(key, out JsonEncodedText formatted)) { - writer.WriteStringValue(transformed); + writer.WriteStringValue(formatted); return; } string original = value.ToString(); if (IsValidIdentifier(original)) { + // We are dealing with a combination of flag constants since + // all constant values were cached during warm-up. JavaScriptEncoder? encoder = options.Encoder; - // We are dealing with flags since all literal values were cached during warm-up. - transformed = _namingPolicy == null - ? JsonEncodedText.Encode(original, encoder) - : FormatEnumValue(original, encoder); + if (_nameCache.Count < NameCacheSizeSoftLimit) + { + formatted = _namingPolicy == null + ? JsonEncodedText.Encode(original, encoder) + : FormatEnumValue(original, encoder); - writer.WriteStringValue(transformed); + writer.WriteStringValue(formatted); - // Since the value represents a valid identifier, malicious user input is not added to the cache. - _nameCache.TryAdd(key, transformed); + _nameCache.TryAdd(key, formatted); + } + else + { + // We also do not create a JsonEncodedText instance here because passing the string + // directly to the writer is cheaper than creating one and not caching it for reuse. + writer.WriteStringValue( + _namingPolicy == null + ? original + : FormatEnumValueToString(original, encoder)); + } return; } @@ -238,11 +246,52 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions } } + // This method is adapted from Enum.ToUInt64 (an internal method): + // https://github.com/dotnet/runtime/blob/bd6cbe3642f51d70839912a6a666e5de747ad581/src/libraries/System.Private.CoreLib/src/System/Enum.cs#L240-L260 + private static ulong ConvertToUInt64(object value) + { + Debug.Assert(value is T); + ulong result = s_enumTypeCode switch + { + TypeCode.Int32 => (ulong)(int)value, + TypeCode.UInt32 => (uint)value, + TypeCode.UInt64 => (ulong)value, + TypeCode.Int64 => (ulong)(long)value, + TypeCode.SByte => (ulong)(sbyte)value, + TypeCode.Byte => (byte)value, + TypeCode.Int16 => (ulong)(short)value, + TypeCode.UInt16 => (ushort)value, + _ => throw new InvalidOperationException(), + }; + return result; + } + + private static bool IsValidIdentifier(string value) + { + // Trying to do this check efficiently. When an enum is converted to + // string the underlying value is given if it can't find a matching + // identifier (or identifiers in the case of flags). + // + // The underlying value will be given back with a digit (e.g. 0-9) possibly + // preceded by a negative sign. Identifiers have to start with a letter + // so we'll just pick the first valid one and check for a negative sign + // if needed. + return (value[0] >= 'A' && + (s_negativeSign == null || !value.StartsWith(s_negativeSign))); + } + private JsonEncodedText FormatEnumValue(string value, JavaScriptEncoder? encoder) { Debug.Assert(_namingPolicy != null); - string converted; + string formatted = FormatEnumValueToString(value, encoder); + return JsonEncodedText.Encode(formatted, encoder); + } + + private string FormatEnumValueToString(string value, JavaScriptEncoder? encoder) + { + Debug.Assert(_namingPolicy != null); + string converted; if (!value.Contains(ValueSeparator)) { converted = _namingPolicy.ConvertName(value); @@ -266,7 +315,7 @@ private JsonEncodedText FormatEnumValue(string value, JavaScriptEncoder? encoder converted = string.Join(ValueSeparator, enumValues); } - return JsonEncodedText.Encode(converted, encoder); + return converted; } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs index 76ac0be4f1be1f..753aeaf2494d93 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.IO; +using System.Threading.Tasks; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -200,5 +201,90 @@ public void EnumWithConverterAttribute() obj = JsonSerializer.Deserialize("2"); Assert.Equal(MyCustomEnum.Second, obj); } + + [Fact] + public static void EnumWithNoValues() + { + var options = new JsonSerializerOptions + { + Converters = { new JsonStringEnumConverter() } + }; + + Assert.Equal("-1", JsonSerializer.Serialize((EmptyEnum)(-1), options)); + Assert.Equal("1", JsonSerializer.Serialize((EmptyEnum)(1), options)); + } + + public enum EmptyEnum { }; + + [Fact] + public static void MoreThan64EnumValuesToSerialize() + { + var options = new JsonSerializerOptions + { + Converters = { new JsonStringEnumConverter() } + }; + + for (int i = 0; i < 128; i++) + { + MyEnum value = (MyEnum)i; + string asStr = value.ToString(); + string expected = char.IsLetter(asStr[0]) ? $@"""{asStr}""" : asStr; + Assert.Equal(expected, JsonSerializer.Serialize(value, options)); + } + } + + [Fact, OuterLoop] + public static void VeryLargeAmountOfEnumsToSerialize() + { + // Ensure we don't throw OutOfMemoryException. + // Multiple threads are used to ensure the approximate size limit + // for the name cache(a concurrent dictionary) is honored. + + var options = new JsonSerializerOptions + { + Converters = { new JsonStringEnumConverter() } + }; + + const int MaxValue = 33554432; // Value for MyEnum.Z + Task[] tasks = new Task[MaxValue]; + + for (int i = 0; i < tasks.Length; i++) + { + tasks[i] = Task.Run(() => JsonSerializer.Serialize((MyEnum)i, options)); + } + + Task.WaitAll(tasks); + } + + [Flags] + public enum MyEnum + { + A = 1 << 0, + B = 1 << 1, + C = 1 << 2, + D = 1 << 3, + E = 1 << 4, + F = 1 << 5, + G = 1 << 6, + H = 1 << 7, + I = 1 << 8, + J = 1 << 9, + K = 1 << 10, + L = 1 << 11, + M = 1 << 12, + N = 1 << 13, + O = 1 << 14, + P = 1 << 15, + Q = 1 << 16, + R = 1 << 17, + S = 1 << 18, + T = 1 << 19, + U = 1 << 20, + V = 1 << 21, + W = 1 << 22, + X = 1 << 23, + Y = 1 << 24, + Z = 1 << 25, + } } }