search

coderedcorp / wagtail-cache

A simple page cache for Wagtail based on the Django cache middleware.
Other
87 stars 29 forks source link

Adds Vary: Cookie to requests #29

Closed thenewguy closed 4 months ago

thenewguy commented 3 years ago

I am seeing Vary: Cookie added to requests when WAGTAIL_CACHE = True for responses that do not contain Vary: Cookie when WAGTAIL_CACHE = False

thenewguy commented 3 years ago

Adding Vary: Cookie prevents the reverse proxy (or CDN - like Cloudfront) from serving the cached response to another session. Introducing Vary: Cookie when it isn't required means that anonymous content cannot be reused. This is important for caching anonymous page views, wagtail's dynamic image serve view, and other assets served by Django that are not user specific.

vsalvino commented 2 years ago

I recently did some work related to this... see pull request #47

In that pull request, we are going to remove the Vary: Cookie header for anything that does not have a session or CSRF token.

I still like your change of checking for existence of a session rather than a user. Could you take a look at that #47 and/or the main branch, and let me know what you think? Please also merge/rebase from main, I would be curious if the tests all still pass in your branch.

Thanks!

vsalvino commented 2 years ago

Going back to the original issue, the Django Session middleware is the thing adding Vary: Cookie to ALL responses. In order to skirt that, we have added the new WAGTAIL_CACHE_IGNORE_COOKIES setting which essentially strips all non-Django cookies from the request, and removes the Vary: Cookie header if there is not a Django session or CSRF token.

thenewguy commented 2 years ago

I just saw this. I've been out of town. If you would merge my tests and the updated checks I'm happy to rework it.

vsalvino commented 4 months ago

Closing this as the Vary header handling has been previously re-worked. Feel free to re-open if you have any new information.