Repository: vercel/next.js
Pull request: next-image: fix dpl handling with unicode

With skew protection enabled, image srcs were double-encoded:

1. The imported value might be `'/_next/static/media/äöüščří.55df2443.png?dpl=1234'`
2. Then `new URL(src, 'http://n')` would normalize the URL, leading to `/_next/static/media/%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD.55df2443.png?dpl=1234`
3. The the existing `?url=${encodeURIComponent(src)}` causes the already percent-encoded unicode characters to be encoded yet again.

It's unfortunate that we have to handroll some URL parsing here, maybe someone has a better idea


```
  ● Image Component Unicode Image URL › production mode › should load static unicode image

    expect(received).toMatch(expected)

    Expected pattern: /_next%2Fstatic%2Fmedia%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD(.+)png/
    Received string:  "/_next/image?url=%2F_next%2Fstatic%2Fmedia%2F%25C3%25A4%25C3%25B6%25C3%25BC%25C5%25A1%25C4%258D%25C5%2599%25C3%25AD.55df2443.png&w=828&q=75&dpl=test-immutable-tkn-7890"

      21 |   it('should load static unicode image', async () => {
      22 |     const src = await browser.elementById('static').getAttribute('src')
    > 23 |     expect(src).toMatch(
         |                 ^
      24 |       /_next%2Fstatic%2Fmedia%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD(.+)png/
      25 |     )
      26 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

      at Object.toMatch (integration/next-image-legacy/unicode/test/index.test.ts:23:17)



  ● Image Component Unicode Image URL › production mode › should load static unicode image

    expect(received).toMatch(expected)

    Expected pattern: /_next%2Fstatic%2Fmedia%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD(.+)png/
    Received string:  "/_next/image?url=%2F_next%2Fstatic%2Fmedia%2F%25C3%25A4%25C3%25B6%25C3%25BC%25C5%25A1%25C4%258D%25C5%2599%25C3%25AD.55df2443.png&w=828&q=75&dpl=test-immutable-tkn-7890"

      22 |   it('should load static unicode image', async () => {
      23 |     const src = await browser.elementById('static').getAttribute('src')
    > 24 |     expect(src).toMatch(
         |                 ^
      25 |       /_next%2Fstatic%2Fmedia%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD(.+)png/
      26 |     )
      27 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

      at Object.toMatch (integration/next-image-new/unicode/test/index.test.ts:24:17)

```

