Fix domain-based whitelisting

src/plugin/PluginClientBase.cpp

 #include "PluginStdAfx.h"
 
 // Internet / FTP
 #include <wininet.h>
 
 // IP adapter
 #include <iphlpapi.h>
 
 #include "PluginSettings.h"
 #include "PluginSystem.h"
@@ skipping 156 matching lines @@
 #ifdef SUPPORT_WHITELIST
 
 bool CPluginClientBase::IsUrlWhiteListed(const CString& url)
 {
   if (url.IsEmpty())
   {
     return false;
   }
 
   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 scheme = url.Find('/',pos) >= 0 ? url.Tokenize(L"/", pos) : L"";
   CString domain = ExtractDomain(url);
-  if (http == L"res:" || http == L"file:")
+  if (scheme == L"res:" || scheme == L"file:")
   {
     return true;
   }
 
+  // TODO: Caching whitelist entries in PluginSettings is redundant and wasteful. We should have an engine call for IsWhitelistedDomain.
   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.

 }
 
 #endif // SUPPORT_WHITELIST