Enable blocking of service worker requests in Chrome #82
Conversation
| // Requests from the service worker use a different protocol | ||
| // (e.g. https). If there is a more direct way to detect requests | ||
| // from the extension, this could be simplified. | ||
| if (isChromium && !this.initiator.startsWith('chrome-extensions://')) { |
There was a problem hiding this comment.
what about Opera and Edge? maybe we should check for http/https instead?
There was a problem hiding this comment.
common/platforms/webextension/platform.es
Line 19 in 319cc4f
isChromium covers Chromium based browsers including Opera and Edge.
There was a problem hiding this comment.
http/https and ws/wss. But I tried to approach it from the other direction and identify requests from the extension. I think a better name for "isBackground" would be something in the line of "is it from the extension?".
There was a problem hiding this comment.
I've checked Opera and Edge and both use same chrome-extensions protocol
There was a problem hiding this comment.
Yes, I think the change here (not the URL filling) would be safe. On isolation, it will not make a difference though because the request will still be skipped because of the missing fields.
| context.tabUrl = context.tabUrl || (page && page.getTabUrl()); | ||
| context.frameUrl = context.frameUrl || (page && page.getFrameUrl(context)); | ||
| context.tabUrl = context.tabUrl || (page && page.getTabUrl()) || context.url; | ||
| context.frameUrl = context.frameUrl || (page && page.getFrameUrl(context)) || context.url; |
There was a problem hiding this comment.
I'm not sure if it is sound to fill it with the url. But the reason that I added it was that there are multiple places where the adblocker assumes that it is present, for instance here:
common/modules/adblocker/sources/adblocker.es
Line 235 in 319cc4f
And later it would throw in the whitelist check:
common/modules/adblocker/sources/adblocker.es
Line 255 in 319cc4f
There was a problem hiding this comment.
Looks like this here is the main problem still. Filling it in like this is risky. Potentially we could apply it only if it is a request from a service worker; but even then, it is not clear which URL to fill in.
The other question is if it is sound to even attempt to fill it. But IMHO you can argue that it is:
- It matches the behavior on Firefox
- The initiator should be very close to what so far was exclusively the tab
But conceptually cleaner might be to put the service worker information in a separate field. (But with the consequence that all places where it is used will need to be revisited. Through the whole stack down to the adblocker library.)
|
Closing in favor of #83 |
POC (to be used together with ghostery/ghostery-extension#1223)
Requests from services workers on Chrome are not associated with a tab. That is why they are currently ignored.