Changed files:
filename: packages/next/src/client/legacy/image.tsx
patch: @@ -129,14 +129,19 @@ function defaultLoader({
 
   // Extract dpl parameter early so validation uses the clean URL
   let deploymentId = getDeploymentId()
-  if (src.startsWith('/')) {
-    const srcUrl = new URL(src, 'http://n')
-    const srcDpl = srcUrl.searchParams.get('dpl')
-    if (srcDpl) {
-      deploymentId = srcDpl
-      // Remove the dpl parameter from the src URL to avoid duplication
-      srcUrl.searchParams.delete('dpl')
-      src = srcUrl.href.slice('http://n'.length)
+  if (src.startsWith('/') && !src.startsWith('//')) {
+    // We unfortunately can't easily use `new URL()` here, because it normalizes the URL which causes
+    // double-encoding with the `encodeURIComponent(src)` below
+    const qIndex = src.indexOf('?')
+    if (qIndex !== -1) {
+      const params = new URLSearchParams(src.slice(qIndex + 1))
+      const srcDpl = params.get('dpl')
+      if (srcDpl) {
+        deploymentId = srcDpl
+        params.delete('dpl')
+        const remaining = params.toString()
+        src = src.slice(0, qIndex) + (remaining ? '?' + remaining : '')
+      }
     }
   }
 
@@ -423,12 +428,20 @@ function generateImgAttrs({
     if (src.startsWith('/') && !src.startsWith('//')) {
       let deploymentId = getDeploymentId()
       if (deploymentId) {
-        const srcUrl = new URL(src, 'http://n')
-        const srcDpl = srcUrl.searchParams.get('dpl')
-        if (!srcDpl) {
+        // We unfortunately can't easily use `new URL()` here, because it normalizes the URL which causes
+        // double-encoding with the `encodeURIComponent(src)` below
+        const qIndex = src.indexOf('?')
+        if (qIndex !== -1) {
+          const params = new URLSearchParams(src.slice(qIndex + 1))
+          const srcDpl = params.get('dpl')
+          if (!srcDpl) {
+            // src is missing the dpl parameter, but we have a deploymentId, so add it to the src URL
+            params.append('dpl', deploymentId)
+            src = src.slice(0, qIndex) + '?' + params.toString()
+          }
+        } else {
           // src is missing the dpl parameter, but we have a deploymentId, so add it to the src URL
-          srcUrl.searchParams.set('dpl', deploymentId)
-          src = srcUrl.href.slice('http://n'.length)
+          src = src + `?dpl=${deploymentId}`
         }
       }
     }

filename: packages/next/src/shared/lib/get-img-props.ts
patch: @@ -234,12 +234,20 @@ function generateImgAttrs({
     if (src.startsWith('/') && !src.startsWith('//')) {
       let deploymentId = getDeploymentId()
       if (deploymentId) {
-        const srcUrl = new URL(src, 'http://n')
-        const srcDpl = srcUrl.searchParams.get('dpl')
-        if (!srcDpl) {
+        // We unfortunately can't easily use `new URL()` here, because it normalizes the URL which causes
+        // double-encoding with the `encodeURIComponent(src)` below
+        const qIndex = src.indexOf('?')
+        if (qIndex !== -1) {
+          const params = new URLSearchParams(src.slice(qIndex + 1))
+          const srcDpl = params.get('dpl')
+          if (!srcDpl) {
+            // src is missing the dpl parameter, but we have a deploymentId, so add it to the src URL
+            params.append('dpl', deploymentId)
+            src = src.slice(0, qIndex) + '?' + params.toString()
+          }
+        } else {
           // src is missing the dpl parameter, but we have a deploymentId, so add it to the src URL
-          srcUrl.searchParams.set('dpl', deploymentId)
-          src = srcUrl.href.slice('http://n'.length)
+          src = src + `?dpl=${deploymentId}`
         }
       }
     }

filename: packages/next/src/shared/lib/image-loader.ts
patch: @@ -28,14 +28,19 @@ function defaultLoader({
 
   // Extract dpl parameter early so validation uses the clean URL
   let deploymentId = getDeploymentId()
-  if (src.startsWith('/')) {
-    const srcUrl = new URL(src, 'http://n')
-    const srcDpl = srcUrl.searchParams.get('dpl')
-    if (srcDpl) {
-      deploymentId = srcDpl
-      // Remove the dpl parameter from the src URL to avoid duplication
-      srcUrl.searchParams.delete('dpl')
-      src = srcUrl.href.slice('http://n'.length)
+  if (src.startsWith('/') && !src.startsWith('//')) {
+    // We unfortunately can't easily use `new URL()` here, because it normalizes the URL which causes
+    // double-encoding with the `encodeURIComponent(src)` below
+    const qIndex = src.indexOf('?')
+    if (qIndex !== -1) {
+      const params = new URLSearchParams(src.slice(qIndex + 1))
+      const srcDpl = params.get('dpl')
+      if (srcDpl) {
+        deploymentId = srcDpl
+        params.delete('dpl')
+        const remaining = params.toString()
+        src = src.slice(0, qIndex) + (remaining ? '?' + remaining : '')
+      }
     }
   }
 

filename: test/unit/next-image-get-img-props.test.ts
patch: @@ -741,6 +741,64 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
+  it('should add query string for imported local image with unicode when deployment id is defined', async () => {
+    try {
+      deploymentId = 'dpl_123'
+      const src = '/_next/static/media/äöüščří.3f1a293b.png'
+      const srcEncoded = encodeURIComponent(src)
+      const { props } = getImageProps({
+        alt: 'a nice desc',
+        src: src,
+        width: 100,
+        height: 200,
+      })
+      expect(warningMessages).toStrictEqual([])
+      expect(Object.entries(props)).toStrictEqual([
+        ['alt', 'a nice desc'],
+        ['loading', 'lazy'],
+        ['width', 100],
+        ['height', 200],
+        ['decoding', 'async'],
+        ['style', { color: 'transparent' }],
+        [
+          'srcSet',
+          `/_next/image?url=${srcEncoded}&w=128&q=75&dpl=dpl_123 1x, /_next/image?url=${srcEncoded}&w=256&q=75&dpl=dpl_123 2x`,
+        ],
+        ['src', `/_next/image?url=${srcEncoded}&w=256&q=75&dpl=dpl_123`],
+      ])
+    } finally {
+      deploymentId = undefined
+    }
+  })
+  it('should respect query string for imported local image with unicode when deployment id is defined', async () => {
+    try {
+      deploymentId = 'dpl_123'
+      const src = '/_next/static/media/äöüščří.3f1a293b.png'
+      const srcEncoded = encodeURIComponent(src)
+      const { props } = getImageProps({
+        alt: 'a nice desc',
+        src: `${src}?dpl=dpl_existing`,
+        width: 100,
+        height: 200,
+      })
+      expect(warningMessages).toStrictEqual([])
+      expect(Object.entries(props)).toStrictEqual([
+        ['alt', 'a nice desc'],
+        ['loading', 'lazy'],
+        ['width', 100],
+        ['height', 200],
+        ['decoding', 'async'],
+        ['style', { color: 'transparent' }],
+        [
+          'srcSet',
+          `/_next/image?url=${srcEncoded}&w=128&q=75&dpl=dpl_existing 1x, /_next/image?url=${srcEncoded}&w=256&q=75&dpl=dpl_existing 2x`,
+        ],
+        ['src', `/_next/image?url=${srcEncoded}&w=256&q=75&dpl=dpl_existing`],
+      ])
+    } finally {
+      deploymentId = undefined
+    }
+  })
   it('should add query string for relative local image when deployment id defined', async () => {
     try {
       deploymentId = 'dpl_123'
@@ -821,6 +879,29 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
+  it('should add query string with question mark for unoptimized relative svg with unicode when deployment id is defined', async () => {
+    try {
+      deploymentId = 'dpl_123'
+      const { props } = getImageProps({
+        alt: 'a nice desc',
+        src: '/äöüščří.svg',
+        width: 100,
+        height: 200,
+      })
+      expect(warningMessages).toStrictEqual([])
+      expect(Object.entries(props)).toStrictEqual([
+        ['alt', 'a nice desc'],
+        ['loading', 'lazy'],
+        ['width', 100],
+        ['height', 200],
+        ['decoding', 'async'],
+        ['style', { color: 'transparent' }],
+        ['src', '/äöüščří.svg?dpl=dpl_123'],
+      ])
+    } finally {
+      deploymentId = undefined
+    }
+  })
   it('should add query string with ampersand for unoptimized relative svg when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
@@ -891,7 +972,6 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-
   it('should respect existing query string for unoptimized relative image when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
@@ -916,4 +996,28 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
+  it('should respect existing query string for unoptimized relative image with unicode when deployment id is defined', async () => {
+    try {
+      deploymentId = 'dpl_123'
+      const { props } = getImageProps({
+        alt: 'a nice desc',
+        src: `/_next/static/media/äöüščří.3f1a293b.png?dpl=dpl_existing`,
+        unoptimized: true,
+        width: 100,
+        height: 200,
+      })
+      expect(warningMessages).toStrictEqual([])
+      expect(Object.entries(props)).toStrictEqual([
+        ['alt', 'a nice desc'],
+        ['loading', 'lazy'],
+        ['width', 100],
+        ['height', 200],
+        ['decoding', 'async'],
+        ['style', { color: 'transparent' }],
+        ['src', '/_next/static/media/äöüščří.3f1a293b.png?dpl=dpl_existing'],
+      ])
+    } finally {
+      deploymentId = undefined
+    }
+  })
 })

filename: test/unit/next-image-legacy.test.ts
patch: @@ -143,8 +143,45 @@ describe('Image Legacy Rendering', () => {
       dpl: 'dpl_123',
       expected: '/_next/static/media/test.3f1a293b.png?dpl=dpl_existing',
     },
+    {
+      name: 'unicode: without deployment add',
+      src: '/_next/static/media/äöüščří.3f1a293b.png',
+      expected: `/_next/image?url=${encodeURIComponent('/_next/static/media/äöüščří.3f1a293b.png')}&w=3840&q=75`,
+    },
+    {
+      name: 'unicode: adding a deployment id',
+      src: '/_next/static/media/äöüščří.3f1a293b.png',
+      dpl: 'dpl_123',
+      expected: `/_next/image?url=${encodeURIComponent('/_next/static/media/äöüščří.3f1a293b.png')}&w=3840&q=75&dpl=dpl_123`,
+    },
+    {
+      name: 'unicode: respect existing deployment id',
+      src: '/_next/static/media/äöüščří.3f1a293b.png?dpl=dpl_existing',
+      dpl: 'dpl_123',
+      expected: `/_next/image?url=${encodeURIComponent('/_next/static/media/äöüščří.3f1a293b.png')}&w=3840&q=75&dpl=dpl_existing`,
+    },
+    {
+      name: 'unicode: unoptimized without deployment add',
+      unoptimized: true,
+      src: '/_next/static/media/äöüščří.3f1a293b.png',
+      expected: '/_next/static/media/äöüščří.3f1a293b.png',
+    },
+    {
+      name: 'unicode: unoptimized adding a deployment id',
+      unoptimized: true,
+      src: '/_next/static/media/äöüščří.3f1a293b.png',
+      dpl: 'dpl_123',
+      expected: '/_next/static/media/äöüščří.3f1a293b.png?dpl=dpl_123',
+    },
+    {
+      name: 'unicode: unoptimized respect existing deployment id',
+      unoptimized: true,
+      src: '/_next/static/media/äöüščří.3f1a293b.png?dpl=dpl_existing',
+      dpl: 'dpl_123',
+      expected: '/_next/static/media/äöüščří.3f1a293b.png?dpl=dpl_existing',
+    },
   ])(
-    'should correctly deployment ids: $name',
+    'should correctly handle deployment ids: $name',
     async ({ src, unoptimized, dpl, expected }) => {
       deploymentId = dpl
       try {