be more conservative with spotting q= in URL (github#21945)

* be more conservative with spotting q= in URL Fixes github/docs-engineering#951 * refactor check * Update tests/routing/redirects.js Co-authored-by: Rachael Sewell <[email protected]> Co-authored-by: Rachael Sewell <[email protected]>
diosystore-com · Oct 18, 2021 · c712da8 · c712da8
1 parent 1f71206
commit c712da8
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 2 deletions.
diff --git a/middleware/redirects/handle-redirects.js b/middleware/redirects/handle-redirects.js
@@ -26,10 +26,13 @@ export default function handleRedirects(req, res, next) {
   let redirect = req.path
   let queryParams = req._parsedUrl.query
 
-  // update old-style query params (#9467)
   // have to do this now because searchPath replacement changes the path as well as the query params
   if (queryParams) {
-    queryParams = '?' + queryParams.replace('q=', 'query=')
+    // update old-style query params (#9467)
+    if ('q' in req.query) {
+      queryParams = queryParams.replace('q=', 'query=')
+    }
+    queryParams = '?' + queryParams
     redirect = (redirect + queryParams).replace(patterns.searchPath, '$1')
   }
 

diff --git a/tests/routing/redirects.js b/tests/routing/redirects.js
@@ -80,6 +80,15 @@ describe('redirects', () => {
       expect(res.headers.location).toBe(expected)
     })
 
+    test('have faq= not converted to query=', async () => {
+      // Don't confuse `?faq=` for `?q=` just because they both start with `q=`
+      // Docs internal #21945
+      const res = await get('/en/enterprise/admin?faq=pulls')
+      expect(res.statusCode).toBe(301)
+      const expected = `/en/enterprise-server@${enterpriseServerReleases.latest}/admin?faq=pulls`
+      expect(res.headers.location).toBe(expected)
+    })
+
     test('work with redirected search paths', async () => {
       const res = await get('/en/enterprise/admin/search?utf8=%E2%9C%93&query=pulls')
       expect(res.statusCode).toBe(301)