Fix InvalidCastException when deserializing some dictionary types (do…

…tnet#42835)
layomia · Dec 2, 2020 · d750ae3 · d750ae3
1 parent b02e13a
commit d750ae3
Show file tree

Hide file tree

Showing 13 changed files with 84 additions and 33 deletions.
diff --git a/...ext.Json/src/System/Text/Json/Serialization/Converters/Collection/IDictionaryConverter.cs b/...ext.Json/src/System/Text/Json/Serialization/Converters/Collection/IDictionaryConverter.cs
@@ -35,6 +35,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
                     ThrowHelper.ThrowNotSupportedException_CannotPopulateCollection(TypeToConvert, ref reader, ref state);
                 }
 
+                // Strings are intentionally used as keys when deserializing non-generic dictionaries.
                 state.Current.ReturnValue = new Dictionary<string, object>();
             }
             else

diff --git a/.../System/Text/Json/Serialization/Converters/Collection/IDictionaryOfTKeyTValueConverter.cs b/.../System/Text/Json/Serialization/Converters/Collection/IDictionaryOfTKeyTValueConverter.cs
@@ -6,7 +6,7 @@
 namespace System.Text.Json.Serialization.Converters
 {
     /// <summary>
-    /// Converter for <cref>System.Collections.Generic.IDictionary{string, TValue}</cref> that
+    /// Converter for <cref>System.Collections.Generic.IDictionary{TKey, TValue}</cref> that
     /// (de)serializes as a JSON object with properties representing the dictionary element key and value.
     /// </summary>
     internal sealed class IDictionaryOfTKeyTValueConverter<TCollection, TKey, TValue>
@@ -30,7 +30,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
                     ThrowHelper.ThrowNotSupportedException_CannotPopulateCollection(TypeToConvert, ref reader, ref state);
                 }
 
-                state.Current.ReturnValue = new Dictionary<string, TValue>();
+                state.Current.ReturnValue = new Dictionary<TKey, TValue>();
             }
             else
             {

diff --git a/...ystem/Text/Json/Serialization/Converters/Collection/IEnumerableConverterFactoryHelpers.cs b/...ystem/Text/Json/Serialization/Converters/Collection/IEnumerableConverterFactoryHelpers.cs
@@ -171,7 +171,7 @@ public static MethodInfo GetImmutableEnumerableCreateRangeMethod(this Type type,
 
         [DynamicDependency(CreateRangeMethodNameForDictionary, ImmutableDictionaryTypeName, ImmutableCollectionsAssembly)]
         [DynamicDependency(CreateRangeMethodNameForDictionary, ImmutableSortedDictionaryTypeName, ImmutableCollectionsAssembly)]
-        public static MethodInfo GetImmutableDictionaryCreateRangeMethod(this Type type, Type elementType)
+        public static MethodInfo GetImmutableDictionaryCreateRangeMethod(this Type type, Type keyType, Type valueType)
         {
             Type? constructingType = GetImmutableDictionaryConstructingType(type);
             if (constructingType != null)
@@ -184,7 +184,7 @@ public static MethodInfo GetImmutableDictionaryCreateRangeMethod(this Type type,
                         method.IsGenericMethod &&
                         method.GetGenericArguments().Length == 2)
                     {
-                        return method.MakeGenericMethod(typeof(string), elementType);
+                        return method.MakeGenericMethod(keyType, valueType);
                     }
                 }
             }

diff --git a/...Text/Json/Serialization/Converters/Collection/IReadOnlyDictionaryOfTKeyTValueConverter.cs b/...Text/Json/Serialization/Converters/Collection/IReadOnlyDictionaryOfTKeyTValueConverter.cs
@@ -22,7 +22,7 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
                 ThrowHelper.ThrowNotSupportedException_CannotPopulateCollection(TypeToConvert, ref reader, ref state);
             }
 
-            state.Current.ReturnValue = new Dictionary<string, TValue>();
+            state.Current.ReturnValue = new Dictionary<TKey, TValue>();
         }
 
         protected internal override bool OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, ref WriteStack state)

