Issue 3710 - Unify hostname logic

lib/abp2blocklist.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 48 matching lines @@
   let excluded = [];
   let rules = [];
 
   parseDomains(filter.domains, included, excluded);
 
   if (excluded.length == 0 && !(filter.selector in elemhideSelectorExceptions))
     return {matchDomains: included.map(matchDomain), selector: filter.selector};
 }
 
 /**
- * Convert the given filter "regexpSource" string into a regular expression.
+ * Convert the given filter "regexpSource" string into a regular expression,
+ * handling the conversion of unicode inside hostnames to punycode.
  * (Also deciding if the regular expression can be safely converted to and
  *  matched as lower case or not.)
  *
  * @param   {string} text regexpSource property of a filter
  * @returns {object} An object containing a regular expression string and a bool
  *                   indicating if the filter can be safely matched as lower
- *                   case: {regexp: "...", caseSenstive: true/false}
+ *                   case: {regexp: "...", canSafelyMatchAsLowercase: true/false}
  */
 function toRegExp(text)
 {
   let result = [];
   let lastIndex = text.length - 1;
-  let hostnameStarted = false;
+  let hostnameStart = null;
   let hostnameFinished = false;
-  let caseSensitive = false;
+  let canSafelyMatchAsLowercase = false;
 
   for (let i = 0; i < text.length; i++)
   {
     let c = text[i];
 
+    // 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 == "/");

[Sebastian Noack, 2016/02/27 23:06:16]

I know we didn't handle it before, but what's if we have a | in the last
position. It implies the end of the URL, and therefore also end of the hostname.

[kzar, 2016/03/07 17:06:48]

So do we want to always consider "|" to end the hostname, or only when it also
happens to be the last character?

[Sebastian Noack, 2016/03/08 09:31:22]

I guess, for simplicity we can just assume that any "|" (except in the
beginning) implies the end of the URL. It cannot occur in a domain, and if it
occurs in the path/query it has to be escaped, anyway.

[kzar, 2016/03/08 12:36:01]

Yea, sounds good to me. Also we already know that we've passed the first
character here as hostnameStart isn't null which makes things even easier.

+      if (!endingChar && i != lastIndex)
+        continue;
+
+      let hostname = text.substring(hostnameStart, endingChar ? i : i + 1);
+      hostnameFinished = true;
+      result.push(escapeRegExp(punycode.toASCII(hostname)));
+      if (!endingChar)
+        break;
+    }
+
     switch (c)
     {
       case "*":
-        if (hostnameStarted)
-          hostnameFinished = true;
         if (result.length > 0 && i < lastIndex && text[i + 1] != "*")
           result.push(".*");
         break;
       case "^":
-        if (hostnameStarted)
-          hostnameFinished = true;
         if (i < lastIndex)
           result.push(".");
         break;
       case "|":
         if (i == 0)
         {
           result.push("^");
           break;
         }
         if (i == lastIndex)
         {
           result.push("$");
           break;
         }
         if (i == 1 && text[0] == "|")
         {
-          hostnameStarted = caseSensitive = true;
+          hostnameStart = i + 1;
+          canSafelyMatchAsLowercase = true;
           result.push("https?://");
           break;
         }
+        result.push("\\|");
+        break;
+      case "/":
+        if (!hostnameFinished &&
+            text.charAt(i-2) == ":" && text.charAt(i-1) == "/")
+        {
+          hostnameStart = i + 1;
+          canSafelyMatchAsLowercase = true;
+        }
+        result.push("/");
+        break;
+      case ".": case "+": case "$": case "?":
+      case "{": case "}": case "(": case ")":
+      case "[": case "]": case "\\":
         result.push("\\", c);
         break;
-      case "?":
-        if (hostnameStarted)
-          hostnameFinished = true;
-      case ".": case "+": case "$": case "{": case "}":
-      case "(": case ")": case "[": case "]": case "\\":
-        result.push("\\", c);
-        break;
-      case "/":
-        if (hostnameStarted)
-          hostnameFinished = true;
-        else if (text.charAt(i-2) == ":" && text.charAt(i-1) == "/")
-          hostnameStarted = caseSensitive = true;
       default:
         if (hostnameFinished && (c >= "a" && c <= "z" ||
                                  c >= "A" && c <= "Z"))
-          caseSensitive = false;
+          canSafelyMatchAsLowercase = false;
         result.push(c);
     }
   }
 
-  return {regexp: result.join(""), caseSensitive: caseSensitive};
+  return {regexp: result.join(""),
+          canSafelyMatchAsLowercase: canSafelyMatchAsLowercase};
 }
 
 function getRegExpTrigger(filter)
 {
-  let result = toRegExp(filter.regexpSource.replace(
-    // Safari expects punycode, filter lists use unicode
-    /^(\|\||\|?https?:\/\/)([\w\-.*\u0080-\uFFFF]+)/i,
-    function (match, prefix, domain)
-    {
-      return prefix + punycode.toASCII(domain);
-    }
-  ));
+  let result = toRegExp(filter.regexpSource);
 
   let trigger = {"url-filter": result.regexp};
 
   // Limit rules to to HTTP(S) URLs
   if (!/^(\^|http)/i.test(trigger["url-filter"]))
     trigger["url-filter"] = "^https?://.*" + trigger["url-filter"];
 
   // For rules containing only a hostname we know that we're matching against
   // a lowercase string unless the matchCase option was passed.
-  if (result.caseSensitive && !filter.matchCase)
+  if (result.canSafelyMatchAsLowercase && !filter.matchCase)
     trigger["url-filter"] = trigger["url-filter"].toLowerCase();
 
-  if (result.caseSensitive || filter.matchCase)
+  if (result.canSafelyMatchAsLowercase || filter.matchCase)
     trigger["url-filter-is-case-sensitive"] = true;
 
   return trigger;
 }
 
 function getResourceTypes(filter)
 {
   let types = [];
 
   if (filter.contentType & typeMap.IMAGE)
@@ skipping 248 matching lines @@
 
   for (let filter of this.elemhideExceptions)
     addRule(convertFilter(filter, "ignore-previous-rules", false));
   for (let filter of this.requestFilters)
     addRule(convertFilter(filter, "block", true));
   for (let filter of this.requestExceptions)
     addRule(convertFilter(filter, "ignore-previous-rules", true));
 
   return rules;
 };