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

fix(http2): "Trailer" header should be allowed in HTTP/2 responses as per RFC 9110 #3648

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

lqs
Copy link
Contributor

@lqs lqs commented Apr 25, 2024

This pull request addresses the incorrect handling of the Trailer header in HTTP/2 responses within the hyper project. According to RFC 9110 "HTTP Semantics", the Trailer header is not listed as a "hop-by-hop" header in section 7.6.1. RFC 9110 specifies that a sender intending to generate one or more trailer fields in a message should use the Trailer header in the header section to indicate potential trailer fields, as mentioned in section 6.6.2.

The current implementation in hyper may result in unnecessary warnings for gRPC calls, as gRPC implementations, such as grpc-go, routinely use the Trailer header to indicate the presence of trailers after the response body. (See this behavior in grpc-go's implementation.

This fix updates hyper to correctly allow the Trailer header in HTTP/2 responses, aligning it with RFC 9110 and preventing misleading warnings in environments using gRPC, such as linkerd2-proxy, which prints a warning Connection header illegal in HTTP/2: trailer on every gRPC call.

Note: Mozilla's documentation mistakenly lists Trailer as a hop-by-hop header, which is incorrect.

@lqs lqs force-pushed the fix/allow-trailer-header branch from 31c7257 to fe9f6df Compare April 25, 2024 08:36
Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Nice catch! When paying attention, it seems obvious. We just blindly trusted the MDN page without thinking, woops!

@seanmonstar seanmonstar enabled auto-merge (squash) April 26, 2024 15:32
@seanmonstar seanmonstar merged commit a3269f7 into hyperium:master Apr 26, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants