Fix duplicated collection items when a constructor parameter name differs only by case from the property#129775
Open
tarekgh wants to merge 1 commit into
Open
Fix duplicated collection items when a constructor parameter name differs only by case from the property#129775tarekgh wants to merge 1 commit into
tarekgh wants to merge 1 commit into
Conversation
…rs only by case from the property The reflection binder decided whether to re-bind a property already bound through a constructor parameter using a case-sensitive name comparison, while the rest of the binder matches parameters and properties case-insensitively. When a constructor parameter such as 'instances' populated a collection property such as 'Instances', the property was bound a second time and the collection items were duplicated. Align the comparison with the case-insensitive matching used elsewhere so the property is not re-bound when a constructor parameter already covers it.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the reflection-based Microsoft.Extensions.Configuration.Binder where collection values could be bound twice (and therefore duplicated) when a constructor parameter name and its corresponding property name differ only by case.
Changes:
- Update
BindPropertiesto treat constructor parameters and properties as matching case-insensitively, consistent with the rest of the binder and configuration key semantics. - Add a regression test covering multiple collection shapes (getter-only, settable, interface-typed, and
paramscollection constructor) to ensure items are not duplicated. - Introduce minimal test helper types used by the regression test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs | Makes the constructor-parameter coverage check case-insensitive to prevent rebinding and collection duplication. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs | Adds a regression test (reflection-only) validating collections aren’t duplicated when ctor parameter casing differs from the property. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs | Adds test-only types exercising the constructor/property casing scenario across collection variants. |
This was referenced Jun 24, 2026
Open
rosebyte
approved these changes
Jun 25, 2026
svick
approved these changes
Jun 25, 2026
svick
reviewed
Jun 25, 2026
| /// the binder must bind the collection once (through the constructor) and must not bind it again | ||
| /// through the property, which would otherwise duplicate the collection items. | ||
| /// </summary> | ||
| [ConditionalFact(typeof(TestHelpers), nameof(TestHelpers.NotSourceGenMode))] |
Member
There was a problem hiding this comment.
I think this could use [ActiveIssue]:
Suggested change
| [ConditionalFact(typeof(TestHelpers), nameof(TestHelpers.NotSourceGenMode))] | |
| [Fact] | |
| [ActiveIssue("https://github.com/dotnet/runtime/issues/83803", typeof(TestHelpers), nameof(TestHelpers.SourceGenMode))] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #129760.
When binding configuration to a type that is created through a constructor whose parameter populates a collection property, the collection items were duplicated if the constructor parameter name differed only by case from the property name (for example a parameter
instancesand a propertyInstances).Root cause
Binding a type with a parameterized constructor happens in two phases:
BindPropertiesiterates the properties and binds any property that is not already covered by a constructor parameter.The check in phase 2 used a case-sensitive comparison to decide whether a constructor parameter already covered a property:
The rest of the binder matches constructor parameters and properties case-insensitively (
DoAllParametersHaveEquivalentPropertiesusesStringComparer.OrdinalIgnoreCase), and configuration keys are case-insensitive as well. Because of the case-sensitive check, a parameterinstanceswas not recognized as covering the propertyInstances, so the property was bound a second time and the items were appended to the collection that the constructor had already populated, producing duplicates. Records were not affected because the compiler generated property keeps the exact parameter casing.Fix
Align the comparison with the case-insensitive matching used everywhere else in the binder so a property is not re-bound when a constructor parameter already covers it:
Scope
This change addresses the reflection based runtime binder only. The configuration source generator has the same problem and is tracked by #83803. The source generator case, including a class (not a record) using a
paramsconstructor, should be addressed when fixing that tracking issue so both paths behave the same.Tests
Added a regression test covering getter only collection, settable collection, getter only interface collection, and a
paramscollection constructor, all with a constructor parameter name that differs only by case from the property. The test is gated to the reflection binder (NotSourceGenMode) because the source generator fix is tracked separately. Verified the test fails without the fix (items duplicated) and passes with it, and that all existing binder tests continue to pass.