Fix FormatException if an invalid JsonConverter attribute is specified (dotnet/corefx...

Commit migrated from https://github.com/dotnet/corefx/commit/dfe3ab2f0205436e23b93c369ee0b2e41a4973e0

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs
index d059708..9be3015 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs
@@ -77,7 +77,7 @@ namespace System.Text.Json
 
                 if (converterAttribute != null)
                 {
-                    converter = GetConverterFromAttribute(converterAttribute, runtimePropertyType, parentClassType, propertyInfo);
+                    converter = GetConverterFromAttribute(converterAttribute, typeToConvert: runtimePropertyType, classTypeAttributeIsOn: parentClassType, propertyInfo);
                 }
             }
 
@@ -127,7 +127,7 @@ namespace System.Text.Json
 
                 if (converterAttribute != null)
                 {
-                    converter = GetConverterFromAttribute(converterAttribute, typeToConvert, typeToConvert, propertyInfo: null);
+                    converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, classTypeAttributeIsOn: typeToConvert, propertyInfo: null);
                 }
             }
 
@@ -189,7 +189,7 @@ namespace System.Text.Json
             return GetConverter(typeToConvert) != null;
         }
 
-        private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classType, PropertyInfo propertyInfo)
+        private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, PropertyInfo propertyInfo)
         {
             JsonConverter converter;
 
@@ -200,7 +200,7 @@ namespace System.Text.Json
                 converter = converterAttribute.CreateConverter(typeToConvert);
                 if (converter == null)
                 {
-                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classType, propertyInfo);
+                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, propertyInfo, typeToConvert);
                 }
             }
             else
@@ -208,7 +208,7 @@ namespace System.Text.Json
                 ConstructorInfo ctor = type.GetConstructor(Type.EmptyTypes);
                 if (!typeof(JsonConverter).IsAssignableFrom(type) || !ctor.IsPublic)
                 {
-                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(classType, propertyInfo);
+                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(classTypeAttributeIsOn, propertyInfo);
                 }
 
                 converter = (JsonConverter)Activator.CreateInstance(type);
@@ -216,7 +216,7 @@ namespace System.Text.Json
 
             if (!converter.CanConvert(typeToConvert))
             {
-                ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classType, propertyInfo);
+                ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, propertyInfo, typeToConvert);
             }
 
             return converter;

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
index 74e2534..ad60c18 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
@@ -86,15 +86,16 @@ namespace System.Text.Json
         }
 
         [MethodImpl(MethodImplOptions.NoInlining)]
-        public static void ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(Type classType, PropertyInfo propertyInfo)
+        public static void ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(Type classTypeAttributeIsOn, PropertyInfo propertyInfo, Type typeToConvert)
         {
-            string location = classType.ToString();
+            string location = classTypeAttributeIsOn.ToString();
+
             if (propertyInfo != null)
             {
-                location += $".{propertyInfo.ToString()}";
+                location += $".{propertyInfo.Name}";
             }
 
-            throw new InvalidOperationException(SR.Format(SR.SerializationConverterOnAttributeNotCompatible, location));
+            throw new InvalidOperationException(SR.Format(SR.SerializationConverterOnAttributeNotCompatible, location, typeToConvert));
         }
 
         [MethodImpl(MethodImplOptions.NoInlining)]

diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.BadConverters.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.BadConverters.cs
index 3740241..6d27d6f 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.BadConverters.cs
+++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.BadConverters.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Generic;
 using Xunit;
 
 namespace System.Text.Json.Serialization.Tests
@@ -57,27 +58,78 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Throws<SuccessException>(() => JsonSerializer.Serialize<Person>(new DerivedCustomer(), options));
         }
 
-        private class CanConvertNullConverterAttribute : JsonConverterAttribute
+        private class InvalidConverterAttribute : JsonConverterAttribute
         {
-            public CanConvertNullConverterAttribute() : base(typeof(int)) { }
-
-            public override JsonConverter CreateConverter(Type typeToConvert)
-            {
-                return null;
-            }
+            // converterType is not valid since typeof(int) is not a type that derives from JsonConverter.
+            public InvalidConverterAttribute() : base(converterType: typeof(int)) { }
         }
 
-        private class PocoWithNullConverter
+        private class PocoWithInvalidConverter
         {
-            [CanConvertNullConverter]
+            [InvalidConverter]
             public int MyInt { get; set; }
         }
 
         [Fact]
         public static void AttributeCreateConverterFail()
         {
-            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new PocoWithNullConverter()));
-            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<PocoWithNullConverter>("{}"));
+            InvalidOperationException ex;
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new PocoWithInvalidConverter()));
+            // Message should be in the form "The converter specified on 'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithInvalidConverter.MyInt' does not derive from JsonConverter or have a public parameterless constructor."
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithInvalidConverter.MyInt'", ex.Message);
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<PocoWithInvalidConverter>("{}"));
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithInvalidConverter.MyInt'", ex.Message);
+        }
+
+        private class InvalidTypeConverterClass
+        {
+            [JsonConverter(typeof(JsonStringEnumConverter))]
+            public ICollection<InvalidTypeConverterEnum> MyEnumValues { get; set; }
+        }
+
+        private enum InvalidTypeConverterEnum
+        {
+            Value1,
+            Value2,
+        }
+
+        [Fact]
+        public static void AttributeOnPropertyFail()
+        {
+            InvalidOperationException ex;
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new InvalidTypeConverterClass()));
+            // Message should be in the form "The converter specified on 'System.Text.Json.Serialization.Tests.CustomConverterTests+InvalidTypeConverterClass.MyEnumValues' is not compatible with the type 'System.Collections.Generic.ICollection`1[System.Text.Json.Serialization.Tests.ExceptionTests+InvalidTypeConverterEnum]'."
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+InvalidTypeConverterClass.MyEnumValues'", ex.Message);
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<InvalidTypeConverterClass>("{}"));
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+InvalidTypeConverterClass.MyEnumValues'", ex.Message);
+            Assert.Contains("'System.Collections.Generic.ICollection`1[System.Text.Json.Serialization.Tests.CustomConverterTests+InvalidTypeConverterEnum]'", ex.Message);
+        }
+
+        [JsonConverter(typeof(JsonStringEnumConverter))]
+        private class InvalidTypeConverterClassWithAttribute { }
+
+        [Fact]
+        public static void AttributeOnClassFail()
+        {
+            const string expectedSubStr = "'System.Text.Json.Serialization.Tests.CustomConverterTests+InvalidTypeConverterClassWithAttribute'";
+
+            InvalidOperationException ex;
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new InvalidTypeConverterClassWithAttribute()));
+            // Message should be in the form "The converter specified on 'System.Text.Json.Serialization.Tests.CustomConverterTests+InvalidTypeConverterClassWithAttribute' is not compatible with the type 'System.Text.Json.Serialization.Tests.CustomConverterTests+InvalidTypeConverterClassWithAttribute'."
+
+            int pos = ex.Message.IndexOf(expectedSubStr);
+            Assert.True(pos > 0);
+            Assert.Contains(expectedSubStr, ex.Message.Substring(pos + expectedSubStr.Length)); // The same string is repeated again.
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<InvalidTypeConverterClassWithAttribute>("{}"));
+            pos = ex.Message.IndexOf(expectedSubStr);
+            Assert.True(pos > 0);
+            Assert.Contains(expectedSubStr, ex.Message.Substring(pos + expectedSubStr.Length));
         }
 
         private class ConverterThatReturnsNull : JsonConverterFactory
@@ -239,7 +291,7 @@ namespace System.Text.Json.Serialization.Tests
 
         private class PocoWithTwoConvertersOnProperty
         {
-            [CanConvertNullConverter]
+            [InvalidConverter]
             [PointConverter]
             public int MyInt { get; set; }
         }
@@ -247,11 +299,19 @@ namespace System.Text.Json.Serialization.Tests
         [Fact]
         public static void PropertyHasMoreThanOneConverter()
         {
-            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new PocoWithTwoConvertersOnProperty()));
-            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<PocoWithTwoConvertersOnProperty>("{}"));
+            InvalidOperationException ex;
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new PocoWithTwoConvertersOnProperty()));
+            // Message should be in the form "The attribute 'System.Text.Json.Serialization.JsonConverterAttribute' cannot exist more than once on 'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithTwoConvertersOnProperty.MyInt'."
+            Assert.Contains("'System.Text.Json.Serialization.JsonConverterAttribute'", ex.Message);
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithTwoConvertersOnProperty.MyInt'", ex.Message);
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<PocoWithTwoConvertersOnProperty>("{}"));
+            Assert.Contains("'System.Text.Json.Serialization.JsonConverterAttribute'", ex.Message);
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithTwoConvertersOnProperty.MyInt'", ex.Message);
         }
 
-        [CanConvertNullConverter]
+        [InvalidConverter]
         [PointConverter]
         private class PocoWithTwoConverters
         {
@@ -261,8 +321,16 @@ namespace System.Text.Json.Serialization.Tests
         [Fact]
         public static void TypeHasMoreThanOneConverter()
         {
-            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new PocoWithTwoConverters()));
-            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<PocoWithTwoConverters>("{}"));
+            InvalidOperationException ex;
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new PocoWithTwoConverters()));
+            // Message should be in the form "The attribute 'System.Text.Json.Serialization.JsonConverterAttribute' cannot exist more than once on 'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithTwoConverters'."
+            Assert.Contains("'System.Text.Json.Serialization.JsonConverterAttribute'", ex.Message);
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithTwoConverters'", ex.Message);
+
+            ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<PocoWithTwoConverters>("{}"));
+            Assert.Contains("'System.Text.Json.Serialization.JsonConverterAttribute'", ex.Message);
+            Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithTwoConverters'", ex.Message);
         }
     }
 }