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

Use lowercase response headers in Rack example #268

Merged
merged 1 commit into from
Nov 25, 2022

Conversation

Sinjo
Copy link
Member

@Sinjo Sinjo commented Nov 24, 2022

Because HTTP/2 requires that headers are sent in lowercase, Rack has started validating that no header contains an uppercase letter.

We were setting uppercase headers as that was a common convention in HTTP/1.1, where they were interpreted in a case-insensitive fashion.

This change makes us compatible with Rack 3.0 without breaking compatibiility for older versions (not that it would matter for this example code).


Fixes #267

@Sinjo Sinjo requested a review from dmagliola November 24, 2022 00:43
Because HTTP/2 requires that headers are sent in lowercase, Rack has
started validating that no header contains an uppercase letter.

We were setting uppercase headers as that was a common convention in
HTTP/1.1, where they were interpreted in a case-insensitive fashion.

This change makes us compatible with Rack 3.0 without breaking
compatibiility for older versions (not that it would matter for this
example code).

Signed-off-by: Chris Sinjakli <chris@sinjakli.co.uk>
@Sinjo Sinjo force-pushed the sinjo-lowercase-example-headers branch from 59b20c9 to c78dbb0 Compare November 24, 2022 00:44
@Sinjo
Copy link
Member Author

Sinjo commented Nov 24, 2022

I think this might come and hit us in our exporter middleware too, but I need to test the compatibility implications of changing that.

I'd be shocked if Prometheus weren't treating them in a case-insensitive way, but I'll give it a try before I PR a change in for those.

@Sinjo Sinjo merged commit 5a04516 into main Nov 25, 2022
@Sinjo Sinjo deleted the sinjo-lowercase-example-headers branch November 25, 2022 17:41
@Sinjo Sinjo mentioned this pull request Dec 20, 2022
2 tasks
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.

The rack example does not work because of Rack::Lint::LintError
2 participants