Code review of whitelisting-related code

- Use `Map()` instead of `{}` for internal data
  structure
- Export as array of directives instead of as
  a string

src/js/background.js

@@ -116,7 +116,7 @@ const µBlock = (function() { // jshint ignore:line
 
         // https://github.com/chrisaljoudi/uBlock/issues/180
         // Whitelist directives need to be loaded once the PSL is available
-        netWhitelist: {},
+        netWhitelist: new Map(),
         netWhitelistModifyTime: 0,
         netWhitelistDefault: [
             'about-scheme',

src/js/messaging.js

@@ -762,9 +762,9 @@ const µb = µBlock;
 
 // Settings
 
-var getLocalData = function(callback) {
+const getLocalData = function(callback) {
-    var onStorageInfoReady = function(bytesInUse) {
+    const onStorageInfoReady = function(bytesInUse) {
-        var o = µb.restoreBackupSettings;
+        const o = µb.restoreBackupSettings;
         callback({
             storageUsed: bytesInUse,
             lastRestoreFile: o.lastRestoreFile,
@@ -779,13 +779,15 @@ var getLocalData = function(callback) {
     µb.getBytesInUse(onStorageInfoReady);
 };
 
-var backupUserData = function(callback) {
+const backupUserData = function(callback) {
-    var userData = {
+    const userData = {
         timeStamp: Date.now(),
         version: vAPI.app.version,
         userSettings: µb.userSettings,
         selectedFilterLists: µb.selectedFilterLists,
         hiddenSettings: µb.hiddenSettings,
+        whitelist: µb.arrayFromWhitelist(µb.netWhitelist),
+        // String representation eventually to be deprecated
         netWhitelist: µb.stringFromWhitelist(µb.netWhitelist),
         dynamicFilteringString: µb.permanentFirewall.toString(),
         urlFilteringString: µb.permanentURLFiltering.toString(),
@@ -793,9 +795,9 @@ var backupUserData = function(callback) {
         userFilters: ''
     };
 
-    var onUserFiltersReady = function(details) {
+    const onUserFiltersReady = function(details) {
         userData.userFilters = details.content;
-        var filename = vAPI.i18n('aboutBackupFilename')
+        const filename = vAPI.i18n('aboutBackupFilename')
             .replace('{{datetime}}', µb.dateNowToSensibleString())
             .replace(/ +/g, '_');
         µb.restoreBackupSettings.lastBackupFile = filename;
@@ -829,9 +831,19 @@ const restoreUserData = function(request) {
                 userData.hiddenSettingsString || ''
             );
         }
+        // Whitelist directives can be represented as an array or as a
+        // (eventually to be deprecated) string.
+        let whitelist = userData.whitelist;
+        if (
+            Array.isArray(whitelist) === false &&
+            typeof userData.netWhitelist === 'string' &&
+            userData.netWhitelist !== ''
+        ) {
+            whitelist = userData.netWhitelist.split('\n');
+        }
         vAPI.storage.set({
             hiddenSettings: hiddenSettings,
-            netWhitelist: userData.netWhitelist || '',
+            netWhitelist: whitelist || [],
             dynamicFilteringString: userData.dynamicFilteringString || '',
             urlFilteringString: userData.urlFilteringString || '',
             hostnameSwitchesString: userData.hostnameSwitchesString || '',

src/js/settings.js

@@ -50,7 +50,10 @@ const handleImportFilePicker = function() {
             if ( typeof userData.userSettings !== 'object' ) {
                 throw 'Invalid';
             }
-            if ( typeof userData.netWhitelist !== 'string' ) {
+            if (
+                Array.isArray(userData.whitelist) === false &&
+                typeof userData.netWhitelist !== 'string'
+            ) {
                 throw 'Invalid';
             }
             if (

src/js/start.js

@@ -239,7 +239,10 @@ const onVersionReady = function(lastVersion) {
 // gorhill 2014-12-15: not anymore
 
 const onNetWhitelistReady = function(netWhitelistRaw) {
-    µb.netWhitelist = µb.whitelistFromString(netWhitelistRaw);
+    if ( typeof netWhitelistRaw === 'string' ) {
+        netWhitelistRaw = netWhitelistRaw.split('\n');
+    }
+    µb.netWhitelist = µb.whitelistFromArray(netWhitelistRaw);
     µb.netWhitelistModifyTime = Date.now();
 };
 
@@ -358,7 +361,7 @@ const createDefaultProps = function() {
         'lastRestoreTime': 0,
         'lastBackupFile': '',
         'lastBackupTime': 0,
-        'netWhitelist': µb.netWhitelistDefault.join('\n'),
+        'netWhitelist': µb.netWhitelistDefault,
         'selfieMagic': 0,
         'version': '0.0.0.0'
     };

src/js/storage.js

@@ -238,7 +238,7 @@
 
 µBlock.saveWhitelist = function() {
     vAPI.storage.set({
-        netWhitelist: this.stringFromWhitelist(this.netWhitelist)
+        netWhitelist: this.arrayFromWhitelist(this.netWhitelist)
     });
     this.netWhitelistModifyTime = Date.now();
 };
@@ -1194,8 +1194,11 @@
             binNotEmpty = true;
         }
 
-        if ( typeof data.netWhitelist === 'string' ) {
+        if ( Array.isArray(data.whitelist) ) {
-            bin.netWhitelist = data.netWhitelist;
+            bin.netWhitelist = data.whitelist;
+            binNotEmpty = true;
+        } else if ( typeof data.netWhitelist === 'string' ) {
+            bin.netWhitelist = data.netWhitelist.split('\n');
             binNotEmpty = true;
         }
 

src/js/ublock.js

@@ -24,29 +24,30 @@
 /******************************************************************************/
 /******************************************************************************/
 
-(function(){
+// *****************************************************************************
+// start of local namespace
 
-/******************************************************************************/
+{
 
 // https://github.com/chrisaljoudi/uBlock/issues/405
 // Be more flexible with whitelist syntax
 
 // Any special regexp char will be escaped
-var whitelistDirectiveEscape = /[-\/\\^$+?.()|[\]{}]/g;
+const whitelistDirectiveEscape = /[-\/\\^$+?.()|[\]{}]/g;
 
 // All `*` will be expanded into `.*`
-var whitelistDirectiveEscapeAsterisk = /\*/g;
+const whitelistDirectiveEscapeAsterisk = /\*/g;
 
 // Remember encountered regexps for reuse.
-var directiveToRegexpMap = new Map();
+const directiveToRegexpMap = new Map();
 
 // Probably manually entered whitelist directive
-var isHandcraftedWhitelistDirective = function(directive) {
+const isHandcraftedWhitelistDirective = function(directive) {
     return directive.startsWith('/') && directive.endsWith('/') ||
            directive.indexOf('/') !== -1 && directive.indexOf('*') !== -1;
 };
 
-var matchDirective = function(url, hostname, directive) {
+const matchDirective = function(url, hostname, directive) {
     // Directive is a plain hostname.
     if ( directive.indexOf('/') === -1 ) {
         return hostname.endsWith(directive) &&
@@ -54,13 +55,16 @@ var matchDirective = function(url, hostname, directive) {
                hostname.charAt(hostname.length - directive.length - 1) === '.');
     }
     // Match URL exactly.
-    if ( directive.startsWith('/') === false && directive.indexOf('*') === -1 ) {
+    if (
+        directive.startsWith('/') === false &&
+        directive.indexOf('*') === -1
+    ) {
         return url === directive;
     }
     // Transpose into a regular expression.
-    var re = directiveToRegexpMap.get(directive);
+    let re = directiveToRegexpMap.get(directive);
     if ( re === undefined ) {
-        var reStr;
+        let reStr;
         if ( directive.startsWith('/') && directive.endsWith('/') ) {
             reStr = directive.slice(1, -1);
         } else {
@@ -73,9 +77,9 @@ var matchDirective = function(url, hostname, directive) {
     return re.test(url);
 };
 
-var matchBucket = function(url, hostname, bucket, start) {
+const matchBucket = function(url, hostname, bucket, start) {
     if ( bucket ) {
-        for ( var i = start || 0, n = bucket.length; i < n; i++ ) {
+        for ( let i = start || 0, n = bucket.length; i < n; i++ ) {
             if ( matchDirective(url, hostname, bucket[i]) ) {
                 return i;
             }
@@ -84,23 +88,20 @@ var matchBucket = function(url, hostname, bucket, start) {
     return -1;
 };
 
-// https://www.youtube.com/watch?v=RL2W_XK-UJ4&list=PLhPp-QAUKF_hRMjWsYvvdazGw0qIjtSXJ
-
 /******************************************************************************/
 
 µBlock.getNetFilteringSwitch = function(url) {
-    var targetHostname = this.URI.hostnameFromURI(url),
+    const hostname = this.URI.hostnameFromURI(url);
-        key = targetHostname,
+    let key = hostname;
-        pos;
     for (;;) {
-        if ( matchBucket(url, targetHostname, this.netWhitelist[key]) !== -1 ) {
+        if ( matchBucket(url, hostname, this.netWhitelist.get(key)) !== -1 ) {
             return false;
         }
-        pos = key.indexOf('.');
+        const pos = key.indexOf('.');
         if ( pos === -1 ) { break; }
         key = key.slice(pos + 1);
     }
-    if ( matchBucket(url, targetHostname, this.netWhitelist['//']) !== -1 ) {
+    if ( matchBucket(url, hostname, this.netWhitelist.get('//')) !== -1 ) {
         return false;
     }
     return true;
@@ -109,7 +110,7 @@ var matchBucket = function(url, hostname, bucket, start) {
 /******************************************************************************/
 
 µBlock.toggleNetFilteringSwitch = function(url, scope, newState) {
-    var currentState = this.getNetFilteringSwitch(url);
+    const currentState = this.getNetFilteringSwitch(url);
     if ( newState === undefined ) {
         newState = !currentState;
     }
@@ -117,58 +118,59 @@ var matchBucket = function(url, hostname, bucket, start) {
         return currentState;
     }
 
-    var netWhitelist = this.netWhitelist,
+    const netWhitelist = this.netWhitelist;
-        pos = url.indexOf('#'),
+    const pos = url.indexOf('#');
-        targetURL = pos !== -1 ? url.slice(0, pos) : url,
+    let targetURL = pos !== -1 ? url.slice(0, pos) : url;
-        targetHostname = this.URI.hostnameFromURI(targetURL),
+    const targetHostname = this.URI.hostnameFromURI(targetURL);
-        key = targetHostname,
+    let key = targetHostname;
-        directive = scope === 'page' ? targetURL : targetHostname;
+    let directive = scope === 'page' ? targetURL : targetHostname;
 
     // Add to directive list
     if ( newState === false ) {
-        if ( netWhitelist[key] === undefined ) {
+        let bucket = netWhitelist.get(key);
-            netWhitelist[key] = [];
+        if ( bucket === undefined ) {
+            bucket = [];
+            netWhitelist.set(key, bucket);
         }
-        netWhitelist[key].push(directive);
+        bucket.push(directive);
         this.saveWhitelist();
         return true;
     }
 
-    // Remove from directive list whatever causes current URL to be whitelisted
+    // Remove all directives which cause current URL to be whitelisted
-    var bucket, i;
     for (;;) {
-        bucket = netWhitelist[key];
+        const bucket = netWhitelist.get(key);
         if ( bucket !== undefined ) {
-            i = undefined;
+            let i;
             for (;;) {
                 i = matchBucket(targetURL, targetHostname, bucket, i);
                 if ( i === -1 ) { break; }
                 directive = bucket.splice(i, 1)[0];
                 if ( isHandcraftedWhitelistDirective(directive) ) {
-                    netWhitelist['#'].push('# ' + directive);
+                    netWhitelist.get('#').push(`# ${directive}`);
                 }
             }
             if ( bucket.length === 0 ) {
-                delete netWhitelist[key];
+                netWhitelist.delete(key);
             }
         }
-        pos = key.indexOf('.');
+        const pos = key.indexOf('.');
         if ( pos === -1 ) { break; }
         key = key.slice(pos + 1);
     }
-    bucket = netWhitelist['//'];
+    const bucket = netWhitelist.get('//');
     if ( bucket !== undefined ) {
-        i = undefined;
+        let i;
         for (;;) {
             i = matchBucket(targetURL, targetHostname, bucket, i);
             if ( i === -1 ) { break; }
             directive = bucket.splice(i, 1)[0];
             if ( isHandcraftedWhitelistDirective(directive) ) {
-                netWhitelist['#'].push('# ' + directive);
+                netWhitelist.get('#').push(`# ${directive}`);
             }
         }
         if ( bucket.length === 0 ) {
-            delete netWhitelist['//'];
+            netWhitelist.delete('//');
         }
     }
     this.saveWhitelist();
@@ -179,8 +181,7 @@ var matchBucket = function(url, hostname, bucket, start) {
 
 µBlock.arrayFromWhitelist = function(whitelist) {
     const out = new Set();
-    for ( const key in whitelist ) {
+    for ( const bucket of whitelist.values() ) {
-        const bucket = whitelist[key];
         for ( const directive of bucket ) {
             out.add(directive);
         }
@@ -194,25 +195,23 @@ var matchBucket = function(url, hostname, bucket, start) {
 
 /******************************************************************************/
 
-µBlock.whitelistFromString = function(s) {
+µBlock.whitelistFromArray = function(lines) {
-    var whitelist = Object.create(null),
+    const whitelist = new Map();
-        lineIter = new this.LineIterator(s),
-        line, matches, key, directive, re;
 
     // Comment bucket must always be ready to be used.
-    whitelist['#'] = [];
+    whitelist.set('#', []);
 
     // New set of directives, scrap cached data.
     directiveToRegexpMap.clear();
 
-    while ( !lineIter.eot() ) {
+    for ( let line of lines ) {
-        line = lineIter.next().trim();
+        line = line.trim();
 
         // https://github.com/gorhill/uBlock/issues/171
         // Skip empty lines
-        if ( line === '' ) {
+        if ( line === '' ) { continue; }
-            continue;
+
-        }
+        let key, directive;
 
         // Don't throw out commented out lines: user might want to fix them
         if ( line.startsWith('#') ) {
@@ -229,11 +228,15 @@ var matchBucket = function(url, hostname, bucket, start) {
             }
         }
         // Regex-based (ensure it is valid)
-        else if ( line.length > 2 && line.startsWith('/') && line.endsWith('/') ) {
+        else if (
+            line.length > 2 &&
+            line.startsWith('/') &&
+            line.endsWith('/')
+        ) {
             key = '//';
             directive = line;
             try {
-                re = new RegExp(directive.slice(1, -1));
+                const re = new RegExp(directive.slice(1, -1));
                 directiveToRegexpMap.set(directive, re);
             } catch(ex) {
                 key = '#';
@@ -244,7 +247,7 @@ var matchBucket = function(url, hostname, bucket, start) {
         // label (or else it would be just impossible to make an efficient
         // dict.
         else {
-            matches = this.reWhitelistHostnameExtractor.exec(line);
+            const matches = this.reWhitelistHostnameExtractor.exec(line);
             if ( !matches || matches.length !== 2 ) {
                 key = '#';
                 directive = '# ' + line;
@@ -260,21 +263,28 @@ var matchBucket = function(url, hostname, bucket, start) {
 
         // Be sure this stays fixed:
         // https://github.com/chrisaljoudi/uBlock/issues/185
-        if ( whitelist[key] === undefined ) {
+        let bucket = whitelist.get(key);
-            whitelist[key] = [];
+        if ( bucket === undefined ) {
+            bucket = [];
+            whitelist.set(key, bucket);
         }
-        whitelist[key].push(directive);
+        bucket.push(directive);
     }
     return whitelist;
 };
 
+µBlock.whitelistFromString = function(s) {
+    return this.whitelistFromArray(s.split('\n'));
+};
+
 // https://github.com/gorhill/uBlock/issues/3717
 µBlock.reWhitelistBadHostname = /[^a-z0-9.\-_\[\]:]/;
 µBlock.reWhitelistHostnameExtractor = /([a-z0-9.\-_\[\]]+)(?::[\d*]+)?\/(?:[^\x00-\x20\/]|$)[^\x00-\x20]*$/;
 
-/******************************************************************************/
+// end of local namespace
+// *****************************************************************************
 
-})();
+}
 
 /******************************************************************************/
 /******************************************************************************/
@@ -391,7 +401,7 @@ var matchBucket = function(url, hostname, bucket, start) {
 // https://www.reddit.com/r/uBlockOrigin/comments/8524cf/my_custom_scriptlets_doesnt_work_what_am_i_doing/
 
 µBlock.changeHiddenSettings = function(hs) {
-    var mustReloadResources =
+    const mustReloadResources =
         hs.userResourcesLocation !== this.hiddenSettings.userResourcesLocation;
     this.hiddenSettings = hs;
     this.saveHiddenSettings();
@@ -522,7 +532,7 @@ var matchBucket = function(url, hostname, bucket, start) {
         );
         break;
     case 'no-large-media':
-        var pageStore = this.pageStoreFromTabId(details.tabId);
+        const pageStore = this.pageStoreFromTabId(details.tabId);
         if ( pageStore !== null ) {
             pageStore.temporarilyAllowLargeMediaElements(!details.state);
         }
@@ -560,43 +570,44 @@ var matchBucket = function(url, hostname, bucket, start) {
 /******************************************************************************/
 
 µBlock.scriptlets = (function() {
-    var pendingEntries = new Map();
+    const pendingEntries = new Map();
 
-    var Entry = function(tabId, scriptlet, callback) {
+    const Entry = class {
-        this.tabId = tabId;
+        constructor(tabId, scriptlet, callback) {
-        this.scriptlet = scriptlet;
+            this.tabId = tabId;
-        this.callback = callback;
+            this.scriptlet = scriptlet;
-        this.timer = vAPI.setTimeout(this.service.bind(this), 1000);
+            this.callback = callback;
-    };
+            this.timer = vAPI.setTimeout(this.service.bind(this), 1000);
-
+        }
-    Entry.prototype.service = function(response) {
+        service(response) {
-        if ( this.timer !== null ) {
+            if ( this.timer !== null ) {
-            clearTimeout(this.timer);
+                clearTimeout(this.timer);
-            this.timer = null;
+                this.timer = null;
+            }
+            pendingEntries.delete(makeKey(this.tabId, this.scriptlet));
+            this.callback(response);
         }
-        pendingEntries.delete(makeKey(this.tabId, this.scriptlet));
-        this.callback(response);
     };
 
-    var makeKey = function(tabId, scriptlet) {
+    const makeKey = function(tabId, scriptlet) {
         return tabId + ' ' + scriptlet;
     };
 
-    var report = function(tabId, scriptlet, response) {
+    const report = function(tabId, scriptlet, response) {
-        var key = makeKey(tabId, scriptlet);
+        const key = makeKey(tabId, scriptlet);
-        var entry = pendingEntries.get(key);
+        const entry = pendingEntries.get(key);
         if ( entry === undefined ) { return; }
         entry.service(response);
     };
 
-    var inject = function(tabId, scriptlet, callback) {
+    const inject = function(tabId, scriptlet, callback) {
         if ( typeof callback === 'function' ) {
             if ( vAPI.isBehindTheSceneTabId(tabId) ) {
                 callback();
                 return;
             }
-            var key = makeKey(tabId, scriptlet),
+            const key = makeKey(tabId, scriptlet);
-                entry = pendingEntries.get(key);
+            const entry = pendingEntries.get(key);
             if ( entry !== undefined ) {
                 if ( callback !== entry.callback ) {
                     callback();
@@ -611,7 +622,7 @@ var matchBucket = function(url, hostname, bucket, start) {
     };
 
     // TODO: think about a callback mechanism.
-    var injectDeep = function(tabId, scriptlet) {
+    const injectDeep = function(tabId, scriptlet) {
         vAPI.tabs.injectScript(tabId, {
             file: '/js/scriptlets/' + scriptlet + '.js',
             allFrames: true