Skip to content

Commit

Permalink
Add vary header to fix incorrectly caching RSC as HTML response (#41479)
Browse files Browse the repository at this point in the history
Add failing test for the back button download bug. Created it as a new
app given that adding it to the existing `app` suite did not reproduce
the issue for some reason.

The underlying reason is that we need to add `Vary: __rsc__,
__next_router_prefetch__` to ensure Chrome does not cache the response
and serve it on back button.

The download was caused by the `application/octet-stream` content type,
but that was just a consequence of serving the wrong response.

<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change that you're making:
-->

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
timneutkens and ijjk committed Oct 18, 2022
1 parent 6101bf6 commit c5896f2
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 2 deletions.
9 changes: 7 additions & 2 deletions packages/next/server/base-server.ts
Expand Up @@ -1054,8 +1054,13 @@ export default abstract class Server<ServerOptions extends Options = Options> {
) &&
(isSSG || hasServerProps)

if (isAppPath && req.headers['__rsc__']) {
if (isSSG) {
if (isAppPath) {
res.setHeader(
'vary',
'__rsc__, __next_router_state_tree__, __next_router_prefetch__'
)

if (isSSG && req.headers['__rsc__']) {
isDataReq = true
// strip header so we generate HTML still
if (
Expand Down
45 changes: 45 additions & 0 deletions test/e2e/app-dir/back-button-download-bug.test.ts
@@ -0,0 +1,45 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import path from 'path'
import webdriver from 'next-webdriver'

describe('app-dir back button download bug', () => {
if ((global as any).isNextDeploy) {
it('should skip next deploy for now', () => {})
return
}

if (process.env.NEXT_TEST_REACT_VERSION === '^17') {
it('should skip for react v17', () => {})
return
}
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: new FileRef(path.join(__dirname, 'back-button-download-bug')),
dependencies: {
react: 'experimental',
'react-dom': 'experimental',
},
skipStart: true,
})

await next.start()
})
afterAll(() => next.destroy())

it('should redirect route when clicking link', async () => {
const browser = await webdriver(next.url, '/')
const text = await browser
.elementByCss('#to-post-1')
.click()
.waitForElementByCss('#post-page')
.text()
expect(text).toBe('This is the post page')

await browser.back()

expect(await browser.waitForElementByCss('#home-page').text()).toBe('Home!')
})
})
8 changes: 8 additions & 0 deletions test/e2e/app-dir/back-button-download-bug/app/layout.js
@@ -0,0 +1,8 @@
export default function RootLayout({ children }) {
return (
<html lang="en">
<head></head>
<body>{children}</body>
</html>
)
}
13 changes: 13 additions & 0 deletions test/e2e/app-dir/back-button-download-bug/app/page.js
@@ -0,0 +1,13 @@
import Link from 'next/link'

export default function Home() {
return (
<>
<h1 id="home-page">Home!</h1>
<Link href="/post/1">
<a id="to-post-1">To post 1</a>
</Link>
<Link href="/">To /</Link>
</>
)
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/back-button-download-bug/next.config.js
@@ -0,0 +1,10 @@
/** @type {import('next').NextConfig} */
module.exports = {
reactStrictMode: true,
experimental: {
appDir: true,
},
images: {
domains: ['res.cloudinary.com', 'avatars.githubusercontent.com'],
},
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/back-button-download-bug/pages/_app.js
@@ -0,0 +1,3 @@
export default function MyApp({ Component, pageProps }) {
return <Component {...pageProps} />
}
17 changes: 17 additions & 0 deletions test/e2e/app-dir/back-button-download-bug/pages/_document.js
@@ -0,0 +1,17 @@
import Document, { Head, Html, Main, NextScript } from 'next/document'

class MyDocument extends Document {
render() {
return (
<Html lang="en">
<Head />
<body>
<Main />
<NextScript />
</body>
</Html>
)
}
}

export default MyDocument
@@ -0,0 +1,3 @@
export default function Post() {
return <h1 id="post-page">This is the post page</h1>
}

0 comments on commit c5896f2

Please sign in to comment.