ConfigurationBinder fails to set values for properties of a type with ISet<T> variations

Description

ConfigurationBinder throws an exception in curtain cases related to ISet<T> usages.
Exception message is:

Object of type 'System.Collections.Generic.List`1[System.String]' cannot be converted to type 'System.Collections.Generic.ISet`1[System.String]'.

So far I found these cases:

class MyOptions
{
    public ISet<string>? Prop1 { get; set; }

    public IDictionary<string, ISet<string>> Prop2 { get; set; } = new Dictionary<string, ISet<string>>();

    public IDictionary<string, ISet<string>>? Prop3 { get; set; }
}

Reproduction Steps

I created a short repro: https://github.com/xakep139/config-binder-set-issue
In MyOptions class all properties that cause an exception are commented out, i.e.:

public ISet<string>? NullableSet { get; set; }
public IDictionary<string, ISet<string>> AssignedISetDict { get; set; } = new Dictionary<string, ISet<string>>();
public IDictionary<string, ISet<string>>? NullableISetDict { get; set; }

Expected behavior

No exception is thrown, values are bound as expected

Actual behavior

The exception is thrown:

System.ArgumentException
  HResult=0x80070057
  Message=Object of type 'System.Collections.Generic.List`1[System.String]' cannot be converted to type 'System.Collections.Generic.ISet`1[System.String]'.
  Source=System.Private.CoreLib
  StackTrace:
   at System.RuntimeType.TryChangeType(Object value, Binder binder, CultureInfo culture, Boolean needsSpecialCast) in /_/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs:line 3596
   at System.RuntimeType.CheckValue(Object value, Binder binder, CultureInfo culture, BindingFlags invokeAttr) in /_/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs:line 3557
   at System.Reflection.MethodBase.CheckArguments(StackAllocedArguments& stackArgs, ReadOnlySpan`1 parameters, Binder binder, BindingFlags invokeAttr, CultureInfo culture, Signature sig) in /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs:line 99
   at System.Reflection.RuntimeMethodInfo.InvokeOneParameter(Object obj, BindingFlags invokeAttr, Binder binder, Object parameter, CultureInfo culture) in /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs:line 469
   at System.Reflection.RuntimePropertyInfo.SetValue(Object obj, Object value, BindingFlags invokeAttr, Binder binder, Object[] index, CultureInfo culture) in /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs:line 382
   at System.Reflection.PropertyInfo.SetValue(Object obj, Object value) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/PropertyInfo.cs:line 52
   at Microsoft.Extensions.Configuration.ConfigurationBinder.BindProperty(PropertyInfo property, Object instance, IConfiguration config, BinderOptions options) in /_/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs:line 267
   at Microsoft.Extensions.Configuration.ConfigurationBinder.BindNonScalar(IConfiguration configuration, Object instance, BinderOptions options) in /_/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs:line 233
   at Microsoft.Extensions.Configuration.ConfigurationBinder.BindInstance(Type type, Object instance, IConfiguration config, BinderOptions options) in /_/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs:line 406
   at Microsoft.Extensions.Configuration.ConfigurationBinder.Bind(IConfiguration configuration, Object instance, Action`1 configureOptions) in /_/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs:line 137
   at Microsoft.Extensions.Options.NamedConfigureFromConfigurationOptions`1.BindFromOptions(TOptions options, IConfiguration config, Action`1 configureBinder) in /_/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/src/NamedConfigureFromConfigurationOptions.cs:line 46
   at Microsoft.Extensions.Options.NamedConfigureFromConfigurationOptions`1.<>c__DisplayClass1_0.<.ctor>b__0(TOptions options) in /_/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/src/NamedConfigureFromConfigurationOptions.cs:line 36
   at Microsoft.Extensions.Options.ConfigureNamedOptions`1.Configure(String name, TOptions options) in /_/src/libraries/Microsoft.Extensions.Options/src/ConfigureNamedOptions.cs:line 52
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name) in /_/src/libraries/Microsoft.Extensions.Options/src/OptionsFactory.cs:line 87
   at Microsoft.Extensions.Options.UnnamedOptionsManager`1.get_Value() in /_/src/libraries/Microsoft.Extensions.Options/src/UnnamedOptionsManager.cs:line 30
   at ConsoleApp1.Program.<Main>d__0.MoveNext() in C:\Users\nbalabaev\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 21
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionDispatchInfo.cs:line 56
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/TaskAwaiter.cs:line 181
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/TaskAwaiter.cs:line 149
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/TaskAwaiter.cs:line 104
   at ConsoleApp1.Program.<Main>(String[] args)

Regression?

No response

Known Workarounds

As a workaround you can:

assign a HashSet<T> to a property or
provide a HashSet<T> as a TValue for IDictionary<TKey, TValue>

Configuration

Runtime version is net6.0, SDK version is 6.0.200
Windows 11 Enterprise 21H2 (OS build 22000.493)

Other information

I guess the label should be area-Extensions-Configuration

This issue could be fixed by adding this code in ConfigurationBinder.cs#L255:

private static object? BindToSet(Type type, IConfiguration config)
{
    Type genericType = typeof(HashSet<>).MakeGenericType(type.GenericTypeArguments[0]);
    object? instance = Activator.CreateInstance(genericType);
    BindCollection(instance, genericType, config);
    return instance;
}

And new case needs to be added in ConfigurationBinder.cs#L306 - before checking for ICollection<>:

collectionInterface = FindOpenGenericInterface(typeof(ISet<>), type);
if (collectionInterface != null)
{
    // ISet<T> is guaranteed to have exactly one parameter
    return BindToSet(type, config);
}

Will create a PR shortly to illustrate an idea.

Created #66151

The FindOpenGenericInterface as currently used and implemented is busted. We should never try assigning a List<string> to an ISet<string> which will never work, but we currently attempt to because ISet<T> has an ICollection<T> implementation.

The current logic just assumes that any interface that implements ICollection<T> or IEnumerable<T> must be assignable from a List<T> which is simply not the case as this demonstrates. We have a similar issue with IDictionary<TKey, TVaue>.

Instead, we should check if typeof(ISet<string>).IsAssignableFrom(typeof<List<string>>), typeof(ISet<string>).IsAssignableFrom(typeof<HashSet<string>>) (if we decide to support ISet) etc... It's a bit more complicated because of open generics and variable types, but that's the general idea.

Edit: This is where we're currently creating the List<T> that we're attempting to assign to the ISet<T>:

runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

Lines 256 to 263 in 8b42dff

    
           [RequiresUnreferencedCode("Cannot statically analyze what the element type is of the object collection in type so its members may be trimmed.")] 
        
           private static object? BindToCollection(Type type, IConfiguration config, BinderOptions options) 
        
           { 
        
               Type genericType = typeof(List<>).MakeGenericType(type.GenericTypeArguments[0]); 
        
               object? instance = Activator.CreateInstance(genericType); 
        
               BindCollection(instance, genericType, config, options); 
        
               return instance; 
        
           }

@maryamariyan

@maryamariyan unfortunately, my current capacity doesn't allow me to implement the approach proposed by @halter73 right now.
Once I have a chance, will continue my work on this issue.

@maryamariyan

@maryamariyan - I'm happy to look at this. It's pretty similar to a few other PRs I've done in this area (I just hope one of them isn't the cause of this issue! 😅 )

PR #68133 fixes this issue. It can't fix the scenario where you have this in your options (from the example above):

public IDictionary<string, ISet<string>> Prop2 { get; set; } = new Dictionary<string, ISet<string>>();

It does, however, fix:

public IDictionary<string, ISet<string>> Prop2 { get; set; }

@SteveDunn

@SteveDunn thank you for your PR. However, it does mostly the same thing as mine #66151, thought it doesn't cover IReadOnlySet case (did I miss something?).
As far as I understand, folks want to have an implementation that @halter73 suggested - using typeof(ISet<string>).IsAssignableFrom(typeof<Set<string>>)

@xakep139

@xakep139 - yes, I noticed your PR immediately after committing mine! :)

Mine now supports IReadOnlySet, but did involve a fair amount of head scratching to get it to work. I added a local function to resolve the generic type for the new collection (HashSet<> or List<>) and used a MethodInfo to call the Add method. This was needed as there is no common type between HashSet<> and List<> that contains an Add method.

Well, I guess there's an ICollection that is common for both HashSet and List and it contains an Add method

Maybe, but I couldn't see one that was obvious (as obvious as the non generic List at least)

…

On Mon, 18 Apr 2022, 09:12 Nikita Balabaev, ***@***.***> wrote: Well, I guess there's an ICollection that is common for both HashSet and List and it contains an Add method — Reply to this email directly, view it on GitHub <#66141 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACAJ6CLVAESSFHTMK6UI6LVFUKNLANCNFSM5P2QTYHQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Navigation Menu

Repository navigation

ConfigurationBinder fails to set values for properties of a type with ISet<T> variations #66141

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	[RequiresUnreferencedCode("Cannot statically analyze what the element type is of the object collection in type so its members may be trimmed.")]
	private static object? BindToCollection(Type type, IConfiguration config, BinderOptions options)
	{
	Type genericType = typeof(List<>).MakeGenericType(type.GenericTypeArguments[0]);
	object? instance = Activator.CreateInstance(genericType);
	BindCollection(instance, genericType, config, options);
	return instance;
	}

ConfigurationBinder fails to set values for properties of a type with ISet<T> variations #66141

Description

Description

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

Activity

xakep139 commented on Mar 3, 2022

xakep139 commented on Mar 3, 2022

halter73 commented on Mar 22, 2022

xakep139 commented on Apr 7, 2022

SteveDunn commented on Apr 17, 2022

SteveDunn commented on Apr 17, 2022

xakep139 commented on Apr 18, 2022

SteveDunn commented on Apr 18, 2022

xakep139 commented on Apr 18, 2022

SteveDunn commented on Apr 18, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions