From d6385708381f206c6f64b906f6844f339ed3a267 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 26 Jul 2019 11:53:37 -0500 Subject: [PATCH] Fix FormatException if an invalid JsonConverter attribute is specified (dotnet/corefx#39789) Commit migrated from https://github.com/dotnet/corefx/commit/dfe3ab2f0205436e23b93c369ee0b2e41a4973e0 --- .../JsonSerializerOptions.Converters.cs | 12 +-- .../System/Text/Json/ThrowHelper.Serialization.cs | 9 +- .../CustomConverterTests.BadConverters.cs | 102 +++++++++++++++++---- 3 files changed, 96 insertions(+), 27 deletions(-) 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 --- 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 --- 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 --- 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(() => JsonSerializer.Serialize(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(() => JsonSerializer.Serialize(new PocoWithNullConverter())); - Assert.Throws(() => JsonSerializer.Deserialize("{}")); + InvalidOperationException ex; + + ex = Assert.Throws(() => 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(() => JsonSerializer.Deserialize("{}")); + Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithInvalidConverter.MyInt'", ex.Message); + } + + private class InvalidTypeConverterClass + { + [JsonConverter(typeof(JsonStringEnumConverter))] + public ICollection MyEnumValues { get; set; } + } + + private enum InvalidTypeConverterEnum + { + Value1, + Value2, + } + + [Fact] + public static void AttributeOnPropertyFail() + { + InvalidOperationException ex; + + ex = Assert.Throws(() => 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(() => JsonSerializer.Deserialize("{}")); + 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(() => 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(() => JsonSerializer.Deserialize("{}")); + 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(() => JsonSerializer.Serialize(new PocoWithTwoConvertersOnProperty())); - Assert.Throws(() => JsonSerializer.Deserialize("{}")); + InvalidOperationException ex; + + ex = Assert.Throws(() => 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(() => JsonSerializer.Deserialize("{}")); + 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(() => JsonSerializer.Serialize(new PocoWithTwoConverters())); - Assert.Throws(() => JsonSerializer.Deserialize("{}")); + InvalidOperationException ex; + + ex = Assert.Throws(() => 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(() => JsonSerializer.Deserialize("{}")); + Assert.Contains("'System.Text.Json.Serialization.JsonConverterAttribute'", ex.Message); + Assert.Contains("'System.Text.Json.Serialization.Tests.CustomConverterTests+PocoWithTwoConverters'", ex.Message); } } } -- 2.7.4