correctly assess node equality when nonce attribute is present (#27573)

* add isEqualNode function * add test * trying to make integration test work * revert * Update test/unit/is-equal-node.unit.test.js Co-authored-by: Steven <[email protected]> * Revert "revert" This reverts commit d67b997. * Fix tests * Use TS for unit test * Revert waitfor * Start tests with "should" * Fix lint * Use cloneNode() Co-authored-by: Eric Biewener <[email protected]> Co-authored-by: Steven <[email protected]>
vercel · Nov 11, 2021 · c791da0 · c791da0
1 parent 0196b03
commit c791da0
Show file tree

Hide file tree

Showing 9 changed files with 219 additions and 1 deletion.
diff --git a/packages/next/client/head-manager.ts b/packages/next/client/head-manager.ts
@@ -40,6 +40,36 @@ 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')) {
+      const cloneTag = newTag.cloneNode(true) as typeof newTag
+      cloneTag.setAttribute('nonce', '')
+      cloneTag.nonce = nonce
+      return nonce === oldTag.nonce && oldTag.isEqualNode(cloneTag)
+    }
+  }
+
+  return oldTag.isEqualNode(newTag)
+}
+
 function updateElements(type: string, components: JSX.Element[]): void {
   const headEl = document.getElementsByTagName('head')[0]
   const headCountEl: HTMLMetaElement = headEl.querySelector(
@@ -70,7 +100,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
         }

diff --git a/test/e2e/nonce-head-manager/app/next.config.js b/test/e2e/nonce-head-manager/app/next.config.js
@@ -0,0 +1,15 @@
+module.exports = {
+  async headers() {
+    return [
+      {
+        source: '/csp',
+        headers: [
+          {
+            key: 'Content-Security-Policy',
+            value: "script-src-elem 'nonce-abc123' 'unsafe-eval'",
+          },
+        ],
+      },
+    ]
+  },
+}
diff --git a/test/e2e/nonce-head-manager/app/pages/_document.js b/test/e2e/nonce-head-manager/app/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="abc123" />
+        <body>
+          <Main />
+          <NextScript nonce="abc123" />
+        </body>
+      </Html>
+    )
+  }
+}
+
+export default NextDocument
diff --git a/test/e2e/nonce-head-manager/app/pages/csp.js b/test/e2e/nonce-head-manager/app/pages/csp.js
@@ -0,0 +1,24 @@
+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="abc123" src={useSrc1 ? '/src-1.js' : '/src-2.js'} />
+      </Head>
+      <h1 id="h1">{'Count ' + counter}</h1>
+      <button id="force-rerender" onClick={() => setCounter(counter + 1)}>
+        Re-render
+      </button>
+      <button id="change-script" onClick={() => setUseSrc1(!useSrc1)}>
+        Change script src
+      </button>
+    </>
+  )
+}
+
+export default Page
diff --git a/test/e2e/nonce-head-manager/app/pages/index.js b/test/e2e/nonce-head-manager/app/pages/index.js
@@ -0,0 +1,24 @@
+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="abc123" src={useSrc1 ? '/src-1.js' : '/src-2.js'} />
+      </Head>
+      <h1 id="h1">{'Count ' + counter}</h1>
+      <button id="force-rerender" onClick={() => setCounter(counter + 1)}>
+        Re-render
+      </button>
+      <button id="change-script" onClick={() => setUseSrc1(!useSrc1)}>
+        Change script src
+      </button>
+    </>
+  )
+}
+
+export default Page
diff --git a/test/e2e/nonce-head-manager/app/public/src-1.js b/test/e2e/nonce-head-manager/app/public/src-1.js
@@ -0,0 +1,2 @@
+window.scriptExecutionIds = window.scriptExecutionIds || []
+window.scriptExecutionIds.push('src-1.js')
diff --git a/test/e2e/nonce-head-manager/app/public/src-2.js b/test/e2e/nonce-head-manager/app/public/src-2.js
@@ -0,0 +1,2 @@
+window.scriptExecutionIds = window.scriptExecutionIds || []
+window.scriptExecutionIds.push('src-2.js')
diff --git a/test/e2e/nonce-head-manager/index.test.ts b/test/e2e/nonce-head-manager/index.test.ts
@@ -0,0 +1,56 @@
+import { createNext, FileRef } from 'e2e-utils'
+import { check } from 'next-test-utils'
+import webdriver from 'next-webdriver'
+import { NextInstance } from 'test/lib/next-modes/base'
+import { join } from 'path'
+
+describe('should set-up next', () => {
+  let next: NextInstance
+
+  beforeAll(async () => {
+    next = await createNext({
+      files: {
+        pages: new FileRef(join(__dirname, 'app/pages')),
+        public: new FileRef(join(__dirname, 'app/public')),
+      },
+      nextConfig: new FileRef(join(__dirname, 'app/next.config.js')),
+    })
+  })
+  afterAll(() => next.destroy())
+
+  async function runTests(url) {
+    const browser = await webdriver(next.url, url)
+    await check(
+      async () =>
+        await browser.eval(`JSON.stringify(window.scriptExecutionIds)`),
+      '["src-1.js"]'
+    )
+
+    await browser.elementByCss('#force-rerender').click()
+    await check(
+      async () =>
+        await browser.eval(`document.getElementById('h1').textContent`),
+      'Count 1'
+    )
+    await check(
+      async () =>
+        await browser.eval(`JSON.stringify(window.scriptExecutionIds)`),
+      '["src-1.js"]'
+    )
+
+    await browser.elementByCss('#change-script').click()
+    await check(
+      async () =>
+        await browser.eval(`JSON.stringify(window.scriptExecutionIds)`),
+      '["src-1.js","src-2.js"]'
+    )
+  }
+
+  it('should not re-execute the script when re-rendering', async () => {
+    await runTests('/')
+  })
+
+  it('should not re-execute the script when re-rendering with CSP header', async () => {
+    await runTests('/csp')
+  })
+})
diff --git a/test/unit/is-equal-node.unit.test.ts b/test/unit/is-equal-node.unit.test.ts
@@ -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('should equal itself', () => {
+    const el = createScriptElement()
+    expect(isEqualNode(el, el)).toBe(true)
+  })
+
+  it('should equal equivalent node that has no nonce', () => {
+    const el1 = createScriptElement()
+    const el2 = createScriptElement()
+    expect(isEqualNode(el1, el2)).toBe(true)
+  })
+
+  it('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('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('should not equal node with different html attribute value', () => {
+    const el1 = createScriptElement({ src: '1.js' })
+    const el2 = createScriptElement({ src: '2.js' })
+    expect(isEqualNode(el1, el2)).toBe(false)
+  })
+})