-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix behavior ObjectCollection for single item contains #59547
Fix behavior ObjectCollection for single item contains #59547
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsContains for a signle item performs a reference-equals, not a semantic equality comparison. Change to use the item's equality definition.
|
It looks like we have a similar issue here: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs#L123 I don't see any other instances of this in the file, but a second pair of eyeballs would be good... |
src/libraries/System.Net.Http/tests/UnitTests/Headers/ObjectCollectionTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http/tests/UnitTests/Headers/ObjectCollectionTest.cs
Outdated
Show resolved
Hide resolved
Noticed that too - there's a few https://source.dot.net/#System.Net.Http/System/Net/Http/Headers/ContentDispositionHeaderValue.cs,104 and in setting date and name in ContentDisposition |
79ef90c
to
d71f442
Compare
/backport to release/6.0-rc2 |
Started backporting to release/6.0-rc2: https://github.com/dotnet/runtime/actions/runs/1268272160 |
src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http/tests/UnitTests/Headers/ObjectCollectionTest.cs
Outdated
Show resolved
Hide resolved
@stephentoub @geoffkizer can you please take a look here and if everything looks OK now approve this and the port? Don't want to keep RC2 in delay. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
Contains for a single item performs a reference-equals, not a semantic equality comparison. Change to use the item's equality definition.