Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConfigurationBinder.Bind on virtual array property duplicates elements #70150

Closed
omegatmm opened this issue Jun 2, 2022 · 2 comments · Fixed by #70592
Closed

ConfigurationBinder.Bind on virtual array property duplicates elements #70150

omegatmm opened this issue Jun 2, 2022 · 2 comments · Fixed by #70592
Labels
area-Extensions-Configuration bug help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@omegatmm
Copy link

omegatmm commented Jun 2, 2022

Description

Microsoft.Extensions.Configuration.ConfigurationBinder.Bind sets virtual properties multiple times when overridden. This appends a duplicate array for each overridden instance of virtual array properties.

This appears to come from ConfigurationBinder.GetAllProperties, it adds overridden properties to its returned list multiple times.

Reproduction Steps

public class C1
{
    public virtual string[] Test { get; set; } = System.Array.Empty<string>();
}

public class C2 : C1
{
    public override string[] Test { get => base.Test; set => base.Test = value; }
}

var configuration = new ConfigurationBuilder()
    .AddInMemoryCollection(new Dictionary<string, string>()
    {
        ["Test:0"] = "1",
    })
    .Build();

var test = new C2();
configuration.Bind(test);
// test.Test has two elements with value of "1"

Expected behavior

I would expect test.Test to have one element "1"

Actual behavior

test.Test has two elements with value of "1"

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 2, 2022
@ghost
Copy link

ghost commented Jun 2, 2022

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Microsoft.Extensions.Configuration.ConfigurationBinder.Bind sets virtual properties multiple times when overridden. This appends a duplicate array for each overridden instance of virtual array properties.

This appears to come from ConfigurationBinder.GetAllProperties, it adds overridden properties to its returned list multiple times.

Reproduction Steps

public class C1
{
    public virtual string[] Test { get; set; } = System.Array.Empty<string>();
}

public class C2 : C1
{
    public override string[] Test { get => base.Test; set => base.Test = value; }
}

var configuration = new ConfigurationBuilder()
    .AddInMemoryCollection(new Dictionary<string, string>()
    {
        ["Test:0"] = "1",
    })
    .Build();

var test = new C2();
configuration.Bind(test);
// test.Test has two elements with value of "1"

Expected behavior

I would expect test.Test to have one element "1"

Actual behavior

test.Test has two elements with value of "1"

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: omegatmm
Assignees: -
Labels:

untriaged, area-Extensions-Configuration

Milestone: -

@maryamariyan maryamariyan added bug help wanted [up-for-grabs] Good issue for external contributors labels Jun 7, 2022
@maryamariyan maryamariyan added this to the Future milestone Jun 7, 2022
@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Jun 7, 2022
@maryamariyan
Copy link
Member

FYI link to code:

private static List<PropertyInfo> GetAllProperties(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]
Type type)
{
var allProperties = new List<PropertyInfo>();
Type? baseType = type;
do
{
allProperties.AddRange(baseType!.GetProperties(DeclaredOnlyLookup));
baseType = baseType.BaseType;
}
while (baseType != typeof(object));
return allProperties;
}

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 10, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 14, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Configuration bug help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant