Skip to content

Commit 4e0790f

Browse files
authored
Reject next image urls in image optimizer (#68628)
### What Follow up on #67465, add more cases that should be rejected by image optimizer when the url is tend to contain the generated next image with `/_next/image` in url. These should also get should get rejected: * with encoded: `%2F_next%2Fimage%3` * with asset prefix: `/assets/_next/image` * with host: `https://<domain>/_next/image` Closes NDX-101 x-ref: [slack](https://vercel.slack.com/archives/C07AYREDX45/p1720111367743119)
1 parent 2cee0de commit 4e0790f

File tree

4 files changed

+66
-8
lines changed

4 files changed

+66
-8
lines changed

packages/next/src/lib/url.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ export function isFullStringUrl(url: string) {
66
return /https?:\/\//.test(url)
77
}
88

9+
export function parseUrl(url: string): URL | undefined {
10+
let parsed = undefined
11+
try {
12+
parsed = new URL(url, DUMMY_ORIGIN)
13+
} catch {}
14+
return parsed
15+
}
16+
917
export function stripNextRscUnionQuery(relativeUrl: string): string {
1018
const urlInstance = new URL(relativeUrl, DUMMY_ORIGIN)
1119
urlInstance.searchParams.delete(NEXT_RSC_UNION_QUERY)

packages/next/src/server/image-optimizer.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { sendEtagResponse } from './send-payload'
2222
import { getContentType, getExtension } from './serve-static'
2323
import * as Log from '../build/output/log'
2424
import isError from '../lib/is-error'
25+
import { parseUrl } from '../lib/url'
2526

2627
type XCacheHeader = 'MISS' | 'HIT' | 'STALE'
2728

@@ -213,9 +214,13 @@ export class ImageOptimizerCache {
213214
}
214215
}
215216

216-
if (url.startsWith('/_next/image')) {
217-
return {
218-
errorMessage: '"url" parameter cannot be recursive',
217+
const parsedUrl = parseUrl(url)
218+
if (parsedUrl) {
219+
const decodedPathname = decodeURIComponent(parsedUrl.pathname)
220+
if (/\/_next\/image($|\/)/.test(decodedPathname)) {
221+
return {
222+
errorMessage: '"url" parameter cannot be recursive',
223+
}
219224
}
220225
}
221226

test/integration/image-optimizer/test/util.ts

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
fetchViaHTTP,
99
File,
1010
findPort,
11+
getFetchUrl,
1112
killApp,
1213
launchApp,
1314
nextBuild,
@@ -1021,11 +1022,46 @@ export function runTests(ctx: RunTestsCtx) {
10211022
)
10221023
})
10231024

1024-
it('should fail when url is recursive', async () => {
1025-
const query = { url: `/_next/image?url=test.pngw=1&q=1`, w: ctx.w, q: 1 }
1026-
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
1027-
expect(res.status).toBe(400)
1028-
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
1025+
describe('recursive url is not allowed', () => {
1026+
it('should fail with relative next image url', async () => {
1027+
const query = { url: `/_next/image?url=test.pngw=1&q=1`, w: ctx.w, q: 1 }
1028+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
1029+
expect(res.status).toBe(400)
1030+
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
1031+
})
1032+
1033+
it('should fail with encoded relative image url', async () => {
1034+
const query = {
1035+
url: '%2F_next%2Fimage%3Furl%3Dtest.pngw%3D1%26q%3D1',
1036+
w: ctx.w,
1037+
q: 1,
1038+
}
1039+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
1040+
expect(res.status).toBe(400)
1041+
expect(await res.text()).toBe(`"url" parameter is invalid`)
1042+
})
1043+
1044+
it('should fail with absolute next image url', async () => {
1045+
const fullUrl = getFetchUrl(
1046+
ctx.appPort,
1047+
'/_next/image?url=test.pngw=1&q=1'
1048+
)
1049+
const query = { url: fullUrl, w: ctx.w, q: 1 }
1050+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
1051+
expect(res.status).toBe(400)
1052+
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
1053+
})
1054+
1055+
it('should fail with relative image url with assetPrefix', async () => {
1056+
const fullUrl = getFetchUrl(
1057+
ctx.appPort,
1058+
`/assets/_next/image?url=test.pngw=1&q=1`
1059+
)
1060+
const query = { url: fullUrl, w: ctx.w, q: 1 }
1061+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
1062+
expect(res.status).toBe(400)
1063+
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
1064+
})
10291065
})
10301066

10311067
it('should fail when internal url is not an image', async () => {

test/lib/next-test-utils.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,15 @@ export function withQuery(
158158
return `${pathname}?${querystring}`
159159
}
160160

161+
export function getFetchUrl(
162+
appPort: string | number,
163+
pathname: string,
164+
query?: Record<string, any> | string | null | undefined
165+
) {
166+
const url = query ? withQuery(pathname, query) : pathname
167+
return getFullUrl(appPort, url)
168+
}
169+
161170
export function fetchViaHTTP(
162171
appPort: string | number,
163172
pathname: string,

0 commit comments

Comments
 (0)