archive/rsnext
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Ensure redirects are handled properly from cache (#18806)

Browse source 
This updates the GS(S)P redirect handling to make sure to handle redirects correctly when coming from the incremental-cache. Additional tests have been added to ensure the redirects work correctly after the cache is populated. 

Fixes: https://github.com/vercel/next.js/issues/18735
Fixes: https://github.com/vercel/next.js/issues/18783
This commit is contained in:
JJ Kasper 2020-11-04 16:18:44 -06:00 committed by GitHub
parent 8f7f1018d2
commit a5cb28d907
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 313 additions and 96 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
packages/next/build/webpack/loaders/next-serverless-loader.ts
View file

@ -477,6 +477,8 @@ const nextServerlessLoader: loader.Loader = function () {
const { denormalizePagePath } = require('next/dist/next-server/server/denormalize-page-path')
const { setLazyProp, getCookieParser } = require('next/dist/next-server/server/api-utils')
const {sendPayload} = require('next/dist/next-server/server/send-payload');
const {getRedirectStatus} = require('next/dist/lib/load-custom-routes');
const {PERMANENT_REDIRECT_STATUS} = require('next/dist/next-server/lib/constants')
const buildManifest = require('${buildManifest}');
const reactLoadableManifest = require('${reactLoadableManifest}');
@ -819,6 +821,21 @@ const nextServerlessLoader: loader.Loader = function () {
revalidate: renderOpts.revalidate,
})
return null
} else if (renderOpts.isRedirect && !_nextData) {
const redirect = {
destination: renderOpts.pageData.pageProps.__N_REDIRECT,
statusCode: renderOpts.pageData.pageProps.__N_REDIRECT_STATUS
}
const statusCode = getRedirectStatus(redirect)
if (statusCode === PERMANENT_REDIRECT_STATUS) {
res.setHeader('Refresh', \`0;url=\${redirect.destination}\`)
}
res.statusCode = statusCode
res.setHeader('Location', redirect.destination)
res.end()
return null
} else {
sendPayload(req, res, _nextData ? JSON.stringify(renderOpts.pageData) : result, _nextData ? 'json' : 'html', ${
generateEtags === 'true' ? true : false

5
packages/next/lib/load-custom-routes.ts
View file

@ -24,7 +24,10 @@ export type Header = {
export const allowedStatusCodes = new Set([301, 302, 303, 307, 308])
export function getRedirectStatus(route: Redirect): number {
export function getRedirectStatus(route: {
statusCode?: number
permanent?: boolean
}): number {
return (
route.statusCode ||
(route.permanent ? PERMANENT_REDIRECT_STATUS : TEMPORARY_REDIRECT_STATUS)

4
packages/next/next-server/server/incremental-cache.ts
View file

@ -14,6 +14,7 @@ type IncrementalCacheValue = {
pageData?: any
isStale?: boolean
isNotFound?: boolean
isRedirect?: boolean
curRevalidate?: number | false
// milliseconds to revalidate after
revalidateAfter: number | false
@ -69,7 +70,7 @@ export class IncrementalCache {
// default to 50MB limit
max: max || 50 * 1024 * 1024,
length(val) {
if (val.isNotFound) return 25
if (val.isNotFound || val.isRedirect) return 25
// rough estimate of size of cache value
return val.html!.length + JSON.stringify(val.pageData).length
},
@ -161,6 +162,7 @@ export class IncrementalCache {
html?: string
pageData?: any
isNotFound?: boolean
isRedirect?: boolean
},
revalidateSeconds?: number | false
) {

109
packages/next/next-server/server/next-server.ts
View file

@ -26,6 +26,7 @@ import {
CLIENT_STATIC_FILES_PATH,
CLIENT_STATIC_FILES_RUNTIME,
PAGES_MANIFEST,
PERMANENT_REDIRECT_STATUS,
PHASE_PRODUCTION_SERVER,
PRERENDER_MANIFEST,
ROUTES_MANIFEST,
@ -1263,6 +1264,22 @@ export default class Server {
return path
}
const handleRedirect = (pageData: any) => {
const redirect = {
destination: pageData.pageProps.__N_REDIRECT,
statusCode: pageData.pageProps.__N_REDIRECT_STATUS,
}
const statusCode = getRedirectStatus(redirect)
if (statusCode === PERMANENT_REDIRECT_STATUS) {
res.setHeader('Refresh', `0;url=${redirect.destination}`)
}
res.statusCode = statusCode
res.setHeader('Location', redirect.destination)
res.end()
}
// remove /_next/data prefix from urlPathname so it matches
// for direct page visit and /_next/data visit
if (isDataReq) {
@ -1299,26 +1316,30 @@ export default class Server {
? JSON.stringify(cachedData.pageData)
: cachedData.html
sendPayload(
req,
res,
data,
isDataReq ? 'json' : 'html',
{
generateEtags: this.renderOpts.generateEtags,
poweredByHeader: this.renderOpts.poweredByHeader,
},
!this.renderOpts.dev
? {
private: isPreviewMode,
stateful: false, // GSP response
revalidate:
cachedData.curRevalidate !== undefined
? cachedData.curRevalidate
: /* default to minimum revalidate (this should be an invariant) */ 1,
}
: undefined
)
if (!isDataReq && cachedData.pageData?.pageProps?.__N_REDIRECT) {
await handleRedirect(cachedData.pageData)
} else {
sendPayload(
req,
res,
data,
isDataReq ? 'json' : 'html',
{
generateEtags: this.renderOpts.generateEtags,
poweredByHeader: this.renderOpts.poweredByHeader,
},
!this.renderOpts.dev
? {
private: isPreviewMode,
stateful: false, // GSP response
revalidate:
cachedData.curRevalidate !== undefined
? cachedData.curRevalidate
: /* default to minimum revalidate (this should be an invariant) */ 1,
}
: undefined
)
}
// Stop the request chain here if the data we sent was up-to-date
if (!cachedData.isStale) {
@ -1340,11 +1361,13 @@ export default class Server {
pageData: any
sprRevalidate: number | false
isNotFound?: boolean
isRedirect?: boolean
}> => {
let pageData: any
let html: string | null
let sprRevalidate: number | false
let isNotFound: boolean | undefined
let isRedirect: boolean | undefined
let renderResult
// handle serverless
@ -1365,6 +1388,7 @@ export default class Server {
pageData = renderResult.renderOpts.pageData
sprRevalidate = renderResult.renderOpts.revalidate
isNotFound = renderResult.renderOpts.isNotFound
isRedirect = renderResult.renderOpts.isRedirect
} else {
const origQuery = parseUrl(req.url || '', true).query
const resolvedUrl = formatUrl({
@ -1407,9 +1431,10 @@ export default class Server {
pageData = (renderOpts as any).pageData
sprRevalidate = (renderOpts as any).revalidate
isNotFound = (renderOpts as any).isNotFound
isRedirect = (renderOpts as any).isRedirect
}
return { html, pageData, sprRevalidate, isNotFound }
return { html, pageData, sprRevalidate, isNotFound, isRedirect }
}
)
@ -1491,7 +1516,7 @@ export default class Server {
const {
isOrigin,
value: { html, pageData, sprRevalidate, isNotFound },
value: { html, pageData, sprRevalidate, isNotFound, isRedirect },
} = await doRender()
let resHtml = html
@ -1500,23 +1525,27 @@ export default class Server {
!isNotFound &&
(isSSG || isDataReq || isServerProps)
) {
sendPayload(
req,
res,
isDataReq ? JSON.stringify(pageData) : html,
isDataReq ? 'json' : 'html',
{
generateEtags: this.renderOpts.generateEtags,
poweredByHeader: this.renderOpts.poweredByHeader,
},
!this.renderOpts.dev || (isServerProps && !isDataReq)
? {
private: isPreviewMode,
stateful: !isSSG,
revalidate: sprRevalidate,
}
: undefined
)
if (isRedirect && !isDataReq) {
await handleRedirect(pageData)
} else {
sendPayload(
req,
res,
isDataReq ? JSON.stringify(pageData) : html,
isDataReq ? 'json' : 'html',
{
generateEtags: this.renderOpts.generateEtags,
poweredByHeader: this.renderOpts.poweredByHeader,
},
!this.renderOpts.dev || (isServerProps && !isDataReq)
? {
private: isPreviewMode,
stateful: !isSSG,
revalidate: sprRevalidate,
}
: undefined
)
}
resHtml = null
}
@ -1524,7 +1553,7 @@ export default class Server {
if (isOrigin && ssgCacheKey) {
await this.incrementalCache.set(
ssgCacheKey,
{ html: html!, pageData, isNotFound },
{ html: html!, pageData, isNotFound, isRedirect },
sprRevalidate
)
}

60
packages/next/next-server/server/render.tsx
View file

@ -20,10 +20,8 @@ import { isInAmpMode } from '../lib/amp'
import { AmpStateContext } from '../lib/amp-context'
import {
AMP_RENDER_TARGET,
PERMANENT_REDIRECT_STATUS,
SERVER_PROPS_ID,
STATIC_PROPS_ID,
TEMPORARY_REDIRECT_STATUS,
} from '../lib/constants'
import { defaultHead } from '../lib/head'
import { HeadManagerContext } from '../lib/head-manager-context'
@ -52,7 +50,10 @@ import { FontManifest, getFontDefinitionFromManifest } from './font-utils'
import { LoadComponentsReturnType, ManifestItem } from './load-components'
import { normalizePagePath } from './normalize-page-path'
import optimizeAmp from './optimize-amp'
import { allowedStatusCodes } from '../../lib/load-custom-routes'
import {
allowedStatusCodes,
getRedirectStatus,
} from '../../lib/load-custom-routes'
function noRouter() {
const message =
@ -351,22 +352,6 @@ function checkRedirectValues(
}
}
function handleRedirect(res: ServerResponse, redirect: Redirect) {
const statusCode = redirect.statusCode
? redirect.statusCode
: redirect.permanent
? PERMANENT_REDIRECT_STATUS
: TEMPORARY_REDIRECT_STATUS
if (statusCode === PERMANENT_REDIRECT_STATUS) {
res.setHeader('Refresh', `0;url=${redirect.destination}`)
}
res.statusCode = statusCode
res.setHeader('Location', redirect.destination)
res.end()
}
export async function renderToHTML(
req: IncomingMessage,
res: ServerResponse,
@ -660,6 +645,16 @@ export async function renderToHTML(
throw new Error(invalidKeysMsg('getStaticProps', invalidKeys))
}
if (process.env.NODE_ENV !== 'production') {
if ('notFound' in data && 'redirect' in data) {
throw new Error(
`\`redirect\` and \`notFound\` can not both be returned from ${
isSSG ? 'getStaticProps' : 'getServerSideProps'
} at the same time. Page: ${pathname}`
)
}
}
if ('notFound' in data && data.notFound) {
if (pathname === '/404') {
throw new Error(
@ -686,14 +681,11 @@ export async function renderToHTML(
)
}
if (isDataReq) {
;(data as any).props = {
__N_REDIRECT: data.redirect.destination,
}
} else {
handleRedirect(res, data.redirect)
return null
;(data as any).props = {
__N_REDIRECT: data.redirect.destination,
__N_REDIRECT_STATUS: getRedirectStatus(data.redirect),
}
;(renderOpts as any).isRedirect = true
}
if (
@ -815,15 +807,11 @@ export async function renderToHTML(
if ('redirect' in data && typeof data.redirect === 'object') {
checkRedirectValues(data.redirect, req, 'getServerSideProps')
if (isDataReq) {
;(data as any).props = {
__N_REDIRECT: data.redirect.destination,
}
} else {
handleRedirect(res, data.redirect)
return null
;(data as any).props = {
__N_REDIRECT: data.redirect.destination,
__N_REDIRECT_STATUS: getRedirectStatus(data.redirect),
}
;(renderOpts as any).isRedirect = true
}
if (
@ -864,7 +852,9 @@ export async function renderToHTML(
// Avoid rendering page un-necessarily for getServerSideProps data request
// and getServerSideProps/getStaticProps redirects
if (isDataReq && (!isSSG || props.pageProps.__N_REDIRECT)) return props
if ((isDataReq && !isSSG) || (renderOpts as any).isRedirect) {
return props
}
// We don't call getStaticProps or getServerSideProps while generating
// the fallback so make sure to set pageProps to an empty object

4
test/integration/gssp-redirect/pages/index.js
View file

@ -1,3 +1,7 @@
export default function Index() {
if (typeof window !== 'undefined' && !window.initialHref) {
window.initialHref = window.location.href
}
return <p id="index">Index Page</p>
}

207
test/integration/gssp-redirect/test/index.test.js
View file

@ -1,5 +1,5 @@
/* eslint-env jest */
import http from 'http'
import url from 'url'
import fs from 'fs-extra'
import webdriver from 'next-webdriver'
@ -21,7 +21,7 @@ const nextConfig = join(appDir, 'next.config.js')
let app
let appPort
const runTests = () => {
const runTests = (isDev) => {
it('should apply temporary redirect when visited directly for GSSP page', async () => {
const res = await fetchViaHTTP(
appPort,
@ -92,7 +92,9 @@ const runTests = () => {
it('should apply redirect when fallback GSP page is visited directly (internal dynamic)', async () => {
const browser = await webdriver(
appPort,
'/gsp-blog/redirect-dest-_gsp-blog_first'
'/gsp-blog/redirect-dest-_gsp-blog_first',
true,
true
)
await browser.waitForElementByCss('#gsp')
@ -108,8 +110,38 @@ const runTests = () => {
expect(pathname).toBe('/gsp-blog/redirect-dest-_gsp-blog_first')
})
if (!isDev) {
it('should apply redirect when fallback GSP page is visited directly (internal dynamic) 2nd visit', async () => {
const browser = await webdriver(
appPort,
'/gsp-blog/redirect-dest-_gsp-blog_first',
true,
true
)
await browser.waitForElementByCss('#gsp')
const props = JSON.parse(await browser.elementByCss('#props').text())
expect(props).toEqual({
params: {
post: 'first',
},
})
const initialHref = await browser.eval(() => window.initialHref)
const { pathname } = url.parse(initialHref)
// since it was cached the initial value is now the redirect
// result
expect(pathname).toBe('/gsp-blog/first')
})
}
it('should apply redirect when fallback GSP page is visited directly (internal normal)', async () => {
const browser = await webdriver(appPort, '/gsp-blog/redirect-dest-_')
const browser = await webdriver(
appPort,
'/gsp-blog/redirect-dest-_',
true,
true
)
await browser.waitForElementByCss('#index')
@ -118,8 +150,30 @@ const runTests = () => {
expect(pathname).toBe('/gsp-blog/redirect-dest-_')
})
if (!isDev) {
it('should apply redirect when fallback GSP page is visited directly (internal normal) 2nd visit', async () => {
const browser = await webdriver(
appPort,
'/gsp-blog/redirect-dest-_',
true,
true
)
await browser.waitForElementByCss('#index')
const initialHref = await browser.eval(() => window.initialHref)
const { pathname } = url.parse(initialHref)
expect(pathname).toBe('/')
})
}
it('should apply redirect when fallback GSP page is visited directly (external)', async () => {
const browser = await webdriver(appPort, '/gsp-blog/redirect-dest-_missing')
const browser = await webdriver(
appPort,
'/gsp-blog/redirect-dest-_missing',
true,
true
)
await check(
() => browser.eval(() => document.documentElement.innerHTML),
@ -137,7 +191,9 @@ const runTests = () => {
it('should apply redirect when GSSP page is navigated to client-side (internal dynamic)', async () => {
const browser = await webdriver(
appPort,
'/gssp-blog/redirect-dest-_gssp-blog_first'
'/gssp-blog/redirect-dest-_gssp-blog_first',
true,
true
)
await browser.waitForElementByCss('#gssp')
@ -151,7 +207,7 @@ const runTests = () => {
})
it('should apply redirect when GSSP page is navigated to client-side (internal normal)', async () => {
const browser = await webdriver(appPort, '/')
const browser = await webdriver(appPort, '/', true, true)
await browser.eval(`(function () {
window.next.router.push('/gssp-blog/redirect-dest-_another')
@ -164,7 +220,7 @@ const runTests = () => {
})
it('should apply redirect when GSSP page is navigated to client-side (external)', async () => {
const browser = await webdriver(appPort, '/')
const browser = await webdriver(appPort, '/', true, true)
await browser.eval(`(function () {
window.next.router.push('/gssp-blog/redirect-dest-_gssp-blog_first')
@ -181,7 +237,7 @@ const runTests = () => {
})
it('should apply redirect when GSP page is navigated to client-side (internal)', async () => {
const browser = await webdriver(appPort, '/')
const browser = await webdriver(appPort, '/', true, true)
await browser.eval(`(function () {
window.next.router.push('/gsp-blog/redirect-dest-_another')
@ -194,7 +250,7 @@ const runTests = () => {
})
it('should apply redirect when GSP page is navigated to client-side (external)', async () => {
const browser = await webdriver(appPort, '/')
const browser = await webdriver(appPort, '/', true, true)
await browser.eval(`(function () {
window.next.router.push('/gsp-blog/redirect-dest-_gsp-blog_first')
@ -211,10 +267,15 @@ const runTests = () => {
})
it('should not replace history of the origin page when GSSP page is navigated to client-side (internal normal)', async () => {
const browser = await webdriver(appPort, '/another?mark_as=root')
const browser = await webdriver(
appPort,
'/another?mark_as=root',
true,
true
)
await browser.eval(`(function () {
window.location.href = '/'
window.next.router.push('/')
})()`)
await browser.waitForElementByCss('#index')
@ -233,10 +294,15 @@ const runTests = () => {
})
it('should not replace history of the origin page when GSSP page is navigated to client-side (external)', async () => {
const browser = await webdriver(appPort, '/another?mark_as=root')
const browser = await webdriver(
appPort,
'/another?mark_as=root',
true,
true
)
await browser.eval(`(function () {
window.location.href = '/'
window.next.router.push('/')
})()`)
await browser.waitForElementByCss('#index')
@ -255,10 +321,15 @@ const runTests = () => {
})
it('should not replace history of the origin page when GSP page is navigated to client-side (internal)', async () => {
const browser = await webdriver(appPort, '/another?mark_as=root')
const browser = await webdriver(
appPort,
'/another?mark_as=root',
true,
true
)
await browser.eval(`(function () {
window.location.href = '/'
window.next.router.push('/')
})()`)
await browser.waitForElementByCss('#index')
@ -277,10 +348,15 @@ const runTests = () => {
})
it('should not replace history of the origin page when GSP page is navigated to client-side (external)', async () => {
const browser = await webdriver(appPort, '/another?mark_as=root')
const browser = await webdriver(
appPort,
'/another?mark_as=root',
true,
true
)
await browser.eval(`(function () {
window.location.href = '/'
window.next.router.push('/')
})()`)
await browser.waitForElementByCss('#index')
@ -307,7 +383,7 @@ describe('GS(S)P Redirect Support', () => {
})
afterAll(() => killApp(app))
runTests()
runTests(true)
})
describe('production mode', () => {
@ -323,6 +399,8 @@ describe('GS(S)P Redirect Support', () => {
})
describe('serverless mode', () => {
let server
beforeAll(async () => {
await fs.writeFile(
nextConfig,
@ -338,6 +416,97 @@ describe('GS(S)P Redirect Support', () => {
afterAll(async () => {
await fs.remove(nextConfig)
await killApp(app)
try {
server.close()
} catch (err) {
console.error('failed to close server', err)
}
})
it('should handle redirect in raw serverless mode correctly', async () => {
server = http.createServer(async (req, res) => {
try {
console.log(req.url)
if (req.url.includes('/gsp-blog')) {
await require(join(
appDir,
'.next/serverless/pages/gsp-blog/[post].js'
)).render(req, res)
} else {
await require(join(
appDir,
'./.next/serverless/pages/gssp-blog/[post].js'
)).render(req, res)
}
} catch (err) {
console.error('failed to render', err)
res.statusCode = 500
res.end('error')
}
})
const port = await findPort()
await new Promise((resolve, reject) => {
server.listen(port, (err) => (err ? reject(err) : resolve()))
})
console.log(`Raw serverless server listening at port ${port}`)
const res1 = await fetchViaHTTP(
port,
'/gsp-blog/redirect-dest-_gsp-blog_first',
undefined,
{
redirect: 'manual',
}
)
expect(res1.status).toBe(307)
const parsed = url.parse(res1.headers.get('location'), true)
expect(parsed.pathname).toBe('/gsp-blog/first')
expect(parsed.query).toEqual({})
expect(res1.headers.get('refresh')).toBe(null)
const res2 = await fetchViaHTTP(
port,
'/gsp-blog/redirect-permanent-dest-_gsp-blog_first',
undefined,
{
redirect: 'manual',
}
)
expect(res2.status).toBe(308)
expect(res2.headers.get('refresh')).toMatch(/url=\/gsp-blog\/first/)
const parsed2 = url.parse(res2.headers.get('location'), true)
expect(parsed2.pathname).toBe('/gsp-blog/first')
expect(parsed2.query).toEqual({})
const res3 = await fetchViaHTTP(
port,
'/gssp-blog/redirect-dest-_gssp-blog_first',
undefined,
{
redirect: 'manual',
}
)
expect(res3.status).toBe(307)
expect(res3.headers.get('refresh')).toBe(null)
const parsed3 = url.parse(res3.headers.get('location'), true)
expect(parsed3.pathname).toBe('/gssp-blog/first')
expect(parsed3.query).toEqual({})
const res4 = await fetchViaHTTP(
port,
'/gssp-blog/redirect-permanent-dest-_gssp-blog_first',
undefined,
{
redirect: 'manual',
}
)
expect(res4.status).toBe(308)
expect(res4.headers.get('refresh')).toMatch(/url=\/gssp-blog\/first/)
const parsed4 = url.parse(res4.headers.get('location'), true)
expect(parsed4.pathname).toBe('/gssp-blog/first')
expect(parsed4.query).toEqual({})
})
runTests()

3
test/lib/next-webdriver.js
View file

@ -7,6 +7,7 @@ import { Builder, By } from 'selenium-webdriver'
import { Options as ChromeOptions } from 'selenium-webdriver/chrome'
import { Options as SafariOptions } from 'selenium-webdriver/safari'
import { Options as FireFoxOptions } from 'selenium-webdriver/firefox'
import { waitFor } from 'next-test-utils'
const {
BROWSER_NAME: browserName = 'chrome',
@ -210,8 +211,10 @@ export default async (
} catch (err) {
if (allowHydrationRetry) {
// re-try in case the page reloaded during check
await waitFor(2000)
await checkHydrated()
} else {
console.error('failed to check hydration')
throw err
}
}