Issue #1234 - Convert strings associated with URL's

src/plugin/PluginClass.cpp

 #include "PluginStdAfx.h"
 
 #include "PluginClass.h"
 #include "PluginSettings.h"
 #include "PluginSystem.h"
 #include "PluginFilter.h"
 #include "PluginMimeFilterClient.h"
 #include "PluginClient.h"
 #include "PluginClientFactory.h"
 #include "PluginMutex.h"
@@ skipping 25 matching lines @@
 ATOM CPluginClass::s_atomPaneClass = NULL;
 HINSTANCE CPluginClass::s_hUxtheme = NULL;
 std::set<CPluginClass*> CPluginClass::s_instances;
 std::map<DWORD, CPluginClass*> CPluginClass::s_threadInstances;
 
 CComAutoCriticalSection CPluginClass::s_criticalSectionLocal;
 CComAutoCriticalSection CPluginClass::s_criticalSectionBrowser;
 CComAutoCriticalSection CPluginClass::s_criticalSectionWindow;
 
 CComQIPtr<IWebBrowser2> CPluginClass::s_asyncWebBrowser2;
-std::map<UINT,CString> CPluginClass::s_menuDomains;
 
 /*
  * Without namespace declaration, the identifier "Rectangle" is ambiguous
  * See http://msdn.microsoft.com/en-us/library/windows/desktop/dd162898(v=vs.85).aspx
  */
 namespace AdblockPlus
 {
   /**
     * Replacement for ATL type CRect.
     */
@@ skipping 145 matching lines @@
 
   return browser;
 }
 
 std::wstring CPluginClass::GetBrowserUrl() const
 {
   std::wstring url;
   CComQIPtr<IWebBrowser2> browser = GetBrowser();
   if (browser)
   {
-    BSTR bstrURL;
+    CComBSTR bstrURL;
     if (SUCCEEDED(browser->get_LocationURL(&bstrURL)) && bstrURL)
     {
       url = std::wstring(bstrURL, SysStringLen(bstrURL));
-      SysFreeString(bstrURL);
       UnescapeUrl(url);
     }
   }
   else
   {
     url = m_tab->GetDocumentUrl();
   }
   return url;
 }
 
@@ skipping 154 matching lines @@
     {
       s_instances.erase(this);
 
       std::map<DWORD,CPluginClass*>::iterator it = s_threadInstances.find(::GetCurrentThreadId());
       if (it != s_threadInstances.end())
       {
         s_threadInstances.erase(it);
       }
       if (s_instances.empty())
       {
+        // TODO: Explicitly releasing a resource when a container becomes empty looks like a job better suited for shared_ptr
         CPluginClientFactory::ReleaseMimeFilterClientInstance();
       }
     }
     s_criticalSectionLocal.Unlock();
 
     // Release browser interface
     s_criticalSectionBrowser.Lock();
     {
       m_webBrowser2.Release();
     }
@@ skipping 113 matching lines @@
       }
     }
     else
     {
       DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_UI, PLUGIN_ERROR_UI_GET_STATUSBAR, "Class::Get statusbar state");
     }
   }
   DEBUG_GENERAL("ShowStatusBar end");
 }
 
+/*
+ * #1163 This class is the implementation for method DISPID_BEFORENAVIGATE2 in CPluginClass::Invoke.
+ * - It validates and convertes its own arguments, rather than unifying them in the Invoke body.
+ * - It's declared void and not HRESULT, so DISPID_BEFORENAVIGATE2 can only return S_OK.
+ */
 void CPluginClass::BeforeNavigate2(DISPPARAMS* pDispParams)
 {
 
   if (pDispParams->cArgs < 7)
   {
     return;
   }
   //Register a mime filter if it's not registered yet
   if (s_mimeFilter == NULL)
   {
@@ skipping 49 matching lines @@
 
     UpdateStatusBar();
   }
   else
   {
     DEBUG_NAVI(L"Navi::Begin navigation url:" + urlLegacy)
     m_tab->CacheFrame(url);
   }
 }
 
