Issue 5332 - Support Unicode characters in path

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 91 matching lines @@
  *                   case, a hostname string (or undefined) and a bool
  *                   indicating if the source only contains a hostname or not:
  *                     {regexp: "...",
  *                      canSafelyMatchAsLowercase: true/false,
  *                      hostname: "...",
  *                      justHostname: true/false}
  */
 function parseFilterRegexpSource(text)
 {
   let regexp = [];
-  let lastIndex = text.length - 1;
+  let characters = Array.from(text);

[Manish Jethani, 2017/06/19 14:41:06]

We just need to convert the string into an array to get each of the Unicode
characters in it.

Array.from is the only option that will work with JS Hydra and Safari 9. We
cannot use the spread operator or let..of.

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

Maybe add a comment about this?

[Manish Jethani, 2017/07/08 06:02:19]

Done.

+  let lastIndex = characters.length - 1;
   let hostname;
   let hostnameStart = null;
   let hostnameFinished = false;
   let justHostname = false;
   let canSafelyMatchAsLowercase = false;
 
-  for (let i = 0; i < text.length; i++)
+  for (let i = 0; i < characters.length; i++)
   {
-    let c = text[i];
+    let c = characters[i];

[Manish Jethani, 2017/06/19 14:41:06]

Note that c.length is 2 if the character is a surrogate pair.

Previously there was a bug because we did not read Unicode characters but rather
16-bit characters.

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

Acknowledged.

 
     if (hostnameFinished)
       justHostname = false;
 
     // If we're currently inside the hostname we have to be careful not to
     // escape any characters until after we have converted it to punycode.
     if (hostnameStart != null && !hostnameFinished)
     {
       let endingChar = (c == "*" || c == "^" ||
                         c == "?" || c == "/" || c == "|");
       if (!endingChar && i != lastIndex)
         continue;
 
       hostname = punycode.toASCII(
-        text.substring(hostnameStart, endingChar ? i : i + 1)
+        characters.slice(hostnameStart, endingChar ? i : i + 1).join("")
       );
       hostnameFinished = justHostname = true;
       regexp.push(escapeRegExp(hostname));
       if (!endingChar)
         break;
     }
 
     switch (c)
     {
       case "*":
-        if (regexp.length > 0 && i < lastIndex && text[i + 1] != "*")
+        if (regexp.length > 0 && i < lastIndex && characters[i + 1] != "*")
           regexp.push(".*");
         break;
       case "^":
         if (i < lastIndex)
           regexp.push(".");
         break;
       case "|":
         if (i == 0)
         {
           regexp.push("^");
           break;
         }
         if (i == lastIndex)
         {
           regexp.push("$");
           break;
         }
-        if (i == 1 && text[0] == "|")
+        if (i == 1 && characters[0] == "|")
         {
           hostnameStart = i + 1;
           canSafelyMatchAsLowercase = true;
           regexp.push("https?://([^/]+\\.)?");
           break;
         }
         regexp.push("\\|");
         break;
       case "/":
         if (!hostnameFinished &&
-            text.charAt(i-2) == ":" && text.charAt(i-1) == "/")
+            characters[i - 2] == ":" && characters[i - 1] == "/")
         {
           hostnameStart = i + 1;
           canSafelyMatchAsLowercase = true;
         }
         regexp.push("/");
         break;
       case ".": case "+": case "$": case "?":
       case "{": case "}": case "(": case ")":
       case "[": case "]": case "\\":
         regexp.push("\\", c);
         break;
       default:
         if (hostnameFinished && (c >= "a" && c <= "z" ||
                                  c >= "A" && c <= "Z"))
           canSafelyMatchAsLowercase = false;
-        regexp.push(c);
+        regexp.push(c == "%" ? c : encodeURI(c));

[Manish Jethani, 2017/06/19 14:41:06]

Do not encode the percent sign because that means this is already an encoded
part of the URL. Browsers do this too.

I've added a unit test to test this case.

     }
   }
 
   return {
     regexp: regexp.join(""),
     canSafelyMatchAsLowercase: canSafelyMatchAsLowercase,
     hostname: hostname,
     justHostname: justHostname
   };
 }
@@ skipping 336 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));
 };