fix: support both decoded and encoded url requests of conventioned fi…

…les (#56187) Co-authored-by: Omar McIver <[email protected]> Co-authored-by: Jiachi Liu <[email protected]>
vercel · Oct 2, 2023 · 3172cfe · 3172cfe
1 parent a2f9ef5
commit 3172cfe
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 83 deletions.
diff --git a/packages/next/src/server/lib/router-utils/filesystem.ts b/packages/next/src/server/lib/router-utils/filesystem.ts
@@ -517,11 +517,25 @@ export async function setupFsCheck(opts: {
           } catch {}
         }
 
+        let matchedItem = items.has(curItemPath)
+
         // check decoded variant as well
-        if (!items.has(curItemPath) && !opts.dev) {
-          curItemPath = curDecodedItemPath
+        if (!matchedItem && !opts.dev) {
+          matchedItem = items.has(curItemPath)
+          if (matchedItem) curItemPath = curDecodedItemPath
+          else {
+            // x-ref: https://github.com/vercel/next.js/issues/54008
+            // There're cases that urls get decoded before requests, we should support both encoded and decoded ones.
+            // e.g. nginx could decode the proxy urls, the below ones should be treated as the same:
+            // decoded version: `/_next/static/chunks/pages/blog/[slug]-d4858831b91b69f6.js`
+            // encoded version: `/_next/static/chunks/pages/blog/%5Bslug%5D-d4858831b91b69f6.js`
+            try {
+              // encode the special characters in the path and retrieve again to determine if path exists.
+              const encodedCurItemPath = encodeURI(curItemPath)
+              matchedItem = items.has(encodedCurItemPath)
+            } catch {}
+          }
         }
-        const matchedItem = items.has(curItemPath)
 
         if (matchedItem || opts.dev) {
           let fsPath: string | undefined

diff --git a/test/e2e/dynamic-route-interpolation/index.test.ts b/test/e2e/dynamic-route-interpolation/index.test.ts
@@ -1,91 +1,78 @@
-import { createNext } from 'e2e-utils'
-import { NextInstance } from 'test/lib/next-modes/base'
-import { renderViaHTTP } from 'next-test-utils'
-import cheerio from 'cheerio'
-import webdriver from 'next-webdriver'
+import { createNextDescribe } from 'e2e-utils'
 
-describe('Dynamic Route Interpolation', () => {
-  let next: NextInstance
-
-  beforeAll(async () => {
-    next = await createNext({
-      files: {
-        'pages/blog/[slug].js': `
-          import Link from "next/link"
-          import { useRouter } from "next/router"
-
-          export function getServerSideProps({ params }) {
-            return { props: { slug: params.slug, now: Date.now() } }
-          }
+createNextDescribe(
+  'Dynamic Route Interpolation',
+  {
+    files: __dirname,
+  },
+  ({ next, isNextStart }) => {
+    it('should work', async () => {
+      const $ = await next.render$('/blog/a')
+      expect($('#slug').text()).toBe('a')
+    })
 
-          export default function Page(props) { 
-            const router = useRouter()
-            return (
-              <>
-                <p id="slug">{props.slug}</p>
-                <Link id="now" href={router.asPath}>
-                  {props.now}
-                </Link>
-              </>
-            )
-          }
-        `,
+    it('should work with parameter itself', async () => {
+      const $ = await next.render$('/blog/[slug]')
+      expect($('#slug').text()).toBe('[slug]')
+    })
 
-        'pages/api/dynamic/[slug].js': `
-          export default function Page(req, res) { 
-            const { slug } = req.query
-            res.end('slug: ' + slug)
-          }
-        `,
-      },
-      dependencies: {},
+    it('should work with brackets', async () => {
+      const $ = await next.render$('/blog/[abc]')
+      expect($('#slug').text()).toBe('[abc]')
     })
-  })
-  afterAll(() => next.destroy())
 
-  it('should work', async () => {
-    const html = await renderViaHTTP(next.url, '/blog/a')
-    const $ = cheerio.load(html)
-    expect($('#slug').text()).toBe('a')
-  })
+    it('should work with parameter itself in API routes', async () => {
+      const text = await next.render('/api/dynamic/[slug]')
+      expect(text).toBe('slug: [slug]')
+    })
 
-  it('should work with parameter itself', async () => {
-    const html = await renderViaHTTP(next.url, '/blog/[slug]')
-    const $ = cheerio.load(html)
-    expect($('#slug').text()).toBe('[slug]')
-  })
+    it('should work with brackets in API routes', async () => {
+      const text = await next.render('/api/dynamic/[abc]')
+      expect(text).toBe('slug: [abc]')
+    })
 
-  it('should work with brackets', async () => {
-    const html = await renderViaHTTP(next.url, '/blog/[abc]')
-    const $ = cheerio.load(html)
-    expect($('#slug').text()).toBe('[abc]')
-  })
+    it('should bust data cache', async () => {
+      const browser = await next.browser('/blog/login')
+      await browser.elementById('now').click() // fetch data once
+      const text = await browser.elementById('now').text()
+      await browser.elementById('now').click() // fetch data again
+      await browser.waitForElementByCss(`#now:not(:text("${text}"))`)
+      await browser.close()
+    })
 
-  it('should work with parameter itself in API routes', async () => {
-    const text = await renderViaHTTP(next.url, '/api/dynamic/[slug]')
-    expect(text).toBe('slug: [slug]')
-  })
+    it('should bust data cache with symbol', async () => {
+      const browser = await next.browser('/blog/@login')
+      await browser.elementById('now').click() // fetch data once
+      const text = await browser.elementById('now').text()
+      await browser.elementById('now').click() // fetch data again
+      await browser.waitForElementByCss(`#now:not(:text("${text}"))`)
+      await browser.close()
+    })
 
-  it('should work with brackets in API routes', async () => {
-    const text = await renderViaHTTP(next.url, '/api/dynamic/[abc]')
-    expect(text).toBe('slug: [abc]')
-  })
+    if (isNextStart) {
+      it('should support both encoded and decoded nextjs reserved path convention characters in path', async () => {
+        const $ = await next.render$('/blog/123')
+        let pagePathScriptSrc
+        for (const script of $('script').toArray()) {
+          const { src } = script.attribs
+          if (src.includes('slug') && src.includes('pages/blog')) {
+            pagePathScriptSrc = src
+            break
+          }
+        }
 
-  it('should bust data cache', async () => {
-    const browser = await webdriver(next.url, '/blog/login')
-    await browser.elementById('now').click() // fetch data once
-    const text = await browser.elementById('now').text()
-    await browser.elementById('now').click() // fetch data again
-    await browser.waitForElementByCss(`#now:not(:text("${text}"))`)
-    await browser.close()
-  })
+        // e.g. /_next/static/chunks/pages/blog/%5Bslug%5D-3d2fedc300f04305.js
+        const { status: encodedPathReqStatus } = await next.fetch(
+          pagePathScriptSrc
+        )
+        // e.g. /_next/static/chunks/pages/blog/[slug]-3d2fedc300f04305.js
+        const { status: decodedPathReqStatus } = await next.fetch(
+          decodeURI(pagePathScriptSrc)
+        )
 
-  it('should bust data cache with symbol', async () => {
-    const browser = await webdriver(next.url, '/blog/@login')
-    await browser.elementById('now').click() // fetch data once
-    const text = await browser.elementById('now').text()
-    await browser.elementById('now').click() // fetch data again
-    await browser.waitForElementByCss(`#now:not(:text("${text}"))`)
-    await browser.close()
-  })
-})
+        expect(encodedPathReqStatus).toBe(200)
+        expect(decodedPathReqStatus).toBe(200)
+      })
+    }
+  }
+)
diff --git a/test/e2e/dynamic-route-interpolation/pages/api/dynamic/[slug].js b/test/e2e/dynamic-route-interpolation/pages/api/dynamic/[slug].js
@@ -0,0 +1,4 @@
+export default function Page(req, res) {
+  const { slug } = req.query
+  res.end('slug: ' + slug)
+}
diff --git a/test/e2e/dynamic-route-interpolation/pages/blog/[slug].js b/test/e2e/dynamic-route-interpolation/pages/blog/[slug].js
@@ -0,0 +1,18 @@
+import Link from 'next/link'
+import { useRouter } from 'next/router'
+
+export function getServerSideProps({ params }) {
+  return { props: { slug: params.slug, now: Date.now() } }
+}
+
+export default function Page(props) {
+  const router = useRouter()
+  return (
+    <>
+      <p id="slug">{props.slug}</p>
+      <Link id="now" href={router.asPath}>
+        {props.now}
+      </Link>
+    </>
+  )
+}