Skip to content

Commit d11cbc9

Browse files
huozhiztanner
authored andcommitted
Reject next image urls in image optimizer (#68628)
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 575385e commit d11cbc9

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
@@ -11,3 +11,11 @@ export function getPathname(url: string) {
1111
export function isFullStringUrl(url: string) {
1212
return /https?:\/\//.test(url)
1313
}
14+
15+
export function parseUrl(url: string): URL | undefined {
16+
let parsed = undefined
17+
try {
18+
parsed = new URL(url, DUMMY_ORIGIN)
19+
} catch {}
20+
return parsed
21+
}

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import type {
2828
import { sendEtagResponse } from './send-payload'
2929
import { getContentType, getExtension } from './serve-static'
3030
import * as Log from '../build/output/log'
31+
import { parseUrl } from '../lib/url'
3132

3233
type XCacheHeader = 'MISS' | 'HIT' | 'STALE'
3334

@@ -197,9 +198,13 @@ export class ImageOptimizerCache {
197198
}
198199
}
199200

200-
if (url.startsWith('/_next/image')) {
201-
return {
202-
errorMessage: '"url" parameter cannot be recursive',
201+
const parsedUrl = parseUrl(url)
202+
if (parsedUrl) {
203+
const decodedPathname = decodeURIComponent(parsedUrl.pathname)
204+
if (/\/_next\/image($|\/)/.test(decodedPathname)) {
205+
return {
206+
errorMessage: '"url" parameter cannot be recursive',
207+
}
203208
}
204209
}
205210

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,
@@ -879,11 +880,46 @@ export function runTests(ctx) {
879880
)
880881
})
881882

882-
it('should fail when url is recursive', async () => {
883-
const query = { url: `/_next/image?url=test.pngw=1&q=1`, w: ctx.w, q: 1 }
884-
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
885-
expect(res.status).toBe(400)
886-
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
883+
describe('recursive url is not allowed', () => {
884+
it('should fail with relative next image url', async () => {
885+
const query = { url: `/_next/image?url=test.pngw=1&q=1`, w: ctx.w, q: 1 }
886+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
887+
expect(res.status).toBe(400)
888+
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
889+
})
890+
891+
it('should fail with encoded relative image url', async () => {
892+
const query = {
893+
url: '%2F_next%2Fimage%3Furl%3Dtest.pngw%3D1%26q%3D1',
894+
w: ctx.w,
895+
q: 1,
896+
}
897+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
898+
expect(res.status).toBe(400)
899+
expect(await res.text()).toBe(`"url" parameter is invalid`)
900+
})
901+
902+
it('should fail with absolute next image url', async () => {
903+
const fullUrl = getFetchUrl(
904+
ctx.appPort,
905+
'/_next/image?url=test.pngw=1&q=1'
906+
)
907+
const query = { url: fullUrl, w: ctx.w, q: 1 }
908+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
909+
expect(res.status).toBe(400)
910+
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
911+
})
912+
913+
it('should fail with relative image url with assetPrefix', async () => {
914+
const fullUrl = getFetchUrl(
915+
ctx.appPort,
916+
`/assets/_next/image?url=test.pngw=1&q=1`
917+
)
918+
const query = { url: fullUrl, w: ctx.w, q: 1 }
919+
const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {})
920+
expect(res.status).toBe(400)
921+
expect(await res.text()).toBe(`"url" parameter cannot be recursive`)
922+
})
887923
})
888924

889925
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
@@ -147,6 +147,15 @@ export function withQuery(
147147
return `${pathname}?${querystring}`
148148
}
149149

150+
export function getFetchUrl(
151+
appPort: string | number,
152+
pathname: string,
153+
query?: Record<string, any> | string | null | undefined
154+
) {
155+
const url = query ? withQuery(pathname, query) : pathname
156+
return getFullUrl(appPort, url)
157+
}
158+
150159
export function fetchViaHTTP(
151160
appPort: string | number,
152161
pathname: string,

0 commit comments

Comments
 (0)