Include message body in redirect responses (#31886)

# 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 <22380829+ijjk@users.noreply.github.com>
vercel · Dec 16, 2021 · d7062dd · d7062dd
1 parent 0f5600e
commit d7062dd
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 8 deletions.
diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts
@@ -376,7 +376,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
 
             res.statusCode = statusCode
             res.setHeader('Location', redirect.destination)
-            res.end()
+            res.end(redirect.destination)
             return null
           } else {
             sendRenderResult({

diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
@@ -547,7 +547,7 @@ export default abstract class Server {
       if (url.locale?.redirect) {
         res.setHeader('Location', url.locale.redirect)
         res.statusCode = TEMPORARY_REDIRECT_STATUS
-        res.end()
+        res.end(url.locale.redirect)
         return
       }
 
@@ -1285,7 +1285,7 @@ export default abstract class Server {
               res.setHeader('Refresh', `0;url=${location}`)
             }
 
-            res.end()
+            res.end(location)
             return {
               finished: true,
             }
@@ -1919,7 +1919,7 @@ export default abstract class Server {
 
       res.statusCode = statusCode
       res.setHeader('Location', redirect.destination)
-      res.end()
+      res.end(redirect.destination)
     }
 
     // remove /_next/data prefix from urlPathname so it matches

diff --git a/packages/next/server/web/spec-compliant/response.ts b/packages/next/server/web/spec-compliant/response.ts
@@ -45,7 +45,7 @@ class BaseResponse extends Body implements Response {
       )
     }
 
-    return new Response(null, {
+    return new Response(url, {
       headers: { Location: url },
       status,
     })

diff --git a/packages/next/server/web/spec-extension/response.ts b/packages/next/server/web/spec-extension/response.ts
@@ -79,9 +79,9 @@ export class NextResponse extends Response {
         'Failed to execute "redirect" on "response": Invalid status code'
       )
     }
-
-    return new NextResponse(null, {
-      headers: { Location: typeof url === 'string' ? url : url.toString() },
+    const destination = typeof url === 'string' ? url : url.toString()
+    return new NextResponse(destination, {
+      headers: { Location: destination },
       status,
     })
   }

diff --git a/test/development/basic-basepath/misc.test.ts b/test/development/basic-basepath/misc.test.ts
@@ -68,6 +68,8 @@ describe('misc basic dev tests', () => {
       expect(res.status).toBe(308)
       expect(pathname).toBe('/docs/%2fexample.com')
       expect(hostname).not.toBe('example.com')
+      const text = await res.text()
+      expect(text).toEqual('/docs/%2fexample.com')
     })
   })
 

diff --git a/test/development/basic/misc.test.ts b/test/development/basic/misc.test.ts
@@ -65,6 +65,8 @@ describe('misc basic dev tests', () => {
       expect(res.status).toBe(308)
       expect(pathname).toBe('/%2fexample.com')
       expect(hostname).not.toBe('example.com')
+      const text = await res.text()
+      expect(text).toEqual('/%2fexample.com')
     })
   })
 

diff --git a/test/e2e/basepath.test.ts b/test/e2e/basepath.test.ts
@@ -254,6 +254,8 @@ describe('basePath', () => {
       const { pathname } = url.parse(res.headers.get('location') || '')
       expect(pathname).toBe(`${basePath}/somewhere-else`)
       expect(res.status).toBe(307)
+      const text = await res.text()
+      expect(text).toEqual(`${basePath}/somewhere-else`)
     })
 
     it('should not redirect without basePath without disabling', async () => {
@@ -284,6 +286,8 @@ describe('basePath', () => {
       const { pathname } = url.parse(res.headers.get('location') || '')
       expect(pathname).toBe('/another-destination')
       expect(res.status).toBe(307)
+      const text = await res.text()
+      expect(text).toEqual('/another-destination')
     })
 
     //
@@ -466,6 +470,8 @@ describe('basePath', () => {
       expect(res.status).toBe(308)
       const { pathname } = new URL(res.headers.get('location'))
       expect(pathname).toBe(`${basePath}/hello`)
+      const text = await res.text()
+      expect(text).toEqual(`${basePath}/hello`)
     })
 
     it('should redirect trailing slash on root correctly', async () => {
@@ -478,6 +484,8 @@ describe('basePath', () => {
       expect(res.status).toBe(308)
       const { pathname } = new URL(res.headers.get('location'))
       expect(pathname).toBe(`${basePath}`)
+      const text = await res.text()
+      expect(text).toEqual(`${basePath}`)
     })
 
     it('should navigate an absolute url', async () => {

diff --git a/test/integration/api-catch-all/test/index.test.js b/test/integration/api-catch-all/test/index.test.js
@@ -29,6 +29,9 @@ function runTests() {
       redirect: 'manual',
     })
     expect(res.status).toBe(308)
+    const text = await res.text()
+    console.log('### ', text)
+    expect(text).toEqual('/api/users')
   })
 
   it('should return data when catch-all with index and trailing slash', async () => {

diff --git a/test/integration/custom-routes-i18n/test/index.test.js b/test/integration/custom-routes-i18n/test/index.test.js
@@ -39,6 +39,8 @@ const runTests = () => {
       expect(res.status).toBe(dest ? 307 : 404)
 
       if (dest) {
+        const text = await res.text()
+        expect(text).toEqual(dest)
         if (dest.startsWith('/')) {
           const parsed = url.parse(res.headers.get('location'))
           expect(parsed.pathname).toBe(dest)

diff --git a/test/integration/gssp-redirect-base-path/test/index.test.js b/test/integration/gssp-redirect-base-path/test/index.test.js
@@ -51,6 +51,9 @@ const runTests = (isDev) => {
     )
     expect(res.status).toBe(307)
 
+    const text = await res.text()
+    expect(text).toEqual(`/404`)
+
     const parsedUrl = url.parse(res.headers.get('location'))
     expect(parsedUrl.pathname).toBe(`/404`)
 
@@ -76,6 +79,9 @@ const runTests = (isDev) => {
     )
     expect(res.status).toBe(308)
 
+    const text = await res.text()
+    expect(text).toEqual(`${basePath}/404`)
+
     const { pathname } = url.parse(res.headers.get('location'))
 
     expect(pathname).toBe(`${basePath}/404`)
@@ -93,6 +99,9 @@ const runTests = (isDev) => {
     )
     expect(res.status).toBe(301)
 
+    const text = await res.text()
+    expect(text).toEqual(`${basePath}/404`)
+
     const { pathname } = url.parse(res.headers.get('location'))
 
     expect(pathname).toBe(`${basePath}/404`)
@@ -110,6 +119,9 @@ const runTests = (isDev) => {
     )
     expect(res.status).toBe(303)
 
+    const text = await res.text()
+    expect(text).toEqual(`${basePath}/404`)
+
     const { pathname } = url.parse(res.headers.get('location'))
 
     expect(pathname).toBe(`${basePath}/404`)
@@ -536,6 +548,8 @@ describe('GS(S)P Redirect Support', () => {
         }
       )
       expect(res1.status).toBe(307)
+      const text1 = await res1.text()
+      expect(text1).toEqual(`${basePath}/gsp-blog/first`)
       const parsed = url.parse(res1.headers.get('location'), true)
       expect(parsed.pathname).toBe(`${basePath}/gsp-blog/first`)
       expect(parsed.query).toEqual({})
@@ -550,6 +564,8 @@ describe('GS(S)P Redirect Support', () => {
         }
       )
       expect(res2.status).toBe(308)
+      const text2 = await res2.text()
+      expect(text2).toEqual(`${basePath}/gsp-blog/first`)
       expect(res2.headers.get('refresh')).toContain(
         `url=${basePath}/gsp-blog/first`
       )
@@ -566,6 +582,8 @@ describe('GS(S)P Redirect Support', () => {
         }
       )
       expect(res3.status).toBe(307)
+      const text3 = await res3.text()
+      expect(text3).toEqual(`${basePath}/gssp-blog/first`)
       expect(res3.headers.get('refresh')).toBe(null)
       const parsed3 = url.parse(res3.headers.get('location'), true)
       expect(parsed3.pathname).toBe(`${basePath}/gssp-blog/first`)
@@ -580,6 +598,8 @@ describe('GS(S)P Redirect Support', () => {
         }
       )
       expect(res4.status).toBe(308)
+      const text4 = await res4.text()
+      expect(text4).toEqual(`${basePath}/gssp-blog/first`)
       expect(res4.headers.get('refresh')).toContain(
         `url=${basePath}/gssp-blog/first`
       )