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;

[sergei, 2018/02/14 15:59:35]

Perhaps, it should be the very first instruction of this function.

[hub, 2018/02/14 17:05:20]

Done.

 
   // Domains part
   bool seenSpaces = false;
   while (!scanner.done())
   {
     String::value_type next = scanner.next();
     if (next == u'#')
     {
       data.mDomainsEnd = scanner.position();
       break;
@@ skipping 93 matching lines @@
     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']':

[sergei, 2018/02/14 15:59:35]

What about filter containing
`[-abp-properties=what-happens-here"useful"and-here]`? I'm not sure that it's
supposed to be correct and I think it's not handled here. What about setting and
checking (either " or ') the value of `quote` before the loop and expecting ]
right after the closing quote is found?

[hub, 2018/02/14 17:05:20]

This would definitely be invalid. I'll make sure this is rejected here as well
(and tested)

[hub, 2018/02/22 18:56:30]

It is definitely handled in that last patch.

       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;
 }