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

Fix trailing slash redirect applying for data request (#45417)

Browse source 
This ensures we don't apply the trailing slash redirect for `_next/data`
requests as it can cause props to fail to resolve on client transition.
This also fixes `missing` fields not being applied correctly for
`headers` and `redirects` as the field wasn't being passed through.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

Closes: https://github.com/vercel/next.js/pull/45398
Fixes: https://github.com/vercel/next.js/issues/45393
x-ref: https://github.com/vercel/next.js/issues/45340
This commit is contained in:
JJ Kasper 2023-01-30 12:10:30 -08:00 committed by GitHub
parent 9e290f9f15
commit d3a9f5a54a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 398 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -668,6 +668,13 @@ export default async function loadCustomRoutes(
permanent: true,
locale: config.i18n ? false : undefined,
internal: true,
// don't run this redirect for _next/data requests
missing: [
{
type: 'header',
key: 'x-nextjs-data',
},
],
} as Redirect,
{
source: '/:notfile((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/\\.]+)',

2
packages/next/src/server/server-route-utils.ts
View file

@ -81,6 +81,7 @@ export const createHeaderRoute = ({
matchesLocaleAPIRoutes: true,
matchesTrailingSlash: true,
has: headerRoute.has,
missing: headerRoute.missing,
type: headerRoute.type,
name: `${headerRoute.type} ${headerRoute.source} header route`,
fn: async (_req, res, params, _parsedUrl) => {
@ -146,6 +147,7 @@ export const createRedirectRoute = ({
matchesLocaleAPIRoutes: true,
matchesTrailingSlash: true,
has: redirectRoute.has,
missing: redirectRoute.missing,
statusCode: redirectRoute.statusCode,
name: `Redirect route ${redirectRoute.source}`,
fn: async (req, res, params, parsedUrl) => {

24
test/e2e/middleware-trailing-slash/app/pages/html-links.js Normal file
View file

@ -0,0 +1,24 @@
import Link from 'next/link'
export default function Page() {
return (
<ul>
<li>
<Link
id="with-html"
href="/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037.html"
>
Does not work
</Link>
</li>
<li>
<Link
id="without-html"
href="/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037"
>
Works
</Link>
</li>
</ul>
)
}

27
test/e2e/middleware-trailing-slash/app/pages/product/[...product-params].tsx Normal file
View file

@ -0,0 +1,27 @@
import { GetServerSideProps } from 'next'
import React from 'react'
export interface ProductPageProps {
test: string
}
const ProductPage = (params: ProductPageProps) => {
return (
<>
<h1 id="text">Param found: {params.test}</h1>
</>
)
}
export const getServerSideProps: GetServerSideProps = async ({ params }) => {
const joined = Array.isArray(params['product-params'])
? params['product-params'].join(', ')
: params['product-params']
return {
props: {
test: joined ? joined : 'Not Found',
},
}
}
export default ProductPage

56
test/e2e/middleware-trailing-slash/test/index.test.ts
View file

@ -24,6 +24,62 @@ describe('Middleware Runtime trailing slash', () => {
})
function runTests() {
describe('with .html extension', () => {
it('should work when requesting the page directly', async () => {
const $ = await next.render$(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037.html'
)
expect($('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037.html'
)
})
it('should work using browser', async () => {
const browser = await next.browser(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037.html'
)
expect(await browser.elementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037.html'
)
})
it('should work when navigating', async () => {
const browser = await next.browser('/html-links')
await browser.elementByCss('#with-html').click()
expect(await browser.waitForElementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037.html'
)
})
})
describe('without .html extension', () => {
it('should work when requesting the page directly', async () => {
const $ = await next.render$(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037'
)
expect($('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037'
)
})
it('should work using browser', async () => {
const browser = await next.browser(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037'
)
expect(await browser.elementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037'
)
})
it('should work when navigating', async () => {
const browser = await next.browser('/html-links')
await browser.elementByCss('#without-html').click()
expect(await browser.waitForElementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037'
)
})
})
if ((global as any).isNextDev) {
it('refreshes the page when middleware changes ', async () => {
const browser = await webdriver(next.url, `/about/`)

82
test/integration/custom-routes/next.config.js
View file

@ -273,6 +273,41 @@ module.exports = {
},
async redirects() {
return [
{
source: '/missing-redirect-1',
missing: [
{
type: 'header',
key: 'x-my-header',
value: '(?<myHeader>.*)',
},
],
destination: '/with-params',
permanent: false,
},
{
source: '/missing-redirect-2',
missing: [
{
type: 'query',
key: 'my-query',
},
],
destination: '/with-params',
permanent: false,
},
{
source: '/missing-redirect-3',
missing: [
{
type: 'cookie',
key: 'loggedIn',
value: '(?<loggedIn>true)',
},
],
destination: '/with-params?authorized=1',
permanent: false,
},
{
source: '/redirect/me/to-about/:lang',
destination: '/:lang/about',
@ -465,6 +500,53 @@ module.exports = {
async headers() {
return [
{
source: '/missing-headers-1',
missing: [
{
type: 'header',
key: 'x-my-header',
value: '(?<myHeader>.*)',
},
],
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
},
{
source: '/missing-headers-2',
missing: [
{
type: 'query',
key: 'my-query',
},
],
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
},
{
source: '/missing-headers-3',
missing: [
{
type: 'cookie',
key: 'loggedIn',
value: '(?<loggedIn>true)',
},
],
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
},
{
source: '/add-header',
headers: [

189
test/integration/custom-routes/test/index.test.js
View file

@ -913,6 +913,101 @@ const runTests = (isDev = false) => {
)
})
it('should match missing header headers correctly', async () => {
const res = await fetchViaHTTP(appPort, '/missing-headers-1', undefined, {
headers: {
'x-my-header': 'hello world!!',
},
})
expect(res.status).toBe(404)
const res2 = await fetchViaHTTP(appPort, '/missing-headers-1', undefined, {
redirect: 'manual',
})
expect(res2.headers.get('x-new-header')).toBe('new-value')
})
it('should match missing query headers correctly', async () => {
const res = await fetchViaHTTP(appPort, '/missing-headers-2', {
'my-query': 'hellooo',
})
expect(res.status).toBe(404)
const res2 = await fetchViaHTTP(appPort, '/missing-headers-2', undefined, {
redirect: 'manual',
})
expect(res2.headers.get('x-new-header')).toBe('new-value')
})
it('should match missing cookie headers correctly', async () => {
const res = await fetchViaHTTP(appPort, '/missing-headers-3', undefined, {
headers: {
cookie: 'loggedIn=true',
},
redirect: 'manual',
})
expect(res.status).toBe(404)
const res2 = await fetchViaHTTP(appPort, '/missing-headers-3', undefined, {
redirect: 'manual',
})
expect(res2.headers.get('x-new-header')).toBe('new-value')
})
it('should match missing header redirect correctly', async () => {
const res = await fetchViaHTTP(appPort, '/missing-rewrite-1', undefined, {
headers: {
'x-my-header': 'hello world!!',
},
})
expect(res.status).toBe(404)
const res2 = await fetchViaHTTP(appPort, '/missing-redirect-1', undefined, {
redirect: 'manual',
})
expect(res2.status).toBe(307)
const url = new URL(res2.headers.get('location'), 'http://n')
expect(url.pathname).toBe('/with-params')
})
it('should match missing query redirect correctly', async () => {
const res = await fetchViaHTTP(appPort, '/missing-redirect-2', {
'my-query': 'hellooo',
})
expect(res.status).toBe(404)
const res2 = await fetchViaHTTP(appPort, '/missing-redirect-2', undefined, {
redirect: 'manual',
})
expect(res2.status).toBe(307)
const url = new URL(res2.headers.get('location'), 'http://n')
expect(url.pathname).toBe('/with-params')
})
it('should match missing cookie redirect correctly', async () => {
const res = await fetchViaHTTP(appPort, '/missing-redirect-3', undefined, {
headers: {
cookie: 'loggedIn=true',
},
redirect: 'manual',
})
expect(res.status).toBe(404)
const res2 = await fetchViaHTTP(appPort, '/missing-redirect-3', undefined, {
redirect: 'manual',
})
expect(res2.status).toBe(307)
const url = new URL(res2.headers.get('location'), 'http://n')
expect(url.pathname).toBe('/with-params')
expect(url.search).toBe('?authorized=1')
})
it('should match missing header rewrite correctly', async () => {
const res = await fetchViaHTTP(appPort, '/missing-rewrite-1', undefined, {
headers: {
@ -1379,6 +1474,50 @@ const runTests = (isDev = false) => {
statusCode: 308,
internal: true,
},
{
destination: '/with-params',
missing: [
{
key: 'x-my-header',
type: 'header',
value: '(?<myHeader>.*)',
},
],
regex: normalizeRegEx(
'^(?!\\/_next)\\/missing-redirect-1(?:\\/)?$'
),
source: '/missing-redirect-1',
statusCode: 307,
},
{
destination: '/with-params',
missing: [
{
key: 'my-query',
type: 'query',
},
],
regex: normalizeRegEx(
'^(?!\\/_next)\\/missing-redirect-2(?:\\/)?$'
),
source: '/missing-redirect-2',
statusCode: 307,
},
{
destination: '/with-params?authorized=1',
missing: [
{
key: 'loggedIn',
type: 'cookie',
value: '(?<loggedIn>true)',
},
],
regex: normalizeRegEx(
'^(?!\\/_next)\\/missing-redirect-3(?:\\/)?$'
),
source: '/missing-redirect-3',
statusCode: 307,
},
{
destination: '/:lang/about',
regex: normalizeRegEx(
@ -1620,6 +1759,56 @@ const runTests = (isDev = false) => {
},
],
headers: [
{
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
missing: [
{
key: 'x-my-header',
type: 'header',
value: '(?<myHeader>.*)',
},
],
regex: normalizeRegEx('^\\/missing-headers-1(?:\\/)?$'),
source: '/missing-headers-1',
},
{
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
missing: [
{
key: 'my-query',
type: 'query',
},
],
regex: normalizeRegEx('^\\/missing-headers-2(?:\\/)?$'),
source: '/missing-headers-2',
},
{
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
missing: [
{
key: 'loggedIn',
type: 'cookie',
value: '(?<loggedIn>true)',
},
],
regex: normalizeRegEx('^\\/missing-headers-3(?:\\/)?$'),
source: '/missing-headers-3',
},
{
headers: [
{

20
test/lib/next-modes/base.ts
View file

@ -66,15 +66,17 @@ export class NextInstance {
constructor(opts: NextInstanceOpts) {
Object.assign(this, opts)
this.env = {
...this.env,
// remove node_modules/.bin repo path from env
// to match CI $PATH value and isolate further
PATH: process.env.PATH.split(path.delimiter)
.filter((part) => {
return !part.includes(path.join('node_modules', '.bin'))
})
.join(path.delimiter),
if (!(global as any).isNextDeploy) {
this.env = {
...this.env,
// remove node_modules/.bin repo path from env
// to match CI $PATH value and isolate further
PATH: process.env.PATH.split(path.delimiter)
.filter((part) => {
return !part.includes(path.join('node_modules', '.bin'))
})
.join(path.delimiter),
}
}
}