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

Add nonce support to bootstrap scripts and external runtime #26738

Merged
merged 17 commits into from
May 1, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ export type ResponseState = {
startInlineScript: PrecomputedChunk,
instructions: InstructionState,

// state for outputting CSP nonce
nonce: string | void,

// state for data streaming format
externalRuntimeConfig: BootstrapScriptDescriptor | null,

Expand Down Expand Up @@ -239,14 +242,21 @@ export function createResponseState(
}
}
if (bootstrapScripts !== undefined) {
const startScriptWithNonce =
nonce === undefined
? startScriptSrc
: stringToPrecomputedChunk(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Precomputed chunks are generally constructed in module scope. The idiomatic way of doing this is to break these chunks up in a static-dynamic-static sandwich

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnoff I updated the code to sandwich style 🥪

I had originally based my code on the pre-existing nonce setup for inline scripts:

const inlineScriptWithNonce =
nonce === undefined
? startInlineScript
: stringToPrecomputedChunk(
'<script nonce="' + escapeTextForBrowser(nonce) + '">',
);

If the updated code looks good, should I apply it there as well?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is different. We use this repeatedly when we emit our fizz runtime scripts. The idea with precomputed chunks is you can do the text encoding once (if configured for the host runtime) so we only gain a benefit if the value is reused. pulling the chunk creation to module scope is our way of reusing the chunk which is why I suggested the sandwhich style for your original use case. Here we are getting reuse by referencing this precomputed chunk on each instruction flush but we can't make it in module scope because we embed the nonce within it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - if I understand correctly, this is because you return startInlineScript from createResponseState and use it other places. Since we don't have to do that with src/module scripts, it's actually more efficient to construct them the way you suggested, since we can precompile the nonce= chunk once inside the module. 👍

That was a tough one to wrap my head around but I'm pretty sure I have it - thank you for your explanations 🙌

'<script nonce="' + escapeTextForBrowser(nonce) + '" src="',
);

for (let i = 0; i < bootstrapScripts.length; i++) {
const scriptConfig = bootstrapScripts[i];
const src =
typeof scriptConfig === 'string' ? scriptConfig : scriptConfig.src;
const integrity =
typeof scriptConfig === 'string' ? undefined : scriptConfig.integrity;
bootstrapChunks.push(
startScriptSrc,
startScriptWithNonce,
stringToChunk(escapeTextForBrowser(src)),
);
if (integrity) {
Expand All @@ -259,14 +269,23 @@ export function createResponseState(
}
}
if (bootstrapModules !== undefined) {
const startModuleWithNonce =
nonce === undefined
? startModuleSrc
: stringToPrecomputedChunk(
'<script nonce="' +
escapeTextForBrowser(nonce) +
'" type="module" src="',
);

for (let i = 0; i < bootstrapModules.length; i++) {
const scriptConfig = bootstrapModules[i];
const src =
typeof scriptConfig === 'string' ? scriptConfig : scriptConfig.src;
const integrity =
typeof scriptConfig === 'string' ? undefined : scriptConfig.integrity;
bootstrapChunks.push(
startModuleSrc,
startModuleWithNonce,
stringToChunk(escapeTextForBrowser(src)),
);
if (integrity) {
Expand Down Expand Up @@ -297,6 +316,7 @@ export function createResponseState(
preloadChunks: [],
hoistableChunks: [],
stylesToHoist: false,
nonce,
};
}

Expand Down Expand Up @@ -3013,6 +3033,14 @@ export function pushStartInstance(
formatContext.noscriptTagInScope,
);
case 'script':
if (responseState.nonce) {
// add nonce to props, but allow override
props = {
nonce: responseState.nonce,
...props,
};
}

return enableFloat
? pushScript(
target,
Expand Down Expand Up @@ -4066,7 +4094,7 @@ export function writePreamble(
// (User code could choose to send this even earlier by calling
// preinit(...), if they know they will suspend).
const {src, integrity} = responseState.externalRuntimeConfig;
internalPreinitScript(resources, src, integrity);
internalPreinitScript(resources, src, integrity, responseState.nonce);
}

const htmlChunks = responseState.htmlChunks;
Expand Down Expand Up @@ -5348,6 +5376,7 @@ function internalPreinitScript(
resources: Resources,
src: string,
integrity: ?string,
nonce: ?string,
): void {
const key = getResourceKey('script', src);
let resource = resources.scriptsMap.get(key);
Expand All @@ -5364,6 +5393,7 @@ function internalPreinitScript(
async: true,
src,
integrity,
nonce,
});
}
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export type ResponseState = {
preloadChunks: Array<Chunk | PrecomputedChunk>,
hoistableChunks: Array<Chunk | PrecomputedChunk>,
stylesToHoist: boolean,
nonce: string | void,
// This is an extra field for the legacy renderer
generateStaticMarkup: boolean,
};
Expand Down Expand Up @@ -94,6 +95,7 @@ export function createResponseState(
preloadChunks: responseState.preloadChunks,
hoistableChunks: responseState.hoistableChunks,
stylesToHoist: responseState.stylesToHoist,
nonce: responseState.nonce,

// This is an extra field for the legacy renderer
generateStaticMarkup,
Expand Down
67 changes: 65 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ describe('ReactDOMFizzServer', () => {
);
});

it('should support nonce scripts', async () => {
it('should support nonce for bootstrap and runtime scripts', async () => {
CSPnonce = 'R4nd0m';
try {
let resolve;
Expand All @@ -591,11 +591,26 @@ describe('ReactDOMFizzServer', () => {
<Lazy text="Hello" />
</Suspense>
</div>,
{nonce: 'R4nd0m'},
{
nonce: 'R4nd0m',
bootstrapScriptContent: 'function noop(){}',
bootstrapScripts: ['init.js'],
bootstrapModules: ['init.mjs'],
},
);
pipe(writable);
});

expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// check that there are 4 scripts with a matching nonce:
// The runtime script, an inline bootstrap script, and two src scripts
expect(
Array.from(container.getElementsByTagName('script')).filter(
node => node.getAttribute('nonce') === CSPnonce,
).length,
).toEqual(4);

await act(() => {
resolve({default: Text});
});
Expand All @@ -605,6 +620,54 @@ describe('ReactDOMFizzServer', () => {
}
});

it('should render scripts with nonce added', async () => {
CSPnonce = 'R4nd0m';
try {
await act(async () => {
const {pipe} = renderToPipeableStream(
<html>
<body>
<script>{'try { foo() } catch (e) {} ;'}</script>
<script src="foo" async={true} />
<script src="bar" />
<script src="baz" integrity="qux" async={true} />
<script type="module" src="quux" async={true} />
<script type="module" src="corge" async={true} />
<script
type="module"
src="grault"
integrity="garply"
async={true}
/>
</body>
</html>,
{
nonce: CSPnonce,
},
);
pipe(writable);
});

expect(
stripExternalRuntimeInNodes(
document.getElementsByTagName('script'),
renderOptions.unstable_externalRuntimeSrc,
).map(n => n.outerHTML),
).toEqual([
// async scripts get inserted first in render
`<script nonce="${CSPnonce}" src="foo" async=""></script>`,
`<script nonce="${CSPnonce}" src="baz" integrity="qux" async=""></script>`,
`<script nonce="${CSPnonce}" type="module" src="quux" async=""></script>`,
`<script nonce="${CSPnonce}" type="module" src="corge" async=""></script>`,
`<script nonce="${CSPnonce}" type="module" src="grault" integrity="garply" async=""></script>`,
`<script nonce="${CSPnonce}">try { foo() } catch (e) {} ;</script>`,
`<script nonce="${CSPnonce}" src="bar"></script>`,
]);
} finally {
CSPnonce = null;
}
});

it('should client render a boundary if a lazy component rejects', async () => {
let rejectComponent;
const LazyComponent = React.lazy(() => {
Expand Down
17 changes: 17 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,4 +486,21 @@ describe('ReactDOMFizzServerBrowser', () => {
'<!DOCTYPE html><html><head><title>foo</title></head><body>bar</body></html>',
);
});

it('should support nonce attribute for bootstrap scripts', async () => {
const nonce = 'R4nd0m';
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>hello world</div>,
{
nonce,
bootstrapScriptContent: 'INIT();',
bootstrapScripts: ['init.js'],
bootstrapModules: ['init.mjs'],
},
);
const result = await readResult(stream);
expect(result).toMatchInlineSnapshot(
`"<div>hello world</div><script nonce="${nonce}">INIT();</script><script nonce="${nonce}" src="init.js" async=""></script><script nonce="${nonce}" type="module" src="init.mjs" async=""></script>"`,
);
});
});
6 changes: 6 additions & 0 deletions packages/react-dom/src/test-utils/FizzTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ async function executeScript(script: Element) {
} else {
const newScript = ownerDocument.createElement('script');
newScript.textContent = script.textContent;
// make sure to add nonce back to script if it exists
const scriptNonce = script.getAttribute('nonce');
if (scriptNonce) {
newScript.setAttribute('nonce', scriptNonce);
}

parent.insertBefore(newScript, script);
parent.removeChild(script);
}
Expand Down