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 356 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 138 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}

[Manish Jethani, 2018/05/11 13:59:29]

This should be {?string} now after rebasing.

[hub, 2018/05/11 15:57:33]

Done.

+   * The rule specifying how to rewrite the URL.
+   * The syntax is similar to the one of String.prototype.replace().
+   * @type {?string}
    */
   rewrite: null,
 
   /**
    * Rewrites an URL.
    * @param {string} url the URL to rewrite

[Sebastian Noack, 2018/05/08 17:09:30]

The type annotated here is incorrect. We expect an URL object, not a string.

[hub, 2018/05/08 17:14:49]

In the onBeforeRequest listener we pass details.url to rewriteUrl, which is a
string. And below we call String.prototype.replace() on it.

So we really expect a string.

[Sebastian Noack, 2018/05/08 17:25:37]

You are right. But didn't Manish suggest to pass in an URL object instead? (I
personally don't mind much either way.)

[Manish Jethani, 2018/05/08 18:52:31]

I was thinking that we already have a URL object on the calling side, we can
just pass it in and then do the rewriting on the href property. This way we
don't have to create a new one here. For consistency we could also return
rewrittenUrl rather than rewrittenUrl.href.

But the change looks good enough to me as it is, I don't mind if we leave it
like this. What do you think, Hubert?

LGTM

    * @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 34 matching lines @@
   domainSeparator: ",",
 
   /**
    * @see ActiveFilter.ignoreTrailingDot
    */
   ignoreTrailingDot: false,
 
   /**
    * Host names or domains the filter should be restricted to (can be null for
    * no restriction)
-   * @type {string}
+   * @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?} domains
+ * @param {string} [domains]
  *   domains part of the text representation
- * @param {string?} type
+ * @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, 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.
@@ skipping 12 matching lines @@
 
     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"
 });