Issue 6592 - Implement $rewrite filter option

lib/filterClasses.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 170 matching lines @@
   text = text.replace(/[^\S ]+/g, "");
 
   // Don't remove spaces inside comments
   if (/^ *!/.test(text))
     return text.trim();
 
   // Special treatment for element hiding filters, right side is allowed to
   // contain spaces
   if (Filter.elemhideRegExp.test(text))
   {
-    let [, domain, separator, selector] = /^(.*?)(#[@?]?#?)(.*)$/.exec(text);
-    return domain.replace(/ +/g, "") + separator + selector.trim();
+    let [, domains, separator, selector] = /^(.*?)(#[@?]?#?)(.*)$/.exec(text);
+    return domains.replace(/ +/g, "") + separator + selector.trim();
   }
 
   // For most regexp filters we strip all spaces, but $csp filter options
   // are allowed to contain single (non trailing) spaces.
   let strippedText = text.replace(/ +/g, "");
   if (!strippedText.includes("$") || !/\bcsp=/i.test(strippedText))
     return strippedText;
 
   let optionsMatch = Filter.optionsRegExp.exec(strippedText);
   if (!optionsMatch)
@@ skipping 164 matching lines @@
     {
       let oldValue = this._lastHit;
       this._lastHit = value;
       FilterNotifier.triggerListeners("filter.lastHit", this, value, oldValue);
     }
     return this._lastHit;
   },
 
   /**
    * String that the domains property should be generated from
-   * @type {string}
+   * @type {?string}
    */
   domainSource: null,
 
   /**
    * Separator character used in domainSource property, must be
    * overridden by subclasses
    * @type {string}
    */
   domainSeparator: null,
 
@@ skipping 76 matching lines @@
 
       this.domainSource = null;
     }
 
     Object.defineProperty(this, "domains", {value: domains, enumerable: true});
     return this.domains;
   },
 
   /**
    * Array containing public keys of websites that this filter should apply to
-   * @type {string[]}
+   * @type {?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} [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(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))
@@ skipping 188 matching lines @@
   contentType: 0x7FFFFFFF,
   /**
    * Defines whether the filter should distinguish between lower and
    * upper case letters
    * @type {boolean}
    */
   matchCase: false,
   /**
    * Defines whether the filter should apply to third-party or
    * first-party content only. Can be null (apply to all content).
-   * @type {boolean}
+   * @type {?boolean}
    */
   thirdParty: null,
 
   /**
    * String that the sitekey property should be generated from
-   * @type {string}
+   * @type {?string}
    */
   sitekeySource: null,
 
   /**
-   * Array containing public keys of websites that this filter should apply to
-   * @type {string[]}
+   * @see ActiveFilter.sitekeys
    */
   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 {number} typeMask bitmask of content / request types to match
