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

Change default cache-control response to 'no-store' #105

Merged
merged 1 commit into from
Jul 21, 2024
Merged

Change default cache-control response to 'no-store' #105

merged 1 commit into from
Jul 21, 2024

Conversation

ahupp
Copy link
Contributor

@ahupp ahupp commented Jul 21, 2024

The default cache-control header returned by sse-starlete is
"no-cache", which (confusingly!) does not actually mean "don't cache",
but instead that caching is allowed but the result must be
re-validated on each fetch. From MDN:

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.

This is probably harmless as long as there's not an ETag or
Last-Modified header that would allow revalidation, but it does make
it much easier to unexpectedly enable caching or globally audit cache
behavior.

This PR:

  • Changes the default to "no-store", which does disallow caching

  • Uses starlette's MutableHeaders to build the headers; this
    normalizes to all lower-case so we're independent of case if an
    explicit cache-control is passed in.

  • Adds a test for header handling.

  • Adds ruff to the dev dependencies and updates make lint to
    resolve a deprecation warning in the command.

The default cache-control header returned by sse-starlete is
"no-cache", which (confusingly!) does not actually mean "don't cache",
but instead that caching is allowed but the result must be
re-validated on each fetch.  [From MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control):

> 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.

This is *probably* harmless as long as there's not an ETag or
Last-Modified header that would allow revalidation, but it does make
it much easier to unexpectedly enable caching or globally audit cache
behavior.

This PR:

 * Changes the default to "no-store", which does disallow caching

 * Uses starlette's `MutableHeaders` to build the headers; this
   normalizes to all lower-case so we're independent of case if an
   explicit cache-control is passed in.

 * Adds a test for header handling.

 * Adds ruff to the dev dependencies and updates `make lint` to
   resolve a deprecation warning in the command.
@sysid sysid merged commit db66f62 into sysid:main Jul 21, 2024
5 checks passed
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.

2 participants