Merge pull request #18219 from github/repo-sync

repo sync

Author: Octomerger

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

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()
+}

package-lock.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",

package.json

@@ -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', () => {