From 7e224521b087d34f466c06406ce505a1314e7d8c Mon Sep 17 00:00:00 2001
From: Steven <steven@ceriously.com>
Date: Wed, 20 Nov 2024 14:11:55 -0500
Subject: [PATCH] fix(next/image): improve svg heuristic for unoptimized
 (#72970)

The `<Image>` component doesn't optimize SVG images since they are
vector images and can already scale up and down to any width.

Therefore, the component will try to detect SVG images based on the file
name to avoid sending them to the Image Optimization API at all. This is
because the default behavior of the Image Optimization API is to error
for SVG images due to the security implications of `<svg>` with nested
`<script>`, etc (unless `dangerouslyAllowSVG` is enabled.

However, we don't want users to reach for `dangerouslyAllowSVG` because
it is not usually what they intended. So instead, we can improve the
heuristic for detecting SVG in the `<Image>` component by ignoring the
query string.

Related to https://github.com/nodejs/nodejs.org/pull/7244
---
 packages/next/src/client/legacy/image.tsx     |  2 +-
 packages/next/src/shared/lib/get-img-props.ts |  6 +-
 test/unit/next-image-get-img-props.test.ts    | 72 +++++++++++++++++++
 3 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/packages/next/src/client/legacy/image.tsx b/packages/next/src/client/legacy/image.tsx
index 41c0fdb85a63e..c0975321ebd8c 100644
--- a/packages/next/src/client/legacy/image.tsx
+++ b/packages/next/src/client/legacy/image.tsx
@@ -192,7 +192,7 @@ function defaultLoader({
     }
   }
 
-  if (src.endsWith('.svg') && !config.dangerouslyAllowSVG) {
+  if (!config.dangerouslyAllowSVG && src.split('?', 1)[0].endsWith('.svg')) {
     // Special case to make svg serve as-is to avoid proxying
     // through the built-in Image Optimization API.
     return src
diff --git a/packages/next/src/shared/lib/get-img-props.ts b/packages/next/src/shared/lib/get-img-props.ts
index 4e91d287d9b49..5e6669667b00a 100644
--- a/packages/next/src/shared/lib/get-img-props.ts
+++ b/packages/next/src/shared/lib/get-img-props.ts
@@ -397,7 +397,11 @@ export function getImgProps(
   if (config.unoptimized) {
     unoptimized = true
   }
-  if (isDefaultLoader && src.endsWith('.svg') && !config.dangerouslyAllowSVG) {
+  if (
+    isDefaultLoader &&
+    !config.dangerouslyAllowSVG &&
+    src.split('?', 1)[0].endsWith('.svg')
+  ) {
     // Special case to make svg serve as-is to avoid proxying
     // through the built-in Image Optimization API.
     unoptimized = true
diff --git a/test/unit/next-image-get-img-props.test.ts b/test/unit/next-image-get-img-props.test.ts
index 6ae0eb35f773b..a72d1d0d3b861 100644
--- a/test/unit/next-image-get-img-props.test.ts
+++ b/test/unit/next-image-get-img-props.test.ts
@@ -349,4 +349,76 @@ describe('getImageProps()', () => {
       ['src', '/_next/image?url=%2Ftest.png&w=256&q=75'],
     ])
   })
+  it('should auto unoptimized for relative svg', async () => {
+    const { props } = getImageProps({
+      alt: 'a nice desc',
+      src: '/test.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', '/test.svg'],
+    ])
+  })
+  it('should auto unoptimized for relative svg with query', async () => {
+    const { props } = getImageProps({
+      alt: 'a nice desc',
+      src: '/test.svg?v=1',
+      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', '/test.svg?v=1'],
+    ])
+  })
+  it('should auto unoptimized for absolute svg', async () => {
+    const { props } = getImageProps({
+      alt: 'a nice desc',
+      src: 'https://example.com/test.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', 'https://example.com/test.svg'],
+    ])
+  })
+  it('should auto unoptimized for absolute svg with query', async () => {
+    const { props } = getImageProps({
+      alt: 'a nice desc',
+      src: 'https://example.com/test.svg?v=1',
+      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', 'https://example.com/test.svg?v=1'],
+    ])
+  })
 })