diff --git a/...Text/Json/Serialization/Converters/Collection/ImmutableDictionaryOfTKeyTValueConverter.cs b/...Text/Json/Serialization/Converters/Collection/ImmutableDictionaryOfTKeyTValueConverter.cs
@@ -19,21 +19,21 @@ protected override void Add(TKey key, in TValue value, JsonSerializerOptions opt
 
         protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state)
         {
-            state.Current.ReturnValue = new Dictionary<string, TValue>();
+            state.Current.ReturnValue = new Dictionary<TKey, TValue>();
         }
 
         protected override void ConvertCollection(ref ReadStack state, JsonSerializerOptions options)
         {
             JsonClassInfo classInfo = state.Current.JsonClassInfo;
 
-            Func<IEnumerable<KeyValuePair<string, TValue>>, TCollection>? creator = (Func<IEnumerable<KeyValuePair<string, TValue>>, TCollection>?)classInfo.CreateObjectWithArgs;
+            Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection>? creator = (Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection>?)classInfo.CreateObjectWithArgs;
             if (creator == null)
             {
-                creator = options.MemberAccessorStrategy.CreateImmutableDictionaryCreateRangeDelegate<TValue, TCollection>();
+                creator = options.MemberAccessorStrategy.CreateImmutableDictionaryCreateRangeDelegate<TCollection, TKey, TValue>();
                 classInfo.CreateObjectWithArgs = creator;
             }
 
-            state.Current.ReturnValue = creator((Dictionary<string, TValue>)state.Current.ReturnValue!);
+            state.Current.ReturnValue = creator((Dictionary<TKey, TValue>)state.Current.ReturnValue!);
         }
 
         protected internal override bool OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, ref WriteStack state)

diff --git a/...c/System/Text/Json/Serialization/Converters/Collection/ImmutableEnumerableOfTConverter.cs b/...c/System/Text/Json/Serialization/Converters/Collection/ImmutableEnumerableOfTConverter.cs
@@ -28,7 +28,7 @@ protected override void ConvertCollection(ref ReadStack state, JsonSerializerOpt
             Func<IEnumerable<TElement>, TCollection>? creator = (Func<IEnumerable<TElement>, TCollection>?)classInfo.CreateObjectWithArgs;
             if (creator == null)
             {
-                creator = options.MemberAccessorStrategy.CreateImmutableEnumerableCreateRangeDelegate<TElement, TCollection>();
+                creator = options.MemberAccessorStrategy.CreateImmutableEnumerableCreateRangeDelegate<TCollection, TElement>();
                 classInfo.CreateObjectWithArgs = creator;
             }
 

diff --git a/...s/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs b/...s/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs
@@ -19,7 +19,10 @@ public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOp
         }
 
         internal override object ReadWithQuotes(ref Utf8JsonReader reader)
-            => throw new NotSupportedException();
+        {
+            ThrowHelper.ThrowNotSupportedException_DictionaryKeyTypeNotSupported(TypeToConvert);
+            return null!;
+        }
 
         internal override void WriteWithQuotes(Utf8JsonWriter writer, object value, JsonSerializerOptions options, ref WriteStack state)
         {

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/MemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/MemberAccessor.cs
@@ -17,9 +17,9 @@ public abstract JsonClassInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1,
 
         public abstract Action<TCollection, object?> CreateAddMethodDelegate<TCollection>();
 
-        public abstract Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerableCreateRangeDelegate<TElement, TCollection>();
+        public abstract Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerableCreateRangeDelegate<TCollection, TElement>();
 
-        public abstract Func<IEnumerable<KeyValuePair<string, TElement>>, TCollection> CreateImmutableDictionaryCreateRangeDelegate<TElement, TCollection>();
+        public abstract Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection> CreateImmutableDictionaryCreateRangeDelegate<TCollection, TKey, TValue>();
 
         public abstract Func<object, TProperty> CreatePropertyGetter<TProperty>(PropertyInfo propertyInfo);
 

diff --git a/...aries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs b/...aries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs
@@ -171,7 +171,7 @@ private static DynamicMethod CreateAddMethodDelegate(Type collectionType)
             return dynamicMethod;
         }
 
-        public override Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerableCreateRangeDelegate<TElement, TCollection>() =>
+        public override Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerableCreateRangeDelegate<TCollection, TElement>() =>
             CreateDelegate<Func<IEnumerable<TElement>, TCollection>>(
                 CreateImmutableEnumerableCreateRangeDelegate(typeof(TCollection), typeof(TElement), typeof(IEnumerable<TElement>)));
 
@@ -195,13 +195,13 @@ private static DynamicMethod CreateImmutableEnumerableCreateRangeDelegate(Type c
             return dynamicMethod;
         }
 
-        public override Func<IEnumerable<KeyValuePair<string, TElement>>, TCollection> CreateImmutableDictionaryCreateRangeDelegate<TElement, TCollection>() =>
-            CreateDelegate<Func<IEnumerable<KeyValuePair<string, TElement>>, TCollection>>(
-                CreateImmutableDictionaryCreateRangeDelegate(typeof(TCollection), typeof(TElement), typeof(IEnumerable<KeyValuePair<string, TElement>>)));
+        public override Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection> CreateImmutableDictionaryCreateRangeDelegate<TCollection, TKey, TValue>() =>
+            CreateDelegate<Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection>>(
+                CreateImmutableDictionaryCreateRangeDelegate(typeof(TCollection), typeof(TKey), typeof(TValue), typeof(IEnumerable<KeyValuePair<TKey, TValue>>)));
 
-        private static DynamicMethod CreateImmutableDictionaryCreateRangeDelegate(Type collectionType, Type elementType, Type enumerableType)
+        private static DynamicMethod CreateImmutableDictionaryCreateRangeDelegate(Type collectionType, Type keyType, Type valueType, Type enumerableType)
         {
-            MethodInfo realMethod = collectionType.GetImmutableDictionaryCreateRangeMethod(elementType);
+            MethodInfo realMethod = collectionType.GetImmutableDictionaryCreateRangeMethod(keyType, valueType);
 
             var dynamicMethod = new DynamicMethod(
                 realMethod.Name,

diff --git a/...libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionMemberAccessor.cs b/...libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionMemberAccessor.cs
@@ -123,18 +123,18 @@ public override JsonClassInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1,
             };
         }
 
-        public override Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerableCreateRangeDelegate<TElement, TCollection>()
+        public override Func<IEnumerable<TElement>, TCollection> CreateImmutableEnumerableCreateRangeDelegate<TCollection, TElement>()
         {
             MethodInfo createRange = typeof(TCollection).GetImmutableEnumerableCreateRangeMethod(typeof(TElement));
             return (Func<IEnumerable<TElement>, TCollection>)createRange.CreateDelegate(
                 typeof(Func<IEnumerable<TElement>, TCollection>));
         }
 
-        public override Func<IEnumerable<KeyValuePair<string, TElement>>, TCollection> CreateImmutableDictionaryCreateRangeDelegate<TElement, TCollection>()
+        public override Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection> CreateImmutableDictionaryCreateRangeDelegate<TCollection, TKey, TValue>()
         {
-            MethodInfo createRange = typeof(TCollection).GetImmutableDictionaryCreateRangeMethod(typeof(TElement));
-            return (Func<IEnumerable<KeyValuePair<string, TElement>>, TCollection>)createRange.CreateDelegate(
-                typeof(Func<IEnumerable<KeyValuePair<string, TElement>>, TCollection>));
+            MethodInfo createRange = typeof(TCollection).GetImmutableDictionaryCreateRangeMethod(typeof(TKey), typeof(TValue));
+            return (Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection>)createRange.CreateDelegate(
+                typeof(Func<IEnumerable<KeyValuePair<TKey, TValue>>, TCollection>));
         }
 
         public override Func<object, TProperty> CreatePropertyGetter<TProperty>(PropertyInfo propertyInfo)

diff --git a/....Text.Json/tests/Serialization/CollectionTests/CollectionTests.Dictionary.NonStringKey.cs b/....Text.Json/tests/Serialization/CollectionTests/CollectionTests.Dictionary.NonStringKey.cs
@@ -3,6 +3,7 @@
 
 using System.Collections;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Collections.Specialized;
 using System.IO;
 using System.Threading.Tasks;
@@ -574,5 +575,49 @@ public class SuffixNamingPolicy : JsonNamingPolicy
             public const string Suffix = "_Suffix";
             public override string ConvertName(string name) => name + Suffix;
         }
+
+        [Fact]
+        public static void RoundtripAllDictionaryConverters()
+        {
+            const string Expected = @"{""1"":1}";
+
+            foreach (Type type in CollectionTestTypes.DeserializableDictionaryTypes<int, int>())
+            {
+                object dict = JsonSerializer.Deserialize(Expected, type);
+                Assert.Equal(Expected, JsonSerializer.Serialize(dict, type));
+            }
+        }
+
+        [Theory]
+        [InlineData(typeof(IDictionary))]
+        [InlineData(typeof(Hashtable))]
+        public static void IDictionary_Keys_ShouldBe_String_WhenDeserializing(Type type)
+        {
+            const string Expected = @"{""1998-02-14"":1}";
+
+            IDictionary dict = (IDictionary)JsonSerializer.Deserialize(Expected, type);
+            Assert.Equal(1, dict.Count);
+            JsonElement element = Assert.IsType<JsonElement>(dict["1998-02-14"]);
+            Assert.Equal(1, element.GetInt32());
+
+            Assert.Equal(Expected, JsonSerializer.Serialize(dict, type));
+        }
+
+        [Fact]
+        public static void GenericDictionary_WithObjectKeys_Throw_WhenDeserializing()
+        {
+            const string Expected = @"{""1998-02-14"":1}";
+
+            var dict = new Dictionary<object, int> { ["1998-02-14"] = 1 };
+            RunTest<IDictionary<object, int>>(dict);
+            RunTest<Dictionary<object, int>>(dict);
+            RunTest<ImmutableDictionary<object, int>>(ImmutableDictionary.CreateRange(dict));
+
+            void RunTest<T>(T dictionary)
+            {
+                Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<T>(Expected));
+                Assert.Equal(Expected, JsonSerializer.Serialize(dictionary));
+            }
+        }
     }
 }
diff --git a/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs b/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs
@@ -426,13 +426,13 @@ private static void RunAllDictionariessRoundTripTest<T>(List<T> numbers)
 
             string jsonNumbersAsStrings = jsonBuilder_NumbersAsStrings.ToString();
 
-            foreach (Type type in CollectionTestTypes.DeserializableDictionaryTypes<T>())
+            foreach (Type type in CollectionTestTypes.DeserializableDictionaryTypes<string, T>())
             {
                 object obj = JsonSerializer.Deserialize(jsonNumbersAsStrings, type, s_optionReadAndWriteFromStr);
                 JsonTestHelper.AssertJsonEqual(jsonNumbersAsStrings, JsonSerializer.Serialize(obj, s_optionReadAndWriteFromStr));
             }
 
-            foreach (Type type in CollectionTestTypes.DeserializableNonDictionaryTypes<T>())
+            foreach (Type type in CollectionTestTypes.DeserializableNonGenericDictionaryTypes())
             {
                 Dictionary<T, T> dict = JsonSerializer.Deserialize<Dictionary<T, T>>(jsonNumbersAsStrings, s_optionReadAndWriteFromStr);
 

diff --git a/src/libraries/System.Text.Json/tests/Serialization/TestClasses/TestClasses.cs b/src/libraries/System.Text.Json/tests/Serialization/TestClasses/TestClasses.cs
@@ -1939,17 +1939,19 @@ public static IEnumerable<Type> DictionaryTypes<TElement>()
             yield return typeof(GenericIReadOnlyDictionaryWrapper<string, TElement>); // IReadOnlyDictionaryOfStringTValueConverter
         }
 
-        public static IEnumerable<Type> DeserializableDictionaryTypes<TElement>()
+        public static IEnumerable<Type> DeserializableDictionaryTypes<TKey, TValue>()
         {
-            yield return typeof(Dictionary<string, TElement>); // DictionaryOfStringTValueConverter
+            yield return typeof(Dictionary<TKey, TValue>); // DictionaryOfStringTValueConverter
             yield return typeof(Hashtable); // IDictionaryConverter
-            yield return typeof(ConcurrentDictionary<string, TElement>); // IDictionaryOfStringTValueConverter
-            yield return typeof(GenericIDictionaryWrapper<string, TElement>); // IDictionaryOfStringTValueConverter
-            yield return typeof(ImmutableDictionary<string, TElement>); // ImmutableDictionaryOfStringTValueConverter
-            yield return typeof(IReadOnlyDictionary<string, TElement>); // IReadOnlyDictionaryOfStringTValueConverter
+            yield return typeof(IDictionary); // IDictionaryConverter
+            yield return typeof(ConcurrentDictionary<TKey, TValue>); // IDictionaryOfStringTValueConverter
+            yield return typeof(IDictionary<TKey, TValue>); // IDictionaryOfStringTValueConverter
+            yield return typeof(GenericIDictionaryWrapper<TKey, TValue>); // IDictionaryOfStringTValueConverter
+            yield return typeof(ImmutableDictionary<TKey, TValue>); // ImmutableDictionaryOfStringTValueConverter
+            yield return typeof(IReadOnlyDictionary<TKey, TValue>); // IReadOnlyDictionaryOfStringTValueConverter
         }
 
-        public static IEnumerable<Type> DeserializableNonDictionaryTypes<TElement>()
+        public static IEnumerable<Type> DeserializableNonGenericDictionaryTypes()
         {
             yield return typeof(Hashtable); // IDictionaryConverter
             yield return typeof(SortedList); // IDictionaryConverter
-Original file line number
+Diff line change
@@ Expand Up @@
                         ThrowHelper.ThrowNotSupportedException_CannotPopulateCollection(TypeToConvert, ref reader, ref state);
                     }
+                    // Strings are intentionally used as keys when deserializing non-generic dictionaries.
                     state.Current.ReturnValue = new Dictionary<string, object>();
                 }
                 else
@@ Expand Down @@