-
-
Notifications
You must be signed in to change notification settings - Fork 270
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
Handling of RFC7540 8.1.2.5 #699
Comments
I don't know if that should be done but at the very least it shouldn't be done by Hyper, as it would be weird for Hyper to do it only for HTTP/2 (as nothing in HTTP/1.1 says they must be concatenated before being passed to other stuff). I feel like this has already been discussed in the past and the conclusion was that it isn't h2 nor Hyper's business to normalize Cookie headers. |
Thanks for linking to this in hyper. Would you be open to a PR documenting the behavior somewhere? I am not sure for the best place inside hyper, as |
Hello. Are there any updates on this? Because this unhandled specification has come up in pingora again, since it's using this crate. Just curious, but shouldn't the h2 crate actually implement all the specifications for HTTP/2 or what was the agreement made here? |
The h2 crate is strictly about HTTP/2, there is no "non-HTTP/2 context, such as an HTTP/1.1 connection, or a generic HTTP server application" here. The place to implement that is in systems that do provide such things, such as Pingora or Hyper. |
We ran into an issue where the HeaderMap returned in hyper as part of
http::Request
contains multiple entries for thecookie
key.The HTTP/2 spec states in 8.1.2.5:
Currently, If I see this right, during decoding each header is simply appended to the
HeaderMap
(see https://github.com/hyperium/h2/blob/master/src/frame/headers.rs#L895C38-L895C38)This
HeaderMap
ends up in the request that is passed to the user of h2 (/hyper) without merging the Cookie header.I created a simple repro at https://github.com/valkum/h2-cookie-violation
You need go installed because curl currently does not use 8.1.2.5. but the go http2 seems to do that (similar to Browsers).
If, for any reason, this is an expected deviation from the spec, I guess h2 and possible hyper should get some docs about this deviation. It seems the current ecosystem (for using cookies in Rust) settled on using
HeaderMap::get_all
for Cookies.The text was updated successfully, but these errors were encountered: