Fix minimumCacheTTL so it doesn't affect browser caching (vercel#27307

)

In a previous PR (vercel#27200), we added `minimumCacheTTL` to configure the time-to-live for the cached image. However, this was setting the `max-age` header.

This PR ensures that `minimumCacheTTL` doesn't affect browser caching, only the upstream header can affect browser caching.

This is a bit safer in case the developer accidentally caches something that shouldn't be and the cache needs to be invalidated. Simply delete the `.next/cache/images` directory.

- Related to vercel#19914
- Related to vercel#22319

docs/basic-features/image-optimization.md

@@ -176,7 +176,7 @@ module.exports = {
 
 ### Minimum Cache TTL
 
-You can configure the time to live (TTL) in seconds for cached optimized images. This will configure the server's image cache as well as the `Cache-Control` header sent to the browser. This is a global setting that will affect all images. In most cases, its better to use a [Static Image Import](#image-Imports) which will handle hashing file contents and caching the file. You can also configure the `Cache-Control` header on an individual upstream image instead.
+You can configure the time to live (TTL) in seconds for cached optimized images. In many cases, its better to use a [Static Image Import](#image-Imports) which will handle hashing file contents and caching the file forever.
 
 ```js
 module.exports = {
@@ -186,6 +186,8 @@ module.exports = {
 }
 ```
 
+If you need to add a `Cache-Control` header for the browser (not recommended), you can configure [`headers`](/docs/api-reference/next.config.js/headers) on the upstream image e.g. `/some-asset.jpg` not `/_next/image` itself.
+
 ### Disable Static Imports
 
 The default behavior allows you to import static files such as `import icon from './icon.png` and then pass that to the `src` property.

packages/next/server/image-optimizer.ts

@@ -212,10 +212,7 @@ export async function imageOptimizer(
       upstreamType =
         detectContentType(upstreamBuffer) ||
         upstreamRes.headers.get('Content-Type')
-      maxAge = getMaxAge(
-        upstreamRes.headers.get('Cache-Control'),
-        minimumCacheTTL
-      )
+      maxAge = getMaxAge(upstreamRes.headers.get('Cache-Control'))
     } else {
       try {
         const resBuffers: Buffer[] = []
@@ -272,15 +269,15 @@ export async function imageOptimizer(
         upstreamBuffer = Buffer.concat(resBuffers)
         upstreamType =
           detectContentType(upstreamBuffer) || mockRes.getHeader('Content-Type')
-        maxAge = getMaxAge(mockRes.getHeader('Cache-Control'), minimumCacheTTL)
+        maxAge = getMaxAge(mockRes.getHeader('Cache-Control'))
       } catch (err) {
         res.statusCode = 500
         res.end('"url" parameter is valid but upstream response is invalid')
         return { finished: true }
       }
     }
 
-    const expireAt = maxAge * 1000 + now
+    const expireAt = Math.max(maxAge, minimumCacheTTL) * 1000 + now
 
     if (upstreamType) {
       const vector = VECTOR_TYPES.includes(upstreamType)
@@ -540,7 +537,7 @@ export function detectContentType(buffer: Buffer) {
   return null
 }
 
-export function getMaxAge(str: string | null, minimumCacheTTL: number): number {
+export function getMaxAge(str: string | null): number {
   const map = parseCacheControl(str)
   if (map) {
     let age = map.get('s-maxage') || map.get('max-age') || ''
@@ -549,8 +546,8 @@ export function getMaxAge(str: string | null, minimumCacheTTL: number): number {
     }
     const n = parseInt(age, 10)
     if (!isNaN(n)) {
-      return Math.max(n, minimumCacheTTL)
+      return n
     }
   }
-  return minimumCacheTTL
+  return 0
 }

test/integration/image-optimizer/test/get-max-age.test.js

@@ -2,43 +2,40 @@
 import { getMaxAge } from '../../../../packages/next/dist/server/image-optimizer.js'
 
 describe('getMaxAge', () => {
-  it('should return default when no cache-control provided', () => {
-    expect(getMaxAge(undefined, 60)).toBe(60)
+  it('should return 0 when no cache-control provided', () => {
+    expect(getMaxAge(undefined)).toBe(0)
   })
-  it('should return default when cache-control is null', () => {
-    expect(getMaxAge(null, 60)).toBe(60)
+  it('should return 0 when cache-control is null', () => {
+    expect(getMaxAge(null)).toBe(0)
   })
-  it('should return default when cache-control is empty string', () => {
-    expect(getMaxAge('', 60)).toBe(60)
+  it('should return 0 when cache-control is empty string', () => {
+    expect(getMaxAge('')).toBe(0)
   })
-  it('should return default when cache-control max-age is less than default', () => {
-    expect(getMaxAge('max-age=30', 60)).toBe(60)
+  it('should return 0 when cache-control max-age is not a number', () => {
+    expect(getMaxAge('max-age=foo')).toBe(0)
   })
-  it('should return default when cache-control max-age is not a number', () => {
-    expect(getMaxAge('max-age=foo', 60)).toBe(60)
-  })
-  it('should return default when cache-control is no-cache', () => {
-    expect(getMaxAge('no-cache', 60)).toBe(60)
+  it('should return 0 when cache-control is no-cache', () => {
+    expect(getMaxAge('no-cache')).toBe(0)
   })
   it('should return cache-control max-age lowercase', () => {
-    expect(getMaxAge('max-age=9999', 60)).toBe(9999)
+    expect(getMaxAge('max-age=9999')).toBe(9999)
   })
   it('should return cache-control MAX-AGE uppercase', () => {
-    expect(getMaxAge('MAX-AGE=9999', 60)).toBe(9999)
+    expect(getMaxAge('MAX-AGE=9999')).toBe(9999)
   })
   it('should return cache-control s-maxage lowercase', () => {
-    expect(getMaxAge('s-maxage=9999', 60)).toBe(9999)
+    expect(getMaxAge('s-maxage=9999')).toBe(9999)
   })
   it('should return cache-control S-MAXAGE', () => {
-    expect(getMaxAge('S-MAXAGE=9999', 60)).toBe(9999)
+    expect(getMaxAge('S-MAXAGE=9999')).toBe(9999)
   })
   it('should return cache-control s-maxage with spaces', () => {
-    expect(getMaxAge('public, max-age=5555, s-maxage=9999', 60)).toBe(9999)
+    expect(getMaxAge('public, max-age=5555, s-maxage=9999')).toBe(9999)
   })
   it('should return cache-control s-maxage without spaces', () => {
-    expect(getMaxAge('public,s-maxage=9999,max-age=5555', 60)).toBe(9999)
+    expect(getMaxAge('public,s-maxage=9999,max-age=5555')).toBe(9999)
   })
   it('should return cache-control for a quoted value', () => {
-    expect(getMaxAge('public, s-maxage="9999", max-age="5555"', 60)).toBe(9999)
+    expect(getMaxAge('public, s-maxage="9999", max-age="5555"')).toBe(9999)
   })
 })