Merge pull request #293 from praschke/permissions-fixups

Permissions fixups

4 files changed, 96 insertions, 263 deletions

diff --git a/ext/js/background/request-builder.js b/ext/js/background/request-builder.js
index 663e242b..7ee89539 100644
--- a/ext/js/background/request-builder.js
+++ b/ext/js/background/request-builder.js
@@ -31,7 +31,6 @@ class RequestBuilder {
      * Creates a new instance.
      */
     constructor() {
-        this._onBeforeSendHeadersExtraInfoSpec = ['blocking', 'requestHeaders', 'extraHeaders'];
         this._textEncoder = new TextEncoder();
         this._ruleIds = new Set();
     }
@@ -42,6 +41,7 @@ class RequestBuilder {
     async prepare() {
         try {
             await this._clearDynamicRules();
+            await this._clearSessionRules();
         } catch (e) {
             // NOP
         }
@@ -54,15 +54,50 @@ class RequestBuilder {
      * @returns {Promise<Response>} The response of the `fetch` call.
      */
     async fetchAnonymous(url, init) {
-        if (isObject(chrome.declarativeNetRequest)) {
-            return await this._fetchAnonymousDeclarative(url, init);
+        const id = this._getNewRuleId();
+        const originUrl = this._getOriginURL(url);
+        url = encodeURI(decodeURI(url));
+
+        this._ruleIds.add(id);
+        try {
+            const addRules = [{
+                id,
+                priority: 1,
+                condition: {
+                    urlFilter: `|${this._escapeDnrUrl(url)}|`,
+                    resourceTypes: ['xmlhttprequest']
+                },
+                action: {
+                    type: 'modifyHeaders',
+                    requestHeaders: [
+                        {
+                            operation: 'remove',
+                            header: 'Cookie'
+                        },
+                        {
+                            operation: 'set',
+                            header: 'Origin',
+                            value: originUrl
+                        }
+                    ],
+                    responseHeaders: [
+                        {
+                            operation: 'remove',
+                            header: 'Set-Cookie'
+                        }
+                    ]
+                }
+            }];
+
+            await this._updateSessionRules({addRules});
+            try {
+                return await fetch(url, init);
+            } finally {
+                await this._tryUpdateSessionRules({removeRuleIds: [id]});
+            }
+        } finally {
+            this._ruleIds.delete(id);
         }
-        const originURL = this._getOriginURL(url);
-        const headerModifications = [
-            ['cookie', null],
-            ['origin', {name: 'Origin', value: originURL}]
-        ];
-        return await this._fetchInternal(url, init, headerModifications);
     }
 
     /**
@@ -125,145 +160,56 @@ class RequestBuilder {
 
     // Private
 
-    async _fetchInternal(url, init, headerModifications) {
-        const filter = {
-            urls: [this._getMatchURL(url)],
-            types: ['xmlhttprequest']
-        };
-
-        let requestId = null;
-        const onBeforeSendHeadersCallback = (details) => {
-            if (requestId !== null || details.url !== url) { return {}; }
-            ({requestId} = details);
-
-            if (headerModifications === null) { return {}; }
-
-            const requestHeaders = details.requestHeaders;
-            this._modifyHeaders(requestHeaders, headerModifications);
-            return {requestHeaders};
-        };
-
-        let errorDetailsTimer = null;
-        let {promise: errorDetailsPromise, resolve: errorDetailsResolve} = deferPromise();
-        const onErrorOccurredCallback = (details) => {
-            if (errorDetailsResolve === null || details.requestId !== requestId) { return; }
-            if (errorDetailsTimer !== null) {
-                clearTimeout(errorDetailsTimer);
-                errorDetailsTimer = null;
-            }
-            errorDetailsResolve(details);
-            errorDetailsResolve = null;
-        };
-
-        const eventListeners = [];
-        const onBeforeSendHeadersExtraInfoSpec = (headerModifications !== null ? this._onBeforeSendHeadersExtraInfoSpec : []);
-        this._addWebRequestEventListener(chrome.webRequest.onBeforeSendHeaders, onBeforeSendHeadersCallback, filter, onBeforeSendHeadersExtraInfoSpec, eventListeners);
-        this._addWebRequestEventListener(chrome.webRequest.onErrorOccurred, onErrorOccurredCallback, filter, void 0, eventListeners);
+    async _clearSessionRules() {
+        const rules = await this._getSessionRules();
 
-        try {
-            return await fetch(url, init);
-        } catch (e) {
-            // onErrorOccurred is not always invoked by this point, so a delay is needed
-            if (errorDetailsResolve !== null) {
-                errorDetailsTimer = setTimeout(() => {
-                    errorDetailsTimer = null;
-                    if (errorDetailsResolve === null) { return; }
-                    errorDetailsResolve(null);
-                    errorDetailsResolve = null;
-                }, 100);
-            }
-            const details = await errorDetailsPromise;
-            if (details !== null) {
-                const data = {details};
-                this._assignErrorData(e, data);
-            }
-            throw e;
-        } finally {
-            this._removeWebRequestEventListeners(eventListeners);
-        }
-    }
+        if (rules.length === 0) { return; }
 
-    _addWebRequestEventListener(target, callback, filter, extraInfoSpec, eventListeners) {
-        try {
-            for (let i = 0; i < 2; ++i) {
-                try {
-                    if (typeof extraInfoSpec === 'undefined') {
-                        target.addListener(callback, filter);
-                    } else {
-                        target.addListener(callback, filter, extraInfoSpec);
-                    }
-                    break;
-                } catch (e) {
-                    // Firefox doesn't support the 'extraHeaders' option and will throw the following error:
-                    // Type error for parameter extraInfoSpec (Error processing 2: Invalid enumeration value "extraHeaders") for [target].
-                    if (i === 0 && `${e.message}`.includes('extraHeaders') && Array.isArray(extraInfoSpec)) {
-                        const index = extraInfoSpec.indexOf('extraHeaders');
-                        if (index >= 0) {
-                            extraInfoSpec.splice(index, 1);
-                            continue;
-                        }
-                    }
-                    throw e;
-                }
-            }
-        } catch (e) {
-            console.log(e);
-            return;
+        const removeRuleIds = [];
+        for (const {id} of rules) {
+            removeRuleIds.push(id);
         }
-        eventListeners.push({target, callback});
-    }
 
-    _removeWebRequestEventListeners(eventListeners) {
-        for (const {target, callback} of eventListeners) {
-            try {
-                target.removeListener(callback);
-            } catch (e) {
-                console.log(e);
-            }
-        }
+        await this._updateSessionRules({removeRuleIds});
     }
 
-    _getMatchURL(url) {
-        const url2 = new URL(url);
-        return `${url2.protocol}//${url2.host}${url2.pathname}${url2.search}`.replace(/\*/g, '%2a');
+    _getSessionRules() {
+        return new Promise((resolve, reject) => {
+            chrome.declarativeNetRequest.getSessionRules((result) => {
+                const e = chrome.runtime.lastError;
+                if (e) {
+                    reject(new Error(e.message));
+                } else {
+                    resolve(result);
+                }
+            });
+        });
     }
 
-    _getOriginURL(url) {
-        const url2 = new URL(url);
-        return `${url2.protocol}//${url2.host}`;
+    _updateSessionRules(options) {
+        return new Promise((resolve, reject) => {
+            chrome.declarativeNetRequest.updateSessionRules(options, () => {
+                const e = chrome.runtime.lastError;
+                if (e) {
+                    reject(new Error(e.message));
+                } else {
+                    resolve();
+                }
+            });
+        });
     }
 
-    _modifyHeaders(headers, modifications) {
-        modifications = new Map(modifications);
-
-        for (let i = 0, ii = headers.length; i < ii; ++i) {
-            const header = headers[i];
-            const name = header.name.toLowerCase();
-            const modification = modifications.get(name);
-            if (typeof modification === 'undefined') { continue; }
-
-            modifications.delete(name);
-
-            if (modification === null) {
-                headers.splice(i, 1);
-                --i;
-                --ii;
-            } else {
-                headers[i] = modification;
-            }
-        }
-
-        for (const header of modifications.values()) {
-            if (header !== null) {
-                headers.push(header);
-            }
+    async _tryUpdateSessionRules(options) {
+        try {
+            await this._updateSessionRules(options);
+            return true;
+        } catch (e) {
+            return false;
         }
     }
 
     async _clearDynamicRules() {
-        if (!isObject(chrome.declarativeNetRequest)) { return; }
-
-        const rules = this._getDynamicRules();
+        const rules = await this._getDynamicRules();
 
         if (rules.length === 0) { return; }
 
@@ -275,53 +221,6 @@ class RequestBuilder {
         await this._updateDynamicRules({removeRuleIds});
     }
 
-    async _fetchAnonymousDeclarative(url, init) {
-        const id = this._getNewRuleId();
-        const originUrl = this._getOriginURL(url);
-        url = encodeURI(decodeURI(url));
-
-        this._ruleIds.add(id);
-        try {
-            const addRules = [{
-                id,
-                priority: 1,
-                condition: {
-                    urlFilter: `|${this._escapeDnrUrl(url)}|`,
-                    resourceTypes: ['xmlhttprequest']
-                },
-                action: {
-                    type: 'modifyHeaders',
-                    requestHeaders: [
-                        {
-                            operation: 'remove',
-                            header: 'Cookie'
-                        },
-                        {
-                            operation: 'set',
-                            header: 'Origin',
-                            value: originUrl
-                        }
-                    ],
-                    responseHeaders: [
-                        {
-                            operation: 'remove',
-                            header: 'Set-Cookie'
-                        }
-                    ]
-                }
-            }];
-
-            await this._updateDynamicRules({addRules});
-            try {
-                return await this._fetchInternal(url, init, null);
-            } finally {
-                await this._tryUpdateDynamicRules({removeRuleIds: [id]});
-            }
-        } finally {
-            this._ruleIds.delete(id);
-        }
-    }
-
     _getDynamicRules() {
         return new Promise((resolve, reject) => {
             chrome.declarativeNetRequest.getDynamicRules((result) => {
@@ -348,15 +247,6 @@ class RequestBuilder {
         });
     }
 
-    async _tryUpdateDynamicRules(options) {
-        try {
-            await this._updateDynamicRules(options);
-            return true;
-        } catch (e) {
-            return false;
-        }
-    }
-
     _getNewRuleId() {
         let id = 1;
         while (this._ruleIds.has(id)) {
@@ -367,6 +257,11 @@ class RequestBuilder {
         return id;
     }
 
+    _getOriginURL(url) {
+        const url2 = new URL(url);
+        return `${url2.protocol}//${url2.host}`;
+    }
+
     _escapeDnrUrl(url) {
         return url.replace(/[|*^]/g, (char) => this._urlEncodeUtf8(char));
     }
@@ -380,25 +275,6 @@ class RequestBuilder {
         return result;
     }
 
-    _assignErrorData(error, data) {
-        try {
-            error.data = data;
-        } catch (e) {
-            // On Firefox, assigning DOMException.data can fail in certain contexts.
-            // https://bugzilla.mozilla.org/show_bug.cgi?id=1776555
-            try {
-                Object.defineProperty(error, 'data', {
-                    configurable: true,
-                    enumerable: true,
-                    writable: true,
-                    value: data
-                });
-            } catch (e2) {
-                // NOP
-            }
-        }
-    }
-
     static _joinUint8Arrays(items, totalLength) {
         if (items.length === 1) {
             const {array, length} = items[0];
diff --git a/ext/js/extension/environment.js b/ext/js/extension/environment.js
index ec1e8612..ad5a19ae 100644
--- a/ext/js/extension/environment.js
+++ b/ext/js/extension/environment.js
@@ -31,8 +31,9 @@ class Environment {
     }
 
     async _loadEnvironmentInfo() {
-        const browser = await this._getBrowser();
         const os = await this._getOperatingSystem();
+        const browser = await this._getBrowser(os);
+
         return {
             browser,
             platform: {os}
@@ -64,7 +65,7 @@ class Environment {
         });
     }
 
-    async _getBrowser() {
+    async _getBrowser(os) {
         try {
             if (chrome.runtime.getURL('/').startsWith('ms-browser-extension://')) {
                 return 'edge-legacy';
@@ -76,17 +77,12 @@ class Environment {
             // NOP
         }
         if (typeof browser !== 'undefined') {
-            try {
-                const info = await browser.runtime.getBrowserInfo();
-                if (info.name === 'Fennec') {
-                    return 'firefox-mobile';
-                }
-            } catch (e) {
-                // NOP
-            }
             if (this._isSafari()) {
                 return 'safari';
             }
+            if (os === 'android') {
+                return 'firefox-mobile';
+            }
             return 'firefox';
         } else {
             return 'chrome';
diff --git a/ext/permissions.html b/ext/permissions.html
index 9ede7d27..376a9de5 100644
--- a/ext/permissions.html
+++ b/ext/permissions.html
@@ -49,33 +49,6 @@
         </div></div>
         <div class="settings-item"><div class="settings-item-inner">
             <div class="settings-item-left">
-                <div class="settings-item-label"><code>webRequest</code></div>
-                <div class="settings-item-description">
-                    <p>
-                        Yomitan uses this permission to collect audio or create Anki notes using
-                        <a href="https://ankiweb.net/shared/info/2055492159" target="_blank" rel="noopener noreferrer">AnkiConnect</a>.
-                        It is also required to surface error information from failed requests.
-                    </p>
-                </div>
-            </div>
-        </div></div>
-        <div class="settings-item" data-show-for-browser="firefox firefox-mobile"><div class="settings-item-inner">
-            <div class="settings-item-left">
-                <div class="settings-item-label"><code>webRequestBlocking</code></div>
-                <div class="settings-item-description">
-                    <p>
-                        Yomitan uses this permission to ensure certain requests have valid and secure headers.
-                        This sometimes involves removing or changing the <code>Origin</code> request header,
-                        as this can be used to fingerprint browser configuration.
-                    </p>
-                    <p>
-                        Example: <code class="overflow-wrap">Origin: <span class="extension-id-example"></span></code>
-                    </p>
-                </div>
-            </div>
-        </div></div>
-        <div class="settings-item" data-show-for-browser="chrome edge"><div class="settings-item-inner">
-            <div class="settings-item-left">
                 <div class="settings-item-label"><code>declarativeNetRequest</code></div>
                 <div class="settings-item-description">
                     <p>
@@ -89,11 +62,11 @@
                 </div>
             </div>
         </div></div>
-        <div class="settings-item" data-show-for-manifest-version="3"><div class="settings-item-inner">
+        <div class="settings-item"><div class="settings-item-inner">
             <div class="settings-item-left">
                 <div class="settings-item-label"><code>scripting</code></div>
                 <div class="settings-item-description">
-                    Yomitan will sometimes need to inject stylesheets into webpages in order to
+                    Yomitan needs to inject content scripts and stylesheets into webpages in order to
                     properly display the search popup.
                 </div>
             </div>
@@ -121,9 +94,9 @@
                 <label class="toggle"><input type="checkbox" class="permissions-toggle" data-required-permissions="clipboardRead"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
             </div>
         </div></div>
-        <div class="settings-item"><div class="settings-item-inner">
+        <div class="settings-item" data-hide-for-browser="firefox-mobile"><div class="settings-item-inner">
             <div class="settings-item-left">
-                <div class="settings-item-label"><code>nativeMessaging</code> <span class="light" data-show-for-browser="chrome edge">(optional)</span></div>
+                <div class="settings-item-label"><code>nativeMessaging</code> <span class="light">(optional)</span></div>
                 <div class="settings-item-description">
                     Yomitan has the ability to communicate with an optional native messaging component in order to support
                     parsing large blocks of Japanese text using
@@ -135,18 +108,6 @@
                 <label class="toggle"><input type="checkbox" class="permissions-toggle" data-required-permissions="nativeMessaging"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
             </div>
         </div></div>
-        <div class="settings-item" data-hide-for-manifest-version="3"><div class="settings-item-inner">
-            <div class="settings-item-left">
-                <div class="settings-item-label"><code>webNavigation</code> <span class="light">(optional)</span></div>
-                <div class="settings-item-description">
-                    Yomitan may require this permission to inject content scripts for certain browsers
-                    if Google Docs accessibility mode is enabled.
-                </div>
-            </div>
-            <div class="settings-item-right">
-                <label class="toggle"><input type="checkbox" class="permissions-toggle" data-required-permissions="webNavigation"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
-            </div>
-        </div></div>
         <div class="settings-item"><div class="settings-item-inner">
             <div class="settings-item-left">
                 <div class="settings-item-label">Allow in private windows <span class="light">(optional)</span></div>
diff --git a/ext/settings.html b/ext/settings.html
index f1001f90..8d5f0a76 100644
--- a/ext/settings.html
+++ b/ext/settings.html
@@ -1372,7 +1372,7 @@
                 </p>
             </div>
         </div>
-        <div class="settings-item advanced-only">
+        <div class="settings-item advanced-only" data-hide-for-browser="firefox-mobile">
             <div class="settings-item-inner">
                 <div class="settings-item-left">
                     <div class="settings-item-invalid-indicator"></div>