Issue 6733 - Allow empty values in filter options

lib/filterClasses.js

Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -92,17 +92,17 @@
  * 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
@@ -796,17 +796,17 @@
             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");
         }
       }
     }