fix: handle non ASCII characters in cache-tag headers (#2645)

* fix: handle non ASCII characters in cache-tag headers * test: add cases for page router non-ascii paths and cache tags * test: add cases for app router non-ascii paths * test: add comma cases --------- Co-authored-by: pieh <[email protected]>
opennextjs · Oct 9, 2024 · fcf2414 · fcf2414
1 parent 0b74e13
commit fcf2414
Show file tree

Hide file tree

Showing 10 changed files with 114 additions and 22 deletions.
diff --git a/playwright.config.ts b/playwright.config.ts
@@ -25,6 +25,7 @@ export default defineConfig({
     extraHTTPHeaders: {
       /* Add debug logging for netlify cache headers */
       'x-nf-debug-logging': '1',
+      'x-next-debug-logging': '1',
     },
   },
   timeout: 10 * 60 * 1000,

diff --git a/src/run/handlers/cache.cts b/src/run/handlers/cache.cts
@@ -139,15 +139,17 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
       cacheValue.kind === 'APP_ROUTE'
     ) {
       if (cacheValue.headers?.[NEXT_CACHE_TAGS_HEADER]) {
-        const cacheTags = (cacheValue.headers[NEXT_CACHE_TAGS_HEADER] as string).split(',')
+        const cacheTags = (cacheValue.headers[NEXT_CACHE_TAGS_HEADER] as string).split(/,|%2c/gi)
         requestContext.responseCacheTags = cacheTags
       } else if (
         (cacheValue.kind === 'PAGE' || cacheValue.kind === 'PAGES') &&
         typeof cacheValue.pageData === 'object'
       ) {
         // pages router doesn't have cache tags headers in PAGE cache value
         // so we need to generate appropriate cache tags for it
-        const cacheTags = [`_N_T_${key === '/index' ? '/' : key}`]
+        // encode here to deal with non ASCII characters in the key
+
+        const cacheTags = [`_N_T_${key === '/index' ? '/' : encodeURI(key)}`]
         requestContext.responseCacheTags = cacheTags
       }
     }
@@ -341,10 +343,11 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
       if (data?.kind === 'PAGE' || data?.kind === 'PAGES') {
         const requestContext = getRequestContext()
         if (requestContext?.didPagesRouterOnDemandRevalidate) {
-          const tag = `_N_T_${key === '/index' ? '/' : key}`
+          // encode here to deal with non ASCII characters in the key
+          const tag = `_N_T_${key === '/index' ? '/' : encodeURI(key)}`
           getLogger().debug(`Purging CDN cache for: [${tag}]`)
           requestContext.trackBackgroundWork(
-            purgeCache({ tags: [tag] }).catch((error) => {
+            purgeCache({ tags: tag.split(/,|%2c/gi) }).catch((error) => {
               // TODO: add reporting here
               getLogger()
                 .withError(error)
@@ -372,7 +375,9 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
   private async doRevalidateTag(tagOrTags: string | string[], ...args: any) {
     getLogger().withFields({ tagOrTags, args }).debug('NetlifyCacheHandler.revalidateTag')
 
-    const tags = Array.isArray(tagOrTags) ? tagOrTags : [tagOrTags]
+    const tags = (Array.isArray(tagOrTags) ? tagOrTags : [tagOrTags]).flatMap((tag) =>
+      tag.split(/,|%2c/gi),
+    )
 
     const data: TagManifest = {
       revalidatedAt: Date.now(),
@@ -419,7 +424,8 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
       cacheEntry.value?.kind === 'ROUTE' ||
       cacheEntry.value?.kind === 'APP_ROUTE'
     ) {
-      cacheTags = (cacheEntry.value.headers?.[NEXT_CACHE_TAGS_HEADER] as string)?.split(',') || []
+      cacheTags =
+        (cacheEntry.value.headers?.[NEXT_CACHE_TAGS_HEADER] as string)?.split(/,|%2c/gi) || []
     } else {
       return false
     }

diff --git a/tests/e2e/on-demand-app.test.ts b/tests/e2e/on-demand-app.test.ts
@@ -45,6 +45,20 @@ test.describe('app router on-demand revalidation', () => {
       revalidateApiPath: '/api/on-demand-revalidate/tag?tag=show-4',
       expectedH1Content: 'Hello, Statically fetched show 4',
     },
+    {
+      label: 'revalidatePath (prerendered page with dynamic path) - non-ASCII variant',
+      prerendered: true,
+      pagePath: '/product/事前レンダリング,test',
+      revalidateApiPath: `/api/on-demand-revalidate/path?path=/product/事前レンダリング,test`,
+      expectedH1Content: 'Product 事前レンダリング,test',
+    },
+    {
+      label: 'revalidatePath (not prerendered page with dynamic path) - non-ASCII variant',
+      prerendered: false,
+      pagePath: '/product/事前レンダリングされていない,test',
+      revalidateApiPath: `/api/on-demand-revalidate/path?path=/product/事前レンダリングされていない,test`,
+      expectedH1Content: 'Product 事前レンダリングされていない,test',
+    },
   ]) {
     test(label, async ({ page, pollUntilHeadersMatch, serverComponents }) => {
       // in case there is retry or some other test did hit that path before

diff --git a/tests/e2e/page-router.test.ts b/tests/e2e/page-router.test.ts
@@ -80,12 +80,28 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
         revalidateApiBasePath: '/api/revalidate-no-await',
         expectedH1Content: 'Product not-prerendered-and-not-awaited-revalidation',
       },
+      {
+        label:
+          'prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant',
+        prerendered: true,
+        pagePath: '/products/事前レンダリング,test',
+        revalidateApiBasePath: '/api/revalidate',
+        expectedH1Content: 'Product 事前レンダリング,test',
+      },
+      {
+        label:
+          'not prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant',
+        prerendered: false,
+        pagePath: '/products/事前レンダリングされていない,test',
+        revalidateApiBasePath: '/api/revalidate',
+        expectedH1Content: 'Product 事前レンダリングされていない,test',
+      },
     ]) {
       test(label, async ({ page, pollUntilHeadersMatch, pageRouter }) => {
         // in case there is retry or some other test did hit that path before
         // we want to make sure that cdn cache is not warmed up
         const purgeCdnCache = await page.goto(
-          new URL(`/api/purge-cdn?path=${pagePath}`, pageRouter.url).href,
+          new URL(`/api/purge-cdn?path=${encodeURI(pagePath)}`, pageRouter.url).href,
         )
         expect(purgeCdnCache?.status()).toBe(200)
 
@@ -110,7 +126,7 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
         const headers1 = response1?.headers() || {}
         expect(response1?.status()).toBe(200)
         expect(headers1['x-nextjs-cache']).toBeUndefined()
-        expect(headers1['netlify-cache-tag']).toBe(`_n_t_${pagePath}`)
+        expect(headers1['netlify-cache-tag']).toBe(`_n_t_${encodeURI(pagePath).toLowerCase()}`)
         expect(headers1['netlify-cdn-cache-control']).toBe(
           's-maxage=31536000, stale-while-revalidate=31536000, durable',
         )
@@ -138,7 +154,7 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
         const headers1Json = response1Json?.headers() || {}
         expect(response1Json?.status()).toBe(200)
         expect(headers1Json['x-nextjs-cache']).toBeUndefined()
-        expect(headers1Json['netlify-cache-tag']).toBe(`_n_t_${pagePath}`)
+        expect(headers1Json['netlify-cache-tag']).toBe(`_n_t_${encodeURI(pagePath).toLowerCase()}`)
         expect(headers1Json['netlify-cdn-cache-control']).toBe(
           's-maxage=31536000, stale-while-revalidate=31536000, durable',
         )
@@ -459,14 +475,32 @@ test.describe('Page Router with basePath and i18n', () => {
         revalidateApiBasePath: '/api/revalidate-no-await',
         expectedH1Content: 'Product not-prerendered-and-not-awaited-revalidation',
       },
+      {
+        label:
+          'prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant',
+        prerendered: true,
+        pagePath: '/products/事前レンダリング,test',
+        revalidateApiBasePath: '/api/revalidate',
+        expectedH1Content: 'Product 事前レンダリング,test',
+      },
+      {
+        label:
+          'not prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant',
+        prerendered: false,
+        pagePath: '/products/事前レンダリングされていない,test',
+        revalidateApiBasePath: '/api/revalidate',
+        expectedH1Content: 'Product 事前レンダリングされていない,test',
+      },
     ]) {
       test.describe(label, () => {
         test(`default locale`, async ({ page, pollUntilHeadersMatch, pageRouterBasePathI18n }) => {
           // in case there is retry or some other test did hit that path before
           // we want to make sure that cdn cache is not warmed up
           const purgeCdnCache = await page.goto(
-            new URL(`/base/path/api/purge-cdn?path=/en${pagePath}`, pageRouterBasePathI18n.url)
-              .href,
+            new URL(
+              `/base/path/api/purge-cdn?path=/en${encodeURI(pagePath)}`,
+              pageRouterBasePathI18n.url,
+            ).href,
           )
           expect(purgeCdnCache?.status()).toBe(200)
 
@@ -494,7 +528,9 @@ test.describe('Page Router with basePath and i18n', () => {
           const headers1ImplicitLocale = response1ImplicitLocale?.headers() || {}
           expect(response1ImplicitLocale?.status()).toBe(200)
           expect(headers1ImplicitLocale['x-nextjs-cache']).toBeUndefined()
-          expect(headers1ImplicitLocale['netlify-cache-tag']).toBe(`_n_t_/en${pagePath}`)
+          expect(headers1ImplicitLocale['netlify-cache-tag']).toBe(
+            `_n_t_/en${encodeURI(pagePath).toLowerCase()}`,
+          )
           expect(headers1ImplicitLocale['netlify-cdn-cache-control']).toBe(
             's-maxage=31536000, stale-while-revalidate=31536000, durable',
           )
@@ -520,7 +556,9 @@ test.describe('Page Router with basePath and i18n', () => {
           const headers1ExplicitLocale = response1ExplicitLocale?.headers() || {}
           expect(response1ExplicitLocale?.status()).toBe(200)
           expect(headers1ExplicitLocale['x-nextjs-cache']).toBeUndefined()
-          expect(headers1ExplicitLocale['netlify-cache-tag']).toBe(`_n_t_/en${pagePath}`)
+          expect(headers1ExplicitLocale['netlify-cache-tag']).toBe(
+            `_n_t_/en${encodeURI(pagePath).toLowerCase()}`,
+          )
           expect(headers1ExplicitLocale['netlify-cdn-cache-control']).toBe(
             's-maxage=31536000, stale-while-revalidate=31536000, durable',
           )
@@ -552,7 +590,9 @@ test.describe('Page Router with basePath and i18n', () => {
           const headers1Json = response1Json?.headers() || {}
           expect(response1Json?.status()).toBe(200)
           expect(headers1Json['x-nextjs-cache']).toBeUndefined()
-          expect(headers1Json['netlify-cache-tag']).toBe(`_n_t_/en${pagePath}`)
+          expect(headers1Json['netlify-cache-tag']).toBe(
+            `_n_t_/en${encodeURI(pagePath).toLowerCase()}`,
+          )
           expect(headers1Json['netlify-cdn-cache-control']).toBe(
             's-maxage=31536000, stale-while-revalidate=31536000, durable',
           )
@@ -870,7 +910,7 @@ test.describe('Page Router with basePath and i18n', () => {
           const headers1 = response1?.headers() || {}
           expect(response1?.status()).toBe(200)
           expect(headers1['x-nextjs-cache']).toBeUndefined()
-          expect(headers1['netlify-cache-tag']).toBe(`_n_t_/de${pagePath}`)
+          expect(headers1['netlify-cache-tag']).toBe(`_n_t_/de${encodeURI(pagePath).toLowerCase()}`)
           expect(headers1['netlify-cdn-cache-control']).toBe(
             's-maxage=31536000, stale-while-revalidate=31536000, durable',
           )
@@ -899,7 +939,9 @@ test.describe('Page Router with basePath and i18n', () => {
           const headers1Json = response1Json?.headers() || {}
           expect(response1Json?.status()).toBe(200)
           expect(headers1Json['x-nextjs-cache']).toBeUndefined()
-          expect(headers1Json['netlify-cache-tag']).toBe(`_n_t_/de${pagePath}`)
+          expect(headers1Json['netlify-cache-tag']).toBe(
+            `_n_t_/de${encodeURI(pagePath).toLowerCase()}`,
+          )
           expect(headers1Json['netlify-cdn-cache-control']).toBe(
             's-maxage=31536000, stale-while-revalidate=31536000, durable',
           )

diff --git a/tests/fixtures/page-router-base-path-i18n/pages/products/[slug].js b/tests/fixtures/page-router-base-path-i18n/pages/products/[slug].js
@@ -17,22 +17,22 @@ export async function getStaticProps({ params }) {
   }
 }
 
-export const getStaticPaths = () => {
+/** @type {import('next').GetStaticPaths} */
+export const getStaticPaths = ({ locales }) => {
   return {
     paths: [
       {
         params: {
           slug: 'prerendered',
         },
-        locale: 'en',
       },
       {
         params: {
-          slug: 'prerendered',
+          // Japanese prerendered (non-ascii) and comma
+          slug: '事前レンダリング,test',
         },
-        locale: 'de',
       },
-    ],
+    ].flatMap((pathDescription) => locales.map((locale) => ({ ...pathDescription, locale }))),
     fallback: 'blocking', // false or "blocking"
   }
 }

diff --git a/tests/fixtures/page-router/pages/products/[slug].js b/tests/fixtures/page-router/pages/products/[slug].js
@@ -30,6 +30,12 @@ export const getStaticPaths = () => {
           slug: 'prerendered',
         },
       },
+      {
+        params: {
+          // Japanese prerendered (non-ascii) and comma
+          slug: '事前レンダリング,test',
+        },
+      },
     ],
     fallback: 'blocking', // false or "blocking"
   }

diff --git a/tests/fixtures/server-components/app/api/purge-cdn/route.ts b/tests/fixtures/server-components/app/api/purge-cdn/route.ts
@@ -15,7 +15,7 @@ export async function GET(request: NextRequest) {
     )
   }
   try {
-    await purgeCache({ tags: [`_N_T_${pathToPurge}`] })
+    await purgeCache({ tags: [`_N_T_${encodeURI(pathToPurge)}`] })
     return NextResponse.json(
       {
         status: 'ok',

diff --git a/tests/fixtures/server-components/app/product/[slug]/page.js b/tests/fixtures/server-components/app/product/[slug]/page.js
@@ -0,0 +1,20 @@
+const Product = ({ params }) => (
+  <div>
+    <h1>Product {decodeURIComponent(params.slug)}</h1>
+    <p>
+      This page uses generateStaticParams() to prerender a Product
+      <span data-testid="date-now">{new Date().toISOString()}</span>
+    </p>
+  </div>
+)
+
+export async function generateStaticParams() {
+  return [
+    {
+      // Japanese prerendered (non-ascii) and comma
+      slug: '事前レンダリング,test',
+    },
+  ]
+}
+
+export default Product
diff --git a/tests/integration/cache-handler.test.ts b/tests/integration/cache-handler.test.ts
@@ -46,6 +46,7 @@ describe('page router', () => {
       // the real key is much longer and ends in a hash, but we only assert on the first 50 chars to make it easier
       '/products/an-incredibly-long-product-',
       '/products/prerendered',
+      '/products/事前レンダリング,te',
       '/static/revalidate-automatic',
       '/static/revalidate-manual',
       '/static/revalidate-slow',
@@ -359,6 +360,7 @@ describe('plugin', () => {
       '/api/static/first',
       '/api/static/second',
       '/index',
+      '/product/事前レンダリング,test',
       '/revalidate-fetch',
       '/static-fetch-1',
       '/static-fetch-2',

diff --git a/tests/integration/static.test.ts b/tests/integration/static.test.ts
@@ -37,6 +37,7 @@ test<FixtureTestContext>('requesting a non existing page route that needs to be
   expect(entries.map(({ key }) => decodeBlobKey(key.substring(0, 50))).sort()).toEqual([
     '/products/an-incredibly-long-product-',
     '/products/prerendered',
+    '/products/事前レンダリング,te',
     '/static/revalidate-automatic',
     '/static/revalidate-manual',
     '/static/revalidate-slow',