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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use no-store for web UI to prevent intermediate caching #4966

Merged
merged 1 commit into from Aug 14, 2021

Conversation

shaunbennett
Copy link
Contributor

@shaunbennett shaunbennett commented Aug 14, 2021

This PR replaces the default Cache-Control for the Web UI to be no-store instead of no-cache. See here.

Why?

This seems like a saner default to use - tell servers that we don't want the page cached anywhere. Using no-cache says that the page can still be cached by intermediates as long as the page is revalidated. We ran into this issue with the Web UI behind Fastly - since the Web UI doesn't support Etags/If-Modified-Since, it seems like cache validation is just being performed by looking at the content length. This means that we were getting cached responses (such as stat data) that were clearly stale but matched the content length of the previous request.

Similarly, it's not uncommon to put the Web UI behind some authentication system (such as sidekiq-ent provides). It looks like sidekiq-ent doesn't modify the cache headers at all, but when authentication is done we should have private (or just no-store) to prevent authenticated pages from being cached and returned to another user.

Alternatively we can use no-cache, private, although I'm not sure it would give much benefit over just not storing anywhere 馃

@mperham mperham merged commit 20f4bab into sidekiq:master Aug 14, 2021
@shaunbennett shaunbennett deleted the no-store-web branch August 14, 2021 16:15
jonhyman pushed a commit to braze-inc/sidekiq that referenced this pull request Sep 16, 2021
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.

None yet

2 participants