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

[Float] Nonce preload support #26939

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Jun 13, 2023

Some browsers, with some CSP configuration, will not preload a script if the prelaod link tag does not provide a valid nonce attribute. This change adds the ability to specify a nonce for ReactDOM.preload(..., { as: "script" })

@gnoff gnoff requested a review from sophiebits June 13, 2023 18:27
@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Jun 13, 2023
@react-sizebot
Copy link

react-sizebot commented Jun 13, 2023

Comparing: a7bf5ba...8ed2797

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 164.23 kB 164.24 kB = 51.73 kB 51.73 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.67 kB 171.68 kB = 53.97 kB 53.97 kB
facebook-www/ReactDOM-prod.classic.js = 570.12 kB 570.15 kB = 100.58 kB 100.58 kB
facebook-www/ReactDOM-prod.modern.js = 553.90 kB 553.93 kB = 97.75 kB 97.75 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 8ed2797

@sebmarkbage
Copy link
Collaborator

So, we do take this as an option for any React generated things like script tags in Fizz. I wonder if this should just be part of that configuration and that it's automatically provided.

The point of it is to provide some guarantee that first party code is inserting it properly and so adding it automatically for any <script> in JSX or whatever might be a step too far since it might have been some kind of hack. However, for this case it seems like maybe you'd just want it to work out of the box when configured.

@gnoff
Copy link
Collaborator Author

gnoff commented Jun 13, 2023

My thinking was that 3p project can use ReactDOM.preload/preinit() or render a <script>. but maybe it is malicious and you don't expect this package to load any scripts. If we automatically nonced every preload and script you could unknowingly let any package you rely upon to opt into executable scripts when you are trying to protect against that with CSP. So the "safe" perspective is that packages that want to do scripty stuff in a CSP compatible way need to accept a nonce option and wire it up. This makes the CSP-sensitive user aware of what is and is not potentially injecting executable code.

We currently only auto-nonce our own runtime scripts which is fine b/c we control it.

@gnoff gnoff force-pushed the nonce-preload-support branch 2 times, most recently from f689565 to 0152f58 Compare June 13, 2023 20:59
… if the prelaod link tag does not provide a valid nonce attribute. This change addes the ability to specify a nonce for `ReactDOM.preload(..., { as: "script" })`
@sebmarkbage
Copy link
Collaborator

The argument here is that why not provide the nonce if you have to provide to <script> anyway.

@gnoff
Copy link
Collaborator Author

gnoff commented Jun 15, 2023

IMO parity but I can see why one could argue otherwise

@gnoff gnoff merged commit 86acc10 into facebook:main Jun 15, 2023
@gnoff gnoff deleted the nonce-preload-support branch June 15, 2023 20:39
github-actions bot pushed a commit that referenced this pull request Jun 15, 2023
Some browsers, with some CSP configuration, will not preload a script if
the prelaod link tag does not provide a valid nonce attribute. This
change adds the ability to specify a nonce for `ReactDOM.preload(..., {
as: "script" })`

DiffTrain build for [86acc10](86acc10)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Some browsers, with some CSP configuration, will not preload a script if
the prelaod link tag does not provide a valid nonce attribute. This
change adds the ability to specify a nonce for `ReactDOM.preload(..., {
as: "script" })`
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Some browsers, with some CSP configuration, will not preload a script if
the prelaod link tag does not provide a valid nonce attribute. This
change adds the ability to specify a nonce for `ReactDOM.preload(..., {
as: "script" })`

DiffTrain build for commit 86acc10.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants