mlgualtieri / CSS-Exfil-Protection

Official repository for the CSS Exfil Protection browser extensions.
MIT License
155 stars 11 forks source link

high cpu usage for some sites #26

Closed atomGit closed 4 years ago

atomGit commented 4 years ago

i sent a mail via your website - i didn't realize there was a GH repo since it isn't linked on AMO

the add-on is using a lot of CPU and seems to prevent the page from loading completely, though only in rare instances - for example:

https://coreboot.org/status/board-status.html

please check if you can reproduce

tested with a fresh profile

nobody43 commented 4 years ago

Can reproduce: can't even load this page fully when extension is enabled.

mlgualtieri commented 4 years ago

I can verify too. The issue occurs in both Firefox and Chrome and even if the extension is disabled through the checkmark. When the extension is disabled in the browser itself the page loads. Unsure what's happening here. I'll dig deeper and see what I can find. Thanks for the report!

mlgualtieri commented 4 years ago

I have a potential fix for this issue that I'm testing. The fix makes no sense, but it does appear to work. I moved the plugin enabled check sooner in the plugin execution. This seems to prevent the issue from occurring if the plugin is enabled or disabled.

I don't like that this fix makes no sense. There must be something else happening. I'm going to test this a bit more and also test in Chrome - right now I've just verified in Firefox.

The plugin enabled check is in the current location as in the past if it was moved earlier in execution sometimes data would leak. I can't replicate the data leak error now, so maybe that's no longer an issue.

mlgualtieri commented 4 years ago

I have a better potential fix now for this issue. It appears that the 'load blocking CSS' that's applied to the page and then removed after sanitization was causing issues. Since the page itself is 12MB, I think that the issue isn't so much the plugin but that Firefox was choking on applying styles to so many elements. This bit of code has actually just been reworked to fix another bug, and that has helped, but I also noticed that for some reason the blocking CSS was being applied and removed multiple times on the page (no clue why). So, I've added a check in that prevents it from loading and unloading multiple times, which greatly helps with the page rendering. I want to do some more testing, but will hopefully have a fix ready for this soon.

atomGit commented 4 years ago

this sounds good - glad you were able to narrow this down at least to some extent

i fired up the 'inspector' dev tool to see if there was any 'lazy-loading' going on because i thought maybe that could be an answer as to why the css was being applied and removed multiple times, however i wasn't able to determine anything since using the tool also caused massive cpu usage and disk activity (why the disk activity i don't know since i store cache in ram) ... however, the fact that using the inspector resulted in a similar behavior as CSS-E-P might provide a further clue perhaps???

mlgualtieri commented 4 years ago

I think when it comes down to it, it's a HUGE block of HTML and it becomes difficult for the browser to render it, especially if plugins are messing with the result. Ironically, there is no CSS on the page, but the plugin doesn't know this beforehand and treats it like any other page.

atomGit commented 4 years ago

hi Mike - i'd like to inquire as to the status of this

mlgualtieri commented 4 years ago

I have an unreleased commit in the tree now that attempted to deal with the issue and clean up some other items. The reason I haven't released it yet is because it doesn't solve the performance issue. I did enhance how the enable/disable code works, so now when the plugin is disabled the performance issue does not cause an issue, but when it's enabled you will see the same slowdowns. I'm not sure there is a good solution here. I did have one new idea I was playing around with many weeks ago. I'll try to explore that asap path again and get this release out.

atomGit commented 4 years ago

i dropped a note over at the 'ghacks user.js' repo - i know there's some talented guys contributing over there that maybe could take a look at the issue

thanks for the update

mlgualtieri commented 4 years ago

Thanks for the thought, but it's not necessary to call in anyone else. I know the extension well, and the exact part it chokes for the problem URL.

The core issue is for the extension to properly shield the user from data leaks the so-called 'load blocking CSS' needs to be loaded. This is removed once sanitization happens. The problem with this site is that the load blocking CSS causes the browser engine to choke. If I remove the load blocker the page loads without issue. But, I can't do that because it will not provide adequate protection for the user.

I think ultimately adding in a whitelist is the only way to resolve this. It's a popular feature request, so maybe I should bite the bullet and get that coded.

mlgualtieri commented 4 years ago

Just pushed a big update to the firefox plugin source. Needs more testing, but it includes new functionality for "domain settings". This will allow you to skip both scan and sanitize on a per domain basis. If set to never scan / never sanitize, the plugin can be enabled now and the coreboot URL loads well. I need to clean some things up, port the new functionality to Chrome, and also do some more testing. Things are looking good. There's a lot in this update... I'm planning to skip the 1.0.18 entirely and finally move to 1.1.0. Stay tuned...

atomGit commented 4 years ago

tis a bit late to mention this, but is there not a way that you can monitor extension resource use and automate the on/off behavior instead of a white/black list?

i mention this because many of us privacy freaks run several privacy-centric extensions with the potential to break a site and the less troubleshooting and white/black listing the user has to do, the better

Firefox has a 'task manager' (about:performance) and i'm wondering if there's an API that would allow you to monitor resource usage

mlgualtieri commented 4 years ago

Simplicity is why I resisted adding whitelisting functionality for a long time :-) I always figured it would be better to solve a bug than have people manage per-site settings. This is the first bug where it's not going to be possible to fix, because the issue has more to do with the browser engine overhead processing the load-blocking rules when pages are huge.

The good news is that issues like this should be exceedingly rare. The only reason it occurs here is that the problem URL is 13MB of HTML. There may be other reasons why people may want to not scan/sanitize a domain though, so I think it's time to add the feature.

I'm not sure it's possible to determine the resource usage within an extension. And it's not possible to understand if the page would cause issue without first causing the issue, if that makes sense. So, it's a setting better left to the user IMHO.

mlgualtieri commented 4 years ago

This issue has been addressed in today's 1.1.0 release. Since it wasn't technically feasible to address in another manner, the new domain settings options to either "Always Scan / Never Sanitize" or "Never Scan / Never Sanitize" will avoid the issue with the coreboot URL in this thread.