Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include message body in redirect responses #31886

Merged
merged 7 commits into from Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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({
Expand Down
6 changes: 3 additions & 3 deletions packages/next/server/base-server.ts
Expand Up @@ -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
}

Expand Down Expand Up @@ -1285,7 +1285,7 @@ export default abstract class Server {
res.setHeader('Refresh', `0;url=${location}`)
}

res.end()
res.end(location)
return {
finished: true,
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/next/server/web/spec-compliant/response.ts
Expand Up @@ -45,7 +45,7 @@ class BaseResponse extends Body implements Response {
)
}

return new Response(null, {
return new Response(url, {
headers: { Location: url },
status,
})
Expand Down
6 changes: 3 additions & 3 deletions packages/next/server/web/spec-extension/response.ts
Expand Up @@ -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,
})
}
Expand Down
2 changes: 2 additions & 0 deletions test/development/basic-basepath/misc.test.ts
Expand Up @@ -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')
})
})

Expand Down
2 changes: 2 additions & 0 deletions test/development/basic/misc.test.ts
Expand Up @@ -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')
})
})

Expand Down
8 changes: 8 additions & 0 deletions test/e2e/basepath.test.ts
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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')
})

//
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
3 changes: 3 additions & 0 deletions test/integration/api-catch-all/test/index.test.js
Expand Up @@ -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 () => {
Expand Down
2 changes: 2 additions & 0 deletions test/integration/custom-routes-i18n/test/index.test.js
Expand Up @@ -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)
Expand Down
20 changes: 20 additions & 0 deletions test/integration/gssp-redirect-base-path/test/index.test.js
Expand Up @@ -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`)

Expand All @@ -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`)
Expand All @@ -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`)
Expand All @@ -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`)
Expand Down Expand Up @@ -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({})
Expand All @@ -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`
)
Expand All @@ -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`)
Expand All @@ -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`
)
Expand Down