Issue 5870 - Implement the new ElemHideEmulation filter type

compiled/filter/ElemHideBase.cpp

 /*
  * 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 57 matching lines @@
 
 ElemHideBase::ElemHideBase(Type type, const String& text, const ElemHideData& data)
     : ActiveFilter(type, text, false), mData(data)
 {
   if (mData.HasDomains())
     ParseDomains(mData.GetDomainsSource(mText), u',');
 }
 
 Filter::Type ElemHideBase::Parse(DependentString& text, ElemHideData& data, bool& needConversion)
 {
+  needConversion = false;
+
   StringScanner scanner(text);
-
-  needConversion = false;
 
   // Domains part
   bool seenSpaces = false;
   while (!scanner.done())
   {
     String::value_type next = scanner.next();
     if (next == u'#')
     {
       data.mDomainsEnd = scanner.position();
       break;
@@ skipping 71 matching lines @@
     String::size_type byte_len() const
     {
       return len() * sizeof(String::value_type);
     }
   };
 }
 
 // Convert filter from the old syntax to the new.
 DependentString ElemHideBase::ConvertFilter(String& text, String::size_type& at)
 {
-  auto length = text.length();
-  Range suffix = {at, length};
   Range prefix = {at, text.find(OLD_PROPS_SELECTOR, at, OLD_PROPS_SELECTOR_LEN)};
   if (prefix.end == text.npos)
     return DependentString(text);
 

[sergei, 2018/02/13 09:12:50]

Do you mind to move `length` and `suffix` variables after that `if`?

[hub, 2018/02/13 15:52:25]

Done.

+  auto length = text.length();
+  Range suffix = {at, length};
   Range properties = { prefix.end + OLD_PROPS_SELECTOR_LEN, 0 };
   String::value_type quote = 0;
   for (auto index = properties.start;
        index < length && (suffix.start == at); index++)
   {
     auto c = text[index];
     switch (c)
     {
     case u'"':
     case u'\'':
       if (quote == 0)
       {
         // syntax error: we already have a quoted section.
         if (properties.end)
           return DependentString();
 
+        if (properties.start != index)
+          return DependentString();
+
         quote = c;
         properties.start = index + 1;
       }
       else if (quote == c)
       {
         // end of quoted.
         quote = 0;
         properties.end = index;
       }
       break;
     case u']':
       if (quote == 0)
       {
         if (properties.end == 0)
+          return DependentString();
+        if (properties.end + 1 != index)
           return DependentString();
         suffix.start = index + 1;
       }
       break;
     default:
       break;
     }
   }
 
   if (suffix.start == at)

[sergei, 2018/02/27 10:56:53]

Just for reference, I think it (what the whole for-loop above is doing) can be
written in a simpler form, but since it's some sort of interim function it's
fine.

     return DependentString();
 
   String::size_type delimiter = text.find(ELEM_HIDE_DELIMITER, 0,
                                           ELEM_HIDE_DELIMITER_LEN);
   // +1 for the replacement of "##" by "#?#"
   if (delimiter != text.npos)
     at++;
   auto new_len = at + prefix.len() + PROPS_SELECTOR_LEN + properties.len() + 1 /* ) */ + suffix.len();
 
   assert2(new_len + 1 == length || (delimiter == text.npos && new_len + 2 == length), u"Inconsistent length in filter conversion."_str);

[sergei, 2018/02/27 10:56:53]

not important just for reference
length == new_len + (delimiter == text.npos ? 2 : 1)
looks shorted and maybe even more logical.

[hub, 2018/02/27 13:32:30]

Acknowledged.

 
   DependentString converted(text, 0, new_len);
 
   if (suffix.len())
   {
     new_len -= suffix.len();
     std::memmove(converted.data() + new_len,
                  text.data() + suffix.start,
                  suffix.byte_len());
   }
@@ skipping 89 matching lines @@
       if (item.second && !item.first.empty())
       {
         if (!result.empty())
           result.append(u',');
         result.append(item.first);
       }
     }
   }
   return result;
 }