Add generic URI schema support when clicking ZIM links

[Jaifroid]

Fixes #1260. This should add generic support for different "protocols" (URI schemata), so that they are forced to open outside the sandbox. This avoids triggering a CSP security exception (CORS), which then leaves the app in a completely unusable state.

The PR generically catches any URI scheme that is different from the app's location.protocol. It is this difference that can trigger CORS protection. Doing it generically means we don't have to list a whole bunch of schemata like skype:, zoomus:, mailto:, etc.., especially since custom protocols can be defined nowadays. While some of these don't cause exceptions in Firefox (e.g. mailto:), they all cause CORS exceptions in Chromium browsers.

[Jaifroid]

See also kiwix/libkiwix#1138 for analogue issue in Kiwix Serve.

[Jaifroid]

Hmm, although this code works in the PWA, it's not working here. I think it is tested too late, after some other conditions have been triggered.

[Jaifroid]

OK, stupidly forgot to copy some detection code in handleClickOnReplayLink. Now working with mailto: in Military Medicine ZIM.

[Jaifroid]

@audiodude If the Linux (Browser Stack) tests fail again it's probably because the tests are a bit flaky (timing issues), and just need to be re-run, which I'll do if necessary in the morning here. Note that there isn't an e2e test covering this PR because we don't currently have a (small) ZIM in the test suite that has a mailto: link, and also because it would be hard to detect whether such a link had performed a custom action in Selenium. So I'm limited to manual testing.

What this PR does is add two rules, one for Replay-based ZIMs (zimit1, but also zimit2 in most circumstances), and one for openZIM ZIMs, which effectively test whether the protocol of a clicked anchor is the same as the protocol of the document loaded in the iframe. If they aren't the same, then (with the exception of some internal protocols such as javascript: or about:) we force the browser to open the link in a new tab or window, i.e. outside of the sandbox. Then it's no longer our problem...

[audiodude]

I assume from your prior comments @Jaifroid that this fixes the case where our CSP says "Don't allow such and such links" but then we serve documents that contain them? Probably because any old things could end up in a ZIM?

[Jaifroid]

I assume from your prior comments @Jaifroid that this fixes the case where our CSP says "Don't allow such and such links" but then we serve documents that contain them? Probably because any old things could end up in a ZIM?

Yes, it's a combination of the strict CSP, sandbox of iframe and CORS protection: designed to ensure that the app can't "call home" or run active content from remote scripts, or download external images / iframes etc.. The main issue we're working around is that CORS protection is triggered when a user tries to load any URI as the source of the iframe that the browser doesn't perceive as same-origin for the app itself. So, since the app must be served from a secure URL (https:...) in order to run Service Workers, trying to load a skype: URI as the source of the iframe (which is what happens if we click on a skype: URL) triggers CORS protection, and puts the app into an unusable and unrecoverable state. If we redirect such URIs to a different tab, then CORS isn't triggered.

[audiodude]

Just a random thought. Would it be better/cheaper to detect those links on DOM load and add a target="blank" attribute to them? Might make debugging easier in the future, and prevent having to remember to add this logic in new places as the architecture changes.

[audiodude]

Let me look closer in the morning

[Jaifroid]

Just a random thought. Would it be better/cheaper to detect those links on DOM load and add a target="blank" attribute to them? Might make debugging easier in the future, and prevent having to remember to add this logic in new places as the architecture changes.

There's something of a taboo on altering DOM content from the ZIM unless it's the only solution, and I've been strongly discouraged from doing so at least in this app (which follows a purist philosophy that is certainly very sound if sometimes a bit frustrating!). One issue with adding target="blank" to all ZIM links is that it will alter the behaviour of a page if it is already loaded at top level, i.e. outside the iframe in a tab or window without any app chrome (in this mode, the SW acts as a transparent proxy). In that case, none of this link filtering is needed: link filtering is only active if there is an iframe to which the click event listener is attached. And it's only necessary because of the CSP protection on the iframe. Otherwise, we just let the browser do its thing and the app acts just like Kiwix Serve does when run in frameless mode, only in our case it's a Service Worker rather than a Web Server that is interfacing with the decompressor.

Apart from those reasons, it wouldn't be cheaper to process every single ZIM link in an article in advance. It is much cheaper only to process the single anchor on which a user has clicked at click time. FYI, we do have DOM-trawling code in the legacy JQuery mode, but the idea of adding a Service Worker was to prevent us having to simulate browser functionality for every click, every image loaded, every script called.... (it was a nightmare!)... Instead, the SW intercepts Fetch requests, decides if they have to come from the ZIM or from the FS, and responds accordingly. Unfortunately, the SW can't intercept out-of-scope URLs...

You can test browsing chromeless by going to https://browser-extension.kiwix.org/current/www/index.html, opening a ZIM and then command-clicking a ZIM link. This should open the clicked article in a new tab which is fully browsable, with the Service Worker serving content that the browser requests from the ZIM.

[audiodude]

Thanks for the extensive explanation! Maybe we can VC at some point and you can explain the architecture to me (again?) because I am definitely a bit lost.

[audiodude]

LGTM

[Jaifroid]

Manual tests to be performed:

• IE11 in Restricted mode
• Chrome/Firefox in Restricted mode
• Safari and Edge Legacy in SW mode (via BrowserStack) - Safari 17 on iPad Pro 12; Edge Legacy 18
• Chromium Extension in ServiceWorkerLocal mode

[Jaifroid]

All manual and automatic tests are passing, so I think it's safe to merge.