Issue 5325 - Add support for separator characters

lib/abp2blocklist.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 130 matching lines @@
         break;
     }
 
     switch (c)
     {
       case "*":
         if (regexp.length > 0 && i < lastIndex && text[i + 1] != "*")
           regexp.push(".*");
         break;
       case "^":
-        if (i < lastIndex)
-          regexp.push(".");
+        let separator = "[^-_.%" + (justHostname ? "" : "A-Z") + "a-z0-9]";

[kzar, 2017/07/07 12:46:18]

I don't understand the logic with justHostname here, could you explain?

[kzar, 2017/07/07 12:46:18]

Like I mentioned in the issue shouldn't the period be escaped?

[Manish Jethani, 2017/07/08 06:29:10]

For filters like "example.co^" and "example.co^*^hello^, when the code
encounters the first "^", justHostname is true. This means we're still in the
hostname part, therefore there's no need to include the "A-Z", as hostnames are
always lower case. Safari actually Punycode-encodes the hostname part before
running the regular expression on it.

We could just leave the "A-Z" there, it wouldn't hurt, but it's cleaner this
way, especially because if the filter contains only the hostname part then the
following code will covert the entire regular expression to lower case thus
leaving us with "a-za-z".

[Manish Jethani, 2017/07/08 06:29:10]

I replied there as well. No, the period must not be escaped when it's inside
square brackets. Within square brackets it has its literal meaning.

[kzar, 2017/07/10 12:39:56]

Ah OK, thanks. Would you mind adding a comment to explain that?

[kzar, 2017/07/10 12:39:56]

Acknowledged.

[Manish Jethani, 2017/07/11 17:10:33]

Done.

I've also made the code a bit more verbose here so it's easier to understand
what "separator" means.

[kzar, 2017/07/12 09:16:31]

Thanks, while a little verbose like you mention I think it's a lot easier to
understand now.

+        if (i == 0)

[Manish Jethani, 2017/06/19 10:39:54]

Support separators at the beginning of the filter text.

This is in fact given as an example in the filters documentation, we need to
support this case.

[kzar, 2017/07/07 12:46:18]

Acknowledged.

+          regexp.push("^https?://(.*" + separator + ")?");
+        else if (i == lastIndex)
+          regexp.push("(" + separator + ".*)?$");
+        else
+          regexp.push(separator);
         break;
       case "|":
         if (i == 0)
         {
           regexp.push("^");
           break;
         }
         if (i == lastIndex)
         {
           regexp.push("$");
@@ skipping 376 matching lines @@
   {
     convertFilterAddRules(rules, filter, "block", true,
                           requestFilterExceptionDomains);
   }
 
   for (let filter of this.requestExceptions)
     convertFilterAddRules(rules, filter, "ignore-previous-rules", true);
 
   return rules.filter(rule => !hasNonASCI(rule));
 };