From 30426a72420a32ff2d81b29fb1fc8f41203bdd0b Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 30 Nov 2022 12:56:48 -0800 Subject: [PATCH] [release/7.0] Fix configuration binding with types implementing IDictionary<,> (#79019) * Fix configuration binding with types implementing IDictionary<,> * OOB package authoring changes Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com> --- .../src/ConfigurationBinder.cs | 24 +++++++++---------- ...oft.Extensions.Configuration.Binder.csproj | 3 ++- .../ConfigurationCollectionBindingTests.cs | 9 ++++++- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index fa5d77b2fddec..9266f9325ad69 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -367,7 +367,7 @@ private static void BindInstance( if (dictionaryInterface != null) { - BindConcreteDictionary(bindingPoint.Value!, dictionaryInterface, config, options); + BindDictionary(bindingPoint.Value!, dictionaryInterface, config, options); } else { @@ -549,12 +549,12 @@ private static bool CanBindToTheseConstructorParameters(ParameterInfo[] construc } } - 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 @@ private static bool CanBindToTheseConstructorParameters(ParameterInfo[] construc // 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,10 @@ private static void BindConcreteDictionary( 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 +623,7 @@ private static void BindConcreteDictionary( options: options); if (valueBindingPoint.HasNewValue) { - setter.SetValue(dictionary, valueBindingPoint.Value, new object[] { key }); + indexerProperty.SetValue(dictionary, valueBindingPoint.Value, new object[] { key }); } } catch diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj index 8bf4911ccb995..213d0026503f1 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj @@ -5,7 +5,8 @@ true true true - 1 + 2 + true Functionality to bind an object to data in configuration providers for Microsoft.Extensions.Configuration. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs index 66bc1402bdd8c..b604f3c054d7d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs @@ -1171,7 +1171,7 @@ public void CanBindInitializedCustomIndirectlyDerivedIEnumerableList() } [Fact] - public void CanBindInitializedIReadOnlyDictionaryAndDoesNotMofifyTheOriginal() + public void CanBindInitializedIReadOnlyDictionaryAndDoesNotModifyTheOriginal() { // A field declared as IEnumerable that is instantiated with a class // that indirectly implements IEnumerable is still bound, but with @@ -1672,6 +1672,13 @@ public class ImplementerOfIDictionaryClass : IDictionary _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 : Dictionary