From 9065bbdd4866123cafa6ee4d2a3aef3dd3bfc3b6 Mon Sep 17 00:00:00 2001 From: Raymond Hill Date: Tue, 25 Jun 2019 11:57:14 -0400 Subject: [PATCH] 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 | 2 +- src/js/messaging.js | 28 +++++-- src/js/settings.js | 5 +- src/js/start.js | 7 +- src/js/storage.js | 9 ++- src/js/ublock.js | 181 +++++++++++++++++++++++-------------------- 6 files changed, 132 insertions(+), 100 deletions(-) diff --git a/src/js/background.js b/src/js/background.js index e7f60d9b5..036c9b309 100644 --- a/src/js/background.js +++ b/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', diff --git a/src/js/messaging.js b/src/js/messaging.js index b824924f9..c196980f6 100644 --- a/src/js/messaging.js +++ b/src/js/messaging.js @@ -762,9 +762,9 @@ const µb = µBlock; // Settings -var getLocalData = function(callback) { - var onStorageInfoReady = function(bytesInUse) { - var o = µb.restoreBackupSettings; +const getLocalData = function(callback) { + const onStorageInfoReady = function(bytesInUse) { + 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) { - var userData = { +const backupUserData = function(callback) { + 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 || '', diff --git a/src/js/settings.js b/src/js/settings.js index adc38102b..aebab6c36 100644 --- a/src/js/settings.js +++ b/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 ( diff --git a/src/js/start.js b/src/js/start.js index 80828c10f..8ec451ed5 100644 --- a/src/js/start.js +++ b/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' }; diff --git a/src/js/storage.js b/src/js/storage.js index aaeef420f..a3e889adc 100644 --- a/src/js/storage.js +++ b/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' ) { - bin.netWhitelist = data.netWhitelist; + if ( Array.isArray(data.whitelist) ) { + bin.netWhitelist = data.whitelist; + binNotEmpty = true; + } else if ( typeof data.netWhitelist === 'string' ) { + bin.netWhitelist = data.netWhitelist.split('\n'); binNotEmpty = true; } diff --git a/src/js/ublock.js b/src/js/ublock.js index 4afad4980..68130cd8b 100644 --- a/src/js/ublock.js +++ b/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), - key = targetHostname, - pos; + const hostname = this.URI.hostnameFromURI(url); + let key = hostname; 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, - pos = url.indexOf('#'), - targetURL = pos !== -1 ? url.slice(0, pos) : url, - targetHostname = this.URI.hostnameFromURI(targetURL), - key = targetHostname, - directive = scope === 'page' ? targetURL : targetHostname; + const netWhitelist = this.netWhitelist; + const pos = url.indexOf('#'); + let targetURL = pos !== -1 ? url.slice(0, pos) : url; + const targetHostname = this.URI.hostnameFromURI(targetURL); + let key = targetHostname; + let directive = scope === 'page' ? targetURL : targetHostname; // Add to directive list if ( newState === false ) { - if ( netWhitelist[key] === undefined ) { - netWhitelist[key] = []; + let bucket = netWhitelist.get(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 - var bucket, i; + // Remove all directives which cause current URL to be whitelisted 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 ) { - const bucket = whitelist[key]; + for ( const bucket of whitelist.values() ) { for ( const directive of bucket ) { out.add(directive); } @@ -194,25 +195,23 @@ var matchBucket = function(url, hostname, bucket, start) { /******************************************************************************/ -µBlock.whitelistFromString = function(s) { - var whitelist = Object.create(null), - lineIter = new this.LineIterator(s), - line, matches, key, directive, re; +µBlock.whitelistFromArray = function(lines) { + const whitelist = new Map(); // Comment bucket must always be ready to be used. - whitelist['#'] = []; + whitelist.set('#', []); // New set of directives, scrap cached data. directiveToRegexpMap.clear(); - while ( !lineIter.eot() ) { - line = lineIter.next().trim(); + for ( let line of lines ) { + line = line.trim(); // https://github.com/gorhill/uBlock/issues/171 // Skip empty lines - if ( line === '' ) { - continue; - } + if ( line === '' ) { 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 ) { - whitelist[key] = []; + let bucket = whitelist.get(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) { - this.tabId = tabId; - this.scriptlet = scriptlet; - this.callback = callback; - this.timer = vAPI.setTimeout(this.service.bind(this), 1000); - }; - - Entry.prototype.service = function(response) { - if ( this.timer !== null ) { - clearTimeout(this.timer); - this.timer = null; + const Entry = class { + constructor(tabId, scriptlet, callback) { + this.tabId = tabId; + this.scriptlet = scriptlet; + this.callback = callback; + this.timer = vAPI.setTimeout(this.service.bind(this), 1000); + } + service(response) { + if ( this.timer !== null ) { + clearTimeout(this.timer); + 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) { - var key = makeKey(tabId, scriptlet); - var entry = pendingEntries.get(key); + const report = function(tabId, scriptlet, response) { + const key = makeKey(tabId, scriptlet); + 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), - entry = pendingEntries.get(key); + const key = makeKey(tabId, scriptlet); + 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