Fix Invalid character in header ["Content-Disposition"] #30287

ihmpavel · 2021-10-25T23:45:45Z

fixes TypeError [ERR_INVALID_CHAR]: Invalid character in header content ["Content-Disposition"] #28985
fixes TypeError [ERR_INVALID_CHAR]: Invalid character in header content ["Content-Disposition"] #30285
fixes TypeError [ERR_INVALID_CHAR]: Invalid character in header content ["Content-Disposition"] #28979

Bug

Related issues linked using
Integration tests added

ihmpavel · 2021-10-26T20:38:56Z

Hi, could some reviewer kindly have a look?
Thanks in advance 🙏

@timneutkens

styfle

Thanks!

Lets add a test for the invalid character so we know that this fixes it 👍

For example:

next.js/test/integration/image-optimizer/test/index.test.js

Lines 65 to 69 in 8d82d76

    
           it('should handle non-ascii characters in image url', async () => { 
        
             const query = { w, q: 90, url: '/äöü.png' } 
        
             const res = await fetchViaHTTP(appPort, '/_next/image', query, {}) 
        
             expect(res.status).toBe(200) 
        
           })

ijjk · 2021-10-26T21:34:18Z

packages/next/package.json

@@ -81,6 +81,7 @@
    "chalk": "2.4.2",
    "chokidar": "3.5.1",
    "constants-browserify": "1.0.0",
+    "content-disposition": "0.5.3",


Can we ncc this and move it to devDependencies? Here's an example package being ncc'd

next.js/packages/next/taskfile.js

Lines 51 to 60 in 8d82d76

// eslint-disable-next-line camelcase

externals['amphtml-validator'] = 'next/dist/compiled/amphtml-validator'

export async function ncc_amphtml_validator(task, opts) {

await task

.source(

opts.src || relative(__dirname, require.resolve('amphtml-validator'))

)

.ncc({ packageName: 'amphtml-validator', externals })

.target('compiled/amphtml-validator')

}

Moved to devDependencies and added to ncc

ihmpavel · 2021-10-27T10:39:08Z

Thanks!

Lets add a test for the invalid character so we know that this fixes it 👍

For example:

next.js/test/integration/image-optimizer/test/index.test.js

Lines 65 to 69 in 8d82d76

it('should handle non-ascii characters in image url', async () => {

const query = { w, q: 90, url: '/äöü.png' }

const res = await fetchViaHTTP(appPort, '/_next/image', query, {})

expect(res.status).toBe(200)

})

I have updated tests with updated image name.

packages/next/server/image-optimizer.ts

packages/next/types/misc.d.ts

ihmpavel · 2021-10-31T00:38:33Z

@styfle Could you please take a look?

ihmpavel · 2021-11-01T19:42:32Z

@styfle That looks like the failing Jest tests and build are irrelevant to this PR (they were ok before). Could you please rerun the tests? (Maybe pulling latest canary is a right idea?)

ihmpavel · 2021-11-01T21:01:25Z

@styfle For unknow reasons one test failed because of me (i do not know how), but the second one was not my fault. Could you please re-run everything? (Auto-build on Azure failed again...)

ijjk · 2021-11-01T21:50:22Z

Failing test suites

Commit: 700f0f1

test/integration/image-component/unicode/test/index.test.js

Image Component Unicode Image URL > dev mode > should load static unicode image
Image Component Unicode Image URL > dev mode > should load internal unicode image
Image Component Unicode Image URL > dev mode > should load external unicode image
Image Component Unicode Image URL > server mode > should load static unicode image
Image Component Unicode Image URL > server mode > should load internal unicode image
Image Component Unicode Image URL > server mode > should load external unicode image

Expand output

● Image Component Unicode Image URL › dev 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%ADC(.+)png/
Received string:  "/_next/image?url=%2F_next%2Fstatic%2Fmedia%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD.eba16dd4.png&w=828&q=75"

  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%ADC(.+)png/
  25 |     )
  26 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

  at Object.<anonymous> (integration/image-component/unicode/test/index.test.js:23:17)

