From 66ef6df55a52aaf31d49756db02ba223fbe71d5b Mon Sep 17 00:00:00 2001 From: Francois Marier Date: Wed, 15 Aug 2018 12:13:16 +0000 Subject: [PATCH] Bug 1482306 - Ensure that tables are enabled when shared between features. r=dimi! The enable/disable logic of the list manager was wrong. If multiple features shared a given table (e.g. tracking protection and tracking annotations) and only one of them was enabled, then updates could either be disabled or enabled depending on which feature was checked first. By disabling everything at the beginning and selectively re-enabling tables, we can ensure that no table gets both enabled and disabled by different feature toggles. Differential Revision: https://phabricator.services.mozilla.com/D3259 --HG-- extra : moz-landing-system : lando --- .../url-classifier/SafeBrowsing.jsm | 26 ++----------------- .../url-classifier/nsIUrlListManager.idl | 7 ++++- .../nsUrlClassifierListManager.js | 16 +++++++++--- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/toolkit/components/url-classifier/SafeBrowsing.jsm b/toolkit/components/url-classifier/SafeBrowsing.jsm index 0abe288cfa40..065cb9ae1c6d 100644 --- a/toolkit/components/url-classifier/SafeBrowsing.jsm +++ b/toolkit/components/url-classifier/SafeBrowsing.jsm @@ -410,88 +410,66 @@ var SafeBrowsing = { let listManager = Cc["@mozilla.org/url-classifier/listmanager;1"]. getService(Ci.nsIUrlListManager); + listManager.disableAllUpdates(); + for (let i = 0; i < this.phishingLists.length; ++i) { if (this.phishingEnabled) { listManager.enableUpdate(this.phishingLists[i]); - } else { - listManager.disableUpdate(this.phishingLists[i]); } } for (let i = 0; i < this.malwareLists.length; ++i) { if (this.malwareEnabled) { listManager.enableUpdate(this.malwareLists[i]); - } else { - listManager.disableUpdate(this.malwareLists[i]); } } for (let i = 0; i < this.downloadBlockLists.length; ++i) { if (this.malwareEnabled && this.downloadsEnabled) { listManager.enableUpdate(this.downloadBlockLists[i]); - } else { - listManager.disableUpdate(this.downloadBlockLists[i]); } } for (let i = 0; i < this.downloadAllowLists.length; ++i) { if (this.malwareEnabled && this.downloadsEnabled) { listManager.enableUpdate(this.downloadAllowLists[i]); - } else { - listManager.disableUpdate(this.downloadAllowLists[i]); } } for (let i = 0; i < this.passwordAllowLists.length; ++i) { if (this.passwordsEnabled) { listManager.enableUpdate(this.passwordAllowLists[i]); - } else { - listManager.disableUpdate(this.passwordAllowLists[i]); } } for (let i = 0; i < this.trackingAnnotationLists.length; ++i) { if (this.trackingAnnotations) { listManager.enableUpdate(this.trackingAnnotationLists[i]); - } else { - listManager.disableUpdate(this.trackingAnnotationLists[i]); } } for (let i = 0; i < this.trackingAnnotationWhitelists.length; ++i) { if (this.trackingAnnotations) { listManager.enableUpdate(this.trackingAnnotationWhitelists[i]); - } else { - listManager.disableUpdate(this.trackingAnnotationWhitelists[i]); } } for (let i = 0; i < this.trackingProtectionLists.length; ++i) { if (this.trackingEnabled) { listManager.enableUpdate(this.trackingProtectionLists[i]); - } else { - listManager.disableUpdate(this.trackingProtectionLists[i]); } } for (let i = 0; i < this.trackingProtectionWhitelists.length; ++i) { if (this.trackingEnabled) { listManager.enableUpdate(this.trackingProtectionWhitelists[i]); - } else { - listManager.disableUpdate(this.trackingProtectionWhitelists[i]); } } for (let i = 0; i < this.blockedLists.length; ++i) { if (this.blockedEnabled) { listManager.enableUpdate(this.blockedLists[i]); - } else { - listManager.disableUpdate(this.blockedLists[i]); } } for (let i = 0; i < this.flashLists.length; ++i) { if (this.flashBlockEnabled) { listManager.enableUpdate(this.flashLists[i]); - } else { - listManager.disableUpdate(this.flashLists[i]); } } for (let i = 0; i < this.flashInfobarLists.length; ++i) { if (this.flashInfobarListEnabled) { listManager.enableUpdate(this.flashInfobarLists[i]); - } else { - listManager.disableUpdate(this.flashInfobarLists[i]); } } listManager.maybeToggleUpdateChecking(); diff --git a/toolkit/components/url-classifier/nsIUrlListManager.idl b/toolkit/components/url-classifier/nsIUrlListManager.idl index 729acb34bf56..a92d1debd43e 100644 --- a/toolkit/components/url-classifier/nsIUrlListManager.idl +++ b/toolkit/components/url-classifier/nsIUrlListManager.idl @@ -51,7 +51,12 @@ interface nsIUrlListManager : nsISupports void enableUpdate(in ACString tableName); /** - * Turn off update checking for a table. + * Turn off update checking for all tables. + */ + void disableAllUpdates(); + + /** + * Turn off update checking for a single table. Only used in tests. */ void disableUpdate(in ACString tableName); diff --git a/toolkit/components/url-classifier/nsUrlClassifierListManager.js b/toolkit/components/url-classifier/nsUrlClassifierListManager.js index ccb0a363603e..59a247398959 100644 --- a/toolkit/components/url-classifier/nsUrlClassifierListManager.js +++ b/toolkit/components/url-classifier/nsUrlClassifierListManager.js @@ -168,8 +168,7 @@ PROT_ListManager.prototype.getUpdateUrl = function(tableName) { }; /** - * Enable updates for some tables - * @param tables - an array of table names that need updating + * Enable updates for a single table. */ PROT_ListManager.prototype.enableUpdate = function(tableName) { var table = this.tablesData[tableName]; @@ -194,8 +193,17 @@ PROT_ListManager.prototype.updatesNeeded_ = function(updateUrl) { }; /** - * Disables updates for some tables - * @param tables - an array of table names that no longer need updating + * Disable updates for all tables. + */ +PROT_ListManager.prototype.disableAllUpdates = function() { + for (const tableName of Object.keys(this.tablesData)) { + this.disableUpdate(tableName); + } +}; + +/** + * Disables updates for a single table. Avoid this internal function + * and use disableAllUpdates() instead. */ PROT_ListManager.prototype.disableUpdate = function(tableName) { var table = this.tablesData[tableName];