fix(csp): fix csp auto-detection and stylesheet injection #4444
Conversation
|
Thanks for the PR!
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
| @@ -760,6 +760,13 @@ function equals(o1, o2) { | |||
| } | |||
|
|
|||
|
|
|||
| function csp() { | |||
| return (document.securityPolicy && document.securityPolicy.isActive) || | |||
There was a problem hiding this comment.
Shouldn't we cache this?
There was a problem hiding this comment.
Ignore that, not a big deal.
|
LGTM |
|
The unit tests seem to be broken. |
| @@ -76,7 +76,7 @@ function $SnifferProvider() { | |||
|
|
|||
| return eventSupport[event]; | |||
| }, | |||
| csp: document.securityPolicy ? document.securityPolicy.isActive : false, | |||
There was a problem hiding this comment.
@tbosch and I looked at this yesterday. document.securityPolicy appears to be undefined even when the Content-Security-Policy header is present and csp is enabled. This is on Chrome 29. So not sure that this fixes it. We couldn't find a good way to detect csp.
There was a problem hiding this comment.
Actually the spec says that this is the correct way of doing it: https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html
However, Chrome and others don't seem to support this yet. I like the change of @IgorMinar as it allows to manually override the autodetection and it is quite simple.
There was a problem hiding this comment.
@chirayuk securityPolicy is a CSPv1.1 feature that is not in non-canary Chrome yet.
There was a problem hiding this comment.
Aha. Good to know. Thanks!
When we refactored , we broke the csp mode because the previous implementation relied on the fact that it was ok to lazy initialize the .csp property, this is not the case any more. Besides, we need to know about csp mode during bootstrap and avoid injecting the stylesheet when csp is active, so I refactored the code to fix both issues. PR angular#4411 will follow up on this commit and add more improvements. Closes angular#917 Closes angular#2963 Closes angular#4394 Closes angular#4444 BREAKING CHANGE: triggering ngCsp directive via `ng:csp` attribute is not supported any more. Please use data-ng-csp instead.
When we refactored , we broke the csp mode because the previous implementation relied on the fact that it was ok to lazy initialize the .csp property, this is not the case any more. Besides, we need to know about csp mode during bootstrap and avoid injecting the stylesheet when csp is active, so I refactored the code to fix both issues. PR angular#4411 will follow up on this commit and add more improvements. Closes angular#917 Closes angular#2963 Closes angular#4394 Closes angular#4444 BREAKING CHANGE: triggering ngCsp directive via `ng:csp` attribute is not supported any more. Please use data-ng-csp instead.
When we refactored , we broke the csp mode because the previous implementation relied on the fact that it was ok to lazy initialize the .csp property, this is not the case any more. Besides, we need to know about csp mode during bootstrap and avoid injecting the stylesheet when csp is active, so I refactored the code to fix both issues. PR angular#4411 will follow up on this commit and add more improvements. Closes angular#917 Closes angular#2963 Closes angular#4394 Closes angular#4444 BREAKING CHANGE: triggering ngCsp directive via `ng:csp` attribute is not supported any more. Please use data-ng-csp instead.
When we refactored , we broke the csp mode because the previous implementation
relied on the fact that it was ok to lazy initialize the .csp property, this
is not the case any more.
Besides, we need to know about csp mode during bootstrap and avoid injecting the
stylesheet when csp is active, so I refactored the code to fix both issues.
PR #4411 will follow up on this commit and add more improvements.
Closes #917
Closes #2963
Closes #4394