Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/filterClasses.js

Issue 4559243822759936: Issue 431/432 - Remove special handling for the $sitekey option (Closed)
Patch Set: Made changes for WebKit bug 132872 Created Sept. 1, 2014, 3:27 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/contentPolicy.js ('k') | lib/matcher.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -215,7 +215,7 @@
/**
* Abstract base class for filters that can get hits
* @param {String} text see Filter()
- * @param {String} domains (optional) Domains that the filter is restricted to separated by domainSeparator e.g. "foo.com|bar.com|~baz.com"
+ * @param {String} [domains] Domains that the filter is restricted to separated by domainSeparator e.g. "foo.com|bar.com|~baz.com"
* @constructor
* @augments Filter
*/
@@ -387,10 +387,24 @@
},
/**
+ * Array containing public keys of websites that this filter should apply to
+ * @type Array of String
+ */
+ sitekeys: null,
+
+ /**
* Checks whether this filter is active on a domain.
+ * @param {String} docDomain domain name of the document that loads the URL
+ * @param {String} [sitekey] public key provided by the document
+ * @return {Boolean} true in case of the filter being active
*/
- isActiveOnDomain: function(/**String*/ docDomain) /**Boolean*/
+ isActiveOnDomain: function(docDomain, sitekey)
{
+ // Sitekeys are case-sensitive so we shouldn't convert them to upper-case to avoid false
+ // positives here. Instead we need to change the way filter options are parsed.
+ if (this.sitekeys && (!sitekey || this.sitekeys.indexOf(sitekey.toUpperCase()) < 0))
+ return false;
+
// If no domains are set the rule matches everywhere
if (!this.domains)
return true;
@@ -457,16 +471,17 @@
* Abstract base class for RegExp-based filters
* @param {String} text see Filter()
* @param {String} regexpSource filter part that the regular expression should be build from
- * @param {Number} contentType (optional) Content types the filter applies to, combination of values from RegExpFilter.typeMap
- * @param {Boolean} matchCase (optional) Defines whether the filter should distinguish between lower and upper case letters
- * @param {String} domains (optional) Domains that the filter is restricted to, e.g. "foo.com|bar.com|~baz.com"
- * @param {Boolean} thirdParty (optional) Defines whether the filter should apply to third-party or first-party content only
+ * @param {Number} [contentType] Content types the filter applies to, combination of values from RegExpFilter.typeMap
+ * @param {Boolean} [matchCase] Defines whether the filter should distinguish between lower and upper case letters
+ * @param {String} [domains] Domains that the filter is restricted to, e.g. "foo.com|bar.com|~baz.com"
+ * @param {Boolean} [thirdParty] Defines whether the filter should apply to third-party or first-party content only
+ * @param {String} [sitekeys] Public keys of websites that this filter should apply to
* @constructor
* @augments ActiveFilter
*/
-function RegExpFilter(text, regexpSource, contentType, matchCase, domains, thirdParty)
+function RegExpFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys)
{
- ActiveFilter.call(this, text, domains);
+ ActiveFilter.call(this, text, domains, sitekeys);
if (contentType != null)
this.contentType = contentType;
@@ -474,6 +489,8 @@
this.matchCase = matchCase;
if (thirdParty != null)
this.thirdParty = thirdParty;
+ if (sitekeys != null)
+ this.sitekeySource = sitekeys;
if (regexpSource.length >= 2 && regexpSource[0] == "/" && regexpSource[regexpSource.length - 1] == "/")
{
@@ -561,19 +578,50 @@
thirdParty: null,
/**
+ * String that the sitekey property should be generated from
+ * @type String
+ */
+ sitekeySource: null,
+
+ /**
+ * Array containing public keys of websites that this filter should apply to
+ * @type Array of String
+ */
+ get sitekeys()
+ {
+ // Despite this property being cached, the getter is called
+ // several times on Safari, due to WebKit bug 132872
+ let prop = Object.getOwnPropertyDescriptor(this, "sitekeys");
+ if (prop)
+ return prop.value;
+
+ let sitekeys = null;
+
+ if (this.sitekeySource)
+ {
+ sitekeys = this.sitekeySource.split("|");
+ this.sitekeySource = null;
+ }
+
+ Object.defineProperty(this, "sitekeys", {value: sitekeys, enumerable: true});
+ return this.sitekeys;
+ },
+
+ /**
* Tests whether the URL matches this filter
* @param {String} location URL to be tested
* @param {String} contentType content type identifier of the URL
* @param {String} docDomain domain name of the document that loads the URL
* @param {Boolean} thirdParty should be true if the URL is a third-party request
+ * @param {String} sitekey public key provided by the document
* @return {Boolean} true in case of a match
*/
- matches: function(location, contentType, docDomain, thirdParty)
+ matches: function(location, contentType, docDomain, thirdParty, sitekey)
{
if (this.regexp.test(location) &&
(RegExpFilter.typeMap[contentType] & this.contentType) != 0 &&
(this.thirdParty == null || this.thirdParty == thirdParty) &&
- this.isActiveOnDomain(docDomain))
+ this.isActiveOnDomain(docDomain, sitekey))
{
return true;
}
@@ -605,7 +653,7 @@
let contentType = null;
let matchCase = null;
let domains = null;
- let siteKeys = null;
+ let sitekeys = null;
let thirdParty = null;
let collapse = null;
let options;
@@ -651,7 +699,7 @@
else if (option == "~COLLAPSE")
collapse = false;
else if (option == "SITEKEY" && typeof value != "undefined")
- siteKeys = value.split(/\|/);
+ sitekeys = value;
else
return new InvalidFilter(origText, "Unknown option " + option.toLowerCase());
}
@@ -665,15 +713,13 @@
contentType = RegExpFilter.prototype.contentType;
contentType &= ~RegExpFilter.typeMap.DOCUMENT;
}
- if (!blocking && siteKeys)
- contentType = RegExpFilter.typeMap.DOCUMENT;
try
{
if (blocking)
- return new BlockingFilter(origText, text, contentType, matchCase, domains, thirdParty, collapse);
+ return new BlockingFilter(origText, text, contentType, matchCase, domains, thirdParty, sitekeys, collapse);
else
- return new WhitelistFilter(origText, text, contentType, matchCase, domains, thirdParty, siteKeys);
+ return new WhitelistFilter(origText, text, contentType, matchCase, domains, thirdParty, sitekeys);
}
catch (e)
{
@@ -717,13 +763,14 @@
* @param {Boolean} matchCase see RegExpFilter()
* @param {String} domains see RegExpFilter()
* @param {Boolean} thirdParty see RegExpFilter()
+ * @param {String} sitekeys see RegExpFilter()
* @param {Boolean} collapse defines whether the filter should collapse blocked content, can be null
* @constructor
* @augments RegExpFilter
*/
-function BlockingFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, collapse)
+function BlockingFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys, collapse)
{
- RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty);
+ RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys);
this.collapse = collapse;
}
@@ -748,34 +795,25 @@
* @param {Boolean} matchCase see RegExpFilter()
* @param {String} domains see RegExpFilter()
* @param {Boolean} thirdParty see RegExpFilter()
- * @param {String[]} siteKeys public keys of websites that this filter should apply to
+ * @param {String} sitekeys see RegExpFilter()
* @constructor
* @augments RegExpFilter
*/
-function WhitelistFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, siteKeys)
+function WhitelistFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys)
{
- RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty);
-
- if (siteKeys != null)
- this.siteKeys = siteKeys;
+ RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys);
}
exports.WhitelistFilter = WhitelistFilter;
WhitelistFilter.prototype =
{
- __proto__: RegExpFilter.prototype,
-
- /**
- * List of public keys of websites that this filter should apply to
- * @type String[]
- */
- siteKeys: null
+ __proto__: RegExpFilter.prototype
}
/**
* Base class for element hiding filters
* @param {String} text see Filter()
- * @param {String} domains (optional) Host names or domains the filter should be restricted to
+ * @param {String} [domains] Host names or domains the filter should be restricted to
* @param {String} selector CSS selector for the HTML elements that should be hidden
* @constructor
* @augments ActiveFilter
« no previous file with comments | « lib/contentPolicy.js ('k') | lib/matcher.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld