Skip to content

Commit

Permalink
fix(#53190): add missing crossOrigin to assetsPrefix resources (#56311)
Browse files Browse the repository at this point in the history
Fixes #53190.

Next.js App Router comprises two categories of resources, same-origin ones (RSC payload, in the form of inline `<script />`) and possibly third-party ones (chunks that respect the `assetPrefix`). The latter should also respect the `crossOrigin` option from `next.config.js`.

Co-authored-by: Jiachi Liu <4800338+huozhi@users.noreply.github.com>
  • Loading branch information
SukkaW and huozhi committed Oct 2, 2023
1 parent e970e05 commit 86274e6
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 40 deletions.
44 changes: 22 additions & 22 deletions packages/next/src/server/app-render/app-render.tsx
Expand Up @@ -467,6 +467,7 @@ export const renderToHTMLOrFlight: AppPageRender = (
href={fullHref}
// @ts-ignore
precedence={precedence}
crossOrigin={renderOpts.crossOrigin}
key={index}
/>
)
Expand Down Expand Up @@ -511,7 +512,7 @@ export const renderToHTMLOrFlight: AppPageRender = (
const ext = /\.(woff|woff2|eot|ttf|otf)$/.exec(fontFilename)![1]
const type = `font/${ext}`
const href = `${assetPrefix}/_next/${fontFilename}`
ComponentMod.preloadFont(href, type)
ComponentMod.preloadFont(href, type, renderOpts.crossOrigin)
}
} else {
try {
Expand Down Expand Up @@ -546,14 +547,15 @@ export const renderToHTMLOrFlight: AppPageRender = (
const precedence =
process.env.NODE_ENV === 'development' ? 'next_' + href : 'next'

ComponentMod.preloadStyle(fullHref)
ComponentMod.preloadStyle(fullHref, renderOpts.crossOrigin)

return (
<link
rel="stylesheet"
href={fullHref}
// @ts-ignore
precedence={precedence}
crossOrigin={renderOpts.crossOrigin}
key={index}
/>
)
Expand Down Expand Up @@ -1449,21 +1451,26 @@ export const renderToHTMLOrFlight: AppPageRender = (
tree: LoaderTree
formState: any
}) => {
const polyfills = buildManifest.polyfillFiles
.filter(
(polyfill) =>
polyfill.endsWith('.js') && !polyfill.endsWith('.module.js')
)
.map((polyfill) => ({
src: `${assetPrefix}/_next/${polyfill}${getAssetQueryString(
false
)}`,
integrity: subresourceIntegrityManifest?.[polyfill],
}))
const polyfills: JSX.IntrinsicElements['script'][] =
buildManifest.polyfillFiles
.filter(
(polyfill) =>
polyfill.endsWith('.js') && !polyfill.endsWith('.module.js')
)
.map((polyfill) => ({
src: `${assetPrefix}/_next/${polyfill}${getAssetQueryString(
false
)}`,
integrity: subresourceIntegrityManifest?.[polyfill],
crossOrigin: renderOpts.crossOrigin,
noModule: true,
nonce,
}))

const [preinitScripts, bootstrapScript] = getRequiredScripts(
buildManifest,
assetPrefix,
renderOpts.crossOrigin,
subresourceIntegrityManifest,
getAssetQueryString(true),
nonce
Expand Down Expand Up @@ -1533,15 +1540,7 @@ export const renderToHTMLOrFlight: AppPageRender = (
{polyfillsFlushed
? null
: polyfills?.map((polyfill) => {
return (
<script
key={polyfill.src}
src={polyfill.src}
integrity={polyfill.integrity}
noModule={true}
nonce={nonce}
/>
)
return <script key={polyfill.src} {...polyfill} />
})}
{renderServerInsertedHTML()}
{errorMetaTags}
Expand Down Expand Up @@ -1651,6 +1650,7 @@ export const renderToHTMLOrFlight: AppPageRender = (
getRequiredScripts(
buildManifest,
assetPrefix,
renderOpts.crossOrigin,
subresourceIntegrityManifest,
getAssetQueryString(false),
nonce
Expand Down
28 changes: 21 additions & 7 deletions packages/next/src/server/app-render/required-scripts.tsx
Expand Up @@ -5,24 +5,35 @@ import ReactDOM from 'react-dom'
export function getRequiredScripts(
buildManifest: BuildManifest,
assetPrefix: string,
crossOrigin: string | undefined,
SRIManifest: undefined | Record<string, string>,
qs: string,
nonce: string | undefined
): [() => void, string | { src: string; integrity: string }] {
): [
() => void,
{ src: string; integrity?: string; crossOrigin?: string | undefined }
] {
let preinitScripts: () => void
let preinitScriptCommands: string[] = []
let bootstrapScript: string | { src: string; integrity: string } = ''
const bootstrapScript: {
src: string
integrity?: string
crossOrigin?: string | undefined
} = {
src: '',
crossOrigin,
}

const files = buildManifest.rootMainFiles
if (files.length === 0) {
throw new Error(
'Invariant: missing bootstrap script. This is a bug in Next.js'
)
}
if (SRIManifest) {
bootstrapScript = {
src: `${assetPrefix}/_next/` + files[0] + qs,
integrity: SRIManifest[files[0]],
}
bootstrapScript.src = `${assetPrefix}/_next/` + files[0] + qs
bootstrapScript.integrity = SRIManifest[files[0]]

for (let i = 1; i < files.length; i++) {
const src = `${assetPrefix}/_next/` + files[i] + qs
const integrity = SRIManifest[files[i]]
Expand All @@ -34,12 +45,14 @@ export function getRequiredScripts(
ReactDOM.preinit(preinitScriptCommands[i], {
as: 'script',
integrity: preinitScriptCommands[i + 1],
crossOrigin,
nonce,
})
}
}
} else {
bootstrapScript = `${assetPrefix}/_next/` + files[0] + qs
bootstrapScript.src = `${assetPrefix}/_next/` + files[0] + qs

for (let i = 1; i < files.length; i++) {
const src = `${assetPrefix}/_next/` + files[i] + qs
preinitScriptCommands.push(src)
Expand All @@ -50,6 +63,7 @@ export function getRequiredScripts(
ReactDOM.preinit(preinitScriptCommands[i], {
as: 'script',
nonce,
crossOrigin,
})
}
}
Expand Down
31 changes: 21 additions & 10 deletions packages/next/src/server/app-render/rsc/preloads.ts
Expand Up @@ -6,18 +6,29 @@ Files in the rsc directory are meant to be packaged as part of the RSC graph usi

import ReactDOM from 'react-dom'

export function preloadStyle(href: string) {
ReactDOM.preload(href, { as: 'style' })
}

export function preloadFont(href: string, type: string) {
;(ReactDOM as any).preload(href, { as: 'font', type })
export function preloadStyle(href: string, crossOrigin?: string | undefined) {
const opts: any = { as: 'style' }
if (typeof crossOrigin === 'string') {
opts.crossOrigin = crossOrigin
}
ReactDOM.preload(href, opts)
}

export function preconnect(href: string, crossOrigin?: string) {
export function preloadFont(
href: string,
type: string,
crossOrigin?: string | undefined
) {
const opts: any = { as: 'font', type }
if (typeof crossOrigin === 'string') {
;(ReactDOM as any).preconnect(href, { crossOrigin })
} else {
;(ReactDOM as any).preconnect(href)
opts.crossOrigin = crossOrigin
}
ReactDOM.preload(href, opts)
}

export function preconnect(href: string, crossOrigin?: string | undefined) {
;(ReactDOM as any).preconnect(
href,
typeof crossOrigin === 'string' ? { crossOrigin } : undefined
)
}
3 changes: 2 additions & 1 deletion packages/next/src/server/app-render/types.ts
Expand Up @@ -101,7 +101,7 @@ export type ChildProp = {
segment: Segment
}

export type RenderOptsPartial = {
export interface RenderOptsPartial {
err?: Error | null
dev?: boolean
buildId: string
Expand All @@ -111,6 +111,7 @@ export type RenderOptsPartial = {
runtime?: ServerRuntime
serverComponents?: boolean
assetPrefix?: string
crossOrigin?: '' | 'anonymous' | 'use-credentials' | undefined
nextFontManifest?: NextFontManifest
isBot?: boolean
incrementalCache?: import('../lib/incremental-cache').IncrementalCache
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/app-dir/app-config-crossorigin/app/layout.js
@@ -0,0 +1,7 @@
export default function RootLayout({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app-config-crossorigin/app/page.js
@@ -0,0 +1,3 @@
export default function Index(props) {
return <p id="title">IndexPage</p>
}
37 changes: 37 additions & 0 deletions test/e2e/app-dir/app-config-crossorigin/index.test.ts
@@ -0,0 +1,37 @@
import { createNextDescribe } from 'e2e-utils'

createNextDescribe(
'app dir - crossOrigin config',
{
files: __dirname,
skipDeployment: true,
},
({ next, isNextStart }) => {
if (isNextStart) {
it('skip in start mode', () => {})
return
}
it('should render correctly with assetPrefix: "/"', async () => {
const $ = await next.render$('/')
// Only potential external (assetPrefix) <script /> and <link /> should have crossorigin attribute
$(
'script[src*="https://example.vercel.sh"], link[href*="https://example.vercel.sh"]'
).each((_, el) => {
const crossOrigin = $(el).attr('crossorigin')
expect(crossOrigin).toBe('use-credentials')
})

// Inline <script /> (including RSC payload) and <link /> should not have crossorigin attribute
$('script:not([src]), link:not([href])').each((_, el) => {
const crossOrigin = $(el).attr('crossorigin')
expect(crossOrigin).toBeUndefined()
})

// Same origin <script /> and <link /> should not have crossorigin attribute either
$('script[src^="/"], link[href^="/"]').each((_, el) => {
const crossOrigin = $(el).attr('crossorigin')
expect(crossOrigin).toBeUndefined()
})
})
}
)
16 changes: 16 additions & 0 deletions test/e2e/app-dir/app-config-crossorigin/next.config.js
@@ -0,0 +1,16 @@
module.exports = {
/**
* The "assetPrefix" here doesn't needs to be real as we doesn't load the page in the browser in this test,
* we only care about if all assets prefixed with the "assetPrefix" are having correct "crossOrigin".
*/
assetPrefix: 'https://example.vercel.sh',

/**
* According to HTML5 Spec (https://html.spec.whatwg.org/multipage/urls-and-fetching.html#cors-settings-attributes),
* crossorigin="" and crossorigin="anonymous" has the same effect. And ReactDOM's preload methods (preload, preconnect, etc.)
* will prefer crossorigin="" to save bytes.
*
* So we use "use-credentials" here for easier testing.
*/
crossOrigin: 'use-credentials',
}

0 comments on commit 86274e6

Please sign in to comment.