+/*
+ * #1163 implements behavior for method DISPID_WINDOWSTATECHANGED in CPluginClass::Invoke
+ * - should validate and convert arguments in Invoke, not here
+ * - does not validate number of arguments before indexing into 'rgvarg'
+ * - does not validate type of argument before using its value
+ */
 STDMETHODIMP CPluginClass::OnTabChanged(DISPPARAMS* pDispParams, WORD wFlags)
 {
   DEBUG_GENERAL("Tab changed");
   bool newtabshown = pDispParams->rgvarg[1].intVal==3;
   if (newtabshown)
   {
     std::map<DWORD,CPluginClass*>::const_iterator it = s_threadInstances.find(GetCurrentThreadId());
     if (it == s_threadInstances.end())
     {
       s_threadInstances[::GetCurrentThreadId()] = this;
-
-
       if (!m_isInitializedOk)
       {
         m_isInitializedOk = true;
-        if (!InitObject(true))
-        {
-          //                                    Unadvice();
-        }
+        InitObject(true);
         UpdateStatusBar();
       }
     }
   }
   notificationMessage.Hide();
   DEBUG_GENERAL("Tab change end");
-  return VARIANT_TRUE;
+  return S_OK;
 }
 
 // This gets called whenever there's a browser event
+// ENTRY POINT
 STDMETHODIMP CPluginClass::Invoke(DISPID dispidMember, REFIID riid, LCID lcid, WORD wFlags, DISPPARAMS* pDispParams, VARIANT* pvarResult, EXCEPINFO* pExcepInfo, UINT* puArgErr)
 {
-  WCHAR tmp[256];
-  wsprintf(tmp, L"Invoke: %d\n", dispidMember);
-  DEBUG_GENERAL(tmp);
-  switch (dispidMember)
-  {
-
-  case DISPID_WINDOWSTATECHANGED:
-    return OnTabChanged(pDispParams, wFlags);
-    break;
-  case DISPID_HTMLDOCUMENTEVENTS2_ONBEFOREUPDATE:
-    return VARIANT_TRUE;
-    break;
-
-  case DISPID_HTMLDOCUMENTEVENTS2_ONCLICK:
-    return VARIANT_TRUE;
-    break;
-
-  case DISPID_EVMETH_ONLOAD:
-    DEBUG_NAVI("Navi::OnLoad")
-      return VARIANT_TRUE;
-    break;
-
-  case DISPID_EVMETH_ONCHANGE:
-    return VARIANT_TRUE;
-
-  case DISPID_EVMETH_ONMOUSEDOWN:
-    return VARIANT_TRUE;
-
-  case DISPID_EVMETH_ONMOUSEENTER:
-    return VARIANT_TRUE;
-
-  case DISPID_IHTMLIMGELEMENT_START:
-    return VARIANT_TRUE;
-
-  case STDDISPID_XOBJ_ERRORUPDATE:
-    return VARIANT_TRUE;
-
-  case STDDISPID_XOBJ_ONPROPERTYCHANGE:
-    return VARIANT_TRUE;
-
-  case DISPID_READYSTATECHANGE:
-    DEBUG_NAVI("Navi::ReadyStateChange")
-      return VARIANT_TRUE;
-
-  case DISPID_BEFORENAVIGATE:
-    DEBUG_NAVI("Navi::BeforeNavigate")
-      return VARIANT_TRUE;
-  case DISPID_COMMANDSTATECHANGE:
-    if (m_hPaneWnd == NULL)
-    {
-      CreateStatusBarPane();
-    }
-    else
-    {
-      if (CPluginClient::GetInstance()->GetIEVersion() > 6)
-      {
-        RECT rect;
-        BOOL rectRes = GetClientRect(m_hStatusBarWnd, &rect);
-        if (rectRes == TRUE)
-        {
-          MoveWindow(m_hPaneWnd, rect.right - 200, 0, m_nPaneWidth, rect.bottom - rect.top, TRUE);
-        }
-      }      
-    }
-    break;
-  case DISPID_STATUSTEXTCHANGE:
-    break;
-
-  case DISPID_BEFORENAVIGATE2:
-    BeforeNavigate2(pDispParams);
-    break;
-
-  case DISPID_DOWNLOADBEGIN:
-    {
-      DEBUG_NAVI("Navi::Download Begin")
-    }
-    break;
-
-  case DISPID_DOWNLOADCOMPLETE:
-    {
-      DEBUG_NAVI("Navi::Download Complete")
-
+  try
+  {
+    WCHAR tmp[256];
+    wsprintf(tmp, L"Invoke: %d\n", dispidMember);
+    DEBUG_GENERAL(tmp);
+    switch (dispidMember)
+    {
+    case DISPID_WINDOWSTATECHANGED:
+      {
+        // #1163 should validate and convert arguments here
+        return OnTabChanged(pDispParams, wFlags);
+      }
+
+    case DISPID_HTMLDOCUMENTEVENTS2_ONBEFOREUPDATE:
+      break;
+
+    case DISPID_HTMLDOCUMENTEVENTS2_ONCLICK:
+      break;
+
+    case DISPID_EVMETH_ONLOAD:
+      DEBUG_NAVI("Navi::OnLoad")
+      break;
+
+    case DISPID_EVMETH_ONCHANGE:
+      break;
+
+    case DISPID_EVMETH_ONMOUSEDOWN:
+      break;
+
+    case DISPID_EVMETH_ONMOUSEENTER:
+      break;
+
+    case DISPID_IHTMLIMGELEMENT_START:
+      break;
+
+    case STDDISPID_XOBJ_ERRORUPDATE:
+      break;
+
+    case STDDISPID_XOBJ_ONPROPERTYCHANGE:
+      break;
+
+    case DISPID_READYSTATECHANGE:
+      DEBUG_NAVI("Navi::ReadyStateChange");
+      break;
+
+    case DISPID_BEFORENAVIGATE:
+      DEBUG_NAVI("Navi::BeforeNavigate");
+      break;
+
+    case DISPID_COMMANDSTATECHANGE:
+      if (m_hPaneWnd == NULL)
+      {
+        CreateStatusBarPane();
+      }
+      else
+      {
+        if (CPluginClient::GetInstance()->GetIEVersion() > 6)
+        {
+          RECT rect;
+          BOOL rectRes = GetClientRect(m_hStatusBarWnd, &rect);
+          if (rectRes == TRUE)
+          {
+            MoveWindow(m_hPaneWnd, rect.right - 200, 0, m_nPaneWidth, rect.bottom - rect.top, TRUE);
+          }
+        }      
+      }
+      break;
+
+    case DISPID_STATUSTEXTCHANGE:
+      break;
+
+    case DISPID_BEFORENAVIGATE2:
+      {
+        // #1163 should validate and convert parameters here
+        BeforeNavigate2(pDispParams);
+      }
+      break;
+
+    case DISPID_DOWNLOADBEGIN:
+      {
+        DEBUG_NAVI("Navi::Download Begin")
+      }
+      break;
+
+    case DISPID_DOWNLOADCOMPLETE:
+      {
+        DEBUG_NAVI("Navi::Download Complete");
         CComQIPtr<IWebBrowser2> browser = GetBrowser();
-      if (browser)
-      {
-        m_tab->OnDownloadComplete(browser);
-      }
-    }
-    break;
-
-  case DISPID_DOCUMENTCOMPLETE:
-    {
-      DEBUG_NAVI("Navi::Document Complete")
-
+        if (browser)
+        {
+          m_tab->OnDownloadComplete(browser);
+        }
+      }
+      break;
+
+    case DISPID_DOCUMENTCOMPLETE:
+      {
+        DEBUG_NAVI("Navi::Document Complete");
         CComQIPtr<IWebBrowser2> browser = GetBrowser();
-
-      if (browser && pDispParams->cArgs >= 2 && pDispParams->rgvarg[1].vt == VT_DISPATCH)
-      {
-        CComQIPtr<IWebBrowser2> pBrowser = pDispParams->rgvarg[1].pdispVal;
-        if (pBrowser)
-        {
-          BSTR bstrUrl;
-          if (SUCCEEDED(pBrowser->get_LocationURL(&bstrUrl)) && ::SysStringLen(bstrUrl) > 0)
+        if (browser && pDispParams->cArgs >= 2 && pDispParams->rgvarg[1].vt == VT_DISPATCH)
+        {
+          CComQIPtr<IWebBrowser2> pBrowser = pDispParams->rgvarg[1].pdispVal;
+          if (pBrowser)
           {
-            std::wstring url(bstrUrl, SysStringLen(bstrUrl));
-            SysFreeString(bstrUrl);
-            UnescapeUrl(url);
-            m_tab->OnDocumentComplete(browser, url, browser.IsEqualObject(pBrowser));
+            CComBSTR bstrUrl;
+            if (SUCCEEDED(pBrowser->get_LocationURL(&bstrUrl)) && bstrUrl && ::SysStringLen(bstrUrl) > 0)
+            {
+              std::wstring url = std::wstring(bstrUrl, SysStringLen(bstrUrl));
+              UnescapeUrl(url);
+              m_tab->OnDocumentComplete(browser, url, browser.IsEqualObject(pBrowser));
+            }
           }
         }
       }
-    }
-    break;
-
-  case DISPID_ONQUIT:
-  case DISPID_QUIT:
-    {
-      Unadvice();
-    }
-    break;
-
-  default:
-    {
-      CString did;
-      did.Format(L"DispId:%u", dispidMember);
-
-      DEBUG_NAVI(L"Navi::Default " + did)
-    }
-
-    // do nothing
-    break;
-  }
-
-  return VARIANT_TRUE;
+      break;
+
+    case DISPID_ONQUIT:
+    case DISPID_QUIT:
+      {
+        Unadvice();
+      }
+      break;
+
+    default:
+      {
+        CString did;
+        did.Format(L"DispId:%u", dispidMember);
+
+        DEBUG_NAVI(L"Navi::Default " + did)
+      }
+      /*
+       * Ordinarily a method not dispatched should return DISP_E_MEMBERNOTFOUND.
+       * As a conservative initial change, we leave it behaving as before,
+       *   which is to do nothing and return S_OK.
+       */
+      // do nothing
+      break;
+    }
+  }
+  catch(...)
+  {
+    DEBUG_GENERAL( "Caught unknown exception in CPluginClass::Invoke" );
+    return E_FAIL;
+  }
+  return S_OK;
 }
 
 bool CPluginClass::InitObject(bool bBHO)
 {
   DEBUG_GENERAL("InitObject");
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   if (!settings->GetPluginEnabled())
   {
     s_mimeFilter->Unregister();
@@ skipping 499 matching lines @@
   // Invalidate and redraw the control
   UpdateStatusBar();
 }
 
 
 bool CPluginClass::SetMenuBar(HMENU hMenu, const std::wstring& url)
 {
   DEBUG_GENERAL("SetMenuBar");
 
   std::wstring ctext;
-
-  s_criticalSectionLocal.Lock();
-  {
-    s_menuDomains.clear();
-  }
-  s_criticalSectionLocal.Unlock();
-
   Dictionary* dictionary = Dictionary::GetInstance();
 
   MENUITEMINFOW fmii = {};
   fmii.cbSize = sizeof(fmii);
 
   MENUITEMINFOW miiSep = {};
   miiSep.cbSize = sizeof(miiSep);
   miiSep.fMask = MIIM_TYPE | MIIM_FTYPE;
   miiSep.fType = MFT_SEPARATOR;
 
@@ skipping 219 matching lines @@
 
 HICON CPluginClass::GetStatusBarIcon(const std::wstring& url)
 {
   // use the disable icon as defualt, if the client doesn't exists
   HICON hIcon = GetIcon(ICON_PLUGIN_DEACTIVATED);
 
   CPluginTab* tab = GetTab(::GetCurrentThreadId());
   if (tab)
   {
     CPluginClient* client = CPluginClient::GetInstance();
-    if (!CPluginSettings::GetInstance()->IsPluginEnabled())
-    {
-    }
-    else if (client->IsWhitelistedUrl(url))
-    {
-      hIcon = GetIcon(ICON_PLUGIN_DISABLED);
-    }
-    else
-    {
-      CPluginSettings* settings = CPluginSettings::GetInstance();
-      hIcon = GetIcon(ICON_PLUGIN_ENABLED);
+    if (CPluginSettings::GetInstance()->IsPluginEnabled())
+    {
+      if (client->IsWhitelistedUrl(url))
+      {
+        hIcon = GetIcon(ICON_PLUGIN_DISABLED);
+      }
+      else
+      {
+        CPluginSettings* settings = CPluginSettings::GetInstance();
+        hIcon = GetIcon(ICON_PLUGIN_ENABLED);
+      }
     }
   }
   return hIcon;
 }
 
 
 LRESULT CALLBACK CPluginClass::PaneWindowProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
 {
   // Find tab
   CPluginClass *pClass = FindInstance(GetParent(hWnd));
@@ skipping 320 matching lines @@
         }
       }
     }
 
     hTabWnd = ::GetWindow(hTabWnd, GW_HWNDNEXT);
   }
 
   return hTabWnd;
 
 }