Changes between Version 20 and Version 21 of Ticket #6994


Ignore:
Timestamp:
10/30/2018 10:25:25 PM (12 months ago)
Author:
mjethani
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #6994 – Description

    v20 v21  
    22About half of all blocking/whitelist filters have (1) no content type, (2) no domain/sitekey, and (3) no third-party flag. As long as the request's location matches the pattern in the filter, it is a match. For these filters, there should be a "shortcut" version of the `RegExpFilter.prototype.matches` method that checks only the location. When such filters are added to the matcher, it should keep them separately so they can be processed separately (with the shortcut version) for each call to the `_checkEntryMatch` method. 
    33 
     4=== What to change === 
     5Add the following method to `RegExpFilter`: 
    46 
     7{{{ 
     8  function isLocationOnly() 
     9  { 
     10    return this.contentType == RegExpFilter.prototype.contentType && 
     11           this.thirdParty == null && 
     12           !this.domains && !this.sitekeys; 
     13  } 
     14}}} 
    515 
    6 === What to change === 
    7 Add a new map for location only filters (`fastFilterByKeyword`) to the matcher class in `lib/matcher.js`  
     16If the method returns `true` for a `RegExpFilter` object, it means the filter has only a location (URL pattern) and none of the other attributes used for matching. 
    817 
     18Then in the `add` and `remove` methods of `Matcher`, keep location-only filters in a separate map. In the `matchesAny` method (specifically in `checkEntryMatch`), check location-only filters first, and do this using the new `matchesLocation` method rather than the `matches` method. If `typeMask & RegExpFilter.prototype.contentType` is zero, skip the location-only filters (since by definition they don't match the type mask). 
    919 
    10 Change `_checkEntryMatch` in lib/matcher.js to initially loop through location only filters checking for matches using filter.regexp.test(location) or if a new location only version of `RegExpFilter.prototype.matches` is used it can call filter.matchesLocation. Additionally, in _checkEntryMatch, if  
    11 `typeMask & RegExpFilter.prototype.contentType == 0`, these filters should be skipped entirely. 
     20Since location-only filters would now be maintained in a separate map, this would also also require changes in `Matcher`'s `findKeyword` method as well as in some tests. 
    1221 
    13 Add a isLocationOnly() property to the RegExpFilter class. Also, filters must not be stored twice so when adding to the fastFilterByKeyword map we should skip adding to the `matcher.filterByKeyword` map. 
     22=== Hints for testers === 
    1423 
    15 Using a new map in the matcher class will require the following changes: 
     24This is an internal optimization in filter matching and the unit tests already cover all the cases. Nevertheless, it might make sense to test it again via the extension. 
    1625 
    17  - `Matcher.findKeyword` will have to be updated to handle fast-filters (location only) 
    18  - `Matcher.add()` and `Matcher.remove(filter)` will have to determine which map to use based on the new `isLocationOnly` method from `RegExpFilter` 
    19  
    20 === Notes === 
    21 While the goal is to not change anything about the tests, changing the findkeyword method as well as having two maps will likely require changes to keyword tests and anywhere filterByKeyword is used. 
     26Make sure that blocking and whitelist filters are getting applied once added and no longer getting applied once removed. For example, add the filter `/foo^` and see that it blocks `https://example.com/foo`. Add the filter `@@||example.com^` and make sure that the filter `/foo^` no longer blocks `https://example.com/foo`. Try changing the whitelist filter to `@@||example.com^$~third-party` (first-party only), load a page from `localhost` that makes a request to `https://example.com/foo` (third-party request), and see that it is now blocked again by the filter `/foo^`. Then change the blocking filter `/foo^` to `/foo^$image` and load the URL `https://example.com/foo` as a ''script'' (use `<script src="https://example.com/foo"></script>`) and see that the URL is no longer blocked. Load the same URL as an image (use `<img src="https://example.com/foo">`) and see that it is blocked now. Remove both the filters and see that they are no longer being applied. Also look out for any errors in the background page console.