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

Listed connection headers not removed #2653

Closed
motinis opened this issue Jun 26, 2022 · 2 comments · Fixed by #3314
Closed

Listed connection headers not removed #2653

motinis opened this issue Jun 26, 2022 · 2 comments · Fixed by #3314
Labels
Milestone

Comments

@motinis
Copy link

motinis commented Jun 26, 2022

RemoveHopByHopFilter removes the connection header, but doesn't remove the listed headers in it (as it should per e.g. https://www.rfc-editor.org/rfc/rfc9110.html#name-connection or https://datatracker.ietf.org/doc/html/rfc2616#section-14.10). For example, if the header was Connection: xyz then the xyz header would need to be removed as well.

Note that the test code in RemoveHopByHopHeadersFilterTests.removesHeadersListedInConnectionHeader passes because the chosen values of upgrade and keep-alive are listed explicitly in RemoveHopByHopFilter and not because they are in the Connection header value - this is fine since those two headers need to be removed anyway per the specs (whether they are listed in connection header or not).

@spencergibb
Copy link
Member

Yup, I missed

Other hop-by-hop header fields MUST be listed in a Connection header field

here https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-p1-messaging-14#section-7.1.3

@NadChel
Copy link
Contributor

NadChel commented Mar 21, 2024

Got it covered

NadChel added a commit to NadChel/spring-cloud-gateway that referenced this issue Mar 21, 2024
@spencergibb spencergibb added this to the 4.1.2 milestone Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants