From a4c61b39fd44960dc17a57c8bdb6eb0113b7dacb Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 14 Jul 2020 23:10:29 +0200 Subject: [PATCH 01/14] Start on repeated slash redirect --- .../next/client/normalize-trailing-slash.ts | 20 +++-- packages/next/lib/load-custom-routes.ts | 37 -------- .../next/next-server/server/next-server.ts | 50 ++++++++++- .../repeated-slash-redirect/next.config.js | 3 + .../pages/[[...slug]].js | 7 ++ .../test/index.test.js | 85 +++++++++++++++++++ .../trailing-slashes/test/index.test.js | 39 --------- 7 files changed, 155 insertions(+), 86 deletions(-) create mode 100644 test/integration/repeated-slash-redirect/next.config.js create mode 100644 test/integration/repeated-slash-redirect/pages/[[...slug]].js create mode 100644 test/integration/repeated-slash-redirect/test/index.test.js diff --git a/packages/next/client/normalize-trailing-slash.ts b/packages/next/client/normalize-trailing-slash.ts index fdb22d9fad44..52282a0a74b4 100644 --- a/packages/next/client/normalize-trailing-slash.ts +++ b/packages/next/client/normalize-trailing-slash.ts @@ -7,20 +7,22 @@ export function removePathTrailingSlash(path: string): string { return path.endsWith('/') && path !== '/' ? path.slice(0, -1) : path } +export function ensurePathTrailingSlash(path: string): string { + if (/\.[^/]+\/?$/.test(path)) { + return removePathTrailingSlash(path) + } else if (path.endsWith('/')) { + return path + } else { + return path + '/' + } +} + /** * Normalizes the trailing slash of a path according to the `trailingSlash` option * in `next.config.js`. */ const normalizePathTrailingSlash = process.env.__NEXT_TRAILING_SLASH - ? (path: string): string => { - if (/\.[^/]+\/?$/.test(path)) { - return removePathTrailingSlash(path) - } else if (path.endsWith('/')) { - return path - } else { - return path + '/' - } - } + ? ensurePathTrailingSlash : removePathTrailingSlash /** diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index 324863c1d060..d6c04b000b19 100644 --- a/packages/next/lib/load-custom-routes.ts +++ b/packages/next/lib/load-custom-routes.ts @@ -357,43 +357,6 @@ export default async function loadCustomRoutes( loadRedirects(config), ]) - if (config.experimental.trailingSlash) { - redirects.unshift( - { - source: '/:path*/:file.:ext/', - destination: '/:path*/:file.:ext', - permanent: true, - }, - { - source: '/:path*/:notfile([^/.]+)', - destination: '/:path*/:notfile/', - permanent: true, - } - ) - if (config.basePath) { - redirects.unshift({ - source: config.basePath, - destination: config.basePath + '/', - permanent: true, - basePath: false, - }) - } - } else { - redirects.unshift({ - source: '/:path+/', - destination: '/:path+', - permanent: true, - }) - if (config.basePath) { - redirects.unshift({ - source: config.basePath + '/', - destination: config.basePath, - permanent: true, - basePath: false, - }) - } - } - return { headers, rewrites, diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index ff0a17ed57e7..f2c6ab61d411 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -62,7 +62,10 @@ import { compile as compilePathToRegex } from 'next/dist/compiled/path-to-regexp import { loadEnvConfig } from '../../lib/load-env-config' import './node-polyfill-fetch' import { PagesManifest } from '../../build/webpack/plugins/pages-manifest-plugin' -import { removePathTrailingSlash } from '../../client/normalize-trailing-slash' +import { + removePathTrailingSlash, + ensurePathTrailingSlash, +} from '../../client/normalize-trailing-slash' import getRouteFromAssetPath from '../lib/router/utils/get-route-from-asset-path' const getCustomRouteMatcher = pathMatch(true) @@ -444,6 +447,11 @@ export default class Server { ...staticFilesRoute, ] + const normalizePathTrailingSlash = this.nextConfig.experimental + .trailingSlash + ? ensurePathTrailingSlash + : removePathTrailingSlash + const getCustomRouteBasePath = (r: { basePath?: false }) => { return r.basePath !== false && this.renderOpts.dev ? this.nextConfig.basePath @@ -541,6 +549,46 @@ export default class Server { } as Route }) + redirects.unshift({ + type: 'redirect', + match: (url) => url.startsWith(this.nextConfig.basePath), + statusCode: 308, + name: `Redirect slashes`, + fn: async (_req, res, params, parsedUrl) => { + const { pathname } = parsedUrl + const slashesNormalized = normalizePathTrailingSlash( + pathname?.replace(/\/\/+/g, '/') || '/' + ) + + if (slashesNormalized === pathname) { + return { finished: false } + } + + const { parsedDestination } = prepareDestination( + slashesNormalized, + params, + parsedUrl.query, + false, + '' + ) + const updatedDestination = formatUrl(parsedDestination) + + res.setHeader('Location', updatedDestination) + res.statusCode = 308 + + // Since IE11 doesn't support the 308 header add backwards + // compatibility using refresh header + if (res.statusCode === 308) { + res.setHeader('Refresh', `0;url=${updatedDestination}`) + } + + res.end() + return { + finished: true, + } + }, + } as Route) + const rewrites = this.customRoutes.rewrites.map((rewrite) => { const rewriteRoute = getCustomRoute(rewrite, 'rewrite') return { diff --git a/test/integration/repeated-slash-redirect/next.config.js b/test/integration/repeated-slash-redirect/next.config.js new file mode 100644 index 000000000000..ed8e3d3f74af --- /dev/null +++ b/test/integration/repeated-slash-redirect/next.config.js @@ -0,0 +1,3 @@ +module.exports = { + // target: 'serverless', +} diff --git a/test/integration/repeated-slash-redirect/pages/[[...slug]].js b/test/integration/repeated-slash-redirect/pages/[[...slug]].js new file mode 100644 index 000000000000..443e21fb7cb8 --- /dev/null +++ b/test/integration/repeated-slash-redirect/pages/[[...slug]].js @@ -0,0 +1,7 @@ +export async function getServerSideProps({ query: { slug } }) { + return { props: { slug: slug || [] } } +} + +export default function Page({ slug }) { + return
{slug.join('/')}
+} diff --git a/test/integration/repeated-slash-redirect/test/index.test.js b/test/integration/repeated-slash-redirect/test/index.test.js new file mode 100644 index 000000000000..a66bccf20212 --- /dev/null +++ b/test/integration/repeated-slash-redirect/test/index.test.js @@ -0,0 +1,85 @@ +/* eslint-env jest */ + +import fs from 'fs-extra' +import { join } from 'path' +import { + launchApp, + killApp, + findPort, + nextBuild, + nextStart, + fetchViaHTTP, +} from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 2) + +let appDir = join(__dirname, '..') +const nextConfigPath = join(appDir, 'next.config.js') +let nextConfigContent +let appPort +let app + +const runTests = (isDev = false) => { + it.each([ + ['/hello//world', '/hello/world'], + ['//hello/world', '/hello/world'], + ['/hello/world//', '/hello/world'], + ['//', '/'], + ['/hello///world', '/hello/world'], + ['/hello///world////foo', '/hello/world/foo'], + ['/hello//world?foo=bar//baz', '/hello/world?foo=bar%2F%2Fbaz'], + ])('it should redirect %s to %s', async (from, to) => { + const res = await fetchViaHTTP(appPort, from, undefined, { + redirect: 'manual', + }) + expect(res.status).toBe(308) + const location = new URL(res.headers.get('location'), 'http://n') + const locPathname = location.href.slice(location.origin.length) + expect(locPathname).toBe(to) + }) +} + +describe('Repeated trailing slashes', () => { + describe('dev mode', () => { + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + afterAll(() => killApp(app)) + runTests(true) + }) + + describe('server mode', () => { + beforeAll(async () => { + await nextBuild(appDir, [], { + stdout: true, + }) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(() => killApp(app)) + runTests() + }) + + describe('serverless mode', () => { + beforeAll(async () => { + nextConfigContent = await fs.readFile(nextConfigPath, 'utf8') + await fs.writeFile( + nextConfigPath, + nextConfigContent.replace(/\/\/ target/, 'target'), + 'utf8' + ) + await nextBuild(appDir, [], { + stdout: true, + }) + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterAll(async () => { + await fs.writeFile(nextConfigPath, nextConfigContent, 'utf8') + await killApp(app) + }) + + runTests() + }) +}) diff --git a/test/integration/trailing-slashes/test/index.test.js b/test/integration/trailing-slashes/test/index.test.js index 97682630deca..e9565f78c6b4 100644 --- a/test/integration/trailing-slashes/test/index.test.js +++ b/test/integration/trailing-slashes/test/index.test.js @@ -232,23 +232,6 @@ describe('Trailing slashes', () => { }) testWithoutTrailingSlash() - - it('should have a redirect in the routesmanifest', async () => { - const manifest = await fs.readJSON( - join(appDir, '.next', 'routes-manifest.json') - ) - expect(manifest).toEqual( - expect.objectContaining({ - redirects: expect.arrayContaining([ - expect.objectContaining({ - source: '/:path+/', - destination: '/:path+', - statusCode: 308, - }), - ]), - }) - ) - }) }) describe('production mode, trailingSlash: true', () => { @@ -270,28 +253,6 @@ describe('Trailing slashes', () => { }) testWithTrailingSlash() - - it('should have a redirect in the routesmanifest', async () => { - const manifest = await fs.readJSON( - join(appDir, '.next', 'routes-manifest.json') - ) - expect(manifest).toEqual( - expect.objectContaining({ - redirects: expect.arrayContaining([ - expect.objectContaining({ - source: '/:path*/:file.:ext/', - destination: '/:path*/:file.:ext', - statusCode: 308, - }), - expect.objectContaining({ - source: '/:path*/:notfile([^/.]+)', - destination: '/:path*/:notfile/', - statusCode: 308, - }), - ]), - }) - ) - }) }) describe('dev mode, with basepath, trailingSlash: true', () => { From c6019c2fbb92507533490af5ceb543427d5cf446 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 14 Jul 2020 23:11:10 +0200 Subject: [PATCH 02/14] Update next-server.ts --- packages/next/next-server/server/next-server.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index f2c6ab61d411..f67e9dbfbd8b 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -551,10 +551,10 @@ export default class Server { redirects.unshift({ type: 'redirect', - match: (url) => url.startsWith(this.nextConfig.basePath), + match: (url) => url!.startsWith(this.nextConfig.basePath), statusCode: 308, name: `Redirect slashes`, - fn: async (_req, res, params, parsedUrl) => { + fn: async (_req, res, _params, parsedUrl) => { const { pathname } = parsedUrl const slashesNormalized = normalizePathTrailingSlash( pathname?.replace(/\/\/+/g, '/') || '/' @@ -566,7 +566,7 @@ export default class Server { const { parsedDestination } = prepareDestination( slashesNormalized, - params, + {}, parsedUrl.query, false, '' From 68077a922ef2803155e9f00fcdc38a2814f0fbc0 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 14 Jul 2020 23:43:06 +0200 Subject: [PATCH 03/14] Fix custom-routes test --- test/integration/custom-routes/test/index.test.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index c7a0026f3b67..641c996671c6 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -495,14 +495,6 @@ const runTests = (isDev = false) => { pages404: true, basePath: '', redirects: [ - { - destination: '/:path+', - regex: normalizeRegEx( - '^(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))\\/$' - ), - source: '/:path+/', - statusCode: 308, - }, { destination: '/:lang/about', regex: normalizeRegEx( From ef47453e99a43b2210e567bf888784bc985aeadf Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Wed, 15 Jul 2020 00:04:46 +0200 Subject: [PATCH 04/14] fix matcher --- packages/next/next-server/server/next-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index f67e9dbfbd8b..c27acfc93cbe 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -551,7 +551,7 @@ export default class Server { redirects.unshift({ type: 'redirect', - match: (url) => url!.startsWith(this.nextConfig.basePath), + match: getCustomRouteMatcher('/(.*)'), statusCode: 308, name: `Redirect slashes`, fn: async (_req, res, _params, parsedUrl) => { From 124fcc11e35a3df0ae696d8312472e4375804d46 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Sat, 18 Jul 2020 18:56:09 +0200 Subject: [PATCH 05/14] revert --- .../next/client/normalize-trailing-slash.ts | 20 ++++---- packages/next/lib/load-custom-routes.ts | 44 ++++++++++++++++ .../next/next-server/server/next-server.ts | 50 +------------------ .../custom-routes/test/index.test.js | 16 ++++++ .../test/index.test.js | 7 +-- .../trailing-slashes/test/index.test.js | 40 +++++++++++++++ 6 files changed, 112 insertions(+), 65 deletions(-) diff --git a/packages/next/client/normalize-trailing-slash.ts b/packages/next/client/normalize-trailing-slash.ts index 53451dd98d7e..6cbd6f72e0dd 100644 --- a/packages/next/client/normalize-trailing-slash.ts +++ b/packages/next/client/normalize-trailing-slash.ts @@ -5,20 +5,18 @@ export function removePathTrailingSlash(path: string): string { return path.endsWith('/') && path !== '/' ? path.slice(0, -1) : path } -export function ensurePathTrailingSlash(path: string): string { - if (/\.[^/]+\/?$/.test(path)) { - return removePathTrailingSlash(path) - } else if (path.endsWith('/')) { - return path - } else { - return path + '/' - } -} - /** * Normalizes the trailing slash of a path according to the `trailingSlash` option * in `next.config.js`. */ export const normalizePathTrailingSlash = process.env.__NEXT_TRAILING_SLASH - ? ensurePathTrailingSlash + ? (path: string): string => { + if (/\.[^/]+\/?$/.test(path)) { + return removePathTrailingSlash(path) + } else if (path.endsWith('/')) { + return path + } else { + return path + '/' + } + } : removePathTrailingSlash diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index d6c04b000b19..d4fe3a4613bb 100644 --- a/packages/next/lib/load-custom-routes.ts +++ b/packages/next/lib/load-custom-routes.ts @@ -357,6 +357,50 @@ export default async function loadCustomRoutes( loadRedirects(config), ]) + if (config.experimental.trailingSlash) { + redirects.unshift( + { + source: '/:path*/:file.:ext/', + destination: '/:path*/:file.:ext', + permanent: true, + }, + { + source: '/:path*/:notfile([^/.]+)', + destination: '/:path*/:notfile/', + permanent: true, + } + ) + if (config.basePath) { + redirects.unshift({ + source: config.basePath, + destination: config.basePath + '/', + permanent: true, + basePath: false, + }) + } + } else { + redirects.unshift({ + source: '/:path+/', + destination: '/:path+', + permanent: true, + }) + if (config.basePath) { + redirects.unshift({ + source: config.basePath + '/', + destination: config.basePath, + permanent: true, + basePath: false, + }) + } + } + + // multiple slashes redirect + redirects.unshift({ + source: '/:before*/{(/+)}:after(.*)', + destination: '/:before*/:after', + permanent: true, + }) + return { headers, rewrites, diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index c27acfc93cbe..ff0a17ed57e7 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -62,10 +62,7 @@ import { compile as compilePathToRegex } from 'next/dist/compiled/path-to-regexp import { loadEnvConfig } from '../../lib/load-env-config' import './node-polyfill-fetch' import { PagesManifest } from '../../build/webpack/plugins/pages-manifest-plugin' -import { - removePathTrailingSlash, - ensurePathTrailingSlash, -} from '../../client/normalize-trailing-slash' +import { removePathTrailingSlash } from '../../client/normalize-trailing-slash' import getRouteFromAssetPath from '../lib/router/utils/get-route-from-asset-path' const getCustomRouteMatcher = pathMatch(true) @@ -447,11 +444,6 @@ export default class Server { ...staticFilesRoute, ] - const normalizePathTrailingSlash = this.nextConfig.experimental - .trailingSlash - ? ensurePathTrailingSlash - : removePathTrailingSlash - const getCustomRouteBasePath = (r: { basePath?: false }) => { return r.basePath !== false && this.renderOpts.dev ? this.nextConfig.basePath @@ -549,46 +541,6 @@ export default class Server { } as Route }) - redirects.unshift({ - type: 'redirect', - match: getCustomRouteMatcher('/(.*)'), - statusCode: 308, - name: `Redirect slashes`, - fn: async (_req, res, _params, parsedUrl) => { - const { pathname } = parsedUrl - const slashesNormalized = normalizePathTrailingSlash( - pathname?.replace(/\/\/+/g, '/') || '/' - ) - - if (slashesNormalized === pathname) { - return { finished: false } - } - - const { parsedDestination } = prepareDestination( - slashesNormalized, - {}, - parsedUrl.query, - false, - '' - ) - const updatedDestination = formatUrl(parsedDestination) - - res.setHeader('Location', updatedDestination) - res.statusCode = 308 - - // Since IE11 doesn't support the 308 header add backwards - // compatibility using refresh header - if (res.statusCode === 308) { - res.setHeader('Refresh', `0;url=${updatedDestination}`) - } - - res.end() - return { - finished: true, - } - }, - } as Route) - const rewrites = this.customRoutes.rewrites.map((rewrite) => { const rewriteRoute = getCustomRoute(rewrite, 'rewrite') return { diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 641c996671c6..c90ccadd545b 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -495,6 +495,22 @@ const runTests = (isDev = false) => { pages404: true, basePath: '', redirects: [ + { + destination: '/:before*/:after', + regex: normalizeRegEx( + '^(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?\\/(\\/+)(.*)$' + ), + source: '/:before*/{(/+)}:after(.*)', + statusCode: 308, + }, + { + destination: '/:path+', + regex: normalizeRegEx( + '^(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))\\/$' + ), + source: '/:path+/', + statusCode: 308, + }, { destination: '/:lang/about', regex: normalizeRegEx( diff --git a/test/integration/repeated-slash-redirect/test/index.test.js b/test/integration/repeated-slash-redirect/test/index.test.js index a66bccf20212..98c25124d1a1 100644 --- a/test/integration/repeated-slash-redirect/test/index.test.js +++ b/test/integration/repeated-slash-redirect/test/index.test.js @@ -29,11 +29,8 @@ const runTests = (isDev = false) => { ['/hello///world////foo', '/hello/world/foo'], ['/hello//world?foo=bar//baz', '/hello/world?foo=bar%2F%2Fbaz'], ])('it should redirect %s to %s', async (from, to) => { - const res = await fetchViaHTTP(appPort, from, undefined, { - redirect: 'manual', - }) - expect(res.status).toBe(308) - const location = new URL(res.headers.get('location'), 'http://n') + const res = await fetchViaHTTP(appPort, from) + const location = new URL(res.url) const locPathname = location.href.slice(location.origin.length) expect(locPathname).toBe(to) }) diff --git a/test/integration/trailing-slashes/test/index.test.js b/test/integration/trailing-slashes/test/index.test.js index 5cb4c3e6c02d..44bc308c71db 100644 --- a/test/integration/trailing-slashes/test/index.test.js +++ b/test/integration/trailing-slashes/test/index.test.js @@ -3,6 +3,7 @@ import webdriver from 'next-webdriver' import cheerio from 'cheerio' +import fs from 'fs-extra' import { fetchViaHTTP, renderViaHTTP, @@ -216,6 +217,23 @@ describe('Trailing slashes', () => { }) testWithoutTrailingSlash() + + it('should have a redirect in the routesmanifest', async () => { + const manifest = await fs.readJSON( + join(appDir, '.next', 'routes-manifest.json') + ) + expect(manifest).toEqual( + expect.objectContaining({ + redirects: expect.arrayContaining([ + expect.objectContaining({ + source: '/:path+/', + destination: '/:path+', + statusCode: 308, + }), + ]), + }) + ) + }) }) describe('production mode, trailingSlash: true', () => { @@ -231,6 +249,28 @@ describe('Trailing slashes', () => { }) testWithTrailingSlash() + + it('should have a redirect in the routesmanifest', async () => { + const manifest = await fs.readJSON( + join(appDir, '.next', 'routes-manifest.json') + ) + expect(manifest).toEqual( + expect.objectContaining({ + redirects: expect.arrayContaining([ + expect.objectContaining({ + source: '/:path*/:file.:ext/', + destination: '/:path*/:file.:ext', + statusCode: 308, + }), + expect.objectContaining({ + source: '/:path*/:notfile([^/.]+)', + destination: '/:path*/:notfile/', + statusCode: 308, + }), + ]), + }) + ) + }) }) describe('dev mode, with basepath, trailingSlash: true', () => { From 23b0aed27091bbd297f24471ca2f025390a6e424 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Sat, 18 Jul 2020 19:38:51 +0200 Subject: [PATCH 06/14] ignore files --- packages/next/lib/load-custom-routes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index d4fe3a4613bb..40899d414c6d 100644 --- a/packages/next/lib/load-custom-routes.ts +++ b/packages/next/lib/load-custom-routes.ts @@ -396,7 +396,7 @@ export default async function loadCustomRoutes( // multiple slashes redirect redirects.unshift({ - source: '/:before*/{(/+)}:after(.*)', + source: '/:before*/{(/+)}:after(|.*/[^/.]+|[^/.]+)', destination: '/:before*/:after', permanent: true, }) From a18cbb09c836195007fd021165132560e2239221 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Sat, 18 Jul 2020 20:06:57 +0200 Subject: [PATCH 07/14] fix custom-routes --- test/integration/custom-routes/test/index.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index c90ccadd545b..ed3550c0d2c5 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -496,11 +496,11 @@ const runTests = (isDev = false) => { basePath: '', redirects: [ { - destination: '/:before*/:after', regex: normalizeRegEx( - '^(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?\\/(\\/+)(.*)$' + '^(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?\\/(\\/+)(|.*\\/[^\\/.]+|[^\\/.]+)$' ), - source: '/:before*/{(/+)}:after(.*)', + source: '/:before*/{(/+)}:after(|.*/[^/.]+|[^/.]+)', + destination: '/:before*/:after', statusCode: 308, }, { From 332995302d10855d25fcf52dc6825135e178328a Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Mon, 20 Jul 2020 10:47:02 +0200 Subject: [PATCH 08/14] WIP --- .../next/client/normalize-trailing-slash.ts | 4 +++ .../next/next-server/lib/router/router.ts | 6 ++--- .../repeated-slash-redirect/pages/linker.js | 25 +++++++++++++++++++ .../test/index.test.js | 16 +++++++++--- 4 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 test/integration/repeated-slash-redirect/pages/linker.js diff --git a/packages/next/client/normalize-trailing-slash.ts b/packages/next/client/normalize-trailing-slash.ts index 6cbd6f72e0dd..5c258fef667f 100644 --- a/packages/next/client/normalize-trailing-slash.ts +++ b/packages/next/client/normalize-trailing-slash.ts @@ -20,3 +20,7 @@ export const normalizePathTrailingSlash = process.env.__NEXT_TRAILING_SLASH } } : removePathTrailingSlash + +export function normalizePathSlashes(path: string): string { + return normalizePathTrailingSlash(path.replace(/\/\/+/g, '/')) +} diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 9b10e5db6d70..bd5d2c83bb99 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -19,7 +19,7 @@ import { searchParamsToUrlQuery } from './utils/search-params-to-url-query' import { parseRelativeUrl } from './utils/parse-relative-url' import { removePathTrailingSlash, - normalizePathTrailingSlash, + normalizePathSlashes, } from '../../../client/normalize-trailing-slash' const basePath = (process.env.__NEXT_ROUTER_BASEPATH as string) || '' @@ -27,7 +27,7 @@ const basePath = (process.env.__NEXT_ROUTER_BASEPATH as string) || '' export function addBasePath(path: string): string { return basePath ? path === '/' - ? normalizePathTrailingSlash(basePath) + ? normalizePathSlashes(basePath) : basePath + path : path } @@ -48,7 +48,7 @@ export function resolveHref(currentPath: string, href: Url): string { const urlAsString = typeof href === 'string' ? href : formatWithValidation(href) const finalUrl = new URL(urlAsString, base) - finalUrl.pathname = normalizePathTrailingSlash(finalUrl.pathname) + finalUrl.pathname = normalizePathSlashes(finalUrl.pathname) // if the origin didn't change, it means we received a relative href return finalUrl.origin === base.origin ? finalUrl.href.slice(finalUrl.origin.length) diff --git a/test/integration/repeated-slash-redirect/pages/linker.js b/test/integration/repeated-slash-redirect/pages/linker.js new file mode 100644 index 000000000000..e70939a79867 --- /dev/null +++ b/test/integration/repeated-slash-redirect/pages/linker.js @@ -0,0 +1,25 @@ +import Link from 'next/link' +import { useRouter } from 'next/router' + +export async function getServerSideProps({ query }) { + return { + props: { href: query.href || '/' }, + } +} + +export default function Linker({ href }) { + const router = useRouter() + const pushRoute = () => { + router.push(href) + } + return ( +
+ + link to {href} + + +
+ ) +} diff --git a/test/integration/repeated-slash-redirect/test/index.test.js b/test/integration/repeated-slash-redirect/test/index.test.js index 98c25124d1a1..e7758cafd4eb 100644 --- a/test/integration/repeated-slash-redirect/test/index.test.js +++ b/test/integration/repeated-slash-redirect/test/index.test.js @@ -1,5 +1,6 @@ /* eslint-env jest */ +import cheerio from 'cheerio' import fs from 'fs-extra' import { join } from 'path' import { @@ -9,6 +10,7 @@ import { nextBuild, nextStart, fetchViaHTTP, + renderViaHTTP, } from 'next-test-utils' jest.setTimeout(1000 * 60 * 2) @@ -20,20 +22,28 @@ let appPort let app const runTests = (isDev = false) => { - it.each([ + const cases = [ ['/hello//world', '/hello/world'], ['//hello/world', '/hello/world'], ['/hello/world//', '/hello/world'], ['//', '/'], ['/hello///world', '/hello/world'], ['/hello///world////foo', '/hello/world/foo'], - ['/hello//world?foo=bar//baz', '/hello/world?foo=bar%2F%2Fbaz'], - ])('it should redirect %s to %s', async (from, to) => { + ['/hello//world?foo=bar//baz', '/hello/world?foo=bar//baz'], + ] + + it.each(cases)('it should redirect %s to %s', async (from, to) => { const res = await fetchViaHTTP(appPort, from) const location = new URL(res.url) const locPathname = location.href.slice(location.origin.length) expect(locPathname).toBe(to) }) + + it.each(cases)('it should rewrite href %s to %s', async (from, to) => { + const content = await renderViaHTTP(appPort, `/linker?href=${from}`) + const $ = cheerio.load(content) + expect($('#link').attr('href')).toBe(to) + }) } describe('Repeated trailing slashes', () => { From 16681d5d13caf5e7cd1b4cb797c51782fedd9d7c Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Thu, 23 Jul 2020 00:16:51 +0200 Subject: [PATCH 09/14] Add more tests --- .../pages/[[...slug]].js | 2 +- .../repeated-slash-redirect/pages/linker.js | 2 +- .../test/index.test.js | 27 +++++++++++++++---- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/test/integration/repeated-slash-redirect/pages/[[...slug]].js b/test/integration/repeated-slash-redirect/pages/[[...slug]].js index 443e21fb7cb8..c9648d10e7a8 100644 --- a/test/integration/repeated-slash-redirect/pages/[[...slug]].js +++ b/test/integration/repeated-slash-redirect/pages/[[...slug]].js @@ -3,5 +3,5 @@ export async function getServerSideProps({ query: { slug } }) { } export default function Page({ slug }) { - return
{slug.join('/')}
+ return
{slug.join('/')}
} diff --git a/test/integration/repeated-slash-redirect/pages/linker.js b/test/integration/repeated-slash-redirect/pages/linker.js index e70939a79867..abec3a4cc08b 100644 --- a/test/integration/repeated-slash-redirect/pages/linker.js +++ b/test/integration/repeated-slash-redirect/pages/linker.js @@ -14,7 +14,7 @@ export default function Linker({ href }) { } return (
- + link to {href}