Issues #276, #1163 - introduce class IncomingParam

src/plugin/PluginUserSettings.cpp

 #include "PluginStdAfx.h"
 #include "PluginUserSettings.h"
 #include <algorithm>
+#include "COM_Value.h"
 #include "PluginSettings.h"
 #include "PluginClient.h"
 #include "../shared/Dictionary.h"
 
 static const CString s_GetMessage = L"GetMessage";
 static const CString s_GetLanguageCount = L"GetLanguageCount";
 static const CString s_GetLanguageByIndex = L"GetLanguageByIndex";
 static const CString s_GetLanguageTitleByIndex = L"GetLanguageTitleByIndex";
 static const CString s_SetLanguage = L"SetLanguage";
 static const CString s_GetLanguage = L"GetLanguage";
@@ skipping 109 matching lines @@
     return E_POINTER;
 
   if (pDispparams->cNamedArgs)
     return DISP_E_NONAMEDARGS;
 
   CPluginSettings* settings = CPluginSettings::GetInstance();
 
   if (dispidMember  < 0  ||  dispidMember >= countof(s_Methods))
     return DISP_E_BADINDEX;
 
+  /*
+   * See Issue #1163.
+   * TODO: Rewrite this linear sequence of string comparisons as a switch statement.
+   * TODO: Add a try-catch block at the top level of the switch body to rationalize argument checking etc.
+   */
   const CString& method = s_Methods[dispidMember];
-
   if (s_GetMessage == method)
   {
-    if (2 != pDispparams->cArgs)
-      return DISP_E_BADPARAMCOUNT;
-
-    if (VT_BSTR != pDispparams->rgvarg[0].vt)
-      return DISP_E_TYPEMISMATCH;
-    if (VT_BSTR != pDispparams->rgvarg[1].vt)
-      return DISP_E_TYPEMISMATCH;
-
+    /*
+     * If the caller ignores the value, we can elide the body since it has no side-effects.
+     */
     if (pVarResult)
     {
-      CComBSTR key = pDispparams->rgvarg[0].bstrVal;
-      CComBSTR section = pDispparams->rgvarg[1].bstrVal;
-      CStringW message = sGetMessage((BSTR)section, (BSTR)key);
+      if (2 != pDispparams->cArgs)
+        return DISP_E_BADPARAMCOUNT;
+
+      AdblockPlus::COM::IncomingParam key(pDispparams->rgvarg[0]);
+      AdblockPlus::COM::IncomingParam section(pDispparams->rgvarg[1]);
+      if (!key.wstringConvertible() || !section.wstringConvertible())
+      {
+        return DISP_E_TYPEMISMATCH;
+      }
+      CStringW message = sGetMessage(to_CString(section.wstringValueNoexcept()), to_CString(key.wstringValueNoexcept()));
 
       pVarResult->vt = VT_BSTR;
       pVarResult->bstrVal = SysAllocString(message);
     }
   }
   else if (s_GetLanguageCount == method)
   {
     if (pDispparams->cArgs)
       return DISP_E_BADPARAMCOUNT;
 
@@ skipping 76 matching lines @@
     }
   }
   else if (s_SetLanguage == method)
   {
     if (1 != pDispparams->cArgs)
       return DISP_E_BADPARAMCOUNT;
 
     if (VT_BSTR != pDispparams->rgvarg[0].vt)
       return DISP_E_TYPEMISMATCH;
 
-    CComBSTR url = pDispparams->rgvarg[0].bstrVal;
-
-    settings->SetSubscription((BSTR)url);
+    AdblockPlus::COM::IncomingParam url(pDispparams->rgvarg[0]);
+    if (!url.wstringConvertible())
+    {
+      return DISP_E_TYPEMISMATCH;
+    }
+    settings->SetSubscription(url.wstringValueNoexcept());
   }
   else if (s_GetLanguage == method)
   {
     if (pDispparams->cArgs)
       return DISP_E_BADPARAMCOUNT;
 
     if (pVarResult)
     {
       CString url = settings->GetSubscription();
 
@@ skipping 24 matching lines @@
     }
   }
   else if (s_AddWhitelistDomain == method)
   {
     if (1 != pDispparams->cArgs)
       return DISP_E_BADPARAMCOUNT;
 
     if (VT_BSTR != pDispparams->rgvarg[0].vt)
       return DISP_E_TYPEMISMATCH;
 
-    CComBSTR domain = pDispparams->rgvarg[0].bstrVal;
-    if (domain.Length())
+    AdblockPlus::COM::IncomingParam domain_arg(pDispparams->rgvarg[0]);
+    if (!domain_arg.wstringConvertible())
     {
-      settings->AddWhiteListedDomain((BSTR)domain);
+      return DISP_E_TYPEMISMATCH;
+    }
+    std::wstring domain = domain_arg.wstringValueNoexcept();
+    if (!domain.empty())
+    {
+      settings->AddWhiteListedDomain(to_CString(domain));
     }
   }
   else if (s_RemoveWhitelistDomain == method)
   {
     if (1 != pDispparams->cArgs)
       return DISP_E_BADPARAMCOUNT;
 
     if (VT_BSTR != pDispparams->rgvarg[0].vt)
       return DISP_E_TYPEMISMATCH;
 
-    CComBSTR domain = pDispparams->rgvarg[0].bstrVal;
-    if (domain.Length())
+    AdblockPlus::COM::IncomingParam domain_arg(pDispparams->rgvarg[0]);
+    if (!domain_arg.wstringConvertible())
     {
-      settings->RemoveWhiteListedDomain((BSTR)domain);
+      return DISP_E_TYPEMISMATCH;
+    }
+    std::wstring domain = domain_arg.wstringValueNoexcept();
+    if (!domain.empty())
+    {
+      settings->RemoveWhiteListedDomain(to_CString(domain));
     }
   }
   else if (s_GetAppLocale == method)
   {
     if (0 != pDispparams->cArgs)
       return DISP_E_BADPARAMCOUNT;
 
     pVarResult->vt = VT_BSTR;
     pVarResult->bstrVal = SysAllocString(settings->GetAppLocale());
   }
@@ skipping 33 matching lines @@
       CPluginClient* client = CPluginClient::GetInstance();
       client->RemoveSubscription(client->GetPref(L"subscriptions_exceptionsurl", L""));
     }
   }
   else
     return DISP_E_MEMBERNOTFOUND;
 
   return S_OK;
 }