Fix domain-based whitelisting

src/plugin/PluginClientBase.cpp

Index: src/plugin/PluginClientBase.cpp
===================================================================
--- a/src/plugin/PluginClientBase.cpp
+++ b/src/plugin/PluginClientBase.cpp
@@ -166,15 +166,6 @@
 
 #ifdef SUPPORT_WHITELIST
 
-void CPluginClientBase::CacheWhiteListedUrl(const CString& url, bool isWhitelisted)
-{
-  m_criticalSectionWhitelist.Lock();
-  {
-    m_cacheWhitelistedUrls[url] = isWhitelisted;
-  }
-  m_criticalSectionWhitelist.Unlock();
-}
-
 bool CPluginClientBase::IsUrlWhiteListed(const CString& url)
 {
   if (url.IsEmpty())
@@ -182,49 +173,17 @@
     return false;
   }
 
-  bool isWhitelisted = false;
-  bool isCached = false;
-
-  m_criticalSectionWhitelist.Lock();
+  int pos = 0;
+  CString http = url.Find('/',pos) >= 0 ? url.Tokenize(L"/", pos) : L"";

[Wladimir Palant, 2013/06/17 09:33:56]

I know that this isn't your code but that variable should be called "scheme".

[Felix Dahlke, 2013/06/17 11:19:01]

Done.

+  CString domain = ExtractDomain(url);
+  if (http == L"res:" || http == L"file:")
   {
-    std::map<CString,bool>::iterator it = m_cacheWhitelistedUrls.find(url);
-
-    isCached = it != m_cacheWhitelistedUrls.end();
-    if (isCached)
-    {
-      isWhitelisted = it->second;
-    }
-  }
-  m_criticalSectionWhitelist.Unlock();
-
-  if (!isCached)
-  {
-    int pos = 0;
-    CString http = url.Find('/',pos) >= 0 ? url.Tokenize(L"/", pos) : L"";
-    CString domain = ExtractDomain(url);
-    if (http == L"res:" || http == L"file:")
-    {
-      isWhitelisted = true;
-    }
-    else
-    {
-      isWhitelisted = CPluginSettings::GetInstance()->IsWhiteListedDomain(domain);
-    }
-
-    CacheWhiteListedUrl(url, isWhitelisted);
+    return true;
   }
 
-  return isWhitelisted;
+  CPluginSettings* pluginSettings = CPluginSettings::GetInstance();
+  pluginSettings->RefreshWhitelist();
+  return pluginSettings->IsWhiteListedDomain(domain);

[Wladimir Palant, 2013/06/17 09:33:56]

Retrieving all exception domains from the engine just to check whether one is on
the list is very wasteful. It should be better to have a special engine call for
IsWhiteListedDomain() and do the check there.

Note the implementation here isn't correct either. An exception rule affecting
the current page won't necessarily have the expected form and might not even be
a user-dependent filter.

[Felix Dahlke, 2013/06/17 11:19:01]

Yes, I agree. I frankly didn't dare to change too much here, so I only got rid
of one of two layers of unnecessary caching, the one that caused problems.

Would you be fine with postponing this though? I'd rather move on with the
release and fix this later, adding a TODO comment now.

 }
 
-void CPluginClientBase::ClearWhiteListCache()
-{
-  m_criticalSectionWhitelist.Lock();
-  {
-    m_cacheWhitelistedUrls.clear();
-  }
-  m_criticalSectionWhitelist.Unlock();
-}
-
-
 #endif // SUPPORT_WHITELIST