Issue 1564-Fix FilterEngine::Matches for allowing request which is whitelisted in the ascendant node

src/ReferrerMapping.cpp

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 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 28 matching lines @@
     mapping.erase(poppedUrl);
   }
 }
 
 ReferrerMapping::Urls ReferrerMapping::BuildFrameStructure(const Url& url) const
 {
   Urls frames;
   // We need to limit the chain length to ensure we don't block indefinitely
   // if there's a referrer loop.
   const int maxChainLength = 10;
-  ReferrerMap::const_iterator currentEntry = mapping.end();
+  ReferrerMap::const_iterator currentEntry = url.empty() ? mapping.end() :
+    mapping.find(url);
   ReferrerMap::const_iterator prevEntry = mapping.end();
-  // process the current url beyond the loop below because we should add it

[Wladimir Palant, 2014/12/09 21:20:03]

beyond => outside

-  // even if `mapping` is empty.
-  if (!url.empty())
-  {
-    currentEntry = mapping.find(url);
-    if (currentEntry != mapping.end())
-    {
-      if (currentEntry->second.IsFrame())
-      {
-        frames.push_back(url);
-      }
-      prevEntry = currentEntry;
-      currentEntry = mapping.find(currentEntry->second.referrer);
-    }
-    else
-    {
-      // If there is no information for the url in `mapping` then assume that
-      // it's a frame, despite it's not necessary to be a frame.

[Wladimir Palant, 2014/12/09 21:20:03]

Nit: "despite it not necessarily being a frame"

-      frames.push_back(url);
-    }
-  }

[Wladimir Palant, 2014/12/09 21:20:03]

Can't we check prevEntry after the loop instead of duplicating its content?
Something along these lines:

  auto currentEntry = mapping.find(url);
  auto prevEntry = mapping.end();
  for (...)
  {
    ...
  }
  if (prevEntry == mapping.end())
  {
    // No data, just assume that the url is a frame.
    frames.push_back(url);
  }
  else if (!prevEntry->second.referrer.empty())
  {
    ...
  }

[sergei, 2014/12/12 17:07:07]

That does work, please check the updated version. Although I would say that the
complexity of the algorithm is higher despite the amount of code is less and
there is no repeating.

-
   for (int i = 0; i < maxChainLength && currentEntry != mapping.end(); i++)
   {
     if (currentEntry->second.IsFrame())
     {
       frames.insert(frames.begin(), currentEntry->first);
     }
     prevEntry = currentEntry;
     currentEntry = mapping.find(currentEntry->second.referrer);
   }
-  if (prevEntry != mapping.end() && !prevEntry->second.referrer.empty())
+  if (prevEntry == mapping.end())
+  {
+    if (!url.empty())
+    {
+      // No data, just assume that the url is a frame.
+      frames.push_back(url);
+    }
+  }
+  else if (!prevEntry->second.referrer.empty())
   {
     frames.insert(frames.begin(), prevEntry->second.referrer);
   }
   return frames;
 }