Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

correctly assess node equality when nonce attribute is present #27573

Merged
merged 16 commits into from Nov 11, 2021
35 changes: 34 additions & 1 deletion packages/next/client/head-manager.ts
Expand Up @@ -40,6 +40,39 @@ function reactElementToDOM({ type, props }: JSX.Element): HTMLElement {
return el
}

/**
* When a `nonce` is present on an element, browsers such as Chrome and Firefox strip it out of the
* actual HTML attributes for security reasons *when the element is added to the document*. Thus,
* given two equivalent elements that have nonces, `Element,isEqualNode()` will return false if one
* of those elements gets added to the document. Although the `element.nonce` property will be the
* same for both elements, the one that was added to the document will return an empty string for
* its nonce HTML attribute value.
*
* This custom `isEqualNode()` function therefore removes the nonce value from the `newTag` before
* comparing it to `oldTag`, restoring it afterwards.
*
* For more information, see:
* https://bugs.chromium.org/p/chromium/issues/detail?id=1211471#c12
*/
export function isEqualNode(oldTag: Element, newTag: Element) {
if (oldTag instanceof HTMLElement && newTag instanceof HTMLElement) {
const nonce = newTag.getAttribute('nonce')
// Only strip the nonce if `oldTag` has had it stripped. An element's nonce attribute will not
// be stripped if there is no content security policy response header that includes a nonce.
if (nonce && !oldTag.getAttribute('nonce')) {
// Remove nonce from HTML attribute for comparison
newTag.setAttribute('nonce', '')
newTag.nonce = nonce
const isEqual = nonce === oldTag.nonce && oldTag.isEqualNode(newTag)
// Restore original nonce
newTag.setAttribute('nonce', nonce)
return isEqual
}
}

return oldTag.isEqualNode(newTag)
}

function updateElements(type: string, components: JSX.Element[]): void {
const headEl = document.getElementsByTagName('head')[0]
const headCountEl: HTMLMetaElement = headEl.querySelector(
Expand Down Expand Up @@ -70,7 +103,7 @@ function updateElements(type: string, components: JSX.Element[]): void {
(newTag) => {
for (let k = 0, len = oldTags.length; k < len; k++) {
const oldTag = oldTags[k]
if (oldTag.isEqualNode(newTag)) {
if (isEqualNode(oldTag, newTag)) {
oldTags.splice(k, 1)
return false
}
Expand Down
17 changes: 17 additions & 0 deletions test/integration/head-manager/pages/_document.js
@@ -0,0 +1,17 @@
import Document, { Head, Html, Main, NextScript } from 'next/document'

class NextDocument extends Document {
render() {
return (
<Html>
<Head nonce="abc" />
<body>
<Main />
<NextScript nonce="abc" />
</body>
</Html>
)
}
}

export default NextDocument
23 changes: 23 additions & 0 deletions test/integration/head-manager/pages/nonce.js
@@ -0,0 +1,23 @@
import React from 'react'
import Head from 'next/head'

const Page = () => {
const [counter, setCounter] = React.useState(0)
const [useSrc1, setUseSrc1] = React.useState(true)

return (
<>
<Head>
<script nonce="abc" src={useSrc1 ? '/src-1.js' : '/src-2.js'} />
</Head>
<button id="force-rerender" onClick={() => setCounter(counter + 1)}>
Re-render
</button>
<button id="change-script-src" onClick={() => setUseSrc1(!useSrc1)}>
Change script src
</button>
</>
)
}

export default Page
2 changes: 2 additions & 0 deletions test/integration/head-manager/public/src-1.js
@@ -0,0 +1,2 @@
window.scriptExecutionIds = window.scriptExecutionIds || []
window.scriptExecutionIds.push('src-1.js')
2 changes: 2 additions & 0 deletions test/integration/head-manager/public/src-2.js
@@ -0,0 +1,2 @@
window.scriptExecutionIds = window.scriptExecutionIds || []
window.scriptExecutionIds.push('src-2.js')
30 changes: 30 additions & 0 deletions test/integration/head-manager/server.js
@@ -0,0 +1,30 @@
const { createServer } = require('http')
ijjk marked this conversation as resolved.
Show resolved Hide resolved
const { parse } = require('url')
const next = require('next')

const dev = process.env.NODE_ENV !== 'production'
const dir = __dirname
const app = next({ dev, dir })
const handle = app.getRequestHandler()

const { PORT, INCLUDE_CSP } = process.env

app.prepare().then(() => {
createServer((req, res) => {
// Be sure to pass `true` as the second argument to `url.parse`.
// This tells it to parse the query portion of the URL.
const parsedUrl = parse(req.url, true)

if (INCLUDE_CSP) {
res.setHeader(
'Content-Security-Policy',
"script-src 'nonce-abc123' 'unsafe-eval'"
)
}

handle(req, res, parsedUrl)
}).listen(PORT, (err) => {
if (err) throw err
console.log(`> Ready on http://localhost:${PORT}`)
})
})
27 changes: 27 additions & 0 deletions test/integration/head-manager/start-server.js
@@ -0,0 +1,27 @@
const { createServer } = require('http')
styfle marked this conversation as resolved.
Show resolved Hide resolved
const { parse } = require('url')
const next = require('next')

const dev = process.env.NODE_ENV !== 'production'
const dir = __dirname
const app = next({ dev, dir })
const handle = app.getRequestHandler()

export const startServer = (port, includeCsp) =>
app.prepare().then(() =>
createServer((req, res) => {
const parsedUrl = parse(req.url, true)

if (includeCsp) {
res.setHeader(
'Content-Security-Policy',
"script-src 'nonce-abc123' 'unsafe-eval'"
)
}

handle(req, res, parsedUrl)
}).listen(port, (err) => {
if (err) throw err
console.log(`> Ready on http://localhost:${port}`)
})
)
51 changes: 51 additions & 0 deletions test/integration/head-manager/test/nonce.test.js
@@ -0,0 +1,51 @@
/* eslint-env jest */
import { join } from 'path'
import {
findPort,
waitFor,
killApp,
initNextServerScript,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import clone from 'clone'

const ctx = {}
jest.setTimeout(1000 * 60 * 5)

const startServer = async (includeCsp = true) => {
const scriptPath = join(__dirname, '../server.js')
ctx.appPort = await findPort()
const env = Object.assign({}, clone(process.env), {
PORT: `${ctx.appPort}`,
INCLUDE_CSP: includeCsp,
})

ctx.server = await initNextServerScript(
scriptPath,
/ready on/i,
env,
/ReferenceError: options is not defined/
)
}

describe('nonce', () => {
afterAll(async () => {
if (ctx.browser) await ctx.browser.close()
killApp(ctx.server)
})

it('Re-rendering should not re-execute the script', async () => {
ctx.server = await startServer()
ctx.browser = await webdriver(ctx.appPort, '/nonce')
await waitFor(5000)

expect(await ctx.browser.eval(`window.scriptExecutionIds`)).toEqual([
'src-1.js',
])
ctx.browser.elementByCss('#force-rerender').click()
await waitFor(100)
expect(await ctx.browser.eval(`window.scriptExecutionIds`)).toEqual([
'src-1.js',
])
})
})
37 changes: 37 additions & 0 deletions test/integration/head-manager/test/nonce2.test.js
@@ -0,0 +1,37 @@
/* eslint-env jest */
import { join } from 'path'
import {
findPort,
waitFor,
killApp,
initNextServerScript,
launchApp,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import { startServer } from '../start-server'

const ctx = {}
// jest.setTimeout(1000 * 60 * 5)

describe('nonce', () => {
afterAll(async () => {
if (ctx.browser) await ctx.browser.close()
ctx.server.close()
})

it('Re-rendering should not re-execute the script', async () => {
ctx.appPort = await findPort()
ctx.server = await startServer(ctx.appPort)
ctx.browser = await webdriver(ctx.appPort, '/nonce')
await waitFor(500)

expect(await ctx.browser.eval(`window.scriptExecutionIds`)).toEqual([
'src-1.js',
])
ctx.browser.elementByCss('#force-rerender').click()
await waitFor(100)
expect(await ctx.browser.eval(`window.scriptExecutionIds`)).toEqual([
'src-1.js',
])
})
})
2 changes: 1 addition & 1 deletion test/integration/image-component/basic/test/index.test.js
Expand Up @@ -22,7 +22,7 @@ const emptyImage =

function runTests() {
it('should render an image tag', async () => {
await waitFor(1000)
await waitFor(100000)
ijjk marked this conversation as resolved.
Show resolved Hide resolved
expect(await browser.hasElementByCssSelector('img')).toBeTruthy()
})
it('should support passing through arbitrary attributes', async () => {
Expand Down
48 changes: 48 additions & 0 deletions test/unit/is-equal-node.unit.test.js
@@ -0,0 +1,48 @@
/**
* @jest-environment jsdom
*/
/* eslint-env jest */
import { isEqualNode } from 'next/dist/client/head-manager'

const createScriptElement = (attrs = {}) => {
const el = document.createElement('script')
for (const k in attrs) el.setAttribute(k, attrs[k])
return el
}

describe('isEqualNode', () => {
it('Node should equal itself', () => {
const el = createScriptElement()
expect(isEqualNode(el, el)).toBe(true)
})

it('Node should equal equivalent node that has no nonce', () => {
const el1 = createScriptElement()
const el2 = createScriptElement()
expect(isEqualNode(el1, el2)).toBe(true)
})

it('Node should equal equivalent node that has same nonce property, even if the original node has no html nonce attribute value', () => {
const el1 = createScriptElement({ nonce: 'abc123' })
// Simulate Chrome/FF browser behavior of stripping off nonce value when adding element to the document
el1.setAttribute('nonce', '')
el1.nonce = 'abc123'
const el2 = createScriptElement({ nonce: 'abc123' })
expect(isEqualNode(el1, el2)).toBe(true)
})

it('Node should not equal node with different nonce value', () => {
const el1 = createScriptElement({ nonce: 'abc123' })
// Simulate Chrome/FF browser behavior of stripping off nonce value when adding element to the document
el1.setAttribute('nonce', '')
el1.nonce = 'abc123'
const el2 = createScriptElement({ nonce: 'xyz' })
expect(isEqualNode(el1, el2)).toBe(false)
})

it('Node should not equal node with differnt html attribute value', () => {
ericbiewener marked this conversation as resolved.
Show resolved Hide resolved
const el1 = createScriptElement({ src: '1.js' })
const el2 = createScriptElement({ src: '2.js' })
expect(isEqualNode(el1, el2)).toBe(false)
})
})