From 3172cfedc8b759fec89fda373c86b86931fc02f1 Mon Sep 17 00:00:00 2001 From: Omar McIver Date: Mon, 2 Oct 2023 17:02:49 -0500 Subject: [PATCH] fix: support both decoded and encoded url requests of conventioned files (#56187) Co-authored-by: Omar McIver Co-authored-by: Jiachi Liu --- .../src/server/lib/router-utils/filesystem.ts | 20 ++- .../dynamic-route-interpolation/index.test.ts | 147 ++++++++---------- .../pages/api/dynamic/[slug].js | 4 + .../pages/blog/[slug].js | 18 +++ 4 files changed, 106 insertions(+), 83 deletions(-) create mode 100644 test/e2e/dynamic-route-interpolation/pages/api/dynamic/[slug].js create mode 100644 test/e2e/dynamic-route-interpolation/pages/blog/[slug].js diff --git a/packages/next/src/server/lib/router-utils/filesystem.ts b/packages/next/src/server/lib/router-utils/filesystem.ts index 270580f1f81a7..dfea25dad2561 100644 --- 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 index a7fb9b82168a4..d8d10f1217268 100644 --- 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 ( - <> -

{props.slug}

- - {props.now} - - - ) - } - `, + 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 new file mode 100644 index 0000000000000..28254676b43ab --- /dev/null +++ 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 new file mode 100644 index 0000000000000..7f9692f73fd56 --- /dev/null +++ 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 ( + <> +

{props.slug}

+ + {props.now} + + + ) +}