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

Fixes #233 – Set Vary response header when compression is enabled #286

Closed
wants to merge 7 commits into from

Conversation

palant
Copy link
Contributor

@palant palant commented Jun 15, 2024

See #229 and #233 for the rationale behind these changes.

The Vary insertion logic might be a bit overzealous, it will insert the header even if upstream produced an encoding that we cannot decompress. This is a rather unlikely scenario however, and having this Vary header won’t hurt.

@eaufavor eaufavor added the bug Something isn't working label Jun 20, 2024
@palant palant marked this pull request as ready for review June 21, 2024 17:37
@palant palant changed the title Fixes #229, #233 – Set proper response headers when compression is enabled Fixes #233 – Set Vary response header when compression is enabled Jun 22, 2024
@palant
Copy link
Contributor Author

palant commented Aug 10, 2024

Test failures are again in unrelated code. 🙄

@andrewhavck
Copy link
Contributor

Thanks, will look into the flaky tests.

johnhurt pushed a commit that referenced this pull request 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 pull request 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
@andrewhavck
Copy link
Contributor

This landed in our sync today, thanks for the contribution and sorry for the delay.

escoffier pushed a commit to escoffier/pingora that referenced this pull request 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

Successfully merging this pull request may close these issues.

3 participants