From f365109144aa6eaafdd10ec02fe15d49e4f95e05 Mon Sep 17 00:00:00 2001 From: Steven Date: Wed, 2 Oct 2024 18:13:58 -0400 Subject: [PATCH] fix(next/image): handle undefined `images.localPatterns` config in `images-manifest.json` (#70730) Since `images.localPatterns` config can be undefined, we need to support that when emitting `images-manifest.json`. - When `undefined`, all local images are allowed - When `[]`, no local images are allowed Note this is different than `remotePatterns` since it treats `undefined` and `[]` the same (the default being no remote images are allowed). --- packages/next/src/build/index.ts | 16 ++++--- packages/next/src/server/config.ts | 21 ++++++--- .../app-dir-localpatterns/test/index.test.ts | 43 +++++++++++++++++++ .../next-image-new/app-dir/test/index.test.ts | 34 ++++++++++++++- .../unoptimized/test/index.test.ts | 42 +++++++++++++++--- test/lib/next-test-utils.ts | 4 ++ 6 files changed, 142 insertions(+), 18 deletions(-) diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index b68e4d6b09590..1f2ab770fea3f 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -501,6 +501,8 @@ async function writeImagesManifest( const images = { ...config.images } const { deviceSizes, imageSizes } = images ;(images as any).sizes = [...deviceSizes, ...imageSizes] + + // By default, remotePatterns will allow no remote images ([]) images.remotePatterns = (config?.images?.remotePatterns || []).map((p) => ({ // Modifying the manifest should also modify matchRemotePattern() protocol: p.protocol, @@ -509,11 +511,15 @@ async function writeImagesManifest( pathname: makeRe(p.pathname ?? '**', { dot: true }).source, search: p.search, })) - images.localPatterns = (config?.images?.localPatterns || []).map((p) => ({ - // Modifying the manifest should also modify matchLocalPattern() - pathname: makeRe(p.pathname ?? '**', { dot: true }).source, - search: p.search, - })) + + // By default, localPatterns will allow all local images (undefined) + if (config?.images?.localPatterns) { + images.localPatterns = config.images.localPatterns.map((p) => ({ + // Modifying the manifest should also modify matchLocalPattern() + pathname: makeRe(p.pathname ?? '**', { dot: true }).source, + search: p.search, + })) + } await writeManifest(path.join(distDir, IMAGES_MANIFEST), { version: 1, diff --git a/packages/next/src/server/config.ts b/packages/next/src/server/config.ts index 6a90d569cfdb2..b033db91b1a6e 100644 --- a/packages/next/src/server/config.ts +++ b/packages/next/src/server/config.ts @@ -333,11 +333,18 @@ function assignDefaults( `Specified images.localPatterns should be an Array received ${typeof images.localPatterns}.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` ) } - // static import images are automatically allowed - images.localPatterns.push({ - pathname: '/_next/static/media/**', - search: '', - }) + // avoid double-pushing the same pattern if it already exists + const hasMatch = images.localPatterns.some( + (pattern) => + pattern.pathname === '/_next/static/media/**' && pattern.search === '' + ) + if (!hasMatch) { + // static import images are automatically allowed + images.localPatterns.push({ + pathname: '/_next/static/media/**', + search: '', + }) + } } if (images.remotePatterns) { @@ -357,9 +364,9 @@ function assignDefaults( matchRemotePattern(pattern, url) ) - // avoid double-pushing the same remote if it already can be matched + // avoid double-pushing the same pattern if it already can be matched if (!hasMatchForAssetPrefix) { - images.remotePatterns?.push({ + images.remotePatterns.push({ hostname: url.hostname, protocol: url.protocol.replace(/:$/, '') as 'http' | 'https', port: url.port, diff --git a/test/integration/next-image-new/app-dir-localpatterns/test/index.test.ts b/test/integration/next-image-new/app-dir-localpatterns/test/index.test.ts index e95edf3ebd2d4..008f4c2b41030 100644 --- a/test/integration/next-image-new/app-dir-localpatterns/test/index.test.ts +++ b/test/integration/next-image-new/app-dir-localpatterns/test/index.test.ts @@ -5,6 +5,7 @@ import { assertNoRedbox, fetchViaHTTP, findPort, + getImagesManifest, getRedboxHeader, killApp, launchApp, @@ -64,6 +65,48 @@ function runTests(mode: 'dev' | 'server') { expect(res.status).toBe(400) } }) + + if (mode === 'server') { + it('should build correct images-manifest.json', async () => { + const manifest = getImagesManifest(appDir) + expect(manifest).toEqual({ + version: 1, + images: { + contentDispositionType: 'attachment', + contentSecurityPolicy: + "script-src 'none'; frame-src 'none'; sandbox;", + dangerouslyAllowSVG: false, + deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840], + disableStaticImages: false, + domains: [], + formats: ['image/webp'], + imageSizes: [16, 32, 48, 64, 96, 128, 256, 384], + loader: 'default', + loaderFile: '', + remotePatterns: [], + localPatterns: [ + { + pathname: + '^(?:\\/assets(?:\\/(?!\\.{1,2}(?:\\/|$))(?:(?:(?!(?:^|\\/)\\.{1,2}(?:\\/|$)).)*?)|$))$', + search: '', + }, + { + pathname: + '^(?:\\/_next\\/static\\/media(?:\\/(?!\\.{1,2}(?:\\/|$))(?:(?:(?!(?:^|\\/)\\.{1,2}(?:\\/|$)).)*?)|$))$', + search: '', + }, + ], + minimumCacheTTL: 60, + path: '/_next/image', + sizes: [ + 640, 750, 828, 1080, 1200, 1920, 2048, 3840, 16, 32, 48, 64, 96, + 128, 256, 384, + ], + unoptimized: false, + }, + }) + }) + } } describe('Image localPatterns config', () => { diff --git a/test/integration/next-image-new/app-dir/test/index.test.ts b/test/integration/next-image-new/app-dir/test/index.test.ts index 9917e4d24ddf6..25660ca80bc8e 100644 --- a/test/integration/next-image-new/app-dir/test/index.test.ts +++ b/test/integration/next-image-new/app-dir/test/index.test.ts @@ -8,6 +8,7 @@ import { check, fetchViaHTTP, findPort, + getImagesManifest, getRedboxHeader, killApp, launchApp, @@ -72,7 +73,7 @@ function getRatio(width, height) { return height / width } -function runTests(mode) { +function runTests(mode: 'dev' | 'server') { it('should load the images', async () => { let browser try { @@ -1588,6 +1589,37 @@ function runTests(mode) { 'callback refs that returned a cleanup should never be called with null' ) }) + + if (mode === 'server') { + it('should build correct images-manifest.json', async () => { + const manifest = getImagesManifest(appDir) + expect(manifest).toEqual({ + version: 1, + images: { + contentDispositionType: 'attachment', + contentSecurityPolicy: + "script-src 'none'; frame-src 'none'; sandbox;", + dangerouslyAllowSVG: false, + deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840], + disableStaticImages: false, + domains: [], + formats: ['image/webp'], + imageSizes: [16, 32, 48, 64, 96, 128, 256, 384], + loader: 'default', + loaderFile: '', + remotePatterns: [], + localPatterns: undefined, + minimumCacheTTL: 60, + path: '/_next/image', + sizes: [ + 640, 750, 828, 1080, 1200, 1920, 2048, 3840, 16, 32, 48, 64, 96, + 128, 256, 384, + ], + unoptimized: false, + }, + }) + }) + } } describe('Image Component Default Tests', () => { diff --git a/test/integration/next-image-new/unoptimized/test/index.test.ts b/test/integration/next-image-new/unoptimized/test/index.test.ts index 593bf0d5b1f34..3894860e7a39c 100644 --- a/test/integration/next-image-new/unoptimized/test/index.test.ts +++ b/test/integration/next-image-new/unoptimized/test/index.test.ts @@ -4,6 +4,7 @@ import { join } from 'path' import { check, findPort, + getImagesManifest, killApp, launchApp, nextBuild, @@ -15,7 +16,7 @@ const appDir = join(__dirname, '../') let appPort let app -function runTests(url: string) { +function runTests(url: string, mode: 'dev' | 'server') { it('should not optimize any image', async () => { const browser = await webdriver(appPort, url) expect( @@ -89,6 +90,37 @@ function runTests(url: string) { await browser.elementById('eager-image').getAttribute('srcset') ).toBeNull() }) + + if (mode === 'server') { + it('should build correct images-manifest.json', async () => { + const manifest = getImagesManifest(appDir) + expect(manifest).toEqual({ + version: 1, + images: { + contentDispositionType: 'attachment', + contentSecurityPolicy: + "script-src 'none'; frame-src 'none'; sandbox;", + dangerouslyAllowSVG: false, + deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840], + disableStaticImages: false, + domains: [], + formats: ['image/webp'], + imageSizes: [16, 32, 48, 64, 96, 128, 256, 384], + loader: 'default', + loaderFile: '', + remotePatterns: [], + localPatterns: undefined, + minimumCacheTTL: 60, + path: '/_next/image', + sizes: [ + 640, 750, 828, 1080, 1200, 1920, 2048, 3840, 16, 32, 48, 64, 96, + 128, 256, 384, + ], + unoptimized: true, + }, + }) + }) + } } describe('Unoptimized Image Tests', () => { @@ -101,7 +133,7 @@ describe('Unoptimized Image Tests', () => { await killApp(app) }) - runTests('/') + runTests('/', 'dev') }) ;(process.env.TURBOPACK_DEV ? describe.skip : describe)( 'production mode - component', @@ -115,7 +147,7 @@ describe('Unoptimized Image Tests', () => { await killApp(app) }) - runTests('/') + runTests('/', 'server') } ) describe('development mode - getImageProps', () => { @@ -127,7 +159,7 @@ describe('Unoptimized Image Tests', () => { await killApp(app) }) - runTests('/get-img-props') + runTests('/get-img-props', 'dev') }) ;(process.env.TURBOPACK_DEV ? describe.skip : describe)( 'production mode - getImageProps', @@ -141,7 +173,7 @@ describe('Unoptimized Image Tests', () => { await killApp(app) }) - runTests('/get-img-props') + runTests('/get-img-props', 'server') } ) }) diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 2525e6a0c69af..e69406a56e326 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -1003,6 +1003,10 @@ export function getBuildManifest(dir: string) { return readJson(path.join(dir, '.next/build-manifest.json')) } +export function getImagesManifest(dir: string) { + return readJson(path.join(dir, '.next/images-manifest.json')) +} + export function getPageFilesFromBuildManifest(dir: string, page: string) { const buildManifest = getBuildManifest(dir) const pageFiles = buildManifest.pages[page]