From 2179959609395b158558ef6537c207c46352e125 Mon Sep 17 00:00:00 2001 From: Michiel Van Gendt Date: Wed, 24 Nov 2021 18:31:50 +0100 Subject: [PATCH 1/5] Include message body in redirect responses --- .../webpack/loaders/next-serverless-loader/page-handler.ts | 2 +- packages/next/server/next-server.ts | 6 +++--- packages/next/server/web/spec-compliant/response.ts | 2 +- packages/next/server/web/spec-extension/response.ts | 6 +++--- 4 files changed, 8 insertions(+), 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 index ec162b4e5a9a1..3ddbc18eee297 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts @@ -372,7 +372,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/next-server.ts b/packages/next/server/next-server.ts index 24a226a18a9b8..3541e9617541b 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -522,7 +522,7 @@ export default 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 } @@ -1228,7 +1228,7 @@ export default class Server { res.setHeader('Refresh', `0;url=${location}`) } - res.end() + res.end(location) return { finished: true, } @@ -1922,7 +1922,7 @@ export default 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 index 23be2f7318675..c7d5c746e9316 100644 --- 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 index 9e4fcc42aef10..aa089b609d0d8 100644 --- 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, }) } From 7818ef80ddfe7cd2b40a7beae8e6aec0c67d4a45 Mon Sep 17 00:00:00 2001 From: Michiel Van Gendt Date: Sun, 28 Nov 2021 14:57:44 +0100 Subject: [PATCH 2/5] Add tests for message body in redirect responses --- test/development/basic-basepath/misc.test.ts | 2 ++ test/development/basic/misc.test.ts | 2 ++ test/e2e/basepath.test.ts | 8 ++++++++ test/integration/api-catch-all/next.config.js | 1 + .../api-catch-all/test/index.test.js | 3 +++ .../custom-routes-i18n/test/index.test.js | 2 ++ .../test/index.test.js | 20 +++++++++++++++++++ 7 files changed, 38 insertions(+) create mode 100644 test/integration/api-catch-all/next.config.js diff --git a/test/development/basic-basepath/misc.test.ts b/test/development/basic-basepath/misc.test.ts index cabec1f3ffdd9..69caa032485b8 100644 --- 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 index c1b67d1e00e84..c811fee0cdf84 100644 --- 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 index 9c6234b3802f5..a9fcf887f19de 100644 --- 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/next.config.js b/test/integration/api-catch-all/next.config.js new file mode 100644 index 0000000000000..91c68aa65f28d --- /dev/null +++ b/test/integration/api-catch-all/next.config.js @@ -0,0 +1 @@ +module.exports = { target: 'serverless' } diff --git a/test/integration/api-catch-all/test/index.test.js b/test/integration/api-catch-all/test/index.test.js index 7278099f7ca73..86e5308c4926a 100644 --- 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 index eb442d7c886e4..26a56f9d59ee9 100644 --- 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 index d7bfe63d01b85..deeac7b875581 100644 --- 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}/401`) + 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` ) From c9bb7190eb6c810f5f544db741e1a3220f12978c Mon Sep 17 00:00:00 2001 From: Michiel Van Gendt Date: Sun, 28 Nov 2021 15:05:17 +0100 Subject: [PATCH 3/5] Delete next.config.js --- test/integration/api-catch-all/next.config.js | 1 - 1 file changed, 1 deletion(-) delete mode 100644 test/integration/api-catch-all/next.config.js diff --git a/test/integration/api-catch-all/next.config.js b/test/integration/api-catch-all/next.config.js deleted file mode 100644 index 91c68aa65f28d..0000000000000 --- a/test/integration/api-catch-all/next.config.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = { target: 'serverless' } From 3721d06103d57139ea7ef159caa4ca5ab559534f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 15 Dec 2021 22:57:57 -0600 Subject: [PATCH 4/5] lint-fix --- packages/next/server/web/spec-extension/response.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/web/spec-extension/response.ts b/packages/next/server/web/spec-extension/response.ts index 1ac1665bbb19c..76b40a8490407 100644 --- a/packages/next/server/web/spec-extension/response.ts +++ b/packages/next/server/web/spec-extension/response.ts @@ -81,7 +81,7 @@ export class NextResponse extends Response { } const destination = typeof url === 'string' ? url : url.toString() return new NextResponse(destination, { - headers: { Location: destination}, + headers: { Location: destination }, status, }) } From 61391d81e033e4ede5baf51efd14ac40357dad63 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 15 Dec 2021 23:00:54 -0600 Subject: [PATCH 5/5] update test --- test/integration/gssp-redirect-base-path/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/gssp-redirect-base-path/test/index.test.js b/test/integration/gssp-redirect-base-path/test/index.test.js index deeac7b875581..71c4c0fe0c757 100644 --- a/test/integration/gssp-redirect-base-path/test/index.test.js +++ b/test/integration/gssp-redirect-base-path/test/index.test.js @@ -100,7 +100,7 @@ const runTests = (isDev) => { expect(res.status).toBe(301) const text = await res.text() - expect(text).toEqual(`${basePath}/401`) + expect(text).toEqual(`${basePath}/404`) const { pathname } = url.parse(res.headers.get('location'))