From f169b089016c39e93c46352540af2eefe0513d9e Mon Sep 17 00:00:00 2001 From: Steven Date: Tue, 14 May 2024 14:00:27 -0400 Subject: [PATCH] fix(next/image)!: error when `src` has leading or trailing space (#65637) BREAKING CHANGE: Using the built-in image optimization API, the URL is parsed with `new URL()` constructor which automatically trims spaces. However, the developer may choose a 3rd party image optimization API via `loader` or `loaderFile` (or perhaps a deployment platform that has its own built in loader), so we shouldn't assume the API will parse the URL in the same way as [WHATWG](https://url.spec.whatwg.org/#:~:text=If%20input%20contains%20any%20leading%20or%20trailing%20C0%20control%20or%20space%2C%20invalid%2DURL%2Dunit%20validation%20error.). While we could trim on the client, its probably best to fail fast and let the developer make a conscience decision if a trailing space should be removed or remain (by explicitly using `%20`). --- packages/next/src/shared/lib/get-img-props.ts | 12 ++++++++++++ .../app/invalid-src-leading-space/page.js | 11 +++++++++++ .../app/invalid-src-trailing-space/page.js | 11 +++++++++++ .../next-image-new/app-dir/test/index.test.ts | 16 ++++++++++++++++ .../default/pages/invalid-src-leading-space.js | 11 +++++++++++ .../default/pages/invalid-src-trailing-space.js | 11 +++++++++++ .../next-image-new/default/test/index.test.ts | 16 ++++++++++++++++ 7 files changed, 88 insertions(+) create mode 100644 test/integration/next-image-new/app-dir/app/invalid-src-leading-space/page.js create mode 100644 test/integration/next-image-new/app-dir/app/invalid-src-trailing-space/page.js create mode 100644 test/integration/next-image-new/default/pages/invalid-src-leading-space.js create mode 100644 test/integration/next-image-new/default/pages/invalid-src-trailing-space.js diff --git a/packages/next/src/shared/lib/get-img-props.ts b/packages/next/src/shared/lib/get-img-props.ts index f0fcad0cde..cdd0cf03a6 100644 --- a/packages/next/src/shared/lib/get-img-props.ts +++ b/packages/next/src/shared/lib/get-img-props.ts @@ -465,6 +465,18 @@ export function getImgProps( `Image with src "${src}" has invalid "height" property. Expected a numeric value in pixels but received "${height}".` ) } + // eslint-disable-next-line no-control-regex + if (/^[\x00-\x20]/.test(src)) { + throw new Error( + `Image with src "${src}" cannot start with a space or control character. Use src.trimStart() to remove it or encodeURIComponent(src) to keep it.` + ) + } + // eslint-disable-next-line no-control-regex + if (/[\x00-\x20]$/.test(src)) { + throw new Error( + `Image with src "${src}" cannot end with a space or control character. Use src.trimEnd() to remove it or encodeURIComponent(src) to keep it.` + ) + } } } if (!VALID_LOADING_VALUES.includes(loading)) { diff --git a/test/integration/next-image-new/app-dir/app/invalid-src-leading-space/page.js b/test/integration/next-image-new/app-dir/app/invalid-src-leading-space/page.js new file mode 100644 index 0000000000..b7682cbc7a --- /dev/null +++ b/test/integration/next-image-new/app-dir/app/invalid-src-leading-space/page.js @@ -0,0 +1,11 @@ +import React from 'react' +import Image from 'next/image' + +export default function Page() { + return ( +
+

Invalid src with leading space

+ +
+ ) +} diff --git a/test/integration/next-image-new/app-dir/app/invalid-src-trailing-space/page.js b/test/integration/next-image-new/app-dir/app/invalid-src-trailing-space/page.js new file mode 100644 index 0000000000..279729491a --- /dev/null +++ b/test/integration/next-image-new/app-dir/app/invalid-src-trailing-space/page.js @@ -0,0 +1,11 @@ +import React from 'react' +import Image from 'next/image' + +export default function Page() { + return ( +
+

Invalid src with trailing space

+ +
+ ) +} diff --git a/test/integration/next-image-new/app-dir/test/index.test.ts b/test/integration/next-image-new/app-dir/test/index.test.ts index ebff18725d..25a8250144 100644 --- a/test/integration/next-image-new/app-dir/test/index.test.ts +++ b/test/integration/next-image-new/app-dir/test/index.test.ts @@ -915,6 +915,22 @@ function runTests(mode) { ) }) + it('should show invalid src with leading space', async () => { + const browser = await webdriver(appPort, '/invalid-src-leading-space') + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toContain( + 'Image with src " /test.jpg" cannot start with a space or control character.' + ) + }) + + it('should show invalid src with trailing space', async () => { + const browser = await webdriver(appPort, '/invalid-src-trailing-space') + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toContain( + 'Image with src "/test.png " cannot end with a space or control character.' + ) + }) + it('should show error when string src and placeholder=blur and blurDataURL is missing', async () => { const browser = await webdriver(appPort, '/invalid-placeholder-blur') diff --git a/test/integration/next-image-new/default/pages/invalid-src-leading-space.js b/test/integration/next-image-new/default/pages/invalid-src-leading-space.js new file mode 100644 index 0000000000..b7682cbc7a --- /dev/null +++ b/test/integration/next-image-new/default/pages/invalid-src-leading-space.js @@ -0,0 +1,11 @@ +import React from 'react' +import Image from 'next/image' + +export default function Page() { + return ( +
+

Invalid src with leading space

+ +
+ ) +} diff --git a/test/integration/next-image-new/default/pages/invalid-src-trailing-space.js b/test/integration/next-image-new/default/pages/invalid-src-trailing-space.js new file mode 100644 index 0000000000..279729491a --- /dev/null +++ b/test/integration/next-image-new/default/pages/invalid-src-trailing-space.js @@ -0,0 +1,11 @@ +import React from 'react' +import Image from 'next/image' + +export default function Page() { + return ( +
+

Invalid src with trailing space

+ +
+ ) +} diff --git a/test/integration/next-image-new/default/test/index.test.ts b/test/integration/next-image-new/default/test/index.test.ts index 539b1a8111..7796fc98a0 100644 --- a/test/integration/next-image-new/default/test/index.test.ts +++ b/test/integration/next-image-new/default/test/index.test.ts @@ -916,6 +916,22 @@ function runTests(mode) { ) }) + it('should show invalid src with leading space', async () => { + const browser = await webdriver(appPort, '/invalid-src-leading-space') + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toContain( + 'Image with src " /test.jpg" cannot start with a space or control character.' + ) + }) + + it('should show invalid src with trailing space', async () => { + const browser = await webdriver(appPort, '/invalid-src-trailing-space') + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toContain( + 'Image with src "/test.png " cannot end with a space or control character.' + ) + }) + it('should show error when string src and placeholder=blur and blurDataURL is missing', async () => { const browser = await webdriver(appPort, '/invalid-placeholder-blur')