● Image Component Unicode Image URL › dev mode › should load internal unicode image

expect(received).toMatch(expected)

Expected substring: "/_next/image?url=%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png"
Received string:    "/_next/image?url=%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD.png&w=828&q=75"

  31 |   it('should load internal unicode image', async () => {
  32 |     const src = await browser.elementById('internal').getAttribute('src')
> 33 |     expect(src).toMatch(
     |                 ^
  34 |       '/_next/image?url=%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png'
  35 |     )
  36 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

  at Object.<anonymous> (integration/image-component/unicode/test/index.test.js:33:17)

● Image Component Unicode Image URL › dev mode › should load external unicode image

expect(received).toMatch(expected)

Expected substring: "/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png"
Received string:    "/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD.png&w=828&q=75"

  41 |   it('should load external unicode image', async () => {
  42 |     const src = await browser.elementById('external').getAttribute('src')
> 43 |     expect(src).toMatch(
     |                 ^
  44 |       '/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png'
  45 |     )
  46 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

  at Object.<anonymous> (integration/image-component/unicode/test/index.test.js:43:17)

● Image Component Unicode Image URL › server 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%ADC(.+)png/
Received string:  "/_next/image?url=%2F_next%2Fstatic%2Fmedia%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD.eba16dd4.png&w=828&q=75"

  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%ADC(.+)png/
  25 |     )
  26 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

  at Object.<anonymous> (integration/image-component/unicode/test/index.test.js:23:17)

● Image Component Unicode Image URL › server mode › should load internal unicode image

expect(received).toMatch(expected)

Expected substring: "/_next/image?url=%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png"
Received string:    "/_next/image?url=%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD.png&w=828&q=75"

  31 |   it('should load internal unicode image', async () => {
  32 |     const src = await browser.elementById('internal').getAttribute('src')
> 33 |     expect(src).toMatch(
     |                 ^
  34 |       '/_next/image?url=%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png'
  35 |     )
  36 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

  at Object.<anonymous> (integration/image-component/unicode/test/index.test.js:33:17)

● Image Component Unicode Image URL › server mode › should load external unicode image

expect(received).toMatch(expected)

Expected substring: "/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png"
Received string:    "/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%AD.png&w=828&q=75"

  41 |   it('should load external unicode image', async () => {
  42 |     const src = await browser.elementById('external').getAttribute('src')
> 43 |     expect(src).toMatch(
     |                 ^
  44 |       '/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2F%C3%A4%C3%B6%C3%BC%C5%A1%C4%8D%C5%99%C3%ADC.png'
  45 |     )
  46 |     const fullSrc = new URL(src, `http://localhost:${appPort}`)

  at Object.<anonymous> (integration/image-component/unicode/test/index.test.js:43:17)

test/integration/image-component/unicode/test/index.test.js

ihmpavel · 2021-11-01T22:07:58Z

@styfle I have run the tests successfully, accidentally added a letter (which caused tests to fail, but I did not notice until Test Integration (2)) and committed. I am really disappointed... On the other hand, everything required was run successfully... Fixed now...

ijjk · 2021-11-01T22:29:54Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
buildDuration	18.1s	18.1s	⚠️ +88ms
buildDurationCached	3.9s	3.8s	-32ms
nodeModulesSize	293 MB	293 MB	⚠️ +6.11 kB

Page Load Tests Overall increase ✓

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.872	2.883	⚠️ +0.01
/ avg req/sec	870.48	867.03	⚠️ -3.45
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.399	1.343	-0.06
/error-in-render avg req/sec	1787.28	1860.93	+73.65

Client Bundles (main, webpack, commons)

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	28 kB	28 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	71.9 kB	71.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
polyfills-a4..dd70.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
_app-HASH.js gzip	1.23 kB	1.23 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	327 B	327 B	✓
dynamic-HASH.js gzip	2.38 kB	2.38 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	635 B	635 B	✓
image-HASH.js gzip	4.44 kB	4.44 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	1.87 kB	1.87 kB	✓
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
334f979574ae..6f4.css gzip	106 B	106 B	✓
Overall change	13.1 kB	13.1 kB	✓

Client Build Manifests

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
index.html gzip	534 B	534 B	✓
link.html gzip	547 B	547 B	✓
withRouter.html gzip	527 B	527 B	✓
Overall change	1.61 kB	1.61 kB	✓

Default Build with SWC (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
buildDuration	19.6s	19.5s	-103ms
buildDurationCached	3.8s	3.9s	⚠️ +76ms
nodeModulesSize	293 MB	293 MB	⚠️ +6.11 kB

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.86	2.827	-0.03
/ avg req/sec	874.13	884.34	+10.21
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.347	1.356	⚠️ +0.01
/error-in-render avg req/sec	1856	1843.86	⚠️ -12.14

Client Bundles (main, webpack, commons)

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	28.2 kB	28.2 kB	✓
webpack-HASH.js gzip	1.43 kB	1.43 kB	✓
Overall change	72.1 kB	72.1 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
polyfills-a4..dd70.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
_app-HASH.js gzip	1.22 kB	1.22 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.38 kB	2.38 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	622 B	622 B	✓
image-HASH.js gzip	4.46 kB	4.46 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	1.91 kB	1.91 kB	✓
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
334f979574ae..6f4.css gzip	106 B	106 B	✓
Overall change	13.1 kB	13.1 kB	✓

Client Build Manifests

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
_buildManifest.js gzip	460 B	460 B	✓
Overall change	460 B	460 B	✓

Rendered Page Sizes

	vercel/next.js canary	ihmpavel/next.js image-content-disposition	Change
index.html gzip	535 B	535 B	✓
link.html gzip	548 B	548 B	✓
withRouter.html gzip	529 B	529 B	✓
Overall change	1.61 kB	1.61 kB	✓

Commit: 8e13629

ihmpavel · 2021-11-01T23:22:20Z

@styfle Can be merged? 🤩

…ercel#30287) * Image content disposition * Add tests * Fixed import * Add TS types * Revert readme.md * Alphabet sorting * Compile `content-disposition` * Rename for tests * Fix test * Fix accidentally added letter Co-authored-by: Steven <steven@ceriously.com>

Image content disposition

4255589

ihmpavel requested review from huozhi, ijjk, padmaia, shuding, styfle and timneutkens as code owners October 25, 2021 23:45

ijjk added the type: next label Oct 25, 2021

Merge branch 'canary' into image-content-disposition

47faa32

ihmpavel mentioned this pull request Oct 25, 2021

TypeError [ERR_INVALID_CHAR]: Invalid character in header content ["Content-Disposition"] #30285

Closed

Merge branch 'vercel:canary' into image-content-disposition

1fb9cd3

Merge branch 'vercel:canary' into image-content-disposition

aa3c0f4

styfle reviewed Oct 26, 2021

View reviewed changes

ijjk reviewed Oct 26, 2021

View reviewed changes

ihmpavel and others added 5 commits October 27, 2021 11:22

Merge branch 'vercel:canary' into image-content-disposition

7947c6e

Merge branch 'vercel:canary' into image-content-disposition

f6ae8fa

Add tests

41a9498

Merge branch 'vercel:canary' into image-content-disposition

ee35e8a

Fixed import

4890abd

ihmpavel requested review from ijjk and styfle October 27, 2021 10:41

ihmpavel commented Oct 27, 2021

View reviewed changes

packages/next/server/image-optimizer.ts Show resolved Hide resolved

Add TS types

182215f

ihmpavel commented Oct 27, 2021

View reviewed changes

packages/next/types/misc.d.ts Show resolved Hide resolved

ihmpavel added 4 commits October 27, 2021 19:45

Merge branch 'vercel:canary' into image-content-disposition

c653c1b

Merge branch 'vercel:canary' into image-content-disposition

4ee0cbb

Merge branch 'vercel:canary' into image-content-disposition

863e5da

Merge branch 'canary' into image-content-disposition

4263c42

ihmpavel added 3 commits October 31, 2021 09:15

Merge branch 'canary' into image-content-disposition

cf77cfc

Merge branch 'canary' into image-content-disposition

8acaeac

Merge branch 'canary' into image-content-disposition

6aab04b

styfle previously approved these changes Nov 1, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Merge branch 'canary' into image-content-disposition

93e6798

This comment has been minimized.

Sign in to view

Fix test

4e41255

ihmpavel dismissed styfle’s stale review via 4e41255 November 1, 2021 20:34

Merge branch 'canary' into image-content-disposition

4060b94

styfle previously approved these changes Nov 1, 2021

View reviewed changes

Merge branch 'canary' into image-content-disposition

700f0f1

This comment has been minimized.

Sign in to view

styfle reviewed Nov 1, 2021

View reviewed changes

test/integration/image-component/unicode/test/index.test.js Outdated Show resolved Hide resolved

Fix accidentally added letter

8e13629

ihmpavel dismissed styfle’s stale review via 8e13629 November 1, 2021 22:07

styfle approved these changes Nov 1, 2021

View reviewed changes

styfle merged commit 2027d1f into vercel:canary Nov 1, 2021

stefanprobst mentioned this pull request Nov 8, 2021

German umlauts break image optimization process #31148

Closed

vercel locked as resolved and limited conversation to collaborators Jan 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Invalid character in header ["Content-Disposition"] #30287

Fix Invalid character in header ["Content-Disposition"] #30287

ihmpavel commented Oct 25, 2021 •

edited by styfle

ihmpavel commented Oct 26, 2021

styfle left a comment •

edited

ijjk Oct 26, 2021

ihmpavel Oct 27, 2021

ihmpavel commented Oct 27, 2021

ihmpavel commented Oct 31, 2021

This comment has been minimized.

This comment has been minimized.

ihmpavel commented Nov 1, 2021

This comment has been minimized.

This comment has been minimized.

ihmpavel commented Nov 1, 2021

This comment has been minimized.

ijjk commented Nov 1, 2021

ihmpavel commented Nov 1, 2021 •

edited

ijjk commented Nov 1, 2021

ihmpavel commented Nov 1, 2021

	it('should handle non-ascii characters in image url', async () => {
	const query = { w, q: 90, url: '/äöü.png' }
	const res = await fetchViaHTTP(appPort, '/_next/image', query, {})
	expect(res.status).toBe(200)
	})

	// eslint-disable-next-line camelcase
	externals['amphtml-validator'] = 'next/dist/compiled/amphtml-validator'
	export async function ncc_amphtml_validator(task, opts) {
	await task
	.source(
	opts.src \|\| relative(__dirname, require.resolve('amphtml-validator'))
	)
	.ncc({ packageName: 'amphtml-validator', externals })
	.target('compiled/amphtml-validator')
	}

Fix Invalid character in header ["Content-Disposition"] #30287

Fix Invalid character in header ["Content-Disposition"] #30287

Conversation

ihmpavel commented Oct 25, 2021 • edited by styfle

Bug

ihmpavel commented Oct 26, 2021

styfle left a comment • edited

Choose a reason for hiding this comment

ijjk Oct 26, 2021

Choose a reason for hiding this comment

ihmpavel Oct 27, 2021

Choose a reason for hiding this comment

ihmpavel commented Oct 27, 2021

ihmpavel commented Oct 31, 2021

This comment has been minimized.

This comment has been minimized.

ihmpavel commented Nov 1, 2021

This comment has been minimized.

This comment has been minimized.

ihmpavel commented Nov 1, 2021

This comment has been minimized.

ijjk commented Nov 1, 2021

Failing test suites

ihmpavel commented Nov 1, 2021 • edited

ijjk commented Nov 1, 2021

Stats from current PR

ihmpavel commented Nov 1, 2021

ihmpavel commented Oct 25, 2021 •

edited by styfle

styfle left a comment •

edited

ihmpavel commented Nov 1, 2021 •

edited