[release/7.0-rc2] Fix Configuration.Binder for collection properties with no setters #75723
Conversation
Binding to an IDictionary/ICollection/ISet in ConfigurationBinder with no setter was failing because we were returning too early. Only returning early now if we were able to set the property, or if the interface is read-only. Fix #75626
ghost
added
the
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration Issue DetailsBackport of #75689 to release/7.0-rc2 /cc @eerhardt Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
Is it worth looking through our tests around this for other test holes? |
I did as much as I could. The original report was for I'm not sure how else to discover other holes in this area. |
I guess there is also the possibility of pulling the original change for 7.0, if you're sufficiently concerned about other bugs we might not know about - leaving the change in main. |
I don't think I'm at that point at this time.
Other bugs could be lurking here. Since this was the 2nd regression from the change, I felt it was necessary to call it out in the "Risk" section. However, I think fixing this bug and moving forward is the right plan here. |
sounds good. I have to ask the questions... please send tactics mail with template. possibly augmented with your answers above.. |
carlossanlop
added
Servicing-approved
|
This was approved by Tactics via email. |
Backport of #75689 to release/7.0-rc2
/cc @eerhardt
Customer Impact
A behavior regression was introduced between 7.0-preview7 and 7.0-rc1 that was reported by a customer.
When using ConfigurationBinder to bind an object with a read-only / get-only property of type
IDictionary<,>orICollection<>, the property is not being bound and remains empty. This is a silent issue that can cause apps to not behave correctly because it is missing configuration information at runtime.Testing
New tests were added to ensure the scenario doesn't break again in the future. The customer's repro steps now produce expected results.
Risk
This is the 2nd customer reported regression caused from refactoring introduced by a new feature. It is very possible there might be another one in this area that we haven't found yet. However, I don't believe reverting the original change is necessary. I think fixing this bug and moving forward is the right plan here.
I assessed our existing test coverage as much as I could. The original report was for
IDictionaryand I noticed the same applied toICollectionandISet, so I added tests and fixes for those as well.For this specific fix, the risk of regressing something else is rather low. All existing tests pass.