fix(next/image): ensure images.maximumResponseBody applies to local images too (#92920)

In a previous PR https://github.com/vercel/next.js/pull/88183, we added
`images.maximumResponseBody` but in only applied to external images.

This PR ensures the same config also applies to internal images.

packages/next/errors.json

@@ -1161,5 +1161,6 @@
   "1160": "Route \"%s\" cannot use \\`export const unstable_prefetch = ...\\` without enabling \\`cacheComponents\\`.",
   "1161": "\"unstable_prefetch\" is a route segment config and can only be used when the segment is a Server Component module. Remove the \"use client\" directive from \"%s\" to use this API.",
   "1162": "Route \"%s\" cannot use \\`export const unstable_instant = ...\\` without enabling \\`cacheComponents\\`.",
-  "1163": "Unexpected cache miss after cache warming phase during prerendering. This is likely caused by non-deterministic arguments that differ between the cache warming phase and the final prerender phase (e.g. unstable array order). Ensure that arguments passed to cached functions are deterministic."
+  "1163": "Unexpected cache miss after cache warming phase during prerendering. This is likely caused by non-deterministic arguments that differ between the cache warming phase and the final prerender phase (e.g. unstable array order). Ensure that arguments passed to cached functions are deterministic.",
+  "1164": "Response body exceeded maximum size of %s bytes"
 }

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

@@ -975,6 +975,7 @@ export async function fetchInternalImage(
   href: string,
   _req: IncomingMessage,
   _res: ServerResponse,
+  maximumResponseBody: number,
   handleRequest: (
     newReq: IncomingMessage,
     newRes: ServerResponse,
@@ -989,6 +990,7 @@ export async function fetchInternalImage(
       url: href,
       method,
       socket: _req.socket,
+      maximumResponseBody,
     })
 
     await handleRequest(mocked.req, mocked.res, nodeUrl.parse(href, true))
@@ -1002,6 +1004,14 @@ export async function fetchInternalImage(
       )
     }
 
+    if (mocked.res.buffers.length === 0) {
+      Log.error('internal image response is empty for', href)
+      throw new ImageError(
+        400,
+        '"url" parameter is valid but internal response is invalid'
+      )
+    }
+
     const buffer = Buffer.concat(mocked.res.buffers)
     const contentType = mocked.res.getHeader('Content-Type')
     const cacheControl = mocked.res.getHeader('Cache-Control')
