Detect specifical character in EnumConverter.cs (#76873)

* add specifical char flag and fail earlier before invoke naming policy

* add unit test

* improve unit test and special char check

* fix format error

* fix unit test

* take comments and improve unit test

* use string.IndexOfAny

* throw exception in constructor and improve unit test

* add custome exception message

* take comments

* Update src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs

* Update src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs

* Update src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

Author: SilentCC

src/libraries/System.Text.Json/src/Resources/Strings.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!-- 
-    Microsoft ResX Schema 
-    
+  <!--
+    Microsoft ResX Schema
+
     Version 2.0
-    
-    The primary goals of this format is to allow a simple XML format 
-    that is mostly human readable. The generation and parsing of the 
-    various data types are done through the TypeConverter classes 
+
+    The primary goals of this format is to allow a simple XML format
+    that is mostly human readable. The generation and parsing of the
+    various data types are done through the TypeConverter classes
     associated with the data types.
-    
+
     Example:
-    
+
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-                
-    There are any number of "resheader" rows that contain simple 
+
+    There are any number of "resheader" rows that contain simple
     name/value pairs.
-    
-    Each data row contains a name, and value. The row also contains a 
-    type or mimetype. Type corresponds to a .NET class that support 
-    text/value conversion through the TypeConverter architecture. 
-    Classes that don't support this are serialized and stored with the 
+
+    Each data row contains a name, and value. The row also contains a
+    type or mimetype. Type corresponds to a .NET class that support
+    text/value conversion through the TypeConverter architecture.
+    Classes that don't support this are serialized and stored with the
     mimetype set.
-    
-    The mimetype is used for serialized objects, and tells the 
-    ResXResourceReader how to depersist the object. This is currently not 
+
+    The mimetype is used for serialized objects, and tells the
+    ResXResourceReader how to depersist the object. This is currently not
     extensible. For a given mimetype the value must be set accordingly:
-    
-    Note - application/x-microsoft.net.object.binary.base64 is the format 
-    that the ResXResourceWriter will generate, however the reader can 
+
+    Note - application/x-microsoft.net.object.binary.base64 is the format
+    that the ResXResourceWriter will generate, however the reader can
     read any of the formats listed below.
-    
+
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-    
+
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array 
+    value   : The object must be serialized into a byte array
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -231,6 +231,9 @@
   <data name="InvalidCharacterWithinString" xml:space="preserve">
     <value>'{0}' is invalid within a JSON string. The string should be correctly escaped.</value>
   </data>
+  <data name="InvalidEnumTypeWithSpecialChar" xml:space="preserve">
+    <value>Enum type '{0}' uses unsupported identifer name '{1}'.</value>
+  </data>
   <data name="InvalidEndOfJsonNonPrimitive" xml:space="preserve">
     <value>'{0}' is an invalid token type for the end of the JSON payload. Expected either 'EndArray' or 'EndObject'.</value>
   </data>

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs

@@ -15,6 +15,8 @@ internal sealed class EnumConverter<T> : JsonConverter<T>
     {
         private static readonly TypeCode s_enumTypeCode = Type.GetTypeCode(typeof(T));
 
+        private static readonly char[] s_specialChars = new[] { ',', ' ' };
+
         // Odd type codes are conveniently signed types (for enum backing types).
         private static readonly bool s_isSignedEnum = ((int)s_enumTypeCode % 2) == 1;
 
@@ -85,6 +87,12 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na
                 string jsonName = FormatJsonName(name, namingPolicy);
                 _nameCacheForWriting.TryAdd(key, JsonEncodedText.Encode(jsonName, encoder));
                 _nameCacheForReading?.TryAdd(jsonName, value);
+
+                // If enum contains special char, make it failed to serialize or deserialize.
+                if (name.IndexOfAny(s_specialChars) != -1)
+                {
+                    ThrowHelper.ThrowInvalidOperationException_InvalidEnumTypeWithSpecialChar(typeof(T), name);
+                }
             }
         }
 

src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs

@@ -805,6 +805,12 @@ public static void ThrowInvalidOperationException_PolymorphicTypeConfigurationDo
             throw new InvalidOperationException(SR.Format(SR.Polymorphism_ConfigurationDoesNotSpecifyDerivedTypes, baseType));
         }
 
