Refactor image optimizer static immutable header (vercel#25909)

Previously we were accepting a `s=1` query string parameter for static imports, but this is not necessary.

Instead, this PR looks at the file path to determine if the header should be `immutable`.

The nice thing here is we don't need to worry about someone trying `s=1` with an external image or 3rd party loader. In that case, we use the upstream `Cache-Control` header as usual.

This change also ensures we don't add the `immutable` header for `next dev`.

Related to PR vercel#24993

packages/next/client/image.tsx

@@ -22,7 +22,6 @@ export type ImageLoaderProps = {
   src: string
   width: number
   quality?: number
-  isStatic?: boolean
 }
 
 type DefaultImageLoaderProps = ImageLoaderProps & { root: string }
@@ -190,7 +189,6 @@ type GenImgAttrsData = {
   unoptimized: boolean
   layout: LayoutValue
   loader: ImageLoader
-  isStatic?: boolean
   width?: number
   quality?: number
   sizes?: string
@@ -210,7 +208,6 @@ function generateImgAttrs({
   quality,
   sizes,
   loader,
-  isStatic,
 }: GenImgAttrsData): GenImgAttrsResult {
   if (unoptimized) {
     return { src, srcSet: undefined, sizes: undefined }
@@ -224,7 +221,7 @@ function generateImgAttrs({
     srcSet: widths
       .map(
         (w, i) =>
-          `${loader({ src, quality, isStatic, width: w })} ${
+          `${loader({ src, quality, width: w })} ${
             kind === 'w' ? w : i + 1
           }${kind}`
       )
@@ -236,7 +233,7 @@ function generateImgAttrs({
     // updated by React. That causes multiple unnecessary requests if `srcSet`
     // and `sizes` are defined.
     // This bug cannot be reproduced in Chrome or Firefox.
-    src: loader({ src, quality, isStatic, width: widths[last] }),
+    src: loader({ src, quality, width: widths[last] }),
   }
 }
 
@@ -311,7 +308,6 @@ export default function Image({
     delete rest['layout']
   }
 
-  const isStatic = typeof src === 'object'
   let staticSrc = ''
   if (isStaticImport(src)) {
     const staticImageData = isStaticRequire(src) ? src.default : src
@@ -339,7 +335,7 @@ export default function Image({
       }
     }
   }
-  src = (isStatic ? staticSrc : src) as string
+  src = typeof src === 'string' ? src : staticSrc
 
   if (process.env.NODE_ENV !== 'production') {
     if (!src) {
@@ -516,7 +512,6 @@ export default function Image({
       quality: qualityInt,
       sizes,
       loader,
-      isStatic,
     })
   }
 
@@ -644,7 +639,6 @@ function cloudinaryLoader({
 
 function defaultLoader({
   root,
-  isStatic,
   src,
   width,
   quality,
@@ -692,7 +686,5 @@ function defaultLoader({
     }
   }
 
-  return `${root}?url=${encodeURIComponent(src)}&w=${width}&q=${quality || 75}${
-    isStatic ? '&s=1' : ''
-  }`
+  return `${root}?url=${encodeURIComponent(src)}&w=${width}&q=${quality || 75}`
 }

packages/next/next-server/server/image-optimizer.ts

@@ -46,7 +46,7 @@ export async function imageOptimizer(
   }
 
   const { headers } = req
-  const { url, w, q, s } = parsedUrl.query
+  const { url, w, q } = parsedUrl.query
   const mimeType = getSupportedMimeType(MODERN_TYPES, headers.accept)
   let href: string
 
@@ -111,13 +111,8 @@ export async function imageOptimizer(
     return { finished: true }
   }
 
-  if (s && s !== '1') {
-    res.statusCode = 400
-    res.end('"s" parameter must be "1" or omitted')
-    return { finished: true }
-  }
-
-  const isStatic = !!s
+  // Should match output from next-image-loader
+  const isStatic = url.startsWith('/_next/static/image')
 
   const width = parseInt(w, 10)
 
@@ -381,7 +376,7 @@ function sendResponse(
   res.setHeader(
     'Cache-Control',
     isStatic
-      ? 'public, immutable, max-age=315360000'
+      ? 'public, max-age=315360000, immutable'
       : 'public, max-age=0, must-revalidate'
   )
   if (sendEtagResponse(req, res, etag)) {

test/integration/image-component/default/test/static.test.js

@@ -37,16 +37,6 @@ const runTests = () => {
     expect(html).toContain('width:200px;height:200px')
     expect(html).not.toContain('width:400px;height:400px')
   })
-  it('Should append "&s=1" to URLs of static images', async () => {
-    expect(
-      await browser.elementById('basic-static').getAttribute('src')
-    ).toContain('&s=1')
-  })
-  it('Should not append "&s=1" to URLs of non-static images', async () => {
-    expect(
-      await browser.elementById('basic-non-static').getAttribute('src')
-    ).not.toContain('&s=1')
-  })
   it('Should add a blurry placeholder to statically imported jpg', async () => {
     expect(html).toContain(
       `style="position:absolute;top:0;left:0;bottom:0;right:0;box-sizing:border-box;padding:0;border:none;margin:auto;display:block;width:0;height:0;min-width:100%;max-width:100%;min-height:100%;max-height:100%;background-size:cover;background-image:url("data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wCEAAoKCgoKCgsMDAsPEA4QDxYUExMUFiIYGhgaGCIzICUgICUgMy03LCksNy1RQDg4QFFeT0pPXnFlZXGPiI+7u/sBCgoKCgoKCwwMCw8QDhAPFhQTExQWIhgaGBoYIjMgJSAgJSAzLTcsKSw3LVFAODhAUV5PSk9ecWVlcY+Ij7u7+//CABEIAAgACAMBIgACEQEDEQH/xAAUAAEAAAAAAAAAAAAAAAAAAAAH/9oACAEBAAAAADX/xAAUAQEAAAAAAAAAAAAAAAAAAAAA/9oACAECEAAAAH//xAAUAQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDEAAAAH//xAAdEAABAgcAAAAAAAAAAAAAAAATEhUAAwUUIzLS/9oACAEBAAE/AB0ZlUac43GqMYuo/8QAFBEBAAAAAAAAAAAAAAAAAAAAAP/aAAgBAgEBPwB//8QAFBEBAAAAAAAAAAAAAAAAAAAAAP/aAAgBAwEBPwB//9k=")"`

test/integration/image-optimizer/pages/index.js

@@ -1,5 +1,13 @@
+import Image from 'next/image'
+import Logo from '../public/test.jpg'
+
 function Home() {
-  return <h1>Image Optimizer Home</h1>
+  return (
+    <>
+      <h1>Image Optimizer Home</h1>
+      <Image src={Logo} />
+    </>
+  )
 }
 
 export default Home

test/integration/image-optimizer/test/index.test.js

@@ -217,13 +217,6 @@ function runTests({ w, isDev, domains }) {
     )
   })
 
-  it('should fail when s is present and not "1"', async () => {
-    const query = { url: '/test.png', w, q: 100, s: 'foo' }
-    const res = await fetchViaHTTP(appPort, '/_next/image', query, {})
-    expect(res.status).toBe(400)
-    expect(await res.text()).toBe(`"s" parameter must be "1" or omitted`)
-  })
-
   it('should fail when domain is not defined in next.config.js', async () => {
     const url = `http://vercel.com/button`
     const query = { url, w, q: 100 }
@@ -511,13 +504,20 @@ function runTests({ w, isDev, domains }) {
   })
 
   it('should set cache-control to immutable for static images', async () => {
-    const query = { url: '/test.jpg', w, q: 100, s: '1' }
-    const opts = { headers: { accept: 'image/webp' } }
-    const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
-    expect(res.status).toBe(200)
-    expect(res.headers.get('cache-control')).toBe(
-      'public, immutable, max-age=315360000'
-    )
+    if (!isDev) {
+      const query = {
+        url:
+          '/_next/static/image/public/test.480a01e5ea850d0231aec0fa94bd23a0.jpg',
+        w,
+        q: 100,
+      }
+      const opts = { headers: { accept: 'image/webp' } }
+      const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
+      expect(res.status).toBe(200)
+      expect(res.headers.get('cache-control')).toBe(
+        'public, max-age=315360000, immutable'
+      )
+    }
   })
 
   it("should error if the resource isn't a valid image", async () => {