@@ -1009,6 +1019,23 @@ export async function fetchInternalImage(
 
     return { buffer, contentType, cacheControl, etag }
   } catch (err) {
+    if (err instanceof ImageError) {
+      throw err
+    }
+
+    if (
+      err &&
+      typeof err === 'object' &&
+      'code' in err &&
+      err.code === 'ERR_MAX_BODY_SIZE_EXCEEDED'
+    ) {
+      Log.error('internal image response exceeded maximum size for', href)
+      throw new ImageError(
+        413,
+        '"url" parameter is valid but internal response is invalid'
+      )
+    }
+
     Log.error('upstream image response failed for', href, err)
     throw new ImageError(
       500,

packages/next/src/server/lib/mock-request.ts

@@ -142,6 +142,7 @@ export interface MockedResponseOptions {
   socket?: Socket | null
   headers?: OutgoingHttpHeaders
   resWriter?: (chunk: Uint8Array | Buffer | string) => boolean
+  maximumResponseBody?: number
 }
 
 export class MockedResponse extends Stream.Writable implements ServerResponse {
@@ -174,6 +175,8 @@ export class MockedResponse extends Stream.Writable implements ServerResponse {
   public readonly headers: Headers
 
   private resWriter: MockedResponseOptions['resWriter']
+  private maximumResponseBody?: number
+  private totalSize: number = 0
 
   public readonly headPromise: Promise<void>
   private headPromiseResolve?: () => void
@@ -186,6 +189,7 @@ export class MockedResponse extends Stream.Writable implements ServerResponse {
     this.headers = res.headers
       ? fromNodeOutgoingHttpHeaders(res.headers)
       : new Headers()
+    this.maximumResponseBody = res.maximumResponseBody
 
     this.headPromise = new Promise<void>((resolve) => {
       this.headPromiseResolve = resolve
@@ -238,12 +242,29 @@ export class MockedResponse extends Stream.Writable implements ServerResponse {
     if (this.resWriter) {
       return this.resWriter(chunk)
     }
-    this.buffers.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))
 
+    const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)
+
+    if (this.maximumResponseBody !== undefined) {
+      this.totalSize += buffer.byteLength
+      if (this.totalSize > this.maximumResponseBody) {
+        const error = new Error(
+          `Response body exceeded maximum size of ${this.maximumResponseBody} bytes`
+        ) as NodeJS.ErrnoException
+        error.code = 'ERR_MAX_BODY_SIZE_EXCEEDED'
+        this.destroy(error)
+        return true
+      }
+    }
+
+    this.buffers.push(buffer)
     return true
   }
 
   public end() {
+    if (this.destroyed) {
+      return this
+    }
     this.finished = true
     return super.end(...arguments)
   }
@@ -260,7 +281,7 @@ export class MockedResponse extends Stream.Writable implements ServerResponse {
   public _write(
     chunk: Buffer | string,
     _encoding: string,
-    callback: () => void
+    callback: (error?: Error | null) => void
   ) {
     this.write(chunk)
 
@@ -269,7 +290,13 @@ export class MockedResponse extends Stream.Writable implements ServerResponse {
     // invoked, the 'finish' event will not be emitted.
     //
     // https://nodejs.org/docs/latest-v16.x/api/stream.html#writable_writechunk-encoding-callback
-    callback()
+    //
+    // If the stream was destroyed due to an error, we should propagate it
+    if (this.destroyed && this.errored) {
+      callback(this.errored)
+    } else {
+      callback()
+    }
   }
 
   public writeHead(
@@ -445,6 +472,7 @@ interface RequestResponseMockerOptions {
   bodyReadable?: Stream.Readable
   resWriter?: (chunk: Uint8Array | Buffer | string) => boolean
   socket?: Socket | null
+  maximumResponseBody?: number
 }
 
 export function createRequestResponseMocks({
@@ -454,6 +482,7 @@ export function createRequestResponseMocks({
   bodyReadable,
   resWriter,
   socket = null,
+  maximumResponseBody,
 }: RequestResponseMockerOptions) {
   return {
     req: new MockedRequest({
@@ -463,6 +492,6 @@ export function createRequestResponseMocks({
       socket,
       readable: bodyReadable,
     }),
-    res: new MockedResponse({ socket, resWriter }),
+    res: new MockedResponse({ socket, resWriter, maximumResponseBody }),
   }
 }

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

@@ -749,6 +749,7 @@ export default class NextNodeServer extends BaseServer<
             href,
             req.originalRequest,
             res.originalResponse,
+            this.nextConfig.images.maximumResponseBody,
             handleInternalReq
           )
 

test/e2e/rewrite-request-smuggling/next.config.js

@@ -0,0 +1,13 @@
+/** @type {import('next').NextConfig} */
+const nextConfig = {
+  async rewrites() {
+    return [
+      {
+        source: '/rewrites/:path*',
+        destination: `http://127.0.0.1:${process.env.TEST_INTERMEDIARY_PORT}/rewrites/:path*`,
+      },
+    ]
+  },
+}
+
+module.exports = nextConfig

test/e2e/rewrite-request-smuggling/pages/index.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>hello world</p>
+}

test/e2e/rewrite-request-smuggling/rewrite-request-smuggling.test.ts

@@ -0,0 +1,234 @@
+import net from 'net'
+import http from 'http'
+import { createNext, NextInstance } from 'e2e-utils'
+import { findPort, retry } from 'next-test-utils'
+
+describe('rewrite-request-smuggling', () => {
+  if ((global as any).isNextDeploy) {
+    it('should skip deploy', () => {})
+    return
+  }
+
+  let backend: http.Server
+  let backendPort: number
+  let intermediary: http.Server
+  let intermediaryPort: number
+  let next: NextInstance
+  const backendRequests: string[] = []
+
+  async function sendSmugglingPayload({
+    nextPort,
+    connectionHeader,
+    method = 'DELETE',
+    rewritePath = '/rewrites/poc',
+  }: {
+    nextPort: number
+    connectionHeader: string
+    method?: 'DELETE' | 'OPTIONS'
+    rewritePath?: string
+  }) {
+    const smuggledRequest = Buffer.from(
+      `GET /secret HTTP/1.1\r\nHost: 127.0.0.1:${nextPort}\r\n\r\n`,
+      'latin1'
+    )
+    const chunkSize = Buffer.from(
+      `${smuggledRequest.length.toString(16).toUpperCase()}\r\n`,
+      'latin1'
+    )
+
+    const payload = Buffer.concat([
+      Buffer.from(
+        `${method} ${rewritePath} HTTP/1.1\r\nHost: 127.0.0.1:${nextPort}\r\nTransfer-Encoding: chunked\r\nConnection: ${connectionHeader}\r\n\r\n`,
+        'latin1'
+      ),
+      chunkSize,
+      smuggledRequest,
+      Buffer.from('\r\n0\r\n\r\n', 'latin1'),
+    ])
+
+    await new Promise<void>((resolve, reject) => {
+      const socket = net.createConnection({
+        host: '127.0.0.1',
+        port: nextPort,
+      })
+
+      socket.once('connect', () => {
+        socket.write(payload)
+      })
+      socket.once('error', reject)
+      socket.setTimeout(1000, () => socket.destroy())
+      socket.once('close', () => resolve())
+    })
+  }
+
+  beforeAll(async () => {
+    backendPort = await findPort()
+    intermediaryPort = await findPort()
+
+    backend = http.createServer((req, res) => {
+      backendRequests.push(`${req.method} ${req.url}`)
+
+      if (req.url?.startsWith('/rewrites/')) {
+        res.statusCode = 200
+        res.end('rewrite-ok')
+        return
+      }
+
+      if (req.url === '/secret') {
+        res.statusCode = 200
+        res.end('secret')
+        return
+      }
+
+      res.statusCode = 404
+      res.end('not-found')
+    })
+
+    intermediary = http.createServer((req, res) => {
+      const connectionHeader = Array.isArray(req.headers['connection'])
+        ? req.headers['connection'].join(',')
+        : req.headers['connection'] || ''
+      const hopByHopHeaders = connectionHeader
+        .split(',')
+        .map((h) => h.trim().toLowerCase())
+        .filter(Boolean)
+      const stripTransferEncodingUnconditionally =
+        req.url?.startsWith('/rewrites/non-rfc-strip') || false
+
+      const forwardHeaders: Record<string, string | string[]> = {}
+      for (const [key, value] of Object.entries(req.headers)) {
+        if (key === 'connection') continue
+        if (stripTransferEncodingUnconditionally && key === 'transfer-encoding')
+          continue
+        if (hopByHopHeaders.includes(key)) continue
+        if (value !== undefined) {
+          forwardHeaders[key] = value
+        }
+      }
+      forwardHeaders.connection = stripTransferEncodingUnconditionally
+        ? connectionHeader.toLowerCase().includes('close')
+          ? 'close'
+          : 'keep-alive'
+        : 'keep-alive'
+
+      const proxyReq = http.request(
+        {
+          hostname: '127.0.0.1',
+          port: backendPort,
+          method: req.method,
+          path: req.url,
+          headers: forwardHeaders,
+        },
+        (proxyRes) => {
+          res.writeHead(proxyRes.statusCode || 500, proxyRes.headers)
+          proxyRes.pipe(res)
+        }
+      )
+
+      proxyReq.on('error', () => {
+        res.statusCode = 502
+        res.end('Bad Gateway')
+      })
+
+      req.pipe(proxyReq)
+    })
+
+    await new Promise<void>((resolve, reject) => {
+      backend.listen(backendPort, '127.0.0.1', resolve)
+      backend.once('error', reject)
+    })
+
+    await new Promise<void>((resolve, reject) => {
+      intermediary.listen(intermediaryPort, '127.0.0.1', resolve)
+      intermediary.once('error', reject)
+    })
+
+    next = await createNext({
+      files: __dirname,
+      env: {
+        TEST_INTERMEDIARY_PORT: String(intermediaryPort),
+      },
+    })
+  })
+
+  afterAll(async () => {
+    await next?.destroy()
+    await new Promise<void>((resolve) => intermediary.close(() => resolve()))
+    await new Promise<void>((resolve) => backend.close(() => resolve()))
+  })
+
+  it('does not smuggle a second request when using keep-alive only', async () => {
+    backendRequests.length = 0
+
+    const nextPort = Number(new URL(next.url).port)
+    await sendSmugglingPayload({ nextPort, connectionHeader: 'keep-alive' })
+
+    await retry(async () => {
+      expect(backendRequests).toContain('DELETE /rewrites/poc')
+    })
+    expect(backendRequests).not.toContain('GET /secret')
+  })
+
+  it('does not smuggle a second request with keep-alive, upgrade', async () => {
+    backendRequests.length = 0
+
+    const nextPort = Number(new URL(next.url).port)
+    await sendSmugglingPayload({
+      nextPort,
+      connectionHeader: 'keep-alive, upgrade',
+    })
+
+    await retry(async () => {
+      expect(backendRequests).toContain('DELETE /rewrites/poc')
+    })
+    expect(backendRequests).not.toContain('GET /secret')
+  })
+
+  it('does not smuggle a second request with Transfer-Encoding, upgrade', async () => {
+    backendRequests.length = 0
+
+    const nextPort = Number(new URL(next.url).port)
+    await sendSmugglingPayload({
+      nextPort,
+      connectionHeader: 'Transfer-Encoding, upgrade',
+    })
+
+    await retry(async () => {
+      expect(backendRequests).toContain('DELETE /rewrites/poc')
+    })
+    expect(backendRequests).not.toContain('GET /secret')
+  })
+
+  it('does not smuggle a second request for OPTIONS with Transfer-Encoding, upgrade', async () => {
+    backendRequests.length = 0
+
+    const nextPort = Number(new URL(next.url).port)
+    await sendSmugglingPayload({
+      nextPort,
+      method: 'OPTIONS',
+      connectionHeader: 'Transfer-Encoding, upgrade',
+    })
+
+    await retry(async () => {
+      expect(backendRequests).toContain('OPTIONS /rewrites/poc')
+    })
+    expect(backendRequests).not.toContain('GET /secret')
+  })
+
+  it('does not smuggle a second request when an intermediary strips transfer-encoding unconditionally', async () => {
+    backendRequests.length = 0
+
+    const nextPort = Number(new URL(next.url).port)
+    await sendSmugglingPayload({
+      nextPort,
+      method: 'OPTIONS',
+      rewritePath: '/rewrites/non-rfc-strip',
+      connectionHeader: 'keep-alive, upgrade',
+    })
+
+    await retry(async () => {
+      expect(backendRequests).toContain('OPTIONS /rewrites/non-rfc-strip')
+    })
+    expect(backendRequests).not.toContain('GET /secret')
+  })
+})

test/unit/image-optimizer/fetch-internal-image.test.ts

@@ -0,0 +1,152 @@
+/* eslint-env jest */
+import {
+  fetchInternalImage,
+  ImageError,
+} from 'next/dist/server/image-optimizer'
+import type { IncomingMessage, ServerResponse } from 'http'
+
+describe('fetchInternalImage', () => {
+  describe('response size limit', () => {
+    it('should throw error when response has no buffers', async () => {
+      const mockReq = {} as IncomingMessage
+      const mockRes = {} as ServerResponse
+      const maximumResponseBody = 300_000_000
+
+      const handleRequest = jest.fn(async (_req: IncomingMessage, res: any) => {
+        res.statusCode = 200
+        res.getHeader = jest.fn(() => 'image/jpeg')
+        res.end()
+      })
+
+      const error = await fetchInternalImage(
+        '/test-image.jpg',
+        mockReq,
+        mockRes,
+        maximumResponseBody,
+        handleRequest
+      ).catch((e) => e)
+
+      expect(error).toBeInstanceOf(ImageError)
+      expect((error as ImageError).statusCode).toBe(400)
+      expect((error as ImageError).message).toBe(
+        '"url" parameter is valid but internal response is invalid'
+      )
+    })
+
+    it('should throw error when exceeding maximumResponseBody config on later chunk', async () => {
+      const maximumResponseBody = 2_000 // 2KB custom limit
+      const chunkSize = 1_000 // 1KB chunks
+      const numChunks = 3 // 3KB total, exceeds custom 2KB limit
+
+      const mockReq = {} as IncomingMessage
+      const mockRes = {} as ServerResponse
+
+      const handleRequest = jest.fn(async (_req: IncomingMessage, res: any) => {
+        res.statusCode = 200
+        res.getHeader = jest.fn(() => 'image/jpeg')
+
+        for (let i = 0; i < numChunks; i++) {
+          res.write(Buffer.alloc(chunkSize))
+        }
+        res.end()
+      })
+
+      const error = await fetchInternalImage(
+        '/test-image.jpg',
+        mockReq,
+        mockRes,
+        maximumResponseBody,
+        handleRequest
+      ).catch((e) => e)
+
+      expect(error).toBeInstanceOf(ImageError)
+      expect((error as ImageError).statusCode).toBe(413)
+      expect((error as ImageError).message).toBe(
+        '"url" parameter is valid but internal response is invalid'
+      )
+    })
+
+    it('should throw error when exceeding maximumResponseBody config on first chunk', async () => {
+      const maximumResponseBody = 2_000 // 2KB custom limit
+
+      const mockReq = {} as IncomingMessage
+      const mockRes = {} as ServerResponse
+
+      const handleRequest = jest.fn(async (_req: IncomingMessage, res: any) => {
+        res.statusCode = 200
+        res.getHeader = jest.fn(() => 'image/jpeg')
+        res.write(Buffer.alloc(maximumResponseBody + 1))
+        res.end()
+      })
+
+      const error = await fetchInternalImage(
+        '/test-image.jpg',
+        mockReq,
+        mockRes,
+        maximumResponseBody,
+        handleRequest
+      ).catch((e) => e)
+
+      expect(error).toBeInstanceOf(ImageError)
+      expect((error as ImageError).statusCode).toBe(413)
+      expect((error as ImageError).message).toBe(
+        '"url" parameter is valid but internal response is invalid'
+      )
+    })
+
+    it('should succeed when exactly matching maximumResponseBody config on first chunk', async () => {
+      const maximumResponseBody = 3_000 // 3KB custom limit
+
+      const mockReq = {} as IncomingMessage
+      const mockRes = {} as ServerResponse
+
+      const handleRequest = jest.fn(async (_req: IncomingMessage, res: any) => {
+        res.statusCode = 200
+        res.getHeader = jest.fn(() => 'image/jpeg')
+        res.write(Buffer.alloc(maximumResponseBody))
+        res.end()
+      })
+
+      const result = await fetchInternalImage(
+        '/test-image.jpg',
+        mockReq,
+        mockRes,
+        maximumResponseBody,
+        handleRequest
+      )
+
+      expect(result.buffer).toBeInstanceOf(Buffer)
+      expect(result.buffer.length).toBe(maximumResponseBody)
+    })
+
+    it('should succeed when exactly matching maximumResponseBody config on later chunk', async () => {
+      const maximumResponseBody = 3_000 // 3KB custom limit
+      const chunkSize = 1_000 // 1KB chunks
+      const numChunks = 3 // 3KB total
+
+      const mockReq = {} as IncomingMessage
+      const mockRes = {} as ServerResponse
+
+      const handleRequest = jest.fn(async (_req: IncomingMessage, res: any) => {
+        res.statusCode = 200
+        res.getHeader = jest.fn(() => 'image/jpeg')
+
+        for (let i = 0; i < numChunks; i++) {
+          res.write(Buffer.alloc(chunkSize))
+        }
+        res.end()
+      })
+
+      const result = await fetchInternalImage(
+        '/test-image.jpg',
+        mockReq,
+        mockRes,
+        maximumResponseBody,
+        handleRequest
+      )
+
+      expect(result.buffer).toBeInstanceOf(Buffer)
+      expect(result.buffer.length).toBe(maximumResponseBody)
+    })
+  })
+})