Issue 6592 - Implement $rewrite filter option

lib/filterClasses.js

Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -766,16 +766,17 @@
 
   let contentType = null;
   let matchCase = null;
   let domains = null;
   let sitekeys = null;
   let thirdParty = null;
   let collapse = null;
   let csp = null;
+  let rewrite = null;
   let options;
   let match = (text.indexOf("$") >= 0 ? Filter.optionsRegExp.exec(text) : null);
   if (match)
   {
     options = match[1].split(",");
     text = match.input.substr(0, match.index);
     for (let option of options)
     {
@@ -813,30 +814,32 @@
       else if (option == "~THIRD_PARTY")
         thirdParty = false;
       else if (option == "COLLAPSE")
         collapse = true;
       else if (option == "~COLLAPSE")
         collapse = false;
       else if (option == "SITEKEY" && value)
         sitekeys = value.toUpperCase();
+      else if (option == "REWRITE" && value)
+        rewrite = value;
       else
         return new InvalidFilter(origText, "filter_unknown_option");
     }
   }
 
   try
   {
     if (blocking)
     {
       if (csp && Filter.invalidCSPRegExp.test(csp))
         return new InvalidFilter(origText, "filter_invalid_csp");
 
       return new BlockingFilter(origText, text, contentType, matchCase, domains,
-                                thirdParty, sitekeys, collapse, csp);
+                                thirdParty, sitekeys, collapse, csp, rewrite);
     }
     return new WhitelistFilter(origText, text, contentType, matchCase, domains,
                                thirdParty, sitekeys);
   }
   catch (e)
   {
     return new InvalidFilter(origText, "filter_invalid_regexp");
   }
@@ -889,27 +892,30 @@
  * @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
  * @constructor
  * @augments RegExpFilter
  */
 function BlockingFilter(text, regexpSource, contentType, matchCase, domains,
-                        thirdParty, sitekeys, collapse, csp)
+                        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.
@@ -917,17 +923,37 @@
    * @type {boolean}
    */
   collapse: null,
 
   /**
    * Content Security Policy to inject for matching requests.
    * @type {string}
    */
-  csp: null
+  csp: null,
+
+  /**
+   * The rewrite expression
+   * @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;
+  }
 });
 
 /**
  * Class for whitelist filters
  * @param {string} text see Filter()
  * @param {string} regexpSource see RegExpFilter()
  * @param {number} contentType see RegExpFilter()
  * @param {boolean} matchCase see RegExpFilter()