Issue 6733 - Allow empty values in filter options

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 79 matching lines @@
 Filter.elemhideRegExp = /^([^/*|@"!]*?)#([@?])?#(.+)$/;
 /**
  * Regular expression that RegExp filters specified as RegExps should match
  * @type {RegExp}
  */
 Filter.regexpRegExp = /^(@@)?\/.*\/(?:\$~?[\w-]+(?:=[^,\s]+)?(?:,~?[\w-]+(?:=[^,\s]+)?)*)?$/;
 /**
  * Regular expression that options on a RegExp filter should match
  * @type {RegExp}
  */
-Filter.optionsRegExp = /\$(~?[\w-]+(?:=[^,]+)?(?:,~?[\w-]+(?:=[^,]+)?)*)$/;
+Filter.optionsRegExp = /\$(~?[\w-]+(?:=[^,]*)?(?:,~?[\w-]+(?:=[^,]*)?)*)$/;
 /**
  * Regular expression that matches an invalid Content Security Policy
  * @type {RegExp}
  */
 Filter.invalidCSPRegExp = /(;|^) ?(base-uri|referrer|report-to|report-uri|upgrade-insecure-requests)\b/i;
 
 /**
  * Creates a filter of correct type from its text representation - does the
  * basic parsing and calls the right constructor then.
  *
@@ skipping 683 matching lines @@
             break;
           case "collapse":
             collapse = !inverse;
             break;
           case "sitekey":
             if (!value)
               return new InvalidFilter(origText, "filter_unknown_option");
             sitekeys = value.toUpperCase();
             break;
           case "rewrite":
-            if (!value)
+            if (value == null)

[Manish Jethani, 2018/06/25 13:13:44]

Blank value is allowed for $rewrite here while still an error for $domain,
$sitekeys, and $csp.

[kzar, 2018/07/12 10:36:13]

So by checking for `value == null` instead of `!value` we'd allow "...$rewrite="
but not "rewrite$", or do I misunderstand? Either way, why don't you check for
`!value` here?

[Manish Jethani, 2018/07/12 10:52:55]

Yes, we're allowing $rewrite= but not $rewrite, because this is not a "flag"
like $match-case, it needs a value but it can take an empty value.

[kzar, 2018/07/12 11:04:20]

Ah right I see, we anticipate the confusion about adding the "=" for the users
who do want an empty value is less than the confusion of users who don't
understand the option at all and attempt to use it without a value.

[Manish Jethani, 2018/07/12 11:11:12]

Yup.

               return new InvalidFilter(origText, "filter_unknown_option");
             rewrite = value;
             break;
           default:
             return new InvalidFilter(origText, "filter_unknown_option");
         }
       }
     }
   }
 
@@ skipping 285 matching lines @@
  */
 function ElemHideEmulationFilter(text, domains, selector)
 {
   ElemHideBase.call(this, text, domains, selector);
 }
 exports.ElemHideEmulationFilter = ElemHideEmulationFilter;
 
 ElemHideEmulationFilter.prototype = extend(ElemHideBase, {
   type: "elemhideemulation"
 });