From c26ad83e2e0c29a6e18a36fcff86554815ea6bea Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Tue, 27 Sep 2022 12:40:23 +1000 Subject: Refactoring response handler to improve readability --- bg/ExternalLicenses.js | 12 +++++++++++- bg/ResponseProcessor.js | 22 ++++++++++++---------- main_background.js | 32 ++++++++++++++++++++------------ 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/bg/ExternalLicenses.js b/bg/ExternalLicenses.js index 6eda008..6d35863 100644 --- a/bg/ExternalLicenses.js +++ b/bg/ExternalLicenses.js @@ -48,7 +48,17 @@ const ExternalLicenses = { cachedHrefs.delete(tabId); }, - // Checks external script using web labels + /** + * Checks external script using web labels + * + * If the script cannot be found in the web labels table, returns null. + * + * If the script can be found in the web labels table, and at least + * one of its licenses can be found in our free license DB, returns + * "free". + * + * Otherwise returns "nonfree". + */ async check(script) { const { url, tabId, frameId, documentUrl } = script; const tabCache = cachedHrefs.get(tabId); diff --git a/bg/ResponseProcessor.js b/bg/ResponseProcessor.js index 3c90d9c..429b778 100644 --- a/bg/ResponseProcessor.js +++ b/bg/ResponseProcessor.js @@ -29,6 +29,11 @@ const { ResponseMetaData } = require('./ResponseMetaData'); const listeners = new WeakMap(); const webRequestEvent = browser.webRequest.onHeadersReceived; +// Some hardcoded return values for the onHeadersReceived callback. +const BLOCKING_RESPONSES = { + ACCEPT: {}, // Do nothing + REJECT: { cancel: true }, // Cancel the request. See docs of BlockingResponse +}; class ResponseProcessor { @@ -53,13 +58,6 @@ class ResponseProcessor { } } -Object.assign(ResponseProcessor, { - // control flow values to be returned by handler.pre() callbacks - ACCEPT: {}, - REJECT: { cancel: true }, - CONTINUE: null -}); - class ResponseTextFilter { constructor(request) { this.request = request; @@ -72,13 +70,16 @@ class ResponseTextFilter { } async process(handler) { - if (!this.canProcess) return ResponseProcessor.ACCEPT; + if (!this.canProcess) return BlockingResponses.ACCEPT; const { metaData, request } = this; const response = { request, metaData }; // we keep it around allowing callbacks to store state if (typeof handler.pre !== 'function' || typeof handler.post !== 'function') { throw new Error('handler should have a pre and post function.'); } + + // If can determine without checking and filtering response + // payload then return. const res = await handler.pre(response); if (res) return res; @@ -132,8 +133,9 @@ class ResponseTextFilter { filter.close(); } - return ResponseProcessor.ACCEPT; + // Accepting the filtered payload. + return BLOCKING_RESPONSES.ACCEPT; } } -module.exports = { ResponseProcessor }; +module.exports = { ResponseProcessor, BLOCKING_RESPONSES }; diff --git a/main_background.js b/main_background.js index 49815d5..40a8397 100644 --- a/main_background.js +++ b/main_background.js @@ -22,7 +22,7 @@ */ const checkLib = require('./common/checks.js'); -const { ResponseProcessor } = require('./bg/ResponseProcessor'); +const { ResponseProcessor, BLOCKING_RESPONSES } = require('./bg/ResponseProcessor'); const { Storage, ListStore, hash } = require('./common/Storage'); const { ListManager } = require('./bg/ListManager'); const { ExternalLicenses } = require('./bg/ExternalLicenses'); @@ -470,15 +470,23 @@ async function blockBlacklistedScripts(request) { } /** -* This listener gets called as soon as we've got all the HTTP headers, can guess -* content type and encoding, and therefore correctly parse HTML documents -* and external script inclusions in search of non-free JavaScript -*/ - + * An onHeadersReceived handler. See bg/ResponseProcessor.js for how + * it is used. + * + * This listener gets called as soon as we've got all the HTTP + * headers, can guess content type and encoding, and therefore + * correctly parse HTML documents and external script inclusions in + * search of non-free JavaScript + */ const ResponseHandler = { /** - * Enforce white/black lists for url/site early (hashes will be handled later) - */ + * Checks black/whitelists and web labels. Returns a + * BlockingResponse (if we can determine) or null (if further work + * is needed). + * + * Enforce white/black lists for url/site early (hashes will be + * handled later) + */ async pre(response) { let { request } = response; let { url, type, tabId, frameId, documentUrl } = request; @@ -494,7 +502,7 @@ const ResponseHandler = { if (blacklisted) { if (type === 'script') { // this shouldn't happen, because we intercept earlier in blockBlacklistedScripts() - return ResponseProcessor.REJECT; + return BLOCKING_RESPONSES.REJECT; } if (type === 'main_frame') { // we handle the page change here too, since we won't call edit_html() activityReports[tabId] = await createReport({ url: fullUrl, tabId }); @@ -520,12 +528,12 @@ const ResponseHandler = { url: topUrl, 'whitelisted': [url, whitelistedSite ? `User whitelisted ${whitelistedSite}` : 'Whitelisted by user'] }); - return ResponseProcessor.ACCEPT; + return BLOCKING_RESPONSES.ACCEPT; } else { // Check for the weblabel method const scriptInfo = await ExternalLicenses.check({ url: fullUrl, tabId, frameId, documentUrl }); if (scriptInfo) { - const [verdict, ret] = scriptInfo.free ? ['accepted', ResponseProcessor.ACCEPT] : ['blocked', ResponseProcessor.REJECT]; + const [verdict, ret] = scriptInfo.free ? ['accepted', BLOCKING_RESPONSES.ACCEPT] : ['blocked', BLOCKING_RESPONSES.REJECT]; const licenseIds = [...scriptInfo.licenses].map(l => l.identifier).sort().join(', '); const msg = licenseIds ? `Free license${scriptInfo.licenses.size > 1 ? 's' : ''} (${licenseIds})` @@ -538,7 +546,7 @@ const ResponseHandler = { } // it's a page (it's too early to report) or an unknown script: // let's keep processing - return ResponseProcessor.CONTINUE; + return null; }, /** -- cgit v1.2.3