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

[dnsdist] Custom response headers must be in lower case #8353

Closed
bortzmeyer opened this issue Sep 29, 2019 · 1 comment · Fixed by #8365
Closed

[dnsdist] Custom response headers must be in lower case #8353

bortzmeyer opened this issue Sep 29, 2019 · 1 comment · Fixed by #8365

Comments

@bortzmeyer
Copy link

bortzmeyer commented Sep 29, 2019

  • Program: dnsdist
  • Issue type: Feature request

Short description

If you put custom headers in upper-case or mixed case, HTTP/2 clients fail. This does not seem documented.

Usecase

Trying to use customResponseHeaders in the server but the problem is probably the same with the extra headers of the internal webserver.

Description

addDOHLocal("[::]:443", "/etc/dnsdist/server-doh.pem", "/etc/dnsdist/server-doh.key", "/", {customResponseHeaders={["Link"]="https://www.bortzmeyer.org/doh-bortzmeyer-fr-policy.html rel="\service-meta"; type="text/html""}})

This will be accepted by dnsdist, no warning sent, but it will break HTTP/2 clients (curl reports "'HTTP/2 stream 0 was not closed cleanly: PROTOCOL_ERROR (err 1)'")

Headers must be in lower case.

Two solutions: document this limitation or, better, lowercase headers automatically

Related

This bug in the ACME client dehydrated is another case of programmers forgetting that headers must be in lower case. dehydrated-io/dehydrated#559

@rgacogne
Copy link
Member

rgacogne commented Sep 29, 2019

rfc7540 indeed states that header field names should be converted to lowercase before encoding 1. I wonder if libh2o should rectify that for us, but let's fix it in dnsdist for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants