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

headers: make sure to forward most target response headers #19

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

paulRbr
Copy link
Member

@paulRbr paulRbr commented Dec 12, 2024

This commit forwards all headers from the target server response, but
removes:

  • the set-cookie header to avoid transfering authentication data via
    the proxy (it's not its role to authenticate any clients)
  • the transfer-encoding header, because it's a “hop-by-hop” header
    and not supposed to be forwarded. The proxy reads the response (and
    should thus consume the transfer-encoding header). If one day the
    proxy supports streaming responses, we will have to manually add this
    header as per the HTTP spec.

@paulRbr paulRbr self-assigned this Dec 12, 2024
This commit forwards all headers from the target server response, but
removes:
- the `set-cookie` header to avoid transfering authentication data via
the proxy (it's not its role to authenticate any clients)
- the `transfer-encoding` header, because it's a [“hop-by-hop” header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Transfer-Encoding)
and not supposed to be forwarded. The proxy reads the response (and
should thus consume the `transfer-encoding` header). If one day the
proxy supports streaming responses, we will have to manually add this
header as per the HTTP spec.
@paulRbr paulRbr force-pushed the fix-missing-target-response-headers branch from 692280d to 9fa550d Compare December 12, 2024 12:05
@paulRbr paulRbr merged commit 075220a into bump-sh:main Dec 12, 2024
1 check passed
@paulRbr paulRbr deleted the fix-missing-target-response-headers branch December 12, 2024 13:00
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 this pull request may close these issues.

2 participants