From 325dc4b6800e96ad69f315d18468d2c0b3319675 Mon Sep 17 00:00:00 2001
From: Zack Tanner <1939140+ztanner@users.noreply.github.com>
Date: Sat, 13 Jul 2024 10:48:38 -0700
Subject: [PATCH] pages router: ensure x-middleware-cache is respected (#67734)

`x-middleware-cache: "no-store"` in Pages router is a way to signal to
the client that it should not store the response in the cache. However
in certain circumstances, namely when `unstable_skipClientCache` is
true, the data request would be awaited and then stored in the
`inflightCache` regardless of the header.

The original implementation of this in the router has logic to delete
the response from the inflight cache after the request has fulfilled
because `inflightCache` stores the unresolved promise. But in this
optimistic prefetch case, when we're only storing it in the cache once
the request is fulfilled, we can prevent a race condition where the
ignored prefetch is erroneously re-added to the cache by ensuring it's
never added to the cache to begin with if the response says not to.

Fixes #66881
Closes NEXT-3550
---
 packages/next/src/shared/lib/router/router.ts |  6 +++-
 .../e2e/middleware-rewrites/app/middleware.js | 13 ++++++++
 .../app/pages/dynamic-no-cache/[id].js        | 22 +++++++++++++
 .../middleware-rewrites/app/pages/index.js    |  1 +
 .../middleware-rewrites/test/index.test.ts    | 32 ++++++++++++++++++-
 5 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100644 test/e2e/middleware-rewrites/app/pages/dynamic-no-cache/[id].js

diff --git a/packages/next/src/shared/lib/router/router.ts b/packages/next/src/shared/lib/router/router.ts
index 5e4ffceaee7fa..19be0fd09deef 100644
--- a/packages/next/src/shared/lib/router/router.ts
+++ b/packages/next/src/shared/lib/router/router.ts
@@ -588,7 +588,11 @@ function fetchNextData({
   // without blocking navigation when stale data is available
   if (unstable_skipClientCache && persistCache) {
     return getData({}).then((data) => {
-      inflightCache[cacheKey] = Promise.resolve(data)
+      if (data.response.headers.get('x-middleware-cache') !== 'no-cache') {
+        // only update cache if not marked as no-cache
+        inflightCache[cacheKey] = Promise.resolve(data)
+      }
+
       return data
     })
   }
diff --git a/test/e2e/middleware-rewrites/app/middleware.js b/test/e2e/middleware-rewrites/app/middleware.js
index 4990063415110..493e3734e15ad 100644
--- a/test/e2e/middleware-rewrites/app/middleware.js
+++ b/test/e2e/middleware-rewrites/app/middleware.js
@@ -109,6 +109,19 @@ export async function middleware(request) {
     return NextResponse.rewrite(url)
   }
 
+  if (url.pathname === '/dynamic-no-cache/1') {
+    const rewriteUrl =
+      request.headers.get('purpose') === 'prefetch'
+        ? '/dynamic-no-cache/1'
+        : '/dynamic-no-cache/2'
+
+    url.pathname = rewriteUrl
+
+    return NextResponse.rewrite(url, {
+      headers: { 'x-middleware-cache': 'no-cache' },
+    })
+  }
+
   if (
     url.pathname === '/rewrite-me-without-hard-navigation' ||
     url.searchParams.get('path') === 'rewrite-me-without-hard-navigation'
diff --git a/test/e2e/middleware-rewrites/app/pages/dynamic-no-cache/[id].js b/test/e2e/middleware-rewrites/app/pages/dynamic-no-cache/[id].js
new file mode 100644
index 0000000000000..6727cd6f22584
--- /dev/null
+++ b/test/e2e/middleware-rewrites/app/pages/dynamic-no-cache/[id].js
@@ -0,0 +1,22 @@
+export default function Dynamic({ id }) {
+  return (
+    <main id="dynamic-page">
+      <h1>Page {id}</h1>
+    </main>
+  )
+}
+
+export const getStaticProps = async ({ params }) => {
+  return {
+    props: {
+      id: params.id,
+    },
+  }
+}
+
+export const getStaticPaths = async () => {
+  return {
+    paths: [{ params: { id: '1' } }, { params: { id: '2' } }],
+    fallback: false,
+  }
+}
diff --git a/test/e2e/middleware-rewrites/app/pages/index.js b/test/e2e/middleware-rewrites/app/pages/index.js
index 2e3d07c7ec261..a3cd14e1e3e15 100644
--- a/test/e2e/middleware-rewrites/app/pages/index.js
+++ b/test/e2e/middleware-rewrites/app/pages/index.js
@@ -66,6 +66,7 @@ export default function Home() {
       <Link href="/ssg" id="normal-ssg-link">
         normal SSG link
       </Link>
+      <Link href="/dynamic-no-cache/1">/dynamic-no-cache/1</Link>
       <div />
       <a
         href=""
diff --git a/test/e2e/middleware-rewrites/test/index.test.ts b/test/e2e/middleware-rewrites/test/index.test.ts
index 5c54b8a718b4b..532a677de452f 100644
--- a/test/e2e/middleware-rewrites/test/index.test.ts
+++ b/test/e2e/middleware-rewrites/test/index.test.ts
@@ -7,7 +7,7 @@ import { NextInstance } from 'e2e-utils'
 import { check, fetchViaHTTP, retry } from 'next-test-utils'
 import { createNext, FileRef } from 'e2e-utils'
 import escapeStringRegexp from 'escape-string-regexp'
-import { Request } from 'playwright'
+import { Request, Response } from 'playwright'
 
 describe('Middleware Rewrite', () => {
   let next: NextInstance
@@ -363,6 +363,36 @@ describe('Middleware Rewrite', () => {
     })
 
     if (!(global as any).isNextDev) {
+      it('should opt out of prefetch caching for dynamic routes', async () => {
+        const browser = await webdriver(next.url, '/')
+        await browser.eval('window.__SAME_PAGE = true')
+        await browser.waitForIdleNetwork()
+        let hasResolvedPrefetch = false
+
+        browser.on('response', async (res: Response) => {
+          const req = res.request()
+          const headers = await req.allHeaders()
+          if (
+            headers['purpose'] === 'prefetch' &&
+            req.url().includes('/dynamic-no-cache/1')
+          ) {
+            hasResolvedPrefetch = true
+          }
+        })
+
+        await browser.elementByCss("[href='/dynamic-no-cache/1']").moveTo()
+
+        await retry(async () => {
+          expect(hasResolvedPrefetch).toBe(true)
+        })
+
+        await browser.elementByCss("[href='/dynamic-no-cache/1']").click()
+
+        await browser.waitForElementByCss('#dynamic-page')
+        expect(await browser.elementById('dynamic-page').text()).toBe('Page 2')
+        expect(await browser.eval('window.__SAME_PAGE')).toBe(true)
+      })
+
       it('should not prefetch non-SSG routes', async () => {
         const browser = await webdriver(next.url, '/')