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

Is "cache-control: no-cache" the right default? #104

Closed
ahupp opened this issue Jul 19, 2024 · 3 comments
Closed

Is "cache-control: no-cache" the right default? #104

ahupp opened this issue Jul 19, 2024 · 3 comments

Comments

@ahupp
Copy link
Contributor

ahupp commented Jul 19, 2024

sse-starlette sets cache-control: no-cache as a default response header.

I was surprised to learn that this doesn't mean "do not cache", but rather that it should be cached, but revalidated before serving a response:

The no-cache response directive indicates that the response can be stored in caches, but the response must be validated with the origin server before each reuse, even when the cache is disconnected from the origin server.

I believe without including an ETag or Last-modified header in the response this won't actually allow for caching, but this does seem like a surprising default. It's easy to imagine someone inadvertently adding a last-modified or etag and enabling caching of supposed-to-be-private data.

The EventSource spec requires "cache-control: no-store" (note: not no-cache) which also indicates the expectation is to bypass caches entirely, but if a client isn't using EventSource that won't take effect.

My questions/issues: Is this header default intentional, or was it intended to be no-store?

@sysid
Copy link
Owner

sysid commented Jul 21, 2024

@ahupp thank you for bringing this to attention.

You are absolutely right, it should be "no-store". Do you want to create are PR? Otherwise I will change it with next release.

@ahupp
Copy link
Contributor Author

ahupp commented Jul 21, 2024

PR in #105

@sysid
Copy link
Owner

sysid commented Jul 22, 2024

PR merged

@sysid sysid closed this as completed Jul 22, 2024
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

No branches or pull requests

2 participants