Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/01-app/03-api-reference/02-components/image.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ module.exports = {

The example above will ensure the `src` property of `next/image` must start with `/assets/images/` and must not have a query string. Attempting to optimize any other path will respond with `400` Bad Request error.

> **Good to know**: Omitting the `search` property allows all search parameters which could allow malicious actors to optimize URLs you did not intend. Try using a specific value like `search: '?v=2'` to ensure an exact match.

#### `remotePatterns`

Use `remotePatterns` in your `next.config.js` file to allow images from specific external paths and block all others. This ensures that only external images from your account can be served.
Expand Down
1 change: 1 addition & 0 deletions errors/next-image-unconfigured-localpatterns.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module.exports = {
localPatterns: [
{
pathname: '/assets/**',
search: '?v=2',
},
],
},
Expand Down
3 changes: 2 additions & 1 deletion packages/next/errors.json
Original file line number Diff line number Diff line change
Expand Up @@ -847,5 +847,6 @@
"846": "Route %s used \\`cookies()\\` inside a function cached with \\`unstable_cache()\\`. Accessing Dynamic data sources inside a cache scope is not supported. If you need this data inside a cached function use \\`cookies()\\` outside of the cached function and pass the required dynamic data in as an argument. See more info here: https://nextjs.org/docs/app/api-reference/functions/unstable_cache",
"847": "Route %s with \\`dynamic = \"error\"\\` couldn't be rendered statically because it used \\`connection()\\`. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering",
"848": "%sused %s. \\`searchParams\\` is a Promise and must be unwrapped with \\`await\\` or \\`React.use()\\` before accessing its properties. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
"849": "Route %s with \\`dynamic = \"error\"\\` couldn't be rendered statically because it used \\`cookies()\\`. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering"
"849": "Route %s with \\`dynamic = \"error\"\\` couldn't be rendered statically because it used \\`cookies()\\`. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering",
"850": "Image with src \"%s\" is using a query string which requires images.localPatterns configuration.\\nRead more: https://nextjs.org/docs/messages/next-image-unconfigured-localpatterns"
}
4 changes: 2 additions & 2 deletions packages/next/src/client/legacy/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -865,8 +865,8 @@ export default function Image({
(config.localPatterns.length === 1 &&
config.localPatterns[0].pathname === '/_next/static/media/**'))
) {
warnOnce(
`Image with src "${src}" is using a query string which is not configured in images.localPatterns. This config will be required starting in Next.js 16.` +
throw new Error(
`Image with src "${src}" is using a query string which requires images.localPatterns configuration.` +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one will also need to be moved (I think legacy might use a different loader)

`\nRead more: https://nextjs.org/docs/messages/next-image-unconfigured-localpatterns`
)
}
Expand Down
10 changes: 10 additions & 0 deletions packages/next/src/server/image-optimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,16 @@ export class ImageOptimizerCache {
if (!hasLocalMatch(localPatterns, url)) {
return { errorMessage: '"url" parameter is not allowed' }
}
if (
url.includes('?') &&
nextConfig.images?.localPatterns?.length === 1 &&
nextConfig.images.localPatterns[0].pathname === '/_next/static/media/**'
) {
return {
errorMessage:
'"url" parameter has search query which requires `images.localPatterns` configuration',
}
}
} else {
let hrefParsed: URL

Expand Down
13 changes: 1 addition & 12 deletions packages/next/src/shared/lib/get-img-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,18 +560,7 @@ export function getImgProps(
`\nRead more: https://nextjs.org/docs/messages/next-image-unconfigured-qualities`
)
}
if (
src.startsWith('/') &&
src.includes('?') &&
(!config?.localPatterns?.length ||
(config.localPatterns.length === 1 &&
config.localPatterns[0].pathname === '/_next/static/media/**'))
) {
warnOnce(
`Image with src "${src}" is using a query string which is not configured in images.localPatterns. This config will be required starting in Next.js 16.` +
`\nRead more: https://nextjs.org/docs/messages/next-image-unconfigured-localpatterns`
)
}

if (placeholder === 'blur' && !blurDataURL) {
const VALID_BLUR_EXT = ['jpeg', 'png', 'webp', 'avif'] // should match next-image-loader

Expand Down
11 changes: 11 additions & 0 deletions packages/next/src/shared/lib/image-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,17 @@ function defaultLoader({
)
}
}

if (
src.includes('?') &&
config.localPatterns.length === 1 &&
config.localPatterns[0].pathname === '/_next/static/media/**'
) {
throw new Error(
`Image with src "${src}" is using a query string which requires images.localPatterns configuration.` +
`\nRead more: https://nextjs.org/docs/messages/next-image-unconfigured-localpatterns`
)
}
Comment on lines +50 to +59
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query string validation for local images only runs when config.localPatterns is defined, but it should also run when localPatterns is undefined (the default state when users don't configure it). This creates an inconsistency with the legacy image component and fails to enforce the security requirement that motivated this change.

View Details
📝 Patch Details
diff --git a/packages/next/src/shared/lib/image-loader.ts b/packages/next/src/shared/lib/image-loader.ts
index 23a6d4af10..afa55be73c 100644
--- a/packages/next/src/shared/lib/image-loader.ts
+++ b/packages/next/src/shared/lib/image-loader.ts
@@ -30,27 +30,30 @@ function defaultLoader({
       )
     }
 
-    if (src.startsWith('/') && config.localPatterns) {
-      if (
-        process.env.NODE_ENV !== 'test' &&
-        // micromatch isn't compatible with edge runtime
-        process.env.NEXT_RUNTIME !== 'edge'
-      ) {
-        // We use dynamic require because this should only error in development
-        const { hasLocalMatch } =
-          require('./match-local-pattern') as typeof import('./match-local-pattern')
-        if (!hasLocalMatch(config.localPatterns, src)) {
-          throw new Error(
-            `Invalid src prop (${src}) on \`next/image\` does not match \`images.localPatterns\` configured in your \`next.config.js\`\n` +
-              `See more info: https://nextjs.org/docs/messages/next-image-unconfigured-localpatterns`
-          )
+    if (src.startsWith('/')) {
+      if (config.localPatterns) {
+        if (
+          process.env.NODE_ENV !== 'test' &&
+          // micromatch isn't compatible with edge runtime
+          process.env.NEXT_RUNTIME !== 'edge'
+        ) {
+          // We use dynamic require because this should only error in development
+          const { hasLocalMatch } =
+            require('./match-local-pattern') as typeof import('./match-local-pattern')
+          if (!hasLocalMatch(config.localPatterns, src)) {
+            throw new Error(
+              `Invalid src prop (${src}) on \`next/image\` does not match \`images.localPatterns\` configured in your \`next.config.js\`\n` +
+                `See more info: https://nextjs.org/docs/messages/next-image-unconfigured-localpatterns`
+            )
+          }
         }
       }
 
       if (
         src.includes('?') &&
-        config.localPatterns.length === 1 &&
-        config.localPatterns[0].pathname === '/_next/static/media/**'
+        (!config.localPatterns?.length ||
+          (config.localPatterns.length === 1 &&
+            config.localPatterns[0].pathname === '/_next/static/media/**'))
       ) {
         throw new Error(
           `Image with src "${src}" is using a query string which requires images.localPatterns configuration.` +

Analysis

Query string validation skipped for local images when localPatterns undefined

What fails: defaultLoader() in packages/next/src/shared/lib/image-loader.ts (lines 33-59) skips query string validation when config.localPatterns is undefined, allowing images like /test.svg?v=1 without throwing an error.

How to reproduce:

// With default config (localPatterns: undefined)
defaultLoader({
  config: { localPatterns: undefined, path: '/_next/image' },
  src: '/test.svg?v=1',
  width: 100,
  quality: 75
});
// Returns: /_next/image?url=%2Ftest.svg%3Fv%3D1&w=100&q=75
// Expected: Should throw error

Result: No error thrown. Query string validation (lines 50-59) only executes when inside the if (src.startsWith('/') && config.localPatterns) block, which evaluates to false when localPatterns is undefined.

Expected: Should throw error: "Image with src "/test.svg?v=1" is using a query string which requires images.localPatterns configuration" matching the legacy image component behavior which explicitly checks !config?.localPatterns?.length.

Impact: Creates inconsistency between new and legacy image components, and allows query strings in unconfigured environments contrary to the security requirements that motivated this validation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@styfle mentioned that '/_next/static/media/**' will be inside config.localPatterns, so it seems fine for now, but the warning logic we moved from has !config?.localPatterns?.length || condition checking the actual length, so could be troublesome when we later remove behavior with '/_next/static/media/**' and results in say an empty array or undefined. Therefore, I was thinking of a clean copy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think we should also check for !config?.localPatterns?.length like the old code did, just in case.

}

if (!src.startsWith('/') && (config.domains || config.remotePatterns)) {
Expand Down
20 changes: 0 additions & 20 deletions test/unit/next-image-get-img-props.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,26 +460,6 @@ describe('getImageProps()', () => {
['src', '/test.svg'],
])
})
it('should auto unoptimized for relative svg with query', async () => {
const { props } = getImageProps({
alt: 'a nice desc',
src: '/test.svg?v=1',
width: 100,
height: 200,
})
expect(warningMessages).toStrictEqual([
'Image with src "/test.svg?v=1" is using a query string which is not configured in images.localPatterns. This config will be required starting in Next.js 16.\nRead more: https://nextjs.org/docs/messages/next-image-unconfigured-localpatterns',
])
expect(Object.entries(props)).toStrictEqual([
['alt', 'a nice desc'],
['loading', 'lazy'],
['width', 100],
['height', 200],
['decoding', 'async'],
['style', { color: 'transparent' }],
['src', '/test.svg?v=1'],
])
})
it('should auto unoptimized for absolute svg', async () => {
const { props } = getImageProps({
alt: 'a nice desc',
Expand Down
Loading