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

caddyhttp: Canonicalize header field names #5176

Conversation

ghostwheel42
Copy link
Contributor

This fixes #5175 by canonicalizing the header field names.

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

I just realized this is very similar to 2153a81.

@francislavoie
Copy link
Member

Seems like this is just a fix for JSON, because the headers don't get set through .Add() and such as they do when unmarshaling the Caddyfile.

Should this be done at provision time instead of at runtime, to save some cycles?

@mholt
Copy link
Member

mholt commented Oct 27, 2022

@francislavoie That's a good idea -- I thought about it too, but it has to be done on the output of the replacer, which depends on the request. So I think we have to do it here.

@ghostwheel42
Copy link
Contributor Author

@francislavoie I've thought about using the appropriate methods for this and/or doing it when provisioning, but as @mholt said it needs to be done on the output of the replacer and to use .Add() you'd need to iterate over the list of values which would probably be slower in the end.

@ghostwheel42 ghostwheel42 force-pushed the fix_http.handlers.static_response branch from 5dc2c1f to d6f0217 Compare October 28, 2022 20:05
@francislavoie francislavoie added the bug 🐞 Something isn't working label Oct 29, 2022
@francislavoie francislavoie added this to the v2.6.3 milestone Oct 29, 2022
@francislavoie francislavoie merged commit 087f126 into caddyserver:master Oct 29, 2022
@ghostwheel42 ghostwheel42 deleted the fix_http.handlers.static_response branch December 14, 2022 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http.handlers.static_response fails to canonicalize header field names
4 participants