From d3a9f5a54aa9ad4638dbc96f165eb922ac48f529 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 30 Jan 2023 12:10:30 -0800 Subject: [PATCH] Fix trailing slash redirect applying for data request (#45417) 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 --- packages/next/src/lib/load-custom-routes.ts | 7 + .../next/src/server/server-route-utils.ts | 2 + .../app/pages/html-links.js | 24 +++ .../app/pages/product/[...product-params].tsx | 27 +++ .../test/index.test.ts | 56 ++++++ test/integration/custom-routes/next.config.js | 82 ++++++++ .../custom-routes/test/index.test.js | 189 ++++++++++++++++++ test/lib/next-modes/base.ts | 20 +- 8 files changed, 398 insertions(+), 9 deletions(-) create mode 100644 test/e2e/middleware-trailing-slash/app/pages/html-links.js create mode 100644 test/e2e/middleware-trailing-slash/app/pages/product/[...product-params].tsx diff --git a/packages/next/src/lib/load-custom-routes.ts b/packages/next/src/lib/load-custom-routes.ts index 71133a30ec9e3..9b84ed87f2af8 100644 --- a/packages/next/src/lib/load-custom-routes.ts +++ b/packages/next/src/lib/load-custom-routes.ts @@ -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(?:/.*)?)(?:[^/]+/)*[^/\\.]+)', diff --git a/packages/next/src/server/server-route-utils.ts b/packages/next/src/server/server-route-utils.ts index 1cfa55a1ffccb..5be9abb43b648 100644 --- a/packages/next/src/server/server-route-utils.ts +++ b/packages/next/src/server/server-route-utils.ts @@ -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) => { diff --git a/test/e2e/middleware-trailing-slash/app/pages/html-links.js b/test/e2e/middleware-trailing-slash/app/pages/html-links.js new file mode 100644 index 0000000000000..1d30b2d7bf814 --- /dev/null +++ b/test/e2e/middleware-trailing-slash/app/pages/html-links.js @@ -0,0 +1,24 @@ +import Link from 'next/link' + +export default function Page() { + return ( + + ) +} diff --git a/test/e2e/middleware-trailing-slash/app/pages/product/[...product-params].tsx b/test/e2e/middleware-trailing-slash/app/pages/product/[...product-params].tsx new file mode 100644 index 0000000000000..2970d137c8846 --- /dev/null +++ b/test/e2e/middleware-trailing-slash/app/pages/product/[...product-params].tsx @@ -0,0 +1,27 @@ +import { GetServerSideProps } from 'next' +import React from 'react' + +export interface ProductPageProps { + test: string +} + +const ProductPage = (params: ProductPageProps) => { + return ( + <> +

Param found: {params.test}

+ + ) +} + +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 diff --git a/test/e2e/middleware-trailing-slash/test/index.test.ts b/test/e2e/middleware-trailing-slash/test/index.test.ts index 5356fbdb398c3..189adf5f31d38 100644 --- a/test/e2e/middleware-trailing-slash/test/index.test.ts +++ b/test/e2e/middleware-trailing-slash/test/index.test.ts @@ -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/`) diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index dcdda22d31824..b484e588fa3d5 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -273,6 +273,41 @@ module.exports = { }, async redirects() { return [ + { + source: '/missing-redirect-1', + missing: [ + { + type: 'header', + key: 'x-my-header', + value: '(?.*)', + }, + ], + 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: '(?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: '(?.*)', + }, + ], + 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: '(?true)', + }, + ], + headers: [ + { + key: 'x-new-header', + value: 'new-value', + }, + ], + }, { source: '/add-header', headers: [ diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 1614533ed2e03..dd240ec9d82ce 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -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: '(?.*)', + }, + ], + 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: '(?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: '(?.*)', + }, + ], + 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: '(?true)', + }, + ], + regex: normalizeRegEx('^\\/missing-headers-3(?:\\/)?$'), + source: '/missing-headers-3', + }, { headers: [ { diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts index 800205906fc21..ba08ec2508051 100644 --- a/test/lib/next-modes/base.ts +++ b/test/lib/next-modes/base.ts @@ -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), + } } }