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

forward_auth copy_headers must be case sensitive #5038

Closed
theetrain opened this issue Sep 13, 2022 · 5 comments
Closed

forward_auth copy_headers must be case sensitive #5038

theetrain opened this issue Sep 13, 2022 · 5 comments
Assignees
Milestone

Comments

@theetrain
Copy link

Related docs: https://caddyserver.com/docs/caddyfile/directives/forward_auth#syntax

When using forward_auth, I noticed the copy_headers key accepts space-separated case-sensitive values. Should this config be case-insensitive? If so, I can help contribute to the docs.

@mholt
Copy link
Member

mholt commented Sep 13, 2022

Hm, the implementation should probably be changed to be case-insensitive. I'll wait for @francislavoie to get back from his trip and see what his thoughts are!

@francislavoie
Copy link
Member

We should probably reject any config that isn't canonical header names during Caddyfile adapt I think. But if someone uses the long-form, since it uses placeholders, we have no way to check that, really. The response headers are added to the replacer with Set() so there's no opportunity to transparently fix this (unless we change it to use replacer map, but then we'd need to make a copy of the response headers to avoid manipulations to the headers causing the placeholders to return the wrong thing)

@mholt
Copy link
Member

mholt commented Sep 13, 2022

I have to admit, "canonical form" is confusing with Http/2 and 3 where header names are serialized in lowercase. RFC 9110 doesn't really talk about canonicalization, it just says it's lowercase now: https://www.rfc-editor.org/rfc/rfc9110.html#name-field-names

If we prefer to make them case-insensitive, can we make that work you think?

@francislavoie
Copy link
Member

Well it doesn't need to be lowercase to be insensitive, you can just pass it through https://pkg.go.dev/net/textproto#CanonicalMIMEHeaderKey

What I'm trying to get at though is that doing that isn't completely trivial due to how the replacer works right now.

@mholt mholt modified the milestones: v2.6.1, v2.6.2 Sep 17, 2022
@mholt
Copy link
Member

mholt commented Sep 29, 2022

@theetrain Can you please try #5097? I haven't tested it but I'm hoping it will help.

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

No branches or pull requests

3 participants