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

Set Vary: Accept-Encoding header when compressing responses #233

Open
palant opened this issue May 7, 2024 · 0 comments
Open

Set Vary: Accept-Encoding header when compressing responses #233

palant opened this issue May 7, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@palant
Copy link
Contributor

palant commented May 7, 2024

What is the problem your feature solves, or the need it fulfills?

When downstream compression is enabled in pingora-proxy, it won’t currently set Vary: Accept-Encoding header. This might result in downstream proxies caching the response incorrectly – e.g. caching a Zstandard response and serving it to a client without Zstandard support.

Describe the solution you'd like

ResponseCompressionCtx.response_header_filter() needs to add Vary: Accept-Encoding header. This could happen only for the responses that actually meet the criteria for being compressed (whether the compression is actually applied or not), or just for any responses going through that method if that’s easier to implement.

Describe alternatives you've considered

The header can be added by the code enabling the compression. It’s unexpected that this has to be done manually however.

Additional context

Vary header spec:

An origin server SHOULD generate a Vary header field on a cacheable response when it wishes that response to be selectively reused for subsequent requests. Generally, that is the case when the response content has been tailored to better fit the preferences expressed by those selecting header fields, such as when an origin server has selected the response's language based on the request's Accept-Language header field.

@palant palant changed the title Set Vary: Accept-Encoding when compressing responses Set Vary: Accept-Encoding header when compressing responses May 7, 2024
@eaufavor eaufavor added the bug Something isn't working label May 13, 2024
palant added a commit to palant/pingora that referenced this issue Jun 15, 2024
johnhurt pushed a commit that referenced this issue Aug 23, 2024
…abled

---
Merge branch 'main' into compression-headers
---
Updated “compression enabled” check for recent changes
---
Fixed clippy warning
---
Reverted changes related to Accept-Ranges header
---
Handle multiple Vary headers
---
Merged main branch

Includes-commit: 4ea0c3f
Includes-commit: 78f09b2
Includes-commit: 973d596
Includes-commit: 9f82578
Includes-commit: d6e94c9
Includes-commit: e88fdb8
Includes-commit: f647f04
Replicated-from: #286
johnhurt pushed a commit that referenced this issue Aug 23, 2024
…abled

---
Merge branch 'main' into compression-headers
---
Updated “compression enabled” check for recent changes
---
Fixed clippy warning
---
Reverted changes related to Accept-Ranges header
---
Handle multiple Vary headers
---
Merged main branch

Includes-commit: 4ea0c3f
Includes-commit: 78f09b2
Includes-commit: 973d596
Includes-commit: 9f82578
Includes-commit: d6e94c9
Includes-commit: e88fdb8
Includes-commit: f647f04
Replicated-from: #286
escoffier pushed a commit to escoffier/pingora that referenced this issue Sep 6, 2024
…en compression is enabled

---
Merge branch 'main' into compression-headers
---
Updated “compression enabled” check for recent changes
---
Fixed clippy warning
---
Reverted changes related to Accept-Ranges header
---
Handle multiple Vary headers
---
Merged main branch

Includes-commit: 4ea0c3f
Includes-commit: 78f09b2
Includes-commit: 973d596
Includes-commit: 9f82578
Includes-commit: d6e94c9
Includes-commit: e88fdb8
Includes-commit: f647f04
Replicated-from: cloudflare#286
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

No branches or pull requests

2 participants