Ensure mixed query/hash values are handled correctly (#38852)

This ensures we correctly detect query and hash values when there are `?` characters in the hash. A regression test to ensure this working properly has been added. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Fixes: #38783
vercel · Jul 21, 2022 · 366a04b · 366a04b
1 parent 14463dd
commit 366a04b
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 6 deletions.
diff --git a/packages/next/shared/lib/router/utils/parse-path.ts b/packages/next/shared/lib/router/utils/parse-path.ts
@@ -6,14 +6,14 @@
 export function parsePath(path: string) {
   const hashIndex = path.indexOf('#')
   const queryIndex = path.indexOf('?')
+  const hasQuery = queryIndex > -1 && (hashIndex < 0 || queryIndex < hashIndex)
 
-  if (queryIndex > -1 || hashIndex > -1) {
+  if (hasQuery || hashIndex > -1) {
     return {
-      pathname: path.substring(0, queryIndex > -1 ? queryIndex : hashIndex),
-      query:
-        queryIndex > -1
-          ? path.substring(queryIndex, hashIndex > -1 ? hashIndex : undefined)
-          : '',
+      pathname: path.substring(0, hasQuery ? queryIndex : hashIndex),
+      query: hasQuery
+        ? path.substring(queryIndex, hashIndex > -1 ? hashIndex : undefined)
+        : '',
       hash: hashIndex > -1 ? path.slice(hashIndex) : '',
     }
   }

diff --git a/test/e2e/basepath.test.ts b/test/e2e/basepath.test.ts
@@ -120,6 +120,40 @@ describe('basePath', () => {
       return
     }
 
+    it.each([
+      { hash: '#hello?' },
+      { hash: '#?' },
+      { hash: '##' },
+      { hash: '##?' },
+      { hash: '##hello?' },
+      { hash: '##hello' },
+      { hash: '#hello?world' },
+      { search: '?hello=world', hash: '#a', query: { hello: 'world' } },
+      { search: '?hello', hash: '#a', query: { hello: '' } },
+      { search: '?hello=', hash: '#a', query: { hello: '' } },
+    ])(
+      'should handle query/hash correctly during query updating $hash $search',
+      async ({ hash, search, query }) => {
+        const browser = await webdriver(
+          next.url,
+          `${basePath}${search || ''}${hash || ''}`
+        )
+
+        await check(
+          () =>
+            browser.eval('window.next.router.isReady ? "ready" : "not ready"'),
+          'ready'
+        )
+        expect(await browser.eval('window.location.pathname')).toBe(basePath)
+        expect(await browser.eval('window.location.search')).toBe(search || '')
+        expect(await browser.eval('window.location.hash')).toBe(hash || '')
+        expect(await browser.eval('next.router.pathname')).toBe('/')
+        expect(
+          JSON.parse(await browser.eval('JSON.stringify(next.router.query)'))
+        ).toEqual(query || {})
+      }
+    )
+
     it('should navigate back correctly to a dynamic route', async () => {
       const browser = await webdriver(next.url, `${basePath}`)
 

diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js
@@ -88,6 +88,37 @@ describe('Production Usage', () => {
     await browser.waitForElementByCss('.about-page')
   })
 
+  it.each([
+    { hash: '#hello?' },
+    { hash: '#?' },
+    { hash: '##' },
+    { hash: '##?' },
+    { hash: '##hello?' },
+    { hash: '##hello' },
+    { hash: '#hello?world' },
+    { search: '?hello=world', hash: '#a', query: { hello: 'world' } },
+    { search: '?hello', hash: '#a', query: { hello: '' } },
+    { search: '?hello=', hash: '#a', query: { hello: '' } },
+  ])(
+    'should handle query/hash correctly during query updating $hash $search',
+    async ({ hash, search, query }) => {
+      const browser = await webdriver(appPort, `/${search || ''}${hash || ''}`)
+
+      await check(
+        () =>
+          browser.eval('window.next.router.isReady ? "ready" : "not ready"'),
+        'ready'
+      )
+      expect(await browser.eval('window.location.pathname')).toBe('/')
+      expect(await browser.eval('window.location.hash')).toBe(hash || '')
+      expect(await browser.eval('window.location.search')).toBe(search || '')
+      expect(await browser.eval('next.router.pathname')).toBe('/')
+      expect(
+        JSON.parse(await browser.eval('JSON.stringify(next.router.query)'))
+      ).toEqual(query || {})
+    }
+  )
+
   it('should not show target deprecation warning', () => {
     expect(output).not.toContain(
       'The `target` config is deprecated and will be removed in a future version'

diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js
@@ -93,6 +93,7 @@ export function getFullUrl(appPortOrUrl, url, hostname) {
     const parsedUrl = new URL(fullUrl)
     const parsedPathQuery = new URL(url, fullUrl)
 
+    parsedUrl.hash = parsedPathQuery.hash
     parsedUrl.search = parsedPathQuery.search
     parsedUrl.pathname = parsedPathQuery.pathname