From 8937abfe154dfc178dcd9e8636945574b2c5d641 Mon Sep 17 00:00:00 2001 From: Steven Date: Fri, 8 Apr 2022 18:19:25 -0400 Subject: [PATCH] Fix leaking internal config to user-defined `loader` prop in `next/image` (#36013) The `config` was changed from a global variable to a function parameter of the `loader()` function in PR https://github.com/vercel/next.js/pull/33559 as an implementation detail, not as a public API. It was not meant to be used by the end user which is why it was [undocumented](https://nextjs.org/docs/api-reference/next/image#loader). This config is meant for the default Image Optimization API. Since the `loader` prop bypasses the default Image Optimization API in favor of a custom function, that config is no longer be relevant because the function can implement the optimization url however it desires. - Fixes #35115 --- packages/next/client/image.tsx | 56 ++++++++++++++----- .../basic/pages/loader-prop.js | 23 ++++++++ .../image-component/basic/test/index.test.js | 11 ++++ 3 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 test/integration/image-component/basic/pages/loader-prop.js diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx index be335edc6321324..5eea24447eab361 100644 --- a/packages/next/client/image.tsx +++ b/packages/next/client/image.tsx @@ -40,13 +40,25 @@ type ImageConfig = ImageConfigComplete & { allSizes: number[] } export type ImageLoader = (resolverProps: ImageLoaderProps) => string export type ImageLoaderProps = { - config: Readonly src: string width: number quality?: number } -const loaders = new Map string>([ +// Do not export - this is an internal type only +// because `next.config.js` is only meant for the +// built-in loaders, not for a custom loader() prop. +type ImageLoaderWithConfig = ( + resolverProps: ImageLoaderPropsWithConfig +) => string +type ImageLoaderPropsWithConfig = ImageLoaderProps & { + config: Readonly +} + +const loaders = new Map< + LoaderValue, + (props: ImageLoaderPropsWithConfig) => string +>([ ['default', defaultLoader], ['imgix', imgixLoader], ['cloudinary', cloudinaryLoader], @@ -132,7 +144,7 @@ export type ImageProps = Omit< onLoadingComplete?: OnLoadingComplete } -type ImageElementProps = Omit & { +type ImageElementProps = Omit & { srcString: string imgAttributes: GenImgAttrsResult heightInt: number | undefined @@ -145,7 +157,7 @@ type ImageElementProps = Omit & { loading: LoadingValue config: ImageConfig unoptimized: boolean - loader: ImageLoader + loader: ImageLoaderWithConfig placeholder: PlaceholderValue onLoadingCompleteRef: React.MutableRefObject setBlurComplete: (b: boolean) => void @@ -209,7 +221,7 @@ type GenImgAttrsData = { src: string unoptimized: boolean layout: LayoutValue - loader: ImageLoader + loader: ImageLoaderWithConfig width?: number quality?: number sizes?: string @@ -269,7 +281,7 @@ function getInt(x: unknown): number | undefined { return undefined } -function defaultImageLoader(loaderProps: ImageLoaderProps) { +function defaultImageLoader(loaderProps: ImageLoaderPropsWithConfig) { const loaderKey = loaderProps.config?.loader || 'default' const load = loaders.get(loaderKey) if (load) { @@ -357,7 +369,6 @@ export default function Image({ objectPosition, onLoadingComplete, onError, - loader = defaultImageLoader, placeholder = 'empty', blurDataURL, ...all @@ -376,8 +387,23 @@ export default function Image({ // Override default layout if the user specified one: if (rest.layout) layout = rest.layout - // Remove property so it's not spread into image: - delete rest['layout'] + // Remove property so it's not spread on : + delete rest.layout + } + + let loader: ImageLoaderWithConfig = defaultImageLoader + if ('loader' in rest) { + if (rest.loader) { + const customImageLoader = rest.loader + loader = (obj) => { + const { config: _, ...opts } = obj + // The config object is internal only so we must + // not pass it to the user-defined loader() + return customImageLoader(opts) + } + } + // Remove property so it's not spread on + delete rest.loader } let staticSrc = '' @@ -957,7 +983,7 @@ function imgixLoader({ src, width, quality, -}: ImageLoaderProps): string { +}: ImageLoaderPropsWithConfig): string { // Demo: https://static.imgix.net/daisy.png?auto=format&fit=max&w=300 const url = new URL(`${config.path}${normalizeSrc(src)}`) const params = url.searchParams @@ -973,7 +999,11 @@ function imgixLoader({ return url.href } -function akamaiLoader({ config, src, width }: ImageLoaderProps): string { +function akamaiLoader({ + config, + src, + width, +}: ImageLoaderPropsWithConfig): string { return `${config.path}${normalizeSrc(src)}?imwidth=${width}` } @@ -982,7 +1012,7 @@ function cloudinaryLoader({ src, width, quality, -}: ImageLoaderProps): string { +}: ImageLoaderPropsWithConfig): string { // Demo: https://res.cloudinary.com/demo/image/upload/w_300,c_limit,q_auto/turtles.jpg const params = ['f_auto', 'c_limit', 'w_' + width, 'q_' + (quality || 'auto')] const paramsString = params.join(',') + '/' @@ -1001,7 +1031,7 @@ function defaultLoader({ src, width, quality, -}: ImageLoaderProps): string { +}: ImageLoaderPropsWithConfig): string { if (process.env.NODE_ENV !== 'production') { const missingValues = [] diff --git a/test/integration/image-component/basic/pages/loader-prop.js b/test/integration/image-component/basic/pages/loader-prop.js new file mode 100644 index 000000000000000..17747c211ca03a0 --- /dev/null +++ b/test/integration/image-component/basic/pages/loader-prop.js @@ -0,0 +1,23 @@ +import Image from 'next/image' + +const LoaderExample = () => { + return ( +
+

Custom loader in both next.config.js and loader prop

+ { + if (config) { + return 'https://example.vercel.sh/error-unexpected-config' + } + return `https://example.vercel.sh/success/${src}?width=${width}` + }} + /> +
+ ) +} + +export default LoaderExample diff --git a/test/integration/image-component/basic/test/index.test.js b/test/integration/image-component/basic/test/index.test.js index 413b4932af33b7a..3a44542c2a89c4c 100644 --- a/test/integration/image-component/basic/test/index.test.js +++ b/test/integration/image-component/basic/test/index.test.js @@ -317,6 +317,17 @@ describe('Image Component Tests', () => { await browser.elementById('basic-image').getAttribute('data-nimg') ).toBe('intrinsic') }) + it('should not pass config to custom loader prop', async () => { + browser = await webdriver(appPort, '/loader-prop') + expect( + await browser.elementById('loader-prop-img').getAttribute('src') + ).toBe('https://example.vercel.sh/success/foo.jpg?width=1024') + expect( + await browser.elementById('loader-prop-img').getAttribute('srcset') + ).toBe( + 'https://example.vercel.sh/success/foo.jpg?width=480 1x, https://example.vercel.sh/success/foo.jpg?width=1024 2x' + ) + }) }) describe('Client-side Image Component Tests', () => { beforeAll(async () => {