Enable custom Cache-Control header for remote images

[pedrosodre]

Feature

• Implements an existing feature request or RFC - Actually I've opened the feature request together with the pull request: Enable custom "Cache-Control" header for remote images on next/image #35595
• Related issues linked using fixes #number
• Integration tests added
• Documentation added
• Telemetry added. In case of a feature if it's used or not.
• Errors have helpful link attached, see contributing.md

Documentation / Examples

• Make sure the linting passes by running yarn lint

[styfle]

This is a breaking change.

Instead, you should configure the minimumCacheTTL option and let the image optimization api handle the caching behavior instead of the client.

See https://nextjs.org/docs/api-reference/next/image#caching-behavior

[pedrosodre]

@styfle I don't see this as a breaking change, since the minimumCacheTTL does not change the behavior for remote images and I kept the default value as it was directly on code. Static images doesn't have any impact on this.

Every project should keep working without any changes with this update. What do you think? Or do you mean use the minimumCacheTTL variable to change the behavior for remote images?

[pedrosodre]

I can also change this new variable to be a boolean that just apply the same behavior of static images on remote images, if make sense. The big problem in my view is this condition:

    isStatic
      ? 'public, max-age=315360000, immutable'
      : `public, max-age=0, must-revalidate`

[styfle]

You're right, its not a breaking change but it is dangerous.

Once you set the this header, there is no way to invalidate the cache because it lives in the browser cache instead of server cache. Its much safer to use minimumCacheTTL which controls the server's cache.

[pedrosodre]

I agree about how danger it can be, but without this we can't use any CDN in front of a NextJs project with remote images being optimized, since it always will try to revalidate and never will hit the cache. Next.js can hit it's own cache, but CDN is way faster since we have a phisical layer there.

Here is an example about what I mean:

If we have a project hosted on any server and a CDN (like CloudFront) is between user and server, based on the current code:

1. First time we access an image, Next will download it from remote, optimize it and do a local cache. CDN will return a miss.
2. All next time we access the same image, until Next decide to invalidate it, Next will get from cache and return. CDN will return a RefreshHit.
   We will never get a real low latency in this case from CDN.

If we have a project hosted on any server and a CDN (like CloudFront) is between user and server, based on the new code:

1. First time we access an image, Next will download it from remote, optimize it and do a local cache. CDN will return a miss.
2. All next time we access the same image, if CDN cache keeps valid, server will not be called. CDN will return a Hit.

I can change the pull request to use minimumCacheTTL to change max-age for remote images if that make sense, but it's important to be able to modify the Cache-Control header. I prefer a new option, even if it's dangerous, because we do the ability for developers decide about it. This option can save a lot of CPU and network on server that runs Next.js.

[styfle]

It should probably return s-maxage so that any proxy in front of Next.js can utilize it but browsers won’t accidentally cache.

Let me discuss with the team to see how we do this for other assets besides images.

[pedrosodre]

In any case, keep max-age=0 would not be that good for Page Speed Insights as well. While you talk to the team I will validate the CDN behavior with s-max-age.

I think if someone enable next/image for remote domains, they have control about the source as they have for static images. So I don't see that as a big deal in most of cases.

[pedrosodre]

Here is the test I did with an image at S3 and CloudFront in front of it:

1. If image does not return any Cache-Control, CloudFront is able to manage the cache by his side and return a Hit.
2. If image return Cache-Control as public, max-age=0, must-revalidate, CloudFront always returns RefreshHit.
3. If image return Cache-Control as public, max-age=0, s-maxage=315360000, must-revalidate, CloudFront is able to return a Hit.
4. If image return Cache-Control as public, max-age=315360000, must-revalidate, CloudFront is able to return a Hit.

