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
text layer style micro-optimization resulted in requiring style-src 'unsafe-inline' in Content Security Policy #8066
Comments
I'm only suggesting taking other routes to optimizing it. For example, why use 4 padding attributes and then apply them with |
Could also still use the static array approach for setting individual style properties instead of doing it all via |
In general there's no guarantee that a particular
The general line of reasoning behind updating everything at once using |
That doesn't mean
Isn't it happening in a fragment, not the actual DOM? |
@thestinger agree, it will help to address the issue if benchmarks with some performance data was available. |
I'm not really able to measure a difference between different approaches here. The different approaches can be microbenchmarked out of context though. I think the conflict with It's easy for me to just revert this commit so it's not a high priority for me but I'll try to find someone to work on it. It would be great to have tests for CSP stuff so it can't break in the future (optimizations like this could still happen if important, they'd just need to be conditional). |
There might be a solution that allows for the optimisation as is while supporting websites using CSP. There are three ways to set inline style as a string:
Plunker to show the three methods: https://plnkr.co/edit/T8xrUmR5eSuqDukSEVuw?p=preview I'd be more than willing to make a pull request changing |
It seems like a browser bug if |
In case anyone else comes across this and wants a solution, this is my current patch rolling this back to the old way of doing it so It can definitely be optimized a bit, but since that doesn't matter for my usage I'm keeping the patch as minimal as possible for ease of maintenance. I think expecting someone that wants this security issue fixed to care about optimizing the performance is a bit backwards. The performance optimization should have been constrained to what can be done without breaking basic security hygiene. It may even be faster to approach this in a different way as I mentioned above. The commit message for the optimization mentions how difficult it was to even measure a difference, but the security regression is quite easily measured. The reason that I'm using pdf.js in the first place is security. It allows reusing the hardened / sandboxed browser rendering with all the PDF-specific rendering in a memory safe language (JavaScript). As part of doing this, I'm using CSP to mitigate dynamic code / style injection which could massively increase the attack surface to being like a web page, which is very counter to the goals. I think many other people have a similar use case for pdf.js for avoiding dangerous native PDF rendering libraries. Avoiding I consider this on the same level as expecting native binaries / libraries to have SSP, ASLR (PIE), |
Fixed by the pull request above. |
cb5f9df started generating style strings automatically, which is forbidden by a standard Content Security Policy. It would be nice to have a fallback or perhaps reconsider the approach.
To reproduce, use
style-src 'self'
in CSP:The text was updated successfully, but these errors were encountered: