vercel/next.js · #98271

feat(next/image): add internal image redirects (to match existing external image redirects)

styfle · merged Nov 4, 202610 files · 333 + / 92 −
packages/next/src/server/image-optimizer.ts129 + / 70 −
@@ -100,6 +100,77 @@ export interface ImageParamsResult {   minimumCacheTTL: number } +type ImageUrlParamResult =+  | { href: string; isAbsolute: boolean }+  | { errorMessage: string }++function validateUrlParam(+  url: string | string[] | undefined,+  imageData: ImageConfigComplete,+  isRedirect = false+): ImageUrlParamResult {+  // Note - We intentionally skip validation for local/remotePatterns when+  // validating redirect locations so that we don't break a src img redirect+  // to a new domain (e.g. github.com => githubusercontent.com).+  const { domains = [], remotePatterns = [], localPatterns } = imageData++  if (!isRedirect && domains.length > 0) {+    Log.warnOnce(+      'The "images.domains" configuration is deprecated. Please use "images.remotePatterns" configuration instead.'+    )+  }++  if (!url) {+    return { errorMessage: '"url" parameter is required' }+  } else if (Array.isArray(url)) {+    return { errorMessage: '"url" parameter cannot be an array' }+  }++  if (url.length > 3072) {+    return { errorMessage: '"url" parameter is too long' }+  }++  if (url.startsWith('//')) {+    return {+      errorMessage: '"url" parameter cannot be a protocol-relative URL (//)',+    }+  }++  if (url.startsWith('/')) {+    if (+      /\/_next\/image($|\/)/.test(+        decodeURIComponent(parseUrl(url)?.pathname ?? '')+      )+    ) {+      return {+        errorMessage: '"url" parameter cannot be recursive',+      }+    }+    if (!isRedirect && !hasLocalMatch(localPatterns, url)) {+      return { errorMessage: '"url" parameter is not allowed' }+    }+    return { href: url, isAbsolute: false }+  }++  let hrefParsed: URL++  try {+    hrefParsed = new URL(url)+  } catch (_error) {+    return { errorMessage: '"url" parameter is invalid' }+  }++  if (!['http:', 'https:'].includes(hrefParsed.protocol)) {+    return { errorMessage: '"url" parameter is invalid' }+  }++  if (!isRedirect && !hasRemoteMatch(domains, remotePatterns, hrefParsed)) {+    return { errorMessage: '"url" parameter is not allowed' }+  }++  return { href: hrefParsed.toString(), isAbsolute: true }+}+ function getSupportedMimeType(options: string[], accept = ''): string {   const mimeType = mediaType(accept, options)   return accept.includes(mimeType) ? mimeType : ''@@ -185,73 +256,14 @@ export class ImageOptimizerCache {     const {       deviceSizes = [],       imageSizes = [],-      domains = [],       minimumCacheTTL = 14400,       formats = ['image/webp'],     } = imageData-    const remotePatterns = nextConfig.images?.remotePatterns || []-    const localPatterns = nextConfig.images?.localPatterns     const qualities = nextConfig.images?.qualities     const { url, w, q } = query-    let href: string--    if (domains.length > 0) {-      Log.warnOnce(-        'The "images.domains" configuration is deprecated. Please use "images.remotePatterns" configuration instead.'-      )-    }--    if (!url) {-      return { errorMessage: '"url" parameter is required' }-    } else if (Array.isArray(url)) {-      return { errorMessage: '"url" parameter cannot be an array' }-    }--    if (url.length > 3072) {-      return { errorMessage: '"url" parameter is too long' }-    }--    if (url.startsWith('//')) {-      return {-        errorMessage: '"url" parameter cannot be a protocol-relative URL (//)',-      }-    }--    let isAbsolute: boolean--    if (url.startsWith('/')) {-      href = url-      isAbsolute = false-      if (-        /\/_next\/image($|\/)/.test(-          decodeURIComponent(parseUrl(url)?.pathname ?? '')-        )-      ) {-        return {-          errorMessage: '"url" parameter cannot be recursive',-        }-      }-      if (!hasLocalMatch(localPatterns, url)) {-        return { errorMessage: '"url" parameter is not allowed' }-      }-    } else {-      let hrefParsed: URL--      try {-        hrefParsed = new URL(url)-        href = hrefParsed.toString()-        isAbsolute = true-      } catch (_error) {-        return { errorMessage: '"url" parameter is invalid' }-      }--      if (!['http:', 'https:'].includes(hrefParsed.protocol)) {-        return { errorMessage: '"url" parameter is invalid' }-      }--      if (!hasRemoteMatch(domains, remotePatterns, hrefParsed)) {-        return { errorMessage: '"url" parameter is not allowed' }-      }+    const urlResult = validateUrlParam(url, imageData)+    if ('errorMessage' in urlResult) {+      return urlResult     }      if (!w) {@@ -318,10 +330,10 @@ export class ImageOptimizerCache {     }      const mimeType = getSupportedMimeType(formats || [], req.headers['accept'])-+    const { href, isAbsolute } = urlResult     const isStatic =-      url.startsWith(`${nextConfig.basePath || ''}/_next/static/media`) ||-      url.startsWith(+      href.startsWith(`${nextConfig.basePath || ''}/_next/static/media`) ||+      href.startsWith(         `${nextConfig.basePath || ''}/_next/static/immutable/media`       ) @@ -532,7 +544,12 @@ export class ImageOptimizerCache {     }   } }-function isRedirect(statusCode: number) {++// Base origin used to resolve the `Location` header of an internal (relative+// `src`) image response, which is handled in-process and has no real origin.+const INTERNAL_IMAGE_BASE_URL = 'http://n'++function isRedirectStatus(statusCode: number) {   return [301, 302, 303, 307, 308].includes(statusCode) } @@ -856,7 +873,7 @@ export async function fetchExternalImage(   }   const locationHeader = res.headers.location   if (-    isRedirect(statusCode) &&+    isRedirectStatus(statusCode) &&     locationHeader &&     URL.canParse(locationHeader, href)   ) {@@ -939,12 +956,13 @@ export async function fetchInternalImage(   href: string,   _req: IncomingMessage,   _res: ServerResponse,-  maximumResponseBody: number,   handleRequest: (     newReq: IncomingMessage,     newRes: ServerResponse,     newParsedUrl?: NextUrlWithParsedQuery-  ) => Promise<void>+  ) => Promise<void>,+  imagesConfig: ImageConfigComplete,+  count = imagesConfig.maximumRedirects ): Promise<ImageUpstream> {   try {     // Coerce HEAD to GET to avoid issues with the image optimizer@@ -966,12 +984,53 @@ export async function fetchInternalImage(         headers: {},         socket: _req.socket,       }),-      res: new MockedResponse({ maximumResponseBody }),+      res: new MockedResponse({+        maximumResponseBody: imagesConfig.maximumResponseBody,+      }),     }      await handleRequest(mocked.req, mocked.res, parseReqUrl(href))     await mocked.res.hasStreamed +    const locationHeader = mocked.res.getHeader('Location')+    if (+      mocked.res.statusCode &&+      isRedirectStatus(mocked.res.statusCode) &&+      locationHeader &&+      URL.canParse(locationHeader, INTERNAL_IMAGE_BASE_URL)+    ) {+      if (count === 0) {+        Log.error('internal image response had too many redirects', href)+        throw new ImageError(+          508,+          '"url" parameter is valid but internal response is invalid'+        )+      }++      const urlResult = validateUrlParam(locationHeader, imagesConfig, true)+      if ('errorMessage' in urlResult) {+        throw new ImageError(400, urlResult.errorMessage)+      }++      if (!urlResult.isAbsolute) {+        return fetchInternalImage(+          urlResult.href,+          _req,+          _res,+          handleRequest,+          imagesConfig,+          count - 1+        )+      }++      return fetchExternalImage(+        urlResult.href,+        imagesConfig.dangerouslyAllowLocalIP,+        imagesConfig.maximumResponseBody,+        count - 1+      )+    }+     if (       !mocked.res.statusCode ||       mocked.res.statusCode < 200 ||
packages/next/src/server/next-server.ts2 + / 2 −
@@ -702,8 +702,8 @@ export default class NextNodeServer extends BaseServer<             href,             req.originalRequest,             res.originalResponse,-            this.nextConfig.images.maximumResponseBody,-            handleInternalReq+            handleInternalReq,+            this.nextConfig.images           )        return imageOptimizer(imageUpstream, paramsResult, this.nextConfig, {
test/e2e/image-optimizer-internal-redirect/app/layout.tsxadded11 + / 0 −
@@ -0,0 +1,11 @@+export default function RootLayout({+  children,+}: {+  children: React.ReactNode+}) {+  return (+    <html>+      <body>{children}</body>+    </html>+  )+}
test/e2e/image-optimizer-internal-redirect/app/page.tsxadded22 + / 0 −
@@ -0,0 +1,22 @@+import Image from 'next/image'++export default function Page() {+  return (+    <>+      <Image+        id="relative-redirect"+        src="/redirect-relative"+        alt="relative redirect"+        width={128}+        height={128}+      />+      <Image+        id="absolute-redirect"+        src="/redirect-absolute"+        alt="absolute redirect"+        width={128}+        height={128}+      />+    </>+  )+}
test/e2e/image-optimizer-internal-redirect/app/redirect-absolute/route.tsadded13 + / 0 −
@@ -0,0 +1,13 @@+import { connection } from 'next/server'++// Responds with a redirect (302) to an absolute URL on a different origin,+// which is allowed by `images.remotePatterns` in `next.config.js`.+export async function GET(req: Request) {+  await connection()++  const { port } = new URL(req.url)+  return new Response(null, {+    status: 302,+    headers: { Location: `http://127.0.0.1:${port}/test.png` },+  })+}
test/e2e/image-optimizer-internal-redirect/app/redirect-relative/route.tsadded12 + / 0 −
@@ -0,0 +1,12 @@+import { connection } from 'next/server'++// Responds with a redirect (302) to a static image on the same origin using a+// relative `Location` header.+export async function GET() {+  await connection()++  return new Response(null, {+    status: 302,+    headers: { Location: '/test.png' },+  })+}
test/e2e/image-optimizer-internal-redirect/image-optimizer-internal-redirect.test.tsadded44 + / 0 −
@@ -0,0 +1,44 @@+/* eslint-env jest */+import { nextTestSetup } from 'e2e-utils'+import sizeOf from 'image-size'++// https://github.com/vercel/next.js/issues/98237+describe('image-optimizer internal redirect', () => {+  const { next, skipped } = nextTestSetup({+    files: __dirname,+    // The image optimizer route is replaced by Vercel's image CDN when+    // deployed, so this only asserts the local Next.js image pipeline.+    skipDeployment: true,+  })++  if (skipped) {+    return+  }++  function fetchOptimizedImage(src: string) {+    const query = new URLSearchParams({ url: src, w: '128', q: '75' })+    return next.fetch(`/_next/image?${query}`, {+      headers: { accept: 'image/webp' },+    })+  }++  it('should follow a relative redirect from a relative image src', async () => {+    const res = await fetchOptimizedImage('/redirect-relative')++    expect(res.status).toBe(200)+    expect(res.headers.get('Content-Type')).toBe('image/webp')++    const buffer = Buffer.from(await res.arrayBuffer())+    expect(sizeOf(buffer).width).toBe(128)+  })++  it('should follow an absolute redirect to an allowed remote pattern from a relative image src', async () => {+    const res = await fetchOptimizedImage('/redirect-absolute')++    expect(res.status).toBe(200)+    expect(res.headers.get('Content-Type')).toBe('image/webp')++    const buffer = Buffer.from(await res.arrayBuffer())+    expect(sizeOf(buffer).width).toBe(128)+  })+})
test/e2e/image-optimizer-internal-redirect/next.config.jsadded11 + / 0 −
@@ -0,0 +1,11 @@+/**+ * @type {import('next').NextConfig}+ */+const nextConfig = {+  images: {+    dangerouslyAllowLocalIP: true,+    remotePatterns: [{ protocol: 'http', hostname: '127.0.0.1' }],+  },+}++module.exports = nextConfig
test/e2e/image-optimizer-internal-redirect/public/test.pngadded0 + / 0 −

GitHub publishes no diff for this file. View it on GitHub ↗

test/unit/image-optimizer/fetch-internal-image.test.ts89 + / 20 −
@@ -3,6 +3,7 @@ import {   fetchInternalImage,   ImageError, } from 'next/dist/server/image-optimizer'+import type { ImageConfigComplete } from 'next/dist/shared/lib/image-config' import { serveStatic } from 'next/dist/server/serve-static' import { EventEmitter } from 'events' import { promises as fs } from 'fs'@@ -11,9 +12,19 @@ import type { Socket } from 'net' import { tmpdir } from 'os' import { join } from 'path' +function getImagesConfig(maximumResponseBody: number, maximumRedirects = 0) {+  return {+    deviceSizes: [128],+    imageSizes: [],+    dangerouslyAllowLocalIP: false,+    maximumRedirects,+    maximumResponseBody,+  } as ImageConfigComplete+}+ describe('fetchInternalImage', () => {-  describe('non-2xx responses', () => {-    it('should throw error when the internal response is a redirect', async () => {+  describe('redirects and non-2xx responses', () => {+    it('should throw error when the internal response exceeds the redirect limit', async () => {       const mockReq = {} as IncomingMessage       const mockRes = {} as ServerResponse       const maximumResponseBody = 300_000_000@@ -29,18 +40,76 @@ describe('fetchInternalImage', () => {         '/test-image.jpg',         mockReq,         mockRes,-        maximumResponseBody,-        handleRequest+        handleRequest,+        getImagesConfig(maximumResponseBody)       ).catch((e) => e)        expect(error).toBeInstanceOf(ImageError)-      // ImageError maps any status below 400 to 500-      expect((error as ImageError).statusCode).toBe(500)+      expect((error as ImageError).statusCode).toBe(508)       expect((error as ImageError).message).toBe(         '"url" parameter is valid but internal response is invalid'       )     }) +    it('should reject protocol-relative redirects', async () => {+      const mockReq = {} as IncomingMessage+      const mockRes = {} as ServerResponse++      const handleRequest = jest.fn(async (_req: IncomingMessage, res: any) => {+        res.statusCode = 307+        res.getHeader = jest.fn(() => '//example.com/test-image.jpg')+        res.end()+      })++      const error = await fetchInternalImage(+        '/test-image.jpg',+        mockReq,+        mockRes,+        handleRequest,+        getImagesConfig(300_000_000, 1)+      ).catch((e) => e)++      expect(error).toBeInstanceOf(ImageError)+      expect((error as ImageError).statusCode).toBe(400)+      expect((error as ImageError).message).toBe(+        '"url" parameter cannot be a protocol-relative URL (//)'+      )+    })++    it('should skip local pattern validation for internal redirects', async () => {+      const mockReq = {+        url: '/_next/image?url=%2Ftest-image.jpg&w=128&q=75',+        headers: { accept: 'image/webp' },+      } as IncomingMessage+      const mockRes = {} as ServerResponse+      const imagesConfig = getImagesConfig(300_000_000, 1)+      imagesConfig.localPatterns = [{ pathname: '/allowed.jpg' }]++      const handleRequest = jest.fn(async (req: IncomingMessage, res: any) => {+        if (req.url === '/test-image.jpg') {+          res.statusCode = 307+          res.getHeader = jest.fn(() => '/blocked.jpg')+          res.end()+        } else {+          res.statusCode = 200+          res.getHeader = jest.fn(() => 'image/jpeg')+          res.write(Buffer.from('image'))+          res.end()+        }+      })++      const result = await fetchInternalImage(+        '/test-image.jpg',+        mockReq,+        mockRes,+        handleRequest,+        imagesConfig+      )++      expect(result.buffer).toEqual(Buffer.from('image'))+      expect(handleRequest).toHaveBeenCalledTimes(2)+    })+     it('should throw error when the internal response is a 404', async () => {       const mockReq = {} as IncomingMessage       const mockRes = {} as ServerResponse@@ -57,8 +126,8 @@ describe('fetchInternalImage', () => {         '/test-image.jpg',         mockReq,         mockRes,-        maximumResponseBody,-        handleRequest+        handleRequest,+        getImagesConfig(maximumResponseBody)       ).catch((e) => e)        expect(error).toBeInstanceOf(ImageError)@@ -85,8 +154,8 @@ describe('fetchInternalImage', () => {         '/test-image.jpg',         mockReq,         mockRes,-        maximumResponseBody,-        handleRequest+        handleRequest,+        getImagesConfig(maximumResponseBody)       ).catch((e) => e)        expect(error).toBeInstanceOf(ImageError)@@ -118,8 +187,8 @@ describe('fetchInternalImage', () => {         '/test-image.jpg',         mockReq,         mockRes,-        maximumResponseBody,-        handleRequest+        handleRequest,+        getImagesConfig(maximumResponseBody)       ).catch((e) => e)        expect(error).toBeInstanceOf(ImageError)@@ -146,8 +215,8 @@ describe('fetchInternalImage', () => {         '/test-image.jpg',         mockReq,         mockRes,-        maximumResponseBody,-        handleRequest+        handleRequest,+        getImagesConfig(maximumResponseBody)       ).catch((e) => e)        expect(error).toBeInstanceOf(ImageError)@@ -174,8 +243,8 @@ describe('fetchInternalImage', () => {         '/test-image.jpg',         mockReq,         mockRes,-        maximumResponseBody,-        handleRequest+        handleRequest,+        getImagesConfig(maximumResponseBody)       )        expect(result.buffer).toBeInstanceOf(Buffer)@@ -204,8 +273,8 @@ describe('fetchInternalImage', () => {         '/test-image.jpg',         mockReq,         mockRes,-        maximumResponseBody,-        handleRequest+        handleRequest,+        getImagesConfig(maximumResponseBody)       )        expect(result.buffer).toBeInstanceOf(Buffer)@@ -241,8 +310,8 @@ describe('fetchInternalImage', () => {             `/${fileName}`,             mockReq,             mockRes,-            300_000_000,-            handleRequest+            handleRequest,+            getImagesConfig(300_000_000)           ),           new Promise<never>((_resolve, reject) => {             const timer = setTimeout(