Skip to content

Commit

Permalink
Fix leaking internal config to user-defined loader prop in `next/im…
Browse files Browse the repository at this point in the history
…age` (vercel#36013)

The `config` was changed from a global variable to a function parameter of the `loader()` function in PR vercel#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 vercel#35115
  • Loading branch information
styfle authored and colinhacks committed Apr 14, 2022
1 parent 198a165 commit 8937abf
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 13 deletions.
56 changes: 43 additions & 13 deletions packages/next/client/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,25 @@ type ImageConfig = ImageConfigComplete & { allSizes: number[] }
export type ImageLoader = (resolverProps: ImageLoaderProps) => string

export type ImageLoaderProps = {
config: Readonly<ImageConfig>
src: string
width: number
quality?: number
}

const loaders = new Map<LoaderValue, (props: ImageLoaderProps) => 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<ImageConfig>
}

const loaders = new Map<
LoaderValue,
(props: ImageLoaderPropsWithConfig) => string
>([
['default', defaultLoader],
['imgix', imgixLoader],
['cloudinary', cloudinaryLoader],
Expand Down Expand Up @@ -132,7 +144,7 @@ export type ImageProps = Omit<
onLoadingComplete?: OnLoadingComplete
}

type ImageElementProps = Omit<ImageProps, 'src'> & {
type ImageElementProps = Omit<ImageProps, 'src' | 'loader'> & {
srcString: string
imgAttributes: GenImgAttrsResult
heightInt: number | undefined
Expand All @@ -145,7 +157,7 @@ type ImageElementProps = Omit<ImageProps, 'src'> & {
loading: LoadingValue
config: ImageConfig
unoptimized: boolean
loader: ImageLoader
loader: ImageLoaderWithConfig
placeholder: PlaceholderValue
onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>
setBlurComplete: (b: boolean) => void
Expand Down Expand Up @@ -209,7 +221,7 @@ type GenImgAttrsData = {
src: string
unoptimized: boolean
layout: LayoutValue
loader: ImageLoader
loader: ImageLoaderWithConfig
width?: number
quality?: number
sizes?: string
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -357,7 +369,6 @@ export default function Image({
objectPosition,
onLoadingComplete,
onError,
loader = defaultImageLoader,
placeholder = 'empty',
blurDataURL,
...all
Expand 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 <img>:
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 <img>
delete rest.loader
}

let staticSrc = ''
Expand Down Expand Up @@ -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
Expand All @@ -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}`
}

Expand All @@ -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(',') + '/'
Expand All @@ -1001,7 +1031,7 @@ function defaultLoader({
src,
width,
quality,
}: ImageLoaderProps): string {
}: ImageLoaderPropsWithConfig): string {
if (process.env.NODE_ENV !== 'production') {
const missingValues = []

Expand Down
23 changes: 23 additions & 0 deletions test/integration/image-component/basic/pages/loader-prop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import Image from 'next/image'

const LoaderExample = () => {
return (
<div>
<p>Custom loader in both next.config.js and loader prop</p>
<Image
id="loader-prop-img"
src="foo.jpg"
width={300}
height={400}
loader={({ config, src, width }) => {
if (config) {
return 'https://example.vercel.sh/error-unexpected-config'
}
return `https://example.vercel.sh/success/${src}?width=${width}`
}}
/>
</div>
)
}

export default LoaderExample
11 changes: 11 additions & 0 deletions test/integration/image-component/basic/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down

0 comments on commit 8937abf

Please sign in to comment.