+        [DoesNotReturn]
+        public static void ThrowInvalidOperationException_InvalidEnumTypeWithSpecialChar(Type enumType, string enumName)
+        {
+            throw new InvalidOperationException(SR.Format(SR.InvalidEnumTypeWithSpecialChar, enumType.Name, enumName));
+        }
+
         [DoesNotReturn]
         public static void ThrowJsonException_UnrecognizedTypeDiscriminator(object typeDiscriminator)
         {

src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/EnumTests.fs

@@ -0,0 +1,60 @@
+module System.Text.Json.Tests.FSharp.EnumTests
+
+open System
+open System.Reflection
+open System.Text.Json
+open System.Text.Json.Serialization
+open Xunit
+
+[<Flags>]
+type BadEnum =
+  | ``There's a comma, in my name`` = 1
+  | ``There's a comma, even here`` = 2
+  | ``ThisisagoodEnumValue`` = 4
+
+let badEnum = BadEnum.``There's a comma, in my name`` ||| BadEnum.``There's a comma, even here``
+let badEnumJsonStr = $"\"{badEnum}\""
+
+let badEnumWithGoodValue = BadEnum.ThisisagoodEnumValue
+let badEnumWithGoodValueJsonStr = $"\"{badEnumWithGoodValue}\""
+
+[<Flags>]
+type GoodEnum =
+  | Thereisnocommainmyname_1 = 1
+  | Thereisnocommaevenhere_2 = 2
+
+let goodEnum = GoodEnum.Thereisnocommainmyname_1 ||| GoodEnum.Thereisnocommaevenhere_2
+let goodEnumJsonStr = $"\"{goodEnum}\""
+
+let options = new JsonSerializerOptions()
+options.Converters.Add(new JsonStringEnumConverter())
+
+[<Fact>]
+let ``Deserialize With Exception If Enum Contains Special Char`` () =
+    let ex = Assert.Throws<TargetInvocationException>(fun () -> JsonSerializer.Deserialize<BadEnum>(badEnumJsonStr, options) |> ignore)
+    Assert.Equal(typeof<InvalidOperationException>, ex.InnerException.GetType())
+    Assert.Equal("Enum type 'BadEnum' uses unsupported identifer name 'There's a comma, in my name'.", ex.InnerException.Message)
+
+
+[<Fact>]
+let ``Serialize With Exception If Enum Contains Special Char`` () =
+    let ex = Assert.Throws<TargetInvocationException>(fun () ->  JsonSerializer.Serialize(badEnum, options) |> ignore)
+    Assert.Equal(typeof<InvalidOperationException>, ex.InnerException.GetType())
+    Assert.Equal("Enum type 'BadEnum' uses unsupported identifer name 'There's a comma, in my name'.", ex.InnerException.Message)
+
+[<Fact>]
+let ``Successful Deserialize Normal Enum`` () =
+    let actual = JsonSerializer.Deserialize<GoodEnum>(goodEnumJsonStr, options)
+    Assert.Equal(GoodEnum.Thereisnocommainmyname_1 ||| GoodEnum.Thereisnocommaevenhere_2, actual)
+
+[<Fact>]
+let ``Fail Deserialize Good Value Of Bad Enum Type`` () =
+    let ex = Assert.Throws<TargetInvocationException>(fun () -> JsonSerializer.Deserialize<BadEnum>(badEnumWithGoodValueJsonStr, options) |> ignore)
+    Assert.Equal(typeof<InvalidOperationException>, ex.InnerException.GetType())
+    Assert.Equal("Enum type 'BadEnum' uses unsupported identifer name 'There's a comma, in my name'.", ex.InnerException.Message)
+
+[<Fact>]
+let ``Fail Serialize Good Value Of Bad Enum Type`` () =
+    let ex = Assert.Throws<TargetInvocationException>(fun () ->  JsonSerializer.Serialize(badEnumWithGoodValue, options) |> ignore)
+    Assert.Equal(typeof<InvalidOperationException>, ex.InnerException.GetType())
+    Assert.Equal("Enum type 'BadEnum' uses unsupported identifer name 'There's a comma, in my name'.", ex.InnerException.Message)

src/libraries/System.Text.Json/tests/System.Text.Json.FSharp.Tests/System.Text.Json.FSharp.Tests.fsproj

@@ -6,6 +6,7 @@
 
   <ItemGroup>
     <Compile Include="Helpers.fs" />
+    <Compile Include="EnumTests.fs" />
     <Compile Include="OptionTests.fs" />
     <Compile Include="ValueOptionTests.fs" />
     <Compile Include="CollectionTests.fs" />