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

Accept-Encoding should not be added to 'Vary' for non-compressable Content-Types #279

Closed
2 tasks done
rasander opened this issue Feb 5, 2024 · 4 comments
Closed
2 tasks done
Labels

Comments

@rasander
Copy link

rasander commented Feb 5, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.25.2

Plugin version

6.5.0

Node.js version

20.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Mac OS X and Linux Ubuntu

Description

"Accept-Encoding" is always added to Vary for both compressable and non-compressable content-types.
This should only be added if content-type is compressable.

This means, that caching is not working properly in nginx or CDN, because some caching applications (in my case Akamai CDN), will only cache if Content-Encoding is matching Accept-Encoding (in case Accept-Encoding is included in Vary).
For non-compressable content-types these will not match.

Steps to Reproduce

$ curl -I "http://localhost:3000/-/media/test.gif" -X GET -H "Accept-Encoding: br"
HTTP/1.1 200 OK
cache-control: public, max-age=604800
content-type: image/gif
vary: accept-encoding
content-length: 2971239
Date: Mon, 05 Feb 2024 14:50:33 GMT
Connection: keep-alive
Keep-Alive: timeout=72

$ curl -I "http://localhost:3000/-/media/test.gif" -X GET 
HTTP/1.1 200 OK
cache-control: public, max-age=604800
content-type: image/gif
vary: accept-encoding
content-length: 2971239
Date: Mon, 05 Feb 2024 14:50:47 GMT
Connection: keep-alive
Keep-Alive: timeout=72

Expected Behavior

$ curl -I "http://localhost:3000/-/media/test.gif" -X GET -H "Accept-Encoding: br"
HTTP/1.1 200 OK
cache-control: public, max-age=604800
content-type: image/gif
content-length: 2971239
Date: Mon, 05 Feb 2024 14:50:33 GMT
Connection: keep-alive
Keep-Alive: timeout=72

$ curl -I "http://localhost:3000/-/media/test.gif" -X GET 
HTTP/1.1 200 OK
cache-control: public, max-age=604800
content-type: image/gif
content-length: 2971239
Date: Mon, 05 Feb 2024 14:50:47 GMT
Connection: keep-alive
Keep-Alive: timeout=72
@mcollina
Copy link
Member

mcollina commented Feb 5, 2024

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@mcollina mcollina added the bug label Feb 5, 2024
@Fdawgs
Copy link
Member

Fdawgs commented Feb 29, 2024

"Accept-Encoding" is always added to Vary for both compressable and non-compressable content-types. This should only be added if content-type is compressable.

@rasander Do you have a link to a spec that states this?

@stanleyxu2005
Copy link
Contributor

stanleyxu2005 commented Mar 27, 2024

@rasander Could you confirm if my understanding is correct?

  1. Wherever response.headers['content-encoding'] is defined (unless the value is identity, then the header vary should contain accept-encoding
  2. vice versa

Open question:
if only one accept-encoding in request header, should vary header be set in response?

I have provided a PR, but I'm not so certain, if the changes in tests are all valid. @mcollina @Fdawgs

@stanleyxu2005
Copy link
Contributor

"Accept-Encoding" is always added to Vary for both compressable and non-compressable content-types. This should only be added if content-type is compressable.

@rasander Do you have a link to a spec that states this?

https://www.fastly.com/blog/best-practices-using-vary-header
See the section How to Use Vary to Solve Problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants