Include message body in redirect responses #25257

michielvangendt · 2021-05-19T10:43:02Z

Description

The redirect responses from the redirect function do not contain a message body. This is in conflict with the RFCs below and causes Traefik (a reverse proxy) to invalidate the responses. In this pull request, I add a response body to the redirect responses.

References

https://datatracker.ietf.org/doc/html/rfc7230#section-3.3

All 1xx (Informational), 204 (No Content), and 304 (Not Modified) responses must not include a message-body. All other responses do include a message-body, although the body may be of zero length.

https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.3

The server's response payload usually contains a short hypertext note with a hyperlink to the different URI(s).

timneutkens

Can you add an integration test for this in test/integration, then it can be landed 👍

michielvangendt · 2021-05-29T13:12:47Z

@timneutkens I've added a check for this in 2 integration tests, does that suffice?

ijjk · 2021-06-22T12:02:54Z

Failing test suites

Commit: 5c8f75a

test/integration/basic/test/index.test.js

Basic Features > should polyfill Node.js modules

Expand output

● Basic Features › should polyfill Node.js modules

thrown: "Exceeded timeout of 300000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  35 |   afterAll(() => killApp(context.server))
  36 |
> 37 |   it('should polyfill Node.js modules', async () => {
     |   ^
  38 |     const browser = await webdriver(context.appPort, '/node-browser-polyfills')
  39 |
  40 |     console.error({

  at integration/basic/test/index.test.js:37:3
  at Object.<anonymous> (integration/basic/test/index.test.js:18:1)

styfle

styfle

Thanks!

tests added

ijjk · 2021-07-09T16:07:42Z

Stats from current PR

Default Build (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	michielvangendt/next.js canary	Change
buildDuration	12.1s	12.2s	⚠️ +139ms
buildDurationCached	2.9s	2.8s	-57ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +3 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	michielvangendt/next.js canary	Change
/ failed reqs	0	0	✓
/ total time (seconds)	1.954	2.076	⚠️ +0.12
/ avg req/sec	1279.37	1204.4	⚠️ -74.97
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.12	1.115	-0.01
/error-in-render avg req/sec	2232.22	2241.93	+9.71

Client Bundles (main, webpack, commons)

	vercel/next.js canary	michielvangendt/next.js canary	Change
359.HASH.js gzip	3.09 kB	3.09 kB	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	20.6 kB	20.6 kB	✓
webpack-HASH.js gzip	1.49 kB	1.49 kB	✓
Overall change	67.2 kB	67.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	michielvangendt/next.js canary	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	michielvangendt/next.js canary	Change
_app-HASH.js gzip	803 B	803 B	✓
_error-HASH.js gzip	3.18 kB	3.18 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	329 B	329 B	✓
hooks-HASH.js gzip	903 B	903 B	✓
image-HASH.js gzip	5.69 kB	5.69 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	1.66 kB	1.66 kB	✓
routerDirect..HASH.js gzip	319 B	319 B	✓
withRouter-HASH.js gzip	320 B	320 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	14.1 kB	14.1 kB	✓

Client Build Manifests

	vercel/next.js canary	michielvangendt/next.js canary	Change
_buildManifest.js gzip	419 B	419 B	✓
Overall change	419 B	419 B	✓

Rendered Page Sizes

	vercel/next.js canary	michielvangendt/next.js canary	Change
index.html gzip	531 B	531 B	✓
link.html gzip	544 B	544 B	✓
withRouter.html gzip	524 B	524 B	✓
Overall change	1.6 kB	1.6 kB	✓

Webpack 4 Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	michielvangendt/next.js canary	Change
buildDuration	9.7s	9.7s	-71ms
buildDurationCached	4.1s	3.7s	-363ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +3 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	michielvangendt/next.js canary	Change
/ failed reqs	0	0	✓
/ total time (seconds)	1.974	1.939	-0.03
/ avg req/sec	1266.51	1289.57	+23.06
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.09	1.173	⚠️ +0.08
/error-in-render avg req/sec	2292.73	2131.73	⚠️ -161

Client Bundles (main, webpack, commons)

	vercel/next.js canary	michielvangendt/next.js canary	Change
14.HASH.js gzip	3.11 kB	3.11 kB	✓
677f882d2ed8..HASH.js gzip	13.9 kB	13.9 kB	✓
framework.HASH.js gzip	41.8 kB	41.8 kB	✓
main-HASH.js gzip	7.81 kB	7.81 kB	✓
webpack-HASH.js gzip	1.19 kB	1.19 kB	✓
Overall change	67.8 kB	67.8 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	michielvangendt/next.js canary	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	michielvangendt/next.js canary	Change
_app-HASH.js gzip	791 B	791 B	✓
_error-HASH.js gzip	3.83 kB	3.83 kB	✓
amp-HASH.js gzip	531 B	531 B	✓
css-HASH.js gzip	333 B	333 B	✓
hooks-HASH.js gzip	910 B	910 B	✓
index-HASH.js gzip	230 B	230 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	297 B	297 B	✓
withRouter-HASH.js gzip	293 B	293 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	8.98 kB	8.98 kB	✓

Client Build Manifests

	vercel/next.js canary	michielvangendt/next.js canary	Change
_buildManifest.js gzip	418 B	418 B	✓
Overall change	418 B	418 B	✓

Rendered Page Sizes

	vercel/next.js canary	michielvangendt/next.js canary	Change
index.html gzip	574 B	574 B	✓
link.html gzip	586 B	586 B	✓
withRouter.html gzip	569 B	569 B	✓
Overall change	1.73 kB	1.73 kB	✓

Commit: 070a6dd

### Description The redirect responses from the redirect function do not contain a message body. This is in conflict with the RFCs below and causes Traefik (a reverse proxy) to invalidate the responses. In this pull request, I add a response body to the redirect responses. ### References - https://datatracker.ietf.org/doc/html/rfc7230#section-3.3 > All 1xx (Informational), 204 (No Content), and 304 (Not Modified) responses must not include a message-body. All other responses do include a message-body, although the body may be of zero length. - https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.3 > The server's response payload usually contains a short hypertext note with a hyperlink to the different URI(s). - traefik/traefik#4456 - auth0/nextjs-auth0#399

# Description The redirect responses do not contain a message body. This is in conflict with the RFCs (below) and causes Traefik (a reverse proxy) to invalidate the responses. In this pull request, I add a response body to the redirect responses. This PR is similar to #25257, it appears that there are some other locations where redirection is handled incorrectly in next.js. # References - https://datatracker.ietf.org/doc/html/rfc7230#section-3.3 > All 1xx (Informational), 204 (No Content), and 304 (Not Modified) responses must not include a message-body. All other responses do include a message-body, although the body may be of zero length. - https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.3 > The server's response payload usually contains a short hypertext note with a hyperlink to the different URI(s). Co-authored-by: JJ Kasper <[email protected]>

Include message body in redirect responses

dcdeded

michielvangendt requested review from divmain, ijjk, lfades, shuding and timneutkens as code owners May 19, 2021 10:43

ijjk added the type: next label May 19, 2021

This comment has been minimized.

Sign in to view

aarsxx previously approved these changes May 21, 2021

View reviewed changes

Merge branch 'canary' into canary

d286359

This comment has been minimized.

Sign in to view

timneutkens previously requested changes May 22, 2021

View reviewed changes

adamjmcgrath mentioned this pull request May 27, 2021

Include message body in 302 responses auth0/nextjs-auth0#399

Closed

michielvangendt and others added 2 commits May 29, 2021 11:55

Merge branch 'vercel:canary' into canary

f4186ac

Add iontegration tests for redirect fix

88deb83

michielvangendt dismissed aarsxx’s stale review via 88deb83 May 29, 2021 13:11

This comment has been minimized.

Sign in to view

Merge branch 'canary' into canary

5c8f75a

michielvangendt requested a review from timneutkens June 22, 2021 07:08

This comment has been minimized.

Sign in to view

Merge branch 'canary' into canary

24daab7

This comment has been minimized.

Sign in to view

Merge branch 'canary' of https://github.com/vercel/next.js into canary

50d6884

michielvangendt requested review from huozhi and padmaia as code owners July 8, 2021 16:34

michielvangendt requested a review from styfle as a code owner July 8, 2021 16:34

styfle reviewed Jul 8, 2021

View reviewed changes

styfle approved these changes Jul 8, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

ijjk approved these changes Jul 9, 2021

View reviewed changes

Merge branch 'canary' into canary

070a6dd

kodiakhq bot merged commit d22ecd9 into vercel:canary Jul 9, 2021

michielvangendt mentioned this pull request Nov 28, 2021

Include message body in redirect responses #31886

Merged

michielvangendt mentioned this pull request Dec 21, 2021

Include message body in 302 responses auth0/nextjs-auth0#564

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include message body in redirect responses #25257

Include message body in redirect responses #25257

michielvangendt commented May 19, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

timneutkens left a comment

michielvangendt commented May 29, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jun 22, 2021

This comment has been minimized.

styfle left a comment •

edited

Loading

styfle left a comment

This comment has been minimized.

ijjk commented Jul 9, 2021

Include message body in redirect responses #25257

Include message body in redirect responses #25257

Conversation

michielvangendt commented May 19, 2021

Description

References

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

timneutkens left a comment

Choose a reason for hiding this comment

michielvangendt commented May 29, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jun 22, 2021

Failing test suites

This comment has been minimized.

styfle left a comment • edited Loading

Choose a reason for hiding this comment

styfle left a comment

Choose a reason for hiding this comment

This comment has been minimized.

ijjk commented Jul 9, 2021

Stats from current PR

styfle left a comment •

edited

Loading