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

Current implementation will strip headers with same key #202

Closed
quangIO opened this issue Apr 1, 2022 · 2 comments · Fixed by #203
Closed

Current implementation will strip headers with same key #202

quangIO opened this issue Apr 1, 2022 · 2 comments · Fixed by #203

Comments

@quangIO
Copy link
Contributor

quangIO commented Apr 1, 2022

The Headers struct uses HashMap to hold key-value pairs. However, headers can have same keys. For conforming headers, we can join them into one string, but it is not the case for all headers, especially when their values can contain comma(s). Thus, sxg-rs should use a multi-map (e.g. HeaderMap in http).

I can help replacing the current implementation if you think it makes sense.

@twifkak
Copy link
Collaborator

twifkak commented Apr 1, 2022

Good catch. Note that the signed exchanges spec disallows duplicate headers, so the processor must join them with a comma before signing.

The HTTP spec considers this operation safe except for Set-Cookie but that's OK since the SXG spec disallows Set-Cookie anyway. (sxg-rs either strips it or cancels signing, depending on configuration.)

@quangIO
Copy link
Contributor Author

quangIO commented Apr 4, 2022

Thank you for the clarification! I just open a small pull request that should support RFC-conforming headers.

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 a pull request may close this issue.

2 participants