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

allow http/1.0 to work with mixed-case Keep-Alive #308

Merged
merged 2 commits into from
Feb 18, 2024

Conversation

gregors
Copy link
Contributor

@gregors gregors commented Feb 18, 2024

tldr; Is that certain clients (in this case apache bench) will send http/1.0 requests with "Keep-Alive" and not "keep-alive" and we currently don't respect that.

I noticed this issue while comparing ab to other servers which seem to handle this correctly. I originally asked about this behavior in the elixir forums

After taking a look and doing some debugging I saw that we fail to lower-case the connection header prior to comparison. Instead of incurring the penalty of lower-casing the entire header I'm assuming the mixed-case pattern match is a performant solution? Strictly speaking we probably should lower-case it to be "compliant"

I did look at the get_connection_header_keys/1 function but it didn't really look like a good fit since it returns that list. Happy to hear feedback and change anything.

@mtrudel
Copy link
Owner

mtrudel commented Feb 18, 2024

Thanks for the PR!

This is good for now. I'm planning on a pretty major overhaul of header processing after #297 et al land, and I may revisit it then.

@mtrudel mtrudel merged commit 7fd1b6c into mtrudel:main Feb 18, 2024
26 of 27 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