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.

Sign up for GitHub

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

Jump to bottom

Fix behavior ObjectCollection for single item contains #59547

Merged
merged 2 commits into from
Sep 24, 2021
Merged

Fix behavior ObjectCollection for single item contains #59547

merged 2 commits into from
Sep 24, 2021

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Sep 23, 2021 •
edited by karelz
Loading

Contains for a single item performs a reference-equals, not a semantic equality comparison. Change to use the item's equality definition.

@ghost
Copy link

ghost commented Sep 23, 2021

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

Issue Details

Contains for a signle item performs a reference-equals, not a semantic equality comparison. Change to use the item's equality definition.

Author: hoyosjs
Assignees: -
Labels:

area-System.Net
Milestone: -

@geoffkizer
Copy link
Contributor

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...

@hoyosjs
Copy link
Member Author

hoyosjs commented Sep 23, 2021 •
edited
Loading

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...

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

@hoyosjs hoyosjs force-pushed the juhoyosa/fix-object-collection-single-item branch from 79ef90c to d71f442 Compare September 23, 2021 23:13
@runfoapp runfoapp bot mentioned this pull request Sep 24, 2021
Closed
@hoyosjs
Copy link
Member Author

hoyosjs commented Sep 24, 2021

/backport to release/6.0-rc2

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc2: https://github.com/dotnet/runtime/actions/runs/1268272160

@github-actions github-actions bot mentioned this pull request Sep 24, 2021
Merged
@karelz karelz added this to the 7.0.0 milestone Sep 24, 2021
@hoyosjs
Copy link
Member Author

hoyosjs commented Sep 24, 2021

@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.

stephentoub
stephentoub approved these changes Sep 24, 2021
View reviewed changes
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@hoyosjs hoyosjs merged commit 49f22ce into dotnet:main Sep 24, 2021
@hoyosjs hoyosjs deleted the juhoyosa/fix-object-collection-single-item branch September 24, 2021 20:35
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@geoffkizer geoffkizer geoffkizer left review comments

@stephentoub stephentoub stephentoub approved these changes

Assignees
No one assigned
Labels
area-System.Net
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@hoyosjs @geoffkizer @stephentoub @karelz