Skip to content

Have kestrel filter out pseudo headers #30417

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

Closed
Tratcher opened this issue Feb 24, 2021 · 2 comments · Fixed by #36166
Closed

Have kestrel filter out pseudo headers #30417

Tratcher opened this issue Feb 24, 2021 · 2 comments · Fixed by #36166
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. HTTP2 HTTP3
Milestone

Comments

@Tratcher
Copy link
Member

Tratcher commented Feb 24, 2021

dotnet/runtime#48009 (comment)

HTTP/2 uses pseudo headers for special values like method, path, authority, scheme, etc., but they have lots of constraints about where they can be placed, only using reserved ones, etc..

https://tools.ietf.org/html/rfc7540#section-8.1.2.1

Pseudo-header fields are not HTTP header fields.

As a result, other http header APIs won't accept them as input. E.g. HttpRequestMessage, WebHeaderCollection, etc.. This means components that copy request headers from Kestrel have to filter these out.

https://github.com/microsoft/reverse-proxy/blob/a3247ba1168325f46ab0ee4a24d3de1d9acdd853/src/ReverseProxy/Service/Proxy/HttpTransformer.cs#L49-L53

// Filter out HTTP/2 pseudo headers like ":method" and ":path", those go into other fields.
if (headerName.Length > 0 && headerName[0] == ':')
{
    continue;
}

https://github.com/CoreWCF/CoreWCF/blob/9957bb14bede95c22a8c6c7b3be4e42e84ba8471/src/CoreWCF.Http/src/CoreWCF/Channels/HttpContextExtensions.cs#L18-L19

if (header.Key.StartsWith(":")) // HTTP/2 pseudo header, skip as they appear on other properties of HttpRequest
    continue;

Http.Sys and IIS also filter out pseudo headers.

Historically we thought we were being more transparent about the protocol by including these values, but I'm not aware of any situations where they have proven useful since they are duplicated elsewhere in the request.

@Tratcher Tratcher added bug This issue describes a behavior which is not expected - a bug. area-servers labels Feb 24, 2021
@BrennanConroy BrennanConroy added this to the Next sprint planning milestone Feb 24, 2021
@ghost
Copy link

ghost commented Feb 24, 2021

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@jkotalik
Copy link
Contributor

jkotalik commented Apr 6, 2021

+1 on this.

@Tratcher Tratcher self-assigned this Aug 16, 2021
@Tratcher Tratcher modified the milestones: Next sprint planning, 6.0-rc2 Sep 3, 2021
Tratcher added a commit to Tratcher/aspnetcore that referenced this issue Sep 3, 2021
wtgodbe pushed a commit that referenced this issue Sep 8, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. HTTP2 HTTP3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants