Fix configuration binding with types implementing IDictionary<,> (#78946)

Co-authored-by: Steve Dunn <steve@dunnhq.com>

diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
index 39b90fa..256bcc1 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
@@ -367,7 +367,7 @@ namespace Microsoft.Extensions.Configuration
 
                 if (dictionaryInterface != null)
                 {
-                    BindConcreteDictionary(bindingPoint.Value!, dictionaryInterface, config, options);
+                    BindDictionary(bindingPoint.Value!, dictionaryInterface, config, options);
                 }
                 else
                 {
@@ -549,12 +549,12 @@ namespace Microsoft.Extensions.Configuration
                 }
             }
 
-            BindConcreteDictionary(dictionary, dictionaryType, config, options);
+            BindDictionary(dictionary, genericType, config, options);
 
             return dictionary;
         }
 
-        // Binds and potentially overwrites a concrete dictionary.
+        // Binds and potentially overwrites a dictionary object.
         // This differs from BindDictionaryInterface because this method doesn't clone
         // the dictionary; it sets and/or overwrites values directly.
         // When a user specifies a concrete dictionary or a concrete class implementing IDictionary<,>
@@ -562,12 +562,15 @@ namespace Microsoft.Extensions.Configuration
         // in their config class, then it is cloned to a new dictionary, the same way as other collections.
         [RequiresDynamicCode(DynamicCodeWarningMessage)]
         [RequiresUnreferencedCode("Cannot statically analyze what the element type is of the value objects in the dictionary so its members may be trimmed.")]
-        private static void BindConcreteDictionary(
-            object? dictionary,
+        private static void BindDictionary(
+            object dictionary,
             [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties)]
             Type dictionaryType,
             IConfiguration config, BinderOptions options)
         {
+            Debug.Assert(dictionaryType.IsGenericType &&
+                         (dictionaryType.GetGenericTypeDefinition() == typeof(IDictionary<,>) || dictionaryType.GetGenericTypeDefinition() == typeof(Dictionary<,>)));
+
             Type keyType = dictionaryType.GenericTypeArguments[0];
             Type valueType = dictionaryType.GenericTypeArguments[1];
             bool keyTypeIsEnum = keyType.IsEnum;
@@ -589,13 +592,11 @@ namespace Microsoft.Extensions.Configuration
 
             Debug.Assert(dictionary is not null);
 
-            Type dictionaryObjectType = dictionary.GetType();
 
-            MethodInfo tryGetValue = dictionaryObjectType.GetMethod("TryGetValue", BindingFlags.Public | BindingFlags.Instance)!;
+            MethodInfo tryGetValue = dictionaryType.GetMethod("TryGetValue", DeclaredOnlyLookup)!;
+            PropertyInfo? indexerProperty  = dictionaryType.GetProperty("Item", DeclaredOnlyLookup);
 
-            // dictionary should be of type Dictionary<,> or of type implementing IDictionary<,>
-            PropertyInfo? setter = dictionaryObjectType.GetProperty("Item", BindingFlags.Public | BindingFlags.Instance);
-            if (setter is null || !setter.CanWrite)
+            if (indexerProperty is null || !indexerProperty.CanWrite)
             {
                 // Cannot set any item on the dictionary object.
                 return;
@@ -623,7 +624,7 @@ namespace Microsoft.Extensions.Configuration
                         options: options);
                     if (valueBindingPoint.HasNewValue)
                     {
-                        setter.SetValue(dictionary, valueBindingPoint.Value, new object[] { key });
+                        indexerProperty.SetValue(dictionary, valueBindingPoint.Value, new object[] { key });
                     }
                 }
                 catch(Exception ex)

diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs
index 66bc140..b604f3c 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs
@@ -1171,7 +1171,7 @@ namespace Microsoft.Extensions.Configuration.Binder.Test
         }
 
         [Fact]
-        public void CanBindInitializedIReadOnlyDictionaryAndDoesNotMofifyTheOriginal()
+        public void CanBindInitializedIReadOnlyDictionaryAndDoesNotModifyTheOriginal()
         {
             // A field declared as IEnumerable<T> that is instantiated with a class
             // that indirectly implements IEnumerable<T> is still bound, but with
@@ -1672,6 +1672,13 @@ namespace Microsoft.Extensions.Configuration.Binder.Test
             public bool TryGetValue(TKey key, out TValue value) => _dict.TryGetValue(key, out value);
 
             System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => _dict.GetEnumerator();
+
+            // The following are members which have the same names as the IDictionary<,> members.
+            // The following members test that there's no System.Reflection.AmbiguousMatchException when binding to the dictionary.
+            private string? v;
+            public string? this[string key] { get => v; set => v = value; }
+            public bool TryGetValue() { return true; }
+
         }
 
         public class ExtendedDictionary<TKey, TValue> : Dictionary<TKey, TValue>