chore(next/image): improve imgopt api bypass detection for unsupported 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.
This commit is contained in:
Steven
2024-12-17 08:43:58 -05:00
committed by GitHub
parent d3afc8b5e2
commit ce881de043
4 changed files with 20 additions and 12 deletions
+11 -7
View File
@@ -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,
}
@@ -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
@@ -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')
Binary file not shown.