diff --git a/middleware/index.js b/middleware/index.js index ed4320b9bf..6a2b76b2c4 100644 --- a/middleware/index.js +++ b/middleware/index.js @@ -19,7 +19,6 @@ import { setDefaultFastlySurrogateKey } from './set-fastly-surrogate-key.js' import setFastlyCacheHeaders from './set-fastly-cache-headers.js' import reqUtils from './req-utils.js' import recordRedirect from './record-redirect.js' -import connectSlashes from 'connect-slashes' import handleErrors from './handle-errors.js' import handleInvalidPaths from './handle-invalid-paths.js' import handleNextDataPath from './handle-next-data-path.js' @@ -67,6 +66,7 @@ import protect from './overload-protection.js' import fastHead from './fast-head.js' import fastlyCacheTest from './fastly-cache-test.js' import fastRootRedirect from './fast-root-redirect.js' +import trailingSlashes from './trailing-slashes.js' const { DEPLOYMENT_ENV, NODE_ENV } = process.env const isDevelopment = NODE_ENV === 'development' @@ -263,7 +263,7 @@ export default function (app) { // *** Redirects, 3xx responses *** // I ordered these by use frequency - app.use(connectSlashes(false)) + app.use(instrument(trailingSlashes, './redirects/trailing-slashes')) app.use(instrument(redirectsExternal, './redirects/external')) app.use(instrument(languageCodeRedirects, './redirects/language-code-redirects')) // Must come before contextualizers app.use(instrument(handleRedirects, './redirects/handle-redirects')) // Must come before contextualizers diff --git a/middleware/trailing-slashes.js b/middleware/trailing-slashes.js new file mode 100644 index 0000000000..22fd01184f --- /dev/null +++ b/middleware/trailing-slashes.js @@ -0,0 +1,25 @@ +import { cacheControlFactory } from './cache-control.js' + +const cacheControl = cacheControlFactory(60 * 60) + +export default function trailingSlashes(req, res, next) { + if (req.method === 'GET' || req.method === 'HEAD' || req.method === 'OPTIONS') { + const split = req.url.split('?') + let pathname = split.shift() + if (pathname !== '/' && pathname.endsWith('/')) { + while (pathname.endsWith('/')) { + pathname = pathname.slice(0, pathname.length - 1) + } + let url = pathname + if (split.length) { + url += `?${split.join('?')}` + } + // So it can be cached in the CDN + res.removeHeader('set-cookie') + cacheControl(res) + return res.redirect(301, url) + } + } + + next() +} diff --git a/package-lock.json b/package-lock.json index b866b6d5e4..c3c7e93b97 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,6 @@ "cheerio": "^1.0.0-rc.10", "classnames": "^2.3.1", "connect-datadog": "0.0.9", - "connect-slashes": "^1.4.0", "cookie-parser": "^1.4.6", "cors": "^2.8.5", "csurf": "^1.11.0", @@ -7454,14 +7453,6 @@ "unix-dgram": "2.0.x" } }, - "node_modules/connect-slashes": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/connect-slashes/-/connect-slashes-1.4.0.tgz", - "integrity": "sha512-BJRbgSczzlsRwyF64DxGNIizBTxUf7f/tAsDzq2Nq8eLrm2160vVfm/4vQcjrT4qVFu6qDCqPK+vDaEWJsnSzA==", - "engines": { - "node": "*" - } - }, "node_modules/console-browserify": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/console-browserify/-/console-browserify-1.2.0.tgz", @@ -28502,11 +28493,6 @@ } } }, - "connect-slashes": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/connect-slashes/-/connect-slashes-1.4.0.tgz", - "integrity": "sha512-BJRbgSczzlsRwyF64DxGNIizBTxUf7f/tAsDzq2Nq8eLrm2160vVfm/4vQcjrT4qVFu6qDCqPK+vDaEWJsnSzA==" - }, "console-browserify": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/console-browserify/-/console-browserify-1.2.0.tgz", diff --git a/package.json b/package.json index bfb1f81c0a..31a10284dd 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,6 @@ "cheerio": "^1.0.0-rc.10", "classnames": "^2.3.1", "connect-datadog": "0.0.9", - "connect-slashes": "^1.4.0", "cookie-parser": "^1.4.6", "cors": "^2.8.5", "csurf": "^1.11.0", diff --git a/tests/routing/redirects.js b/tests/routing/redirects.js index 23f4ed7661..7f8bc1be93 100644 --- a/tests/routing/redirects.js +++ b/tests/routing/redirects.js @@ -154,6 +154,22 @@ describe('redirects', () => { expect(res.headers.location).toBe('/ja') expect(res.headers['cache-control']).toBe('private, no-store') }) + test('trailing slash on languaged homepage should permantently redirect', async () => { + const res = await get('/en/') + expect(res.statusCode).toBe(301) + expect(res.headers.location).toBe('/en') + expect(res.headers['set-cookie']).toBeUndefined() + expect(res.headers['cache-control']).toContain('public') + expect(res.headers['cache-control']).toMatch(/max-age=\d+/) + }) + test('trailing slash with query string on languaged homepage should permantently redirect', async () => { + const res = await get('/ja/?foo=bar&bar=foo') + expect(res.statusCode).toBe(301) + expect(res.headers.location).toBe('/ja?foo=bar&bar=foo') + expect(res.headers['set-cookie']).toBeUndefined() + expect(res.headers['cache-control']).toContain('public') + expect(res.headers['cache-control']).toMatch(/max-age=\d+/) + }) }) describe('external redirects', () => {