[3.2][WebProfilerBundle] Fix bundle usage in Content-Security-Policy context without unsafe-inline #18568
Conversation
| if (!$this->authorizesInline($directives, $type)) { | ||
| if (!isset($headers[$header][$type])) { | ||
| if (isset($headers[$header]['default-src'])) { | ||
| $headers[$header][$type] = $headers[$header]['default-src']; |
There was a problem hiding this comment.
As this is nested pretty deeply, it might be more readable if the first if becomes a guard clause with a continue
|
How could we ensure that strict CSP compliancy is preserved on the long run? |
e305560 to
3c75a1e
Compare
|
Toggling to Need Work : I will disable CSP when browsing profiler pages and will introduce nonce only for toolbar injection. |
cda45ee to
e5e4f73
Compare
|
Work done, it can now be reviewed |
@nicolas-grekas you asked this question. Checking if the injected script has the appropriate nonce, headers are included and rendered toolbar include nonce should be alright. |
0a024c0 to
3083453
Compare
| @@ -27,7 +27,6 @@ | ||
| {{ dump.data|raw }} | ||
| </div> | ||
| {% endfor %} | ||
| <img src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" onload="var h = this.parentNode.innerHTML, rx=/<script>(.*?)<\/script>/g, s; while (s = rx.exec(h)) {eval(s[1]);};" /> |
There was a problem hiding this comment.
@nicolas-grekas what was the reason for this one ?
There was a problem hiding this comment.
Toggle nested levels in the toolbar
| $this->cleanHeaders($response); | ||
| $this->updateCspHeaders($response, $nonces); | ||
|
|
||
| return $nonces; |
There was a problem hiding this comment.
returning something from an event listener looks weird to me
There was a problem hiding this comment.
OK, this method is actually not an event listener. So the phpdoc is wrong (and the naming is not that good either)
|
Thanks @stof for this review, I'm going to take care of this |
|
|
||
| /** | ||
| * @param Response $response | ||
| */ |
|
👍 |
e48e16e to
52ae783
Compare
|
PR updated, comments addressed |
…ron) This PR was merged into the 2.8 branch. Discussion ---------- [2.8][WebProfilerBundle] Fix invalid CSS style | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A Related to #18568 (comment) Commits ------- aa35a16 [WebProfilerBundle] Fix invalid CSS style
52ae783 to
ef3827f
Compare
| * | ||
| * @return array | ||
| */ | ||
| private function updateCspHeaders(Response $response, array $nonces = array()) { |
ef3827f to
bb94559
Compare
…xt without unsafe-inline
bb94559 to
571a1f2
Compare
|
Thank you @romainneutron. |
…ecurity-Policy context without unsafe-inline (romainneutron) This PR was merged into the 3.2-dev branch. Discussion ---------- [3.2][WebProfilerBundle] Fix bundle usage in Content-Security-Policy context without unsafe-inline | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15397 | License | MIT | Doc PR | N/A Hello, this PR fixes the compatibility of the WebprofilerBundle in a context where Content-Security-Policy headers are could prevent `unsafe-inline` of `script-src` or `style-src` directives. This PR has been originally proposed in 2.8 in #18434 Commits ------- 571a1f2 [WebProfilerBundle] Fix bundle usage in Content-Security-Policy context without unsafe-inline
|
Could this be broken since 4.1 in contexts where unsafe-eval is not allowed? I had problems after upgrading because of CSP violations, but need to retest tomorrow |
Hello, this PR fixes the compatibility of the WebprofilerBundle in a context where Content-Security-Policy headers are could prevent
unsafe-inlineofscript-srcorstyle-srcdirectives.This PR has been originally proposed in 2.8 in #18434