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

Implement forgotten settings for case preserving #2511

Merged
merged 2 commits into from Apr 22, 2021

Conversation

nox
Copy link
Contributor

@nox nox commented Apr 22, 2021

Turns out I didn't implement the server-side public setters for #2480, oopsie.

@zonyitoo
Copy link
Contributor

What's the differences between http1_title_case_headers and http1_preserve_header_case?

@nox
Copy link
Contributor Author

nox commented Apr 22, 2021

http1_title_case_headers: Sec-WebSocket-Key becomes Sec-Websocket-Key.
http1_preserve_header_case: Sec-WebSocket-Key is sent as is.

Note that you can combine the two, so that existing headers in a given request will preserve their original case, but headers added by Hyper itself (say, date) will use the title case.

@zonyitoo
Copy link
Contributor

So http1_title_case_headers won't make date become Date?

@nox
Copy link
Contributor Author

nox commented Apr 22, 2021

It will, but http1_preserve_header_case won't on its own, that's why you need the two. Preserve means there was an original casing to begin with (i.e. the casing that was found when on the wire when parsing the HTTP message), that's not the case for headers emitted by Hyper itself.

@nox
Copy link
Contributor Author

nox commented Apr 22, 2021

Also, note that Hyper still doesn't let you pass arbitrary header case, as HeaderCaseMap is still private, so http1_preserve_header_case is only useful for proxies right now, where you receive a request from one end, and send it with the client on the other end.

@nox
Copy link
Contributor Author

nox commented Apr 22, 2021

This now includes @zonyitoo's fix from #2510.

@seanmonstar seanmonstar merged commit 4fd6c4c into hyperium:master Apr 22, 2021
@seanmonstar
Copy link
Member

Thanks for the quick fix!

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.

None yet

3 participants