-   * @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
+   * @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(location, typeMask, docDomain, thirdParty, sitekey)
   {
     if (this.contentType & typeMask &&
         (this.thirdParty == null || this.thirdParty == thirdParty) &&
         this.isActiveOnDomain(docDomain, sitekey) && this.regexp.test(location))
     {
       return true;
     }
@@ skipping 98 matching lines @@
   }
   catch (e)
   {
     return new InvalidFilter(origText, "filter_invalid_regexp");
   }
 };
 
 /**
  * Maps type strings like "SCRIPT" or "OBJECT" to bit masks
  */
 RegExpFilter.typeMap = {

[Manish Jethani, 2018/04/30 20:10:55]

We should add REWRITE here. Then we should exclude it below from contentType,
similar to CSP.

[hub, 2018/05/01 18:32:15]

I don't think it make sense to do so. This is just an option, not a type.

[Manish Jethani, 2018/05/02 15:31:09]

Maybe for the sake of consistency? CSP is also not a content type, just an
option.

[hub, 2018/05/02 22:14:32]

I still don't think this is necessary. We don't need to filter rewrite out when
matching (unlike CSP).

   OTHER: 1,
   SCRIPT: 2,
   IMAGE: 4,
   STYLESHEET: 8,
   OBJECT: 16,
   SUBDOCUMENT: 32,
   DOCUMENT: 64,
   WEBSOCKET: 128,
   WEBRTC: 256,
   CSP: 512,
@@ skipping 19 matching lines @@
                                         RegExpFilter.typeMap.DOCUMENT |
                                         RegExpFilter.typeMap.ELEMHIDE |
                                         RegExpFilter.typeMap.POPUP |
                                         RegExpFilter.typeMap.GENERICHIDE |
                                         RegExpFilter.typeMap.GENERICBLOCK);
 
 /**
  * Class for blocking filters
  * @param {string} text see Filter()
  * @param {string} regexpSource see RegExpFilter()
- * @param {number} contentType see RegExpFilter()
- * @param {boolean} matchCase see RegExpFilter()
- * @param {string} domains see RegExpFilter()
- * @param {boolean} thirdParty see RegExpFilter()
- * @param {string} sitekeys see RegExpFilter()
- * @param {boolean} collapse
+ * @param {number} [contentType] see RegExpFilter()
+ * @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
  * @param {string} [csp]
  *   Content Security Policy to inject when the filter matches
- * @param {string} [rewrite]
- *   The rewrite expression
+ * @param {?string} [rewrite]
+ *   The (optional) rule specifying how to rewrite the URL. See
+ *   BlockingFilter.prototype.rewrite.
  * @constructor
  * @augments RegExpFilter
  */
 function BlockingFilter(text, regexpSource, contentType, matchCase, domains,
                         thirdParty, sitekeys, collapse, csp, rewrite)
 {
   RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains,
                     thirdParty, sitekeys);
 
   this.collapse = collapse;
   this.csp = csp;
   this.rewrite = rewrite;
 }
 exports.BlockingFilter = BlockingFilter;
 
 BlockingFilter.prototype = extend(RegExpFilter, {
   type: "blocking",
 
   /**
    * Defines whether the filter should collapse blocked content.
    * Can be null (use the global preference).
-   * @type {boolean}
+   * @type {?boolean}
    */
   collapse: null,
 
   /**
    * Content Security Policy to inject for matching requests.
-   * @type {string}
+   * @type {?string}
    */
   csp: null,
 
   /**
-   * The rewrite expression
-   * @type {string}
+   * The rule specifying how to rewrite the URL.
+   * The syntax is similar to the one of String.prototype.replace().
+   * @type {?string}
    */
   rewrite: null,
 
   /**
-   * Perform the URL rewrite
-   * @param {string} urlString the string URL to rewrite
-   * @returns {string?} the rewritten URL, or null if it doesn't match.
-   */
-  doRewrite(urlString)

[Manish Jethani, 2018/05/02 17:52:23]

For what it's worth I would rather call this rewriteUrl rather than doRewrite,
it's a matter of preference though. 

[hub, 2018/05/02 22:14:32]

Done.

-  {
-    let matches = this.regexp.exec(urlString);
-    if (matches)
-      return this.rewrite.replace("$1", matches[0]);

[hub, 2018/04/27 00:48:22]

This is simplistic though. It does work, but the idea would be that $1, $2, etc
are the matches from the RegExp. But I couldn't get a filter with a RegExp to
work properly, which reduce versatility of the filter.

[Manish Jethani, 2018/04/30 20:07:11]

What's wrong with the following?

  doRewrite(urlString)
  {
    return urlString.replace(this.regexp, this.rewrite);
  }

So a filter like
/\b(social\.y5zone\.sg\/ads\/js)\/element\.write\.js\b/$rewrite=$1/foo.js will
rewrite http://social.y5zone.sg/ads/js/element.write.js?ts=5778 as
http://social.y5zone.sg/ads/js/foo.js?ts=5778

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

Also:

  doRewrite(urlString)
  {
    let rewritten = urlString.replace(this.regexp, this.rewrite);

    if (rewritten != urlString)
    {
      try 
      {
        // Disallow rewriting to a different origin.
        if (new URL(rewritten).origin != new URL(urlString).origin)
          rewritten = urlString;
      }  
      catch (error)
      {                                                    
      }   
    }     
    
    return rewritten;
  }

[hub, 2018/05/01 18:32:15]

I thought about this initially but then couldn't make it work as it was always
adding what wasn't matching. But I see now how the regexp should be written.

Fixed.
On error we should return the original urlString IMHO (ie perform no rewrite).

But then I wasn't sure whether we should enforce this restriction or trust the
filters.... What if the user really want to rewrite to a different location with
a custom filter?

[Manish Jethani, 2018/05/02 15:31:09]

Agreed.
Maybe we should allow it only for custom filters if we can do it easily (maybe
we should file a separate issue for that). In general I think it's a good idea
to have this restriction. I'm not so concerned about "malicious" filters, more
about a filter accidentally leaking the user's private data to a different
origin. Filter authors shouldn't have to worry about this possibility.

[hub, 2018/05/02 22:14:32]

Done.

-
-    return null;
+   * Rewrites an URL.
+   * @param {string} url the URL to rewrite
+   * @return {string} the rewritten URL, or the original in case of failure
+   */
+  rewriteUrl(url)
+  {
+    try
+    {
+      let rewrittenUrl = new URL(url.replace(this.regexp, this.rewrite), url);

[kzar, 2018/05/17 12:56:06]

I forgot to mention before, I thought how you did this part was clever. Just
reusing the existing generated regexp.

[hub, 2018/05/17 12:58:39]

thanks!

+      if (rewrittenUrl.origin == new URL(url).origin)
+        return rewrittenUrl.href;
+    }
+    catch (e)
+    {
+    }
+
+    return url;
   }
 });
 
 /**
  * Class for whitelist filters
  * @param {string} text see Filter()
  * @param {string} regexpSource see RegExpFilter()
- * @param {number} contentType see RegExpFilter()
- * @param {boolean} matchCase see RegExpFilter()
- * @param {string} domains see RegExpFilter()
- * @param {boolean} thirdParty see RegExpFilter()
- * @param {string} sitekeys see RegExpFilter()
+ * @param {number} [contentType] see RegExpFilter()
+ * @param {boolean} [matchCase] see RegExpFilter()
+ * @param {string} [domains] see RegExpFilter()
+ * @param {boolean} [thirdParty] see RegExpFilter()
+ * @param {string} [sitekeys] see RegExpFilter()
  * @constructor
  * @augments RegExpFilter
  */
 function WhitelistFilter(text, regexpSource, contentType, matchCase, domains,
                          thirdParty, sitekeys)
 {
   RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains,
                     thirdParty, sitekeys);
 }
 exports.WhitelistFilter = WhitelistFilter;
@@ skipping 11 matching lines @@
  *                            hidden
  * @constructor
  * @augments ActiveFilter
  */
 function ElemHideBase(text, domains, selector)
 {
   ActiveFilter.call(this, text, domains || null);
 
   if (domains)
   {
-    this.selectorDomain = domains.replace(/,~[^,]+/g, "")
-                                 .replace(/^~[^,]+,?/, "").toLowerCase();
+    this.selectorDomains = domains.replace(/,~[^,]+/g, "")
+                                  .replace(/^~[^,]+,?/, "").toLowerCase();
   }
 
   // Braces are being escaped to prevent CSS rule injection.
   this.selector = selector.replace("{", "\\7B ").replace("}", "\\7D ");
 }
 exports.ElemHideBase = ElemHideBase;
 
 ElemHideBase.prototype = extend(ActiveFilter, {
   /**
    * @see ActiveFilter.domainSeparator
    */
   domainSeparator: ",",
 
   /**
    * @see ActiveFilter.ignoreTrailingDot
    */
   ignoreTrailingDot: false,
 
   /**
-   * Host name or domain the filter should be restricted to (can be null for
+   * Host names or domains the filter should be restricted to (can be null for
    * no restriction)
-   * @type {string}
-   */
-  selectorDomain: null,
+   * @type {?string}
+   */
+  selectorDomains: null,
   /**
    * CSS selector for the HTML elements that should be hidden
    * @type {string}
    */
   selector: null
 });
 
 /**
  * Creates an element hiding filter from a pre-parsed text representation
  *
  * @param {string} text         same as in Filter()
- * @param {string?} domain
- *   domain part of the text representation
- * @param {string?} type
+ * @param {string} [domains]
+ *   domains part of the text representation
+ * @param {string} [type]
 *    rule type, either empty or @ (exception) or ? (emulation rule)
  * @param {string} selector     raw CSS selector
  * @return {ElemHideFilter|ElemHideException|
  *          ElemHideEmulationFilter|InvalidFilter}
  */
-ElemHideBase.fromText = function(text, domain, type, selector)
+ElemHideBase.fromText = function(text, domains, type, selector)
 {
   // We don't allow ElemHide filters which have any empty domains.
   // Note: The ElemHide.prototype.domainSeparator is duplicated here, if that
   // changes this must be changed too.
-  if (domain && /(^|,)~?(,|$)/.test(domain))
+  if (domains && /(^|,)~?(,|$)/.test(domains))
     return new InvalidFilter(text, "filter_invalid_domain");
 
   if (type == "@")
-    return new ElemHideException(text, domain, selector);
+    return new ElemHideException(text, domains, selector);
 
   if (type == "?")
   {
     // Element hiding emulation filters are inefficient so we need to make sure
     // that they're only applied if they specify active domains
-    if (!/,[^~][^,.]*\.[^,]/.test("," + domain))
+    if (!/,[^~][^,.]*\.[^,]/.test("," + domains))
       return new InvalidFilter(text, "filter_elemhideemulation_nodomain");
 
-    return new ElemHideEmulationFilter(text, domain, selector);
-  }
-
-  return new ElemHideFilter(text, domain, selector);
+    return new ElemHideEmulationFilter(text, domains, selector);
+  }
+
+  return new ElemHideFilter(text, domains, selector);
 };
 
 /**
  * Class for element hiding filters
  * @param {string} text see Filter()
- * @param {string} domains  see ElemHideBase()
+ * @param {string} [domains]  see ElemHideBase()
  * @param {string} selector see ElemHideBase()
  * @constructor
  * @augments ElemHideBase
  */
 function ElemHideFilter(text, domains, selector)
 {
   ElemHideBase.call(this, text, domains, selector);
 }
 exports.ElemHideFilter = ElemHideFilter;
 
 ElemHideFilter.prototype = extend(ElemHideBase, {
   type: "elemhide"
 });
 
 /**
  * Class for element hiding exceptions
  * @param {string} text see Filter()
- * @param {string} domains  see ElemHideBase()
+ * @param {string} [domains]  see ElemHideBase()
  * @param {string} selector see ElemHideBase()
  * @constructor
  * @augments ElemHideBase
  */
 function ElemHideException(text, domains, selector)
 {
   ElemHideBase.call(this, text, domains, selector);
 }
 exports.ElemHideException = ElemHideException;
 
@@ skipping 11 matching lines @@
  */
 function ElemHideEmulationFilter(text, domains, selector)
 {
   ElemHideBase.call(this, text, domains, selector);
 }
 exports.ElemHideEmulationFilter = ElemHideEmulationFilter;
 
 ElemHideEmulationFilter.prototype = extend(ElemHideBase, {
   type: "elemhideemulation"
 });