Issue 6735 - Store domains in lower case

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 382 matching lines @@
    */
   domainSourceIsLowerCase: false,
 
   /**
    * Map containing domains that this filter should match on/not match
    * on or null if the filter should match on all domains
    * @type {?Map.<string,boolean>}
    */
   get domains()
   {
-    // Despite this property being cached, the getter is called
-    // several times on Safari, due to WebKit bug 132872
-    let prop = Object.getOwnPropertyDescriptor(this, "domains");
-    if (prop)
-      return prop.value;
-
     let domains = null;
 
     if (this.domainSource)
     {
       let source = this.domainSource;
       if (!this.domainSourceIsLowerCase)
       {
         // RegExpFilter already have lowercase domains
         source = source.toLowerCase();
       }
@@ skipping 220 matching lines @@
    * for delayed creation of the regexp property
    * @type {string}
    */
   regexpSource: null,
   /**
    * Regular expression to be used when testing against this filter
    * @type {RegExp}
    */
   get regexp()
   {
-    // Despite this property being cached, the getter is called
-    // several times on Safari, due to WebKit bug 132872
-    let prop = Object.getOwnPropertyDescriptor(this, "regexp");
-    if (prop)
-      return prop.value;
-
     let source = Filter.toRegExp(this.regexpSource);
     let regexp = new RegExp(source, this.matchCase ? "" : "i");
     Object.defineProperty(this, "regexp", {value: regexp});
     this.regexpSource = null;
     return regexp;
   },
   /**
    * Content types the filter applies to, combination of values from
    * RegExpFilter.typeMap
    * @type {number}
@@ skipping 16 matching lines @@
    * String that the sitekey property should be generated from
    * @type {?string}
    */
   sitekeySource: null,
 
   /**
    * @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}
@@ skipping 30 matching lines @@
 
 /**
  * Creates a RegExp filter from its text representation
  * @param {string} text   same as in Filter()
  * @return {Filter}
  */
 RegExpFilter.fromText = function(text)
 {
   let blocking = true;
   let origText = text;
-  if (text.indexOf("@@") == 0)
+  if (text[0] == "@" && text[1] == "@")
   {
     blocking = false;
     text = text.substr(2);
   }
 
   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);
+  let match = text.includes("$") ? Filter.optionsRegExp.exec(text) : null;
   if (match)
   {
     options = match[1].split(",");
     text = match.input.substr(0, match.index);
     for (let option of options)
     {
       let value = null;
       let separatorIndex = option.indexOf("=");
       if (separatorIndex >= 0)
       {
         value = option.substr(separatorIndex + 1);
         option = option.substr(0, separatorIndex);
       }
-      let type = (option[0] == "~" ? option.substr(1) : option)

[Manish Jethani, 2018/06/06 14:59:13]

This part may seem unrelated but the goal is to avoid uppercasing and then
lowercasing again.

-                 .replace(/-/, "_").toUpperCase();
-      option = option.toLowerCase();
-      if (type in RegExpFilter.typeMap)
+
+      let inverse = option[0] == "~";
+      if (inverse)
+        option = option.substr(1);
+
+      let type = RegExpFilter.typeMap[option.replace(/-/, "_").toUpperCase()];
+      if (type)
       {
-        if (option[0] == "~")
+        if (inverse)
         {
           if (contentType == null)
             ({contentType} = RegExpFilter.prototype);
-          contentType &= ~RegExpFilter.typeMap[type];
+          contentType &= ~type;
         }
         else
         {
-          if (contentType == null)
-            contentType = 0;
-          contentType |= RegExpFilter.typeMap[type];
-
-          if (type == "CSP" && value)
+          contentType |= type;
+
+          if (type == RegExpFilter.typeMap.CSP && value)

[Manish Jethani, 2018/06/08 14:21:14]

By the way in the case of csp we quietly ignore the option if it has no value,
so we are not being consistent. Anyway we should address these things
separately, not as part of this patch.

[kzar, 2018/06/15 10:44:03]

Yea, agreed. By the way Rosie is reimplementing this stuff in Python ATM, so if
you change the behaviour ping her in IRC.

             csp = value;
         }
       }
-      else if (option == "match-case")
-        matchCase = true;
-      else if (option == "~match-case")
-        matchCase = false;
-      else if (option == "domain" && value)
-        domains = value.toLowerCase();
-      else if (option == "third-party")
-        thirdParty = true;
-      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");
+      {
+        switch (option.toLowerCase())
+        {
+          case "match-case":
+            matchCase = !inverse;
+            break;
+          case "domain":
+            if (!value)
+              return new InvalidFilter(origText, "filter_unknown_option");
+            domains = value.toLowerCase();
+            break;
+          case "third-party":
+            thirdParty = !inverse;
+            break;
+          case "collapse":
+            collapse = !inverse;
+            break;
+          case "sitekey":
+            if (!value)

[Manish Jethani, 2018/06/08 14:19:42]

There's a bit of code repetition here but I'm OK with it.

[kzar, 2018/06/15 10:44:03]

Yea, I think it's fine.

+              return new InvalidFilter(origText, "filter_unknown_option");
+            sitekeys = value.toUpperCase();
+            break;
+          case "rewrite":
+            if (!value)

[Manish Jethani, 2018/06/08 14:19:42]

Note that this will get updated for the rewrite option in the other patch.

+              return new InvalidFilter(origText, "filter_unknown_option");
+            rewrite = value;
+            break;
+          default:
+            return new InvalidFilter(origText, "filter_unknown_option");
+        }
+      }
     }
   }
 
   // For security reasons, never match $rewrite filters
   // against requests that might load any code to be executed.
   if (rewrite != null)
   {
     if (contentType == null)
       ({contentType} = RegExpFilter.prototype);
     contentType &= ~(RegExpFilter.typeMap.SCRIPT |
@@ skipping 278 matching lines @@
  */
 function ElemHideEmulationFilter(text, domains, selector)
 {
   ElemHideBase.call(this, text, domains, selector);
 }
 exports.ElemHideEmulationFilter = ElemHideEmulationFilter;
 
 ElemHideEmulationFilter.prototype = extend(ElemHideBase, {
   type: "elemhideemulation"
 });