Issue 7208 - Drop superfluous wildcards before processing pattern

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 672 matching lines @@
  *   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
  * @param {?string} [rewrite]
  *   The (optional) rule specifying how to rewrite the URL. See
- *   BlockingFilter.prototype.rewrite.
+ *   RegExpFilter.prototype.rewrite.
  * @constructor
  * @augments ActiveFilter
  */
 function RegExpFilter(text, regexpSource, contentType, matchCase, domains,

[Manish Jethani, 2019/01/10 22:52:22]

I don't like this, but unfortunately the superclass needs to know the value of
the rewrite option. It does already in the regexp getter.

                       thirdParty, sitekeys, rewrite)
 {
   ActiveFilter.call(this, text, domains);
 
   if (contentType != null)
     this.contentType = contentType;
   if (matchCase)
     this.matchCase = matchCase;
   if (thirdParty != null)
     this.thirdParty = thirdParty;
   if (sitekeys != null)
     this.sitekeySource = sitekeys;
+  if (rewrite != null)
+    this.rewrite = rewrite;
 
   if (regexpSource.length >= 2 &&
       regexpSource[0] == "/" &&
       regexpSource[regexpSource.length - 1] == "/")
   {
     // The filter is a regular expression - convert it immediately to
     // catch syntax errors
     let regexp = new RegExp(regexpSource.substr(1, regexpSource.length - 2),
                             this.matchCase ? "" : "i");
     Object.defineProperty(this, "regexp", {value: regexp});
   }
   else
   {
     // Patterns like /foo/bar/* exist so that they are not treated as regular
-    // expressions. We drop any superflous wildcards here so our optimizations
+    // expressions. We drop any superfluous wildcards here so our optimizations
     // can kick in.
-    if (rewrite == null)
+    if (this.rewrite == null)
       regexpSource = regexpSource.replace(/^\*+/, "").replace(/\*+$/, "");
 
     if (!this.matchCase && isLiteralPattern(regexpSource))
       regexpSource = regexpSource.toLowerCase();
 
     // No need to convert this filter to regular expression yet, do it on demand
     this.pattern = regexpSource;
   }
 }
 exports.RegExpFilter = RegExpFilter;
@@ skipping 71 matching lines @@
     {
       sitekeys = this.sitekeySource.split("|");
       this.sitekeySource = null;
     }
 
     Object.defineProperty(
       this, "sitekeys", {value: sitekeys, enumerable: true}
     );
     return this.sitekeys;
   },
+
+  /**
+   * The rule specifying how to rewrite the URL.
+   * The syntax is similar to the one of String.prototype.replace().
+   * @type {?string}
+   */
+  rewrite: null,
 
   /**
    * 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
    * @return {boolean} true in case of a match
@@ skipping 171 matching lines @@
   }
 
   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, rewrite);
+                                thirdParty, sitekeys, rewrite, collapse, csp);
     }
     return new WhitelistFilter(origText, text, contentType, matchCase, domains,
                                thirdParty, sitekeys);
   }
   catch (e)
   {
     return new InvalidFilter(origText, "filter_invalid_regexp");
   }
 };
 
@@ skipping 38 matching lines @@
 
 /**
  * Class for blocking filters
  * @param {string} text see {@link Filter Filter()}
  * @param {string} regexpSource see {@link RegExpFilter RegExpFilter()}
  * @param {number} [contentType] see {@link RegExpFilter RegExpFilter()}
  * @param {boolean} [matchCase] see {@link RegExpFilter RegExpFilter()}
  * @param {string} [domains] see {@link RegExpFilter RegExpFilter()}
  * @param {boolean} [thirdParty] see {@link RegExpFilter RegExpFilter()}
  * @param {string} [sitekeys] see {@link RegExpFilter RegExpFilter()}
+ * @param {?string} [rewrite]
+ *   The (optional) rule specifying how to rewrite the URL. See
+ *   RegExpFilter.prototype.rewrite.
  * @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 (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)
+                        thirdParty, sitekeys, rewrite, collapse, csp)
 {
   RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains,
                     thirdParty, sitekeys, rewrite);
 
   if (collapse != null)
     this.collapse = collapse;
 
   if (csp != null)
     this.csp = csp;
-
-  if (rewrite != null)
-    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}
    */
   collapse: null,
 
   /**
    * Content Security Policy to inject for matching requests.
    * @type {?string}
    */
   csp: null,
-
-  /**
-   * 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
    * @return {string} the rewritten URL, or the original in case of failure
    */
   rewriteUrl(url)
   {
     try
     {
@@ skipping 235 matching lines @@
 
   /**
    * Script that should be executed
    * @type {string}
    */
   get script()
   {
     return this.body;
   }
 });