chore(next/image): improve imgopt api bypass detection for unsupporte…

…d images (#73909)

We currently bypass svg and animated images because vector doesn't need
to be rasterized and animated will turn to still.

However, there are other image types that we implicitly bypass once
sharp throws an error message and we catch it here:


https://github.com/vercel/next.js/blob/7d1ee4ff209d369d8cfce9ec2d00252cc1b8876f/packages/next/src/server/image-optimizer.ts#L798-L815

We already have existing tests for this behavior here:


https://github.com/vercel/next.js/blob/9435b7b9602136aeac77f5ff55bb553dcaa2b01a/test/integration/image-optimizer/test/util.ts#L201

This PR bypasses early so we can avoid sending the buffer, waiting for
it to throw the error, and then catching the error.

Also note this uncovered some discrepancies in how we handle the max age
cache-control so this unifies the behavior.

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

@@ -37,11 +37,12 @@ const JPEG = 'image/jpeg'
 const GIF = 'image/gif'
 const SVG = 'image/svg+xml'
 const ICO = 'image/x-icon'
+const ICNS = 'image/x-icns'
 const TIFF = 'image/tiff'
 const BMP = 'image/bmp'
 const CACHE_VERSION = 4
 const ANIMATABLE_TYPES = [WEBP, PNG, GIF]
-const VECTOR_TYPES = [SVG]
+const BYPASS_TYPES = [SVG, ICO, ICNS, BMP]
 const BLUR_IMG_SIZE = 8 // should match `next-image-loader`
 const BLUR_QUALITY = 70 // should match `next-image-loader`
 
@@ -186,6 +187,9 @@ export function detectContentType(buffer: Buffer) {
   if ([0x00, 0x00, 0x01, 0x00].every((b, i) => buffer[i] === b)) {
     return ICO
   }
+  if ([0x69, 0x63, 0x6e, 0x73].every((b, i) => buffer[i] === b)) {
+    return ICNS
+  }
   if ([0x49, 0x49, 0x2a, 0x00].every((b, i) => buffer[i] === b)) {
     return TIFF
   }
@@ -673,7 +677,10 @@ export async function imageOptimizer(
 }> {
   const { href, quality, width, mimeType } = paramsResult
   const { buffer: upstreamBuffer, etag: upstreamEtag } = imageUpstream
-  const maxAge = getMaxAge(imageUpstream.cacheControl)
+  const maxAge = Math.max(
+    nextConfig.images.minimumCacheTTL,
+    getMaxAge(imageUpstream.cacheControl)
+  )
 
   const upstreamType =
     detectContentType(upstreamBuffer) ||
@@ -708,10 +715,7 @@ export async function imageOptimizer(
         upstreamEtag,
       }
     }
-    if (VECTOR_TYPES.includes(upstreamType)) {
-      // We don't warn here because we already know that "dangerouslyAllowSVG"
-      // was enabled above, therefore the user explicitly opted in.
-      // If we add more VECTOR_TYPES besides SVG, perhaps we could warn for those.
+    if (BYPASS_TYPES.includes(upstreamType)) {
       return {
         buffer: upstreamBuffer,
         contentType: upstreamType,
@@ -790,7 +794,7 @@ export async function imageOptimizer(
     return {
       buffer: optimizedBuffer,
       contentType,
-      maxAge: Math.max(maxAge, nextConfig.images.minimumCacheTTL),
+      maxAge,
       etag: getImageEtag(optimizedBuffer),
       upstreamEtag,
     }

test/integration/image-optimizer/test/util.ts

@@ -241,7 +241,7 @@ export function runTests(ctx: RunTestsCtx) {
     expect(res.status).toBe(200)
     expect(res.headers.get('content-type')).toContain('image/gif')
     expect(res.headers.get('Cache-Control')).toBe(
-      `public, max-age=0, must-revalidate`
+      `public, max-age=${isDev ? 0 : minimumCacheTTL}, must-revalidate`
     )
     expect(res.headers.get('Vary')).toBe('Accept')
     expect(res.headers.get('etag')).toBeTruthy()
@@ -258,7 +258,7 @@ export function runTests(ctx: RunTestsCtx) {
     expect(res.status).toBe(200)
     expect(res.headers.get('content-type')).toContain('image/png')
     expect(res.headers.get('Cache-Control')).toBe(
-      `public, max-age=0, must-revalidate`
+      `public, max-age=${isDev ? 0 : minimumCacheTTL}, must-revalidate`
     )
     expect(res.headers.get('Vary')).toBe('Accept')
     expect(res.headers.get('etag')).toBeTruthy()
@@ -275,7 +275,7 @@ export function runTests(ctx: RunTestsCtx) {
     expect(res.status).toBe(200)
     expect(res.headers.get('content-type')).toContain('image/png')
     expect(res.headers.get('Cache-Control')).toBe(
-      `public, max-age=0, must-revalidate`
+      `public, max-age=${isDev ? 0 : minimumCacheTTL}, must-revalidate`
     )
     expect(res.headers.get('Vary')).toBe('Accept')
     expect(res.headers.get('etag')).toBeTruthy()
@@ -292,7 +292,7 @@ export function runTests(ctx: RunTestsCtx) {
     expect(res.status).toBe(200)
     expect(res.headers.get('content-type')).toContain('image/webp')
     expect(res.headers.get('Cache-Control')).toBe(
-      `public, max-age=0, must-revalidate`
+      `public, max-age=${isDev ? 0 : minimumCacheTTL}, must-revalidate`
     )
     expect(res.headers.get('Vary')).toBe('Accept')
     expect(res.headers.get('etag')).toBeTruthy()
@@ -312,7 +312,7 @@ export function runTests(ctx: RunTestsCtx) {
       expect(res.headers.get('Content-Length')).toBe('603')
       expect(res.headers.get('Content-Type')).toContain('image/svg+xml')
       expect(res.headers.get('Cache-Control')).toBe(
-        `public, max-age=0, must-revalidate`
+        `public, max-age=${isDev ? 0 : minimumCacheTTL}, must-revalidate`
       )
       // SVG is compressible so will have accept-encoding set from
       // compression

test/unit/image-optimizer/detect-content-type.test.ts

@@ -34,6 +34,10 @@ describe('detectContentType', () => {
     const buffer = await getImage('./images/test.ico')
     expect(detectContentType(buffer)).toBe('image/x-icon')
   })
+  it('should return icns', async () => {
+    const buffer = await getImage('./images/test.icns')
+    expect(detectContentType(buffer)).toBe('image/x-icns')
+  })
   it('should return tiff', async () => {
     const buffer = await getImage('./images/test.tiff')
     expect(detectContentType(buffer)).toBe('image/tiff')