Based on the tests, I see some options (that I'm able to change in my pull-request, if needed):

1. Change the variable to a new one (like minimumSharedCacheTTL) where developer is able to customize a s-maxage value for remote images and keep max-age as 0 (or use minimumCacheTTL for it, if make sense).
2. Keep the new remoteCacheControl variable to let developer decide what he/she wants on header and improve the documentation telling about the risks.
3. Create a new variable just to remove the Cache-Control header and pass along the value get from remote image or let CDN (or any reverse proxy) decide what to do.

I like the two first options, the only problem about first one is the fact that we lose the browser cache (I really think if an image came from a remote the API or whereelse returned the URL have control to change the name or a query string if needed). What do you think?

[styfle]

I think the easiest option here is to pass through the value used to cache on disk in the s-maxage header so that downstream proxies can cache the result as long as the Image Optimization API will cache, but it will make sure browsers don't cache. This might be a breaking change because you no longer control the cache so deleting .next/cache/images will still have it cached at the edge.

Another option is to investigate adding a CDN-Cache-Control which is meant for this purpose but its not clear if Cloudfront or others support it https://datatracker.ietf.org/doc/html/draft-cdn-control-header-00

[pedrosodre]

I think if we use s-maxage based on value used to cache on disk and add a new option to enable this value on Cache-Control header we avoid a breaking change and solve the problem for CDN, we keep only the browser cache problem, but it's something already. I don't see CDN-Cache-Control as a good option for now because it's too specific and not well adopted yet.

Should I change my code and check how it will be?

[belgattitude]

Wondering something... Could it be a function too ?

remoteCacheControl: string | (originalUrl: string, urlReturnedByTheLoader: string) => string

To allow customization based on the given url (or the transformed one).

i.e:

remoteCacheControl: (originalUrl: string, urlReturnedByTheLoader: string) => {
  if (isLocalImage(url)) {
    return 'public, max-age=0'
  }
  if(isS3Url(url)) {
    return 'public, max-age=315360000';
  } 
  // ...other cases
}

Just a concept, not a proper API... Might need more thoughts (are other headers useful ?...).

PS: Personally I would have preferred the loader function to return something like:

{ 
   url: 'transformed', 
   responseHeaders: Record<string, string> // So we can add any headers
}

But I guess it would be a bc.

[pedrosodre]

@belgattitude I've checked the code and I'm rewriting my anwser:

I think we can let the remoteCacheControl variable be a string or a function, but not related with the loader, since the problem this PR tries to solve only happens when we use next/image to optimize/transform remote images. If we use a loader to point image to an external CDN without optimize it using next, the problem does not happens (since next/image points directly to external URL).

In any case, if we understand that remoteCacheControl is the way to go ahead with this pull request, I think that make sense let user decide the Cache-Control header based on remote image URL, that way we would be able to set different caching controls for each origin, like that:

remoteCacheControl: (url: string) => {
  if (isLocalImage(url)) {
    return 'public, max-age=0'
  }
  if(isS3Url(url)) {
    return 'public, max-age=315360000';
  } 
  // ...other cases
}

What do you guys think?

[styfle]

I don't think a function is useful here because its not controlling Next.js built-in cache.

Furthermore, I don't think we should allow customizing the cache header for local images since those are hashed by content and already use the immutable header.

[pedrosodre]

@styfle Actually the example was not good, but we can have multiple remote locations, like a CDN from AWS, an image from API and any others, and the cache-control can be different for them, but the function works only for remote images.

I'll modify my code to allow another review.

[pbright]

I found my way to this pull request after noticing that all remote images are served with max-age: 0 from Next 12.1 onwards.

This is causing us issues because we have a use case which requires us to preload images in the browser (so they render immediately when required), which we can't do because the browser cache is effectively disabled, even though our source images are intentionally set to have long cache expiry times.

I appreciate this isn't a common use case, but I wouldn't have thought it was particularly unusual either.

I found this change very confusing because the documentation refers to using the upstream server's Cache-Control headers
https://nextjs.org/docs/api-reference/next/image#caching-behavior

"The expiration (or rather Max Age) is defined by either the minimumCacheTTL configuration or the upstream server's Cache-Control header, whichever is larger. Specifically, the max-age value of the Cache-Control header is used"

Does the documentation need updating?

Like @pedrosodre, for me the problem is caused by this line:
res.setHeader('Cache-Control', isStatic ? 'public, max-age=315360000, immutable' : 'public, max-age=0, must-revalidate');

which prior to Next 12.1 was:

res.setHeader('Cache-Control', isStatic ? 'public, max-age=315360000, immutable' : 'public, max-age=${isDev ? 0 : maxAge}, must-revalidate');

where the value of the maxAge variable was determined as described in the current documentation.

I'm not sure I understand the risk in the old behaviour - if the Cache-Control header is effectively inherited from the underlying image then Next isn't introducing any risk that doesn't already exist if the underlying image was served directly.

[iscekic]

Any update on this PR? If there's more work to be done, I'm willing to do it in order to get this merged.

[styfle]

I did some digging through to see when and why this changed and found the ISR/SWR implementation here: #34075

The reason why Next.js used to cache on the client is because revalidation was really slow. But now the server serves stale and revalidates in the background, so it should always* be fast.

*However, if you have a single instance of Next.js, this revalidation could still be slow if the client is not near the server.

If you're self hosting, we recommend running multiple instances of Next.js to cover multiple regions and share the ISR cache between those instances.

---

if the Cache-Control header is effectively inherited from the underlying image then Next isn't introducing any risk that doesn't already exist if the underlying image was served directly.

There is some risk because the image isn't served as-is, its optimized. Imagine when Next.js has a bug in the Image Optimization API and those images are cached by the client for years. There is no way to revalidate.

The other risk is caching for 2x longer than expected. Imagine an image with maxage of 1 hour and a client that visits the image 1 second before expiration. The client will cache it for an additional hour, even though the server revalidated 1 second later.

---

All that being said, I think the best way to move forward here is to add a stale-while-revaliate directive to remote images.

public, max-age=0, stale-while-revalidate=<upstream-max-age-header>

This will avoid any slowness between client and server since the revalidation happens in the background. But it also avoids the two issues I mentioned listed above.

[pbright]

Thanks for your response @styfle.

The reason why Next.js used to cache on the client is because revalidation was really slow. But now the server serves stale and revalidates in the background, so it should always* be fast.

Even if serving the image from Next.js is fast, the client still needs to redownload the image if the headers don't permit it to be cached on the client side. Is that not undesirable, especially on mobile devices? As @pedrosodre pointed out above, currently Lighthouse and Page Speed Insights complain about the cache headers, correctly in my view.

There is some risk because the image isn't served as-is, its optimized. Imagine when Next.js has a bug in the Image Optimization API and those images are cached by the client for years. There is no way to revalidate.

While there is no way to revalidate, the image url is generated by Next.js so if there was need for a fix on the Next.js end couldn't this be avoided by simply adjusting the url slightly, for example introducing a v=N parameter?

The other risk is caching for 2x longer than expected. Imagine an image with maxage of 1 hour and a client that visits the image 1 second before expiration. The client will cache it for an additional hour, even though the server revalidated 1 second later.

Isn't that the exact same situation as if I served the underlying image with a maxage of 1 hour? Either way if the image is changed immediately after client visits it then the client will have the old version until the cache expiry. I wouldn't view this as a problem, rather working as intended.

All that being said, I think the best way to move forward here is to add a stale-while-revaliate directive to remote images.

I would need to test to be sure, but I suspect that changing to stale-while-revalidate would cause preloading images to work correctly, which was my use case, in those browsers that currently support it. Obviously that's an improvement and I would be grateful for that change, but notably current browser support doesn't include Safari (desktop or iOS).

I would also be surprised if that change resolved the issue with CDN behaviour. From what I can tell Cloudfront doesn't currently support stale-while-revalidate, for example.

[iscekic]

For context, my setup is a CDN/load balancer in front of multiple next.js instances. No shared ISR cache, as the CDN serves that purpose by respecting headers sent from next.

The reason why Next.js used to cache on the client is because revalidation was really slow. But now the server serves stale and revalidates in the background, so it should always* be fast.

*However, if you have a single instance of Next.js, this revalidation could still be slow if the client is not near the server.

This holds true in the case of a shared ISR cache, but in my case, the load balancer can hit an instance which hasn't yet generated an image, causing needless work to be done many times.

If you're self hosting, we recommend running multiple instances of Next.js to cover multiple regions and share the ISR cache between those instances.

This would work, but I would need to introduce complexity into my deployment - something I don't want/need right now.

There is some risk because the image isn't served as-is, its optimized. Imagine when Next.js has a bug in the Image Optimization API and those images are cached by the client for years. There is no way to revalidate.

I can always change the filename (or add a query param), busting the cache. The chances of this happening are low, and it's a risk I'm willing to live with. 😄 It's also appears to affect remote and images present at build time equally.

The other risk is caching for 2x longer than expected. Imagine an image with maxage of 1 hour and a client that visits the image 1 second before expiration. The client will cache it for an additional hour, even though the server revalidated 1 second later.

This is just how caching works, I see no issue here.

If it were up to me, I'd add a new property to the images config, allowing me to override default behavior where necessary (something similar was already suggested above):

// image-optimizer.ts
isStatic ? 'public, max-age=315360000, immutable' : (remoteCacheControl?.(url) || 'public, max-age=0, must-revalidate')

// next.config.js
images: {
  remoteCacheControl: (originalUrl: string) => string | undefined;
}

After stale-while-revalidate gets better support, the default can be switched to it, while keeping the remoteCacheControl escape hatch.

[styfle]

Thanks for the feedback! I created PR #37625 and I'll see what the Next.js team thinks

[pedrosodre]

I've following all these comments on my PR and I'm just waiting a follow-up from next's team to decide what I should change on my code.

I still think it's a good approach to have remoteCacheControl variable on next.config.js.