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

[release/6.0] Fix method HttpHeadersNonValidated uses to get HeaderDescriptor #62194

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 30, 2021

Backport of #62150 to release/6.0

/cc @stephentoub

Fixes #62024

Customer Impact

This API new in 6.0 is not working correctly.

HttpHeaders collections can be associated with a particular kind of header, e.g. HttpRequestMessage.Headers is for request headers. Adding a response header into such a collection should still "work", in that you should still be able to add it and retrieve it, and it should be treated like any arbitrary header. However, the new HttpHeaders.NonValidated property returns a view over the headers that is failing to roundtrip such entries, e.g. if you add a value that would normally be considered a response header into the request headers:

var h = new HttpRequestMessage().Headers;
h.TryAddWithoutValidation(responseHeaderName, value);

and then try to look it up via NonValidated, it won't find the header:

bool found = h.NonValidated.Contains(responseHeaderName);
Assert.True(found); // this fails today

Testing

Added new unit tests.

Risk

Minimal. There were two subtly different internal APIs for retrieving the descriptor object for a given header name, and we were calling the wrong one in two places.

We have two non-public methods for getting a HeaderDescriptor from a string name, one static on HeaderDescriptor and one instance on HttpHeaders.  HttpHeadersNonValidated was using the former, but this then doesn't take into account custom logic used by HttpHeaders in other methods, like TryAddWithoutValidation, which means some methods on HttpHeadersNonValidated can't find the corresponding headers.
@ghost
Copy link

ghost commented Nov 30, 2021

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

Issue Details

Backport of #62150 to release/6.0

/cc @stephentoub

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@stephentoub stephentoub added this to the 6.0.x milestone Nov 30, 2021
@stephentoub stephentoub added the Servicing-consider Issue for next servicing release review label Nov 30, 2021
@karelz karelz requested a review from MihaZupan November 30, 2021 17:50
@Pilchie Pilchie added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 2, 2021
@Pilchie Pilchie modified the milestones: 6.0.x, 6.0.2 Dec 2, 2021
@safern safern merged commit 24a7935 into release/6.0 Dec 15, 2021
@safern safern deleted the backport/pr-62150-to-release/6.0 branch December 15, 2021 18:45
@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants