Feat/replace blocking web request listeners

[kabir-afk]

Description

As the title suggests we had to replace blocking web request listeners with delarativeNetRequest API. The declarativeNetRequest API is used to block or modify network requests by specifying declarative rules. This lets extensions modify network requests without intercepting them and viewing their content, thus providing more privacy. Also it turns out to be more performant than what was being used earlier. If we talk about browser compatibility , then majority of its methods are accessible and compatible across all browsers. Some methods like getMatchedRules is not compatible in FF and similar methods exist for other browsers as well. Not that they were used here, but still , it is something to be kept in mind.

Also , despite the issue #100 being raised in context of background-script.js , the changes apply to both chromium browsers and FF, since changes are being made to manifest.json, they apply to both.

Question that still remains

Why were we modifying headers to begin with ? Like I am aware that we often need to modify headers but what were we achieving here by doing that ?

Changes made :

Anyways , despite all of this, I went ahead to change the current piece of code. I believe all the commit messages are self explanatory. I'd like to emphasize on test : confirming whether the headers have been modified by DNR. As per DNR API , minimum number of static rules that we can add is 30,000 as per mdn. So in order to confirm that our given rule has been added , we should get a number less than 30k i.e., 29,999. There are other ways to confirm it as well , like checking the CSP response header under network tab of DevTools.

Issue

While everything remains same on the front-end , things indeed change with its introduction. Videos stop working on youtube site. They stop at thumbnail. While nobody is going to use our extension on Youtube definitely , if installed and is active can lead to client side confusion. It has something to do with the way I am modifying the header, I haven't fully understood this yet ,hope @hugolpz and @Ishan-Saini can look into it , but with this present #100 can't be closed yet. I'll be refactoring this later , adding either more rules or some kind of urlFilter but until then this is what I could come up with.

Challenges faced

• declarativeNetRequest API is not fully functional. We can't append headers or use regex to match cases. Despite mentioning in the docs it seems like it is not fully reliable. The conversation here sums it pretty well.

The other alternatives were to either use blob URL and iframe tag.

• blob URL works in such a way that it creates a blob object out of the given data (video in our case), which can then be used to make a file or URL.The URL created through blob is a part of the host site , so there is no chance that it can violate CSP header. While it worked on youtube , we got the same CSP Violation error on Github , so this was the end of road for blob url. With this Test blob video in SignItVideosGallery.js #102 can be closed.
• Solution : iframe tag embeds the video inside the content script ,so there is no chance of violation of CSP media-src directive. It indeed works on all the sites with strict CSP headers like youtube / github etc. Although it comes with it's own set of problems.
  • As mentioned here , the embedded document should belong to the parent document , otherwise it returns null. And since our videos belong to origin with wikimedia subdomains, we can never have the same origin, due to which accessing the inner video element is not possible.
  • Due to this inability , feature like twospeed won't work properly , since they worked around video element and not iframe.

Conclusion

Hello @kabir-afk , thanks for the in depth analysis. iframe seems a good idea. Could iframe interfer or prevent the twospeed feature ? But if it does then lets just put that feature aside for sanity sake. It will provide something to do for the GSoC25 : )

If we can do so , then iframe remains the best and final choice, With this #100 can be closed now.

[hugolpz]

Hello @0x010C ,
We are advancing well thank to @kabir-afk who is fighting forward
. There is something we don't understand in SignIt, maybe you would be willing to give some rapid legacy code insight on the following :

Why were we modifying headers to begin with ?
Like I am aware that we often need to modify headers but what were we achieving here by doing that ?

[0x010C]

Hi @kabir-afk and @hugolpz,
I can explain this CSP header modification story to you, but be careful: everything I'm going to write below was valid in 2019 when I wrote the first version of SignIt, I haven't followed the changes since and I I'm not up to date on Maniphest v3.

CSP is a mechanism that allows system administrators of a website to indicate to browsers on which domains they are authorized to retrieve scripts and media, via an HTTP header on the main HTML document of the page. This is a protection to limit the impact of a possible XSS vulnerability. See MDN doc for more details.

To display the popups, SignIt locally injects a JS script into the web page, which adds some cosmetics and, most importantly, the video player.
Because of this, the video player runs in the same context as the web page, and is therefore subject to its CSP rules. If there is a rule prohibiting media from all domains except (for example) youtube.com, then all requests to fetch a video from commons.wikimedia.org will be rejected by the browser.

This was the objective of this commit: to intercept responses to add the domain commons.wikimedia.org in the CSP header (if set) before they are taken into account by the browser. Thus, our domain is always in the list of authorized domains and SignIt can work without problems on all websites.

I hope this answers your questions and I wish you the best with your work on SignIt.
Best regards,
0x010C

[hugolpz]

Thank Kabiraaa for this PR and associated explanations. Thanks too to @0x010C .

I reviewed the changes on github, LGTM but I didn't test it.

@Ishan-Saini hello , could you review this PR as well ?

[EDIT:] Ishan clarified that further commits are expected on this PR.

[kabir-afk]

hello @hugolpz @0x010C @Ishan-Saini , as mentioned above by Antoinne , we were modifying CSP headers to make our extension work on sites that had strict CSP headers , like twitter or youtube, such that they accepted media content from wikimedia sub-domains. I wanted to ask whether there was a specific reason to use video tag instead of iframe 🤔🤔? As , when the video is embedded in iframe ,it doesn't interfere with the host sites' CSP header and we can fetch video from wherever we want. We wont also need to work with declarativeNetRequest API ,or blob urls or patching chrome binary that way.

Like I am aware we'll have to re-write a bunch of stuff , but I think that it is still a better alternative.

[hugolpz]

Hello @kabir-afk , thanks for the in depth analysis.
iframe seems a good idea.
Could iframe interfer or prevent the twospeed feature ? But if it does then lets just put that feature aside for sanity sake. It will provide something to do for the GSoC25 : )

[kabir-afk]

I haven't looked into that feature yet , was waiting for you guys to reply. I'll see what I can do. So should I proceed with iframe?

[hugolpz]

Per Ishan :

I have reviewed the PR
Looks good to me
The only thing left for Kabir is to look into CSS of iframe
He can create a new PR for that
For now you can merge the current one

Thank @kabir-afk & @Ishan-Saini for this PR !