Issue 1793 - check whether the frame is whitelisted before injecting CSS

src/engine/Main.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 22 matching lines @@
 #include "Updater.h"
 
 namespace
 {
   std::auto_ptr<AdblockPlus::FilterEngine> filterEngine;
   std::auto_ptr<Updater> updater;
   int activeConnections = 0;
   CriticalSection activeConnectionsLock;
   HWND callbackWindow;
 
+  std::string GetWhitelistingFilter(const std::string& urlArg,
+    const std::vector<std::string>& frameHierarchy, AdblockPlus::FilterEngine::ContentType type)
+  {
+    auto GetWhitelistingFilter = [&type](const std::string& url, const std::string& parent)->std::string

[Eric, 2015/05/14 17:07:26]

Code style comment.

It's not a good idea to use the exact same name as the function for a local
variable, even if it is substituting for a local function. I can immediately see
two fixes possible:
1. Rename it to reflect the fact that it's not working with frames.
2. Extract it as its own function, with the same name and its distinct
signature.

Or do both.

[sergei, 2015/05/15 11:55:55]

extracted as a separate function.

+    {
+      AdblockPlus::FilterPtr match = filterEngine->Matches(url, type, parent);
+      if (match && match->GetType() == AdblockPlus::Filter::TYPE_EXCEPTION)
+      {
+        return match->GetProperty("text")->AsString();
+      }
+      return "";
+    };

[Eric, 2015/05/14 17:07:26]

The whole loop below would be clearer if implemented with 'std::find_if', but
I'm not sure if the overhead is worth it.

You'd have to memoize the result of "filterEngine->Matches()" above to avoid a
redundant call, for example.

[sergei, 2015/05/15 11:55:55]

It seems such overhead will only complicate it.

+    if (frameHierarchy.empty())
+    {
+      return GetWhitelistingFilter(urlArg, "");
+    }
+    auto frameIterator = frameHierarchy.begin();
+    std::string parentUrl;
+    std::string url = urlArg;
+    while (frameIterator != frameHierarchy.end())
+    {
+      parentUrl = *frameIterator;
+      auto filterText = GetWhitelistingFilter(url, parentUrl);
+      if (!filterText.empty())
+      {
+        return filterText;
+      }
+      url = parentUrl;
+      ++frameIterator;
+    }
+    return "";
+  }
+
   void WriteSubscriptions(Communication::OutputBuffer& response,
       const std::vector<AdblockPlus::SubscriptionPtr>& subscriptions)
   {
     int32_t count = static_cast<int32_t>(subscriptions.size());
     response << count;
     for (int32_t i = 0; i < count; i++)
     {
       AdblockPlus::SubscriptionPtr subscription = subscriptions[i];
       response << subscription->GetProperty("url")->AsString()
                << subscription->GetProperty("title")->AsString()
@@ skipping 143 matching lines @@
           }
         }
 
         response << domains;
         break;
       }
       case Communication::PROC_GET_WHITELISTING_FITER:
       {
         std::string url;
         request >> url;
-        AdblockPlus::FilterPtr match = filterEngine->Matches(url,
-          AdblockPlus::FilterEngine::ContentType::CONTENT_TYPE_DOCUMENT, url);
-        std::string filterText;
-        if (match && match->GetType() == AdblockPlus::Filter::TYPE_EXCEPTION)
-        {
-          filterText = match->GetProperty("text")->AsString();
-        }
-        response << filterText;
+        std::vector<std::string> frameHierarchy;
+        request >> frameHierarchy;
+        response << GetWhitelistingFilter(url, frameHierarchy, AdblockPlus::FilterEngine::CONTENT_TYPE_DOCUMENT);
         break;
       }
       case Communication::PROC_IS_ELEMHIDE_WHITELISTED_ON_URL:
       {
         std::string url;
         request >> url;
-        AdblockPlus::FilterPtr match = filterEngine->Matches(url,
-          AdblockPlus::FilterEngine::ContentType::CONTENT_TYPE_ELEMHIDE, url);
-        response << (match && match->GetType() == AdblockPlus::Filter::TYPE_EXCEPTION);
+        std::vector<std::string> frameHierarchy;
+        request >> frameHierarchy;
+        response << !GetWhitelistingFilter(url, frameHierarchy, AdblockPlus::FilterEngine::CONTENT_TYPE_ELEMHIDE).empty();
         break;
       }
       case Communication::PROC_ADD_FILTER:
       {
         std::string text;
         request >> text;
 
         filterEngine->GetFilter(text)->AddToList();
         break;
       }
@@ skipping 269 matching lines @@
     }
     catch (const std::runtime_error& e)
     {
       DebugException(e);
       return 1;
     }
   }
 
   return 0;
 }