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

Cloudflare ignores Cache-Control when Surrogate-Control is present #282

Open
timkelty opened this issue Feb 11, 2025 · 7 comments · May be fixed by #284
Open

Cloudflare ignores Cache-Control when Surrogate-Control is present #282

timkelty opened this issue Feb 11, 2025 · 7 comments · May be fixed by #284

Comments

@timkelty
Copy link

Cloudflare will ignore any Cache-Control directives if any Surrogate-Control header is present: https://developers.cloudflare.com/cache/concepts/cdn-cache-control/#header-precedence

Additionally, surrogates will not honor Cache-Control headers in the response from an origin. For example, if the Surrogate-Control header is present within the response, Cloudflare ignores any Cache-Control directives, even if the Surrogate-Control header does not contain directives.

This becomes a problem if you're relying on origin cache control via the Cache-Control header.

I've been able to work around this so far by using the CDN-Cache-Control to control Cloudflare caching instead.

That said, the library seems to assume Cache-Control is being used in a few places.

To make working around this a bit easier, what do you think about:

  • making the Surrogate-Control header configurable, so you could have your origin send something like X-Surrogate-Control, to get around Cloudflare's behavior of ignoring Cache-Control
  • making the Cache-Control header configurable, so you could override it to use CDN-Cache-Control or Cloudflare-CDN-Cache-Control if you were using those
@timkelty timkelty changed the title Cloudflare Cache-Control + Surrogate-Control behavior Cloudflare ignores Cache-Control when Surrogate-Control is present Feb 11, 2025
@cdloh
Copy link
Owner

cdloh commented Feb 11, 2025

👍 I've been getting around this with something like rsp.headers.append(SURROGATE_CONTROL_HEADER, `max-age=${cacheTTL}`); in the custom fetcher.

Where cacheTTL is the calculated TTL based off Cache-Control and some other headers.

100% agreed re making the surrogate control header configurable.

I'm not sure about making Cache-Control configurable. I think the behaviour would need to be heavily documented and I'm not clear on what the behaviour should be.

Are you saying that if a custom Cache-Control header is set then we should just treat it as the normal header and the behaviour remains the same? What if they have a custom header set but then also send Cache-Control. Do we ignore it?

@timkelty
Copy link
Author

👍 I've been getting around this with something like rsp.headers.append(SURROGATE_CONTROL_HEADER, max-age=${cacheTTL}); in the custom fetcher.

Interesting. I suppose I could do something similar, but I think I'd still have to resort to cdn-cache-control for other directives (like stale-while-revalidate).

@cdloh
Copy link
Owner

cdloh commented Feb 12, 2025

Interesting I didn't realise that cdn-cache-control supported stale directives.

If I made cache-control configurable would you expect the library to just treat the configured header exactly the same as Cache-Control ?

I'm not sure that would work as peoples browsers would still likely cache things which causes issues.

Since CDN-Cache-Control is only used by CDNs anyway that would likely be consumed by the CDN PRIOR to the worker even modifying anything anyway.

So the only thing that matters is down stream (ie peoples browsers).

@timkelty
Copy link
Author

Yeah, cdn-cache-control has the same directives as cache-control. And Cloudflare (kind of*) supports stale-while-revalidate: https://developers.cloudflare.com/cache/concepts/revalidation/#example-1

So the only thing that matters is down stream (ie peoples browsers).

That sounds right – if the reason to set private, max-age=0 here is so it gets back to the browser, then it shouldn't be configurable.

  • Cloudflare's implementation re-validates synchronously for the first request, so there will still be one user that gets an un-cached response. If you want a fully async revalidation, you have to implement it yourself in your worker.

@timkelty
Copy link
Author

Interestingly, it seems this behavior comes from the Surrogate-Control spec, not just from Cloudflare: https://www.w3.org/TR/edge-arch/

Surrogates should ignore any HTTP Cache-Control request header directives.

…which I suppose is probably why CDNs need CDN-Cache-Control to be a thing.

@cdloh
Copy link
Owner

cdloh commented Feb 12, 2025

@timkelty I'll get the Surrogate-Control header changes released later on this week when I merge it in.

I'm not convinced there is a need to move the Cache-Control header off to a different header as by the time it hits the worker it's POST CDN Consumption and the only thing down stream is the end users browser. That browser is only going to be interested in Cache-Control regardless.

If you still think there is a valid use case for it please let me know.

@timkelty
Copy link
Author

Thanks so much!

Yep – agreed about Cache-Control.

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 a pull request may close this issue.

2 participants