You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@Timer
To bring forth the arguments from #5144 , let me elaborate on a few issues we encountered:
I understand that adding a postbuild script is not difficult. We already have to add it to four repositories, I guess there are companies out there that use more. Furthermore, patching itself is not difficult, getting it right is.
Example: myHTML.replace(/(<script>(?:.*?)<\/script>)/, `<script src="${manifest['runtime~main.js']}"></script>`). Adding an ID will help us to find the correct <script>...</script> so it is better than nothing. If for some reason the runtime script contains itself a </script> like let script = "<script src=" + src + "></script>" this will break.
The solid solution is to parse the index.html as HTML, find the correct node, replace it, convert the content to a string and write it back. Then, I would say, we are no longer in the realm of an easy solution.
If more scripts are inlined, this solution will break as well.
I completely understand the desire to not create additional environment variables or configuration options as they surely add complexity. But in this case it is essentially punishing users that try to maintain a high security standard - regardless if they want to do it or are even (legally) obligated to do so).
I can absolutely understand that saving one additional request for a small file is worth it. But while going from <script src="/static/js/runtime~main.js"></script> to an inlined script is very easy, the other way around becomes rather fragile.
I do not intend to press this any further, I just do not think this is really a small edge-case.
Thank you for your time & feedback 👍
Reacted by Ed Morley, Gabor Dolla, Alexander Forbes-Reed, Eric Bock, Jonathan Svenheden, Jianing Wang, Ashley Finafrock, citizen-dror, fractalawareness, Quest Henkart and 1 more
Just for comparison, Angular CLI has always extracted the script to a separate file precisely for this issue. Before v6 it was even called somewhat confusingly inline.bundle.js.
This is a slightly off-topic comment but I do wonder if it's even a good thing to inline the JavaScript at all in the HTTP2 era. It makes it impossible to make the JS code async and if the .js file is served on the same domain there is extremely little overhead to serving it as a file over HTTP2. The HTML can be parsed sooner because it's smaller which means it can get started, in parallel, to download (and start parsing) other assets. Would love to see a simple option to disable all inlining.
@edmorley You're right. I rushed that. Fixed my blog post now.
But also, I changed my mind about bothering anyway. I took @PerfectPixel 's heed and wrote a script that "uninlines" the inline script tag.
Now my index.html is 1420 bytes (676 gzipped) instead of 2496 bytes (1194 gzipped). It's so small that it's really hard to measure any big difference and the context is a fun little side-project but I wanted to use this to put thoughts into code.
Reacted by Alexander Forbes-Reed, Jianing Wang and LPRKR
Thanks @peterbe, that looks quite helpful. Although it seems quite insane the hoops you have to jump through just to get back to where we were with CRA1. This change only encourages a less secure web - with less experienced/lazy devs most likely to just turn off CSP or allow unsafe-inline...
Reacted by Gustav Boström, Jesse Stuart, Florian Klampfer and citizen-dror
Any update on this? I'd like to see the webpack config for inlining the runtime chunk be opt-in. IMHO having this inlined is a premature optimization at the cost of easily serving the app with a sane CSP.
Activity
PerfectPixel commentedon Oct 5, 2018
@Timer
To bring forth the arguments from #5144 , let me elaborate on a few issues we encountered:
I understand that adding a
postbuildscript is not difficult. We already have to add it to four repositories, I guess there are companies out there that use more. Furthermore, patching itself is not difficult, getting it right is.Example:
myHTML.replace(/(<script>(?:.*?)<\/script>)/, `<script src="${manifest['runtime~main.js']}"></script>`). Adding an ID will help us to find the correct<script>...</script>so it is better than nothing. If for some reason the runtime script contains itself a</script>likelet script = "<script src=" + src + "></script>"this will break.The solid solution is to parse the
index.htmlas HTML, find the correct node, replace it, convert the content to a string and write it back. Then, I would say, we are no longer in the realm of an easy solution.If more scripts are inlined, this solution will break as well.
I completely understand the desire to not create additional environment variables or configuration options as they surely add complexity. But in this case it is essentially punishing users that try to maintain a high security standard - regardless if they want to do it or are even (legally) obligated to do so).
I can absolutely understand that saving one additional request for a small file is worth it. But while going from
<script src="/static/js/runtime~main.js"></script>to an inlined script is very easy, the other way around becomes rather fragile.I do not intend to press this any further, I just do not think this is really a small edge-case.
Thank you for your time & feedback 👍
mgol commentedon Oct 5, 2018
Just for comparison, Angular CLI has always extracted the script to a separate file precisely for this issue. Before v6 it was even called somewhat confusingly
inline.bundle.js.peterbe commentedon Oct 5, 2018
This is a slightly off-topic comment but I do wonder if it's even a good thing to inline the JavaScript at all in the HTTP2 era. It makes it impossible to make the JS code async and if the .js file is served on the same domain there is extremely little overhead to serving it as a file over HTTP2. The HTML can be parsed sooner because it's smaller which means it can get started, in parallel, to download (and start parsing) other assets. Would love to see a simple option to disable all inlining.
Also, because #5144 is now closed I'll post it here: If others are struggling with busted Content Security Polices as of this recent change, you might get some inspirational ideas from this blog post: https://www.peterbe.com/plog/inline-scripts-in-create-react-app-2.0-and-csp-nonces
edmorley commentedon Oct 5, 2018
I could be wrong, but I believe that using a static
nonce-<hash>like that is insecure (the spec says it must be unique each time). The generated hash needs to be put into a'<hash-algorithm>-<base64-value>'attribute instead. See:#5144 (comment)
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Sources
peterbe commentedon Oct 5, 2018
@edmorley You're right. I rushed that. Fixed my blog post now.
But also, I changed my mind about bothering anyway. I took @PerfectPixel 's heed and wrote a script that "uninlines" the inline script tag.
Now my index.html is 1420 bytes (676 gzipped) instead of 2496 bytes (1194 gzipped). It's so small that it's really hard to measure any big difference and the context is a fun little side-project but I wanted to use this to put thoughts into code.
0xdeafcafe commentedon Oct 8, 2018
Thanks @peterbe, that looks quite helpful. Although it seems quite insane the hoops you have to jump through just to get back to where we were with CRA1. This change only encourages a less secure web - with less experienced/lazy devs most likely to just turn off CSP or allow
unsafe-inline...peterbe commentedon Oct 8, 2018
@Timer Where should we redirect our issues about A) an option to disable inlining small scripts and B) lobby to have that option "on" by default.
Timer commentedon Oct 8, 2018
This issue, @peterbe.
gaearon commentedon Oct 8, 2018
I say let’s add a way to opt out. #5309 (comment)
I still think the majority would be better served by the inlined script though.
Timer commentedon Oct 8, 2018
We can close this if we're going to add an environment variable.
edit: actually it might still be useful, tagged for later milestone
jvatic commentedon Jan 24, 2020
Any update on this? I'd like to see the webpack config for inlining the runtime chunk be opt-in. IMHO having this inlined is a premature optimization at the cost of easily serving the app with a sane CSP.
The only other option to prevent having
unsafe-inlinein the CSP, besides ejecting and removing this portion of the config, is to add a cryptographic hash of each runtime script to the CSP (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Sources). Neither option seems to be a sane default.kddnewton commentedon Jan 24, 2020
@jvatic you can opt out with
INLINE_RUNTIME_CHUNK=falsein your.env.dimaqq commentedon May 12, 2020
Or add a config override, like
draft.plugins = draft.plugins.filter(p => p.constructor.name !== "InlineChunkHtmlPlugin");caub commentedon Dec 22, 2021
a little ugly, but it's probably fine to do:
(coming from #5144 nonce issue)
edit: actually using INLINE_RUNTIME_CHUNK=false is better (seen it in https://deploy-preview-27627--material-ui.netlify.app/guides/content-security-policy/#create-react-app-cra)
caub commentedon Feb 24, 2022
Re #5144 the problem is not only for the runtime script, but also for lazy-loaded scripts, what can we do with react-scripts?