Changeset 85735 in vbox

Timestamp:

Aug 12, 2020 8:48:55 PM (5 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

139883

Message:

Main/HostUpdateImpl.cpp: Make sure the RTHTTP instance is always destroyed, even in std::bad_alloc and whatnot cases. bugref:7983

Location:

trunk/src/VBox/Main

Files:

• include/HostUpdateImpl.h (modified) (2 diffs)
• src-server/HostUpdateImpl.cpp (modified) (12 diffs)

trunk/src/VBox/Main/include/HostUpdateImpl.h

@@ -23 +23 @@
 
 #include "HostUpdateWrap.h"
+#include <iprt/http.h> /* RTHTTP */
 
 
@@ -52 +53 @@
     HRESULT i_updateCheckTask(UpdateCheckTask *pTask);
     HRESULT i_checkForVBoxUpdate();
+    HRESULT i_checkForVBoxUpdateInner(RTHTTP hHttp, com::Utf8Str const &strUrl, com::Utf8Str const &strUserAgent,
+                                      ComPtr<ISystemProperties> const &ptrSystemProperties);
 
     /** @name Data members.

trunk/src/VBox/Main/src-server/HostUpdateImpl.cpp

@@ -285 +285 @@
 
     // acquire the System Properties interface
-    ComPtr<ISystemProperties> pSystemProperties;
-    rc = mVirtualBox->COMGETTER(SystemProperties)(pSystemProperties.asOutParam());
+    ComPtr<ISystemProperties> ptrSystemProperties;
+    rc = mVirtualBox->COMGETTER(SystemProperties)(ptrSystemProperties.asOutParam());
     if (FAILED(rc))
         return setErrorVrc(rc, tr("%s: IVirtualBox::systemProperties() failed: %Rrc"), __FUNCTION__, rc);
@@ -297 +297 @@
     RTTimeToString(RTTimeExplode(&Time, RTTimeNow(&TimeNow)), szTimeStr, sizeof(szTimeStr));
     LogRelFunc(("VBox updating UpdateDate with TimeString = %s\n", szTimeStr));
-    rc = pSystemProperties->COMSETTER(VBoxUpdateLastCheckDate)(Bstr(szTimeStr).raw());
+    rc = ptrSystemProperties->COMSETTER(VBoxUpdateLastCheckDate)(Bstr(szTimeStr).raw());
     if (FAILED(rc))
         return rc; // ISystemProperties::setLastCheckDate calls setError() on failure
@@ -303 +303 @@
     // Update the queryURL and the VBoxUpdate setting 'VBoxUpdateCount'
     ULONG cVBoxUpdateCount = 0;
-    rc = pSystemProperties->COMGETTER(VBoxUpdateCount)(&cVBoxUpdateCount);
+    rc = ptrSystemProperties->COMGETTER(VBoxUpdateCount)(&cVBoxUpdateCount);
     if (FAILED(rc))
         return setErrorVrc(rc, tr("%s: retrieving ISystemProperties::VBoxUpdateCount failed: %Rrc"), __FUNCTION__, rc);
@@ -309 +309 @@
     cVBoxUpdateCount++;
 
-    rc = pSystemProperties->COMSETTER(VBoxUpdateCount)(cVBoxUpdateCount);
+    rc = ptrSystemProperties->COMSETTER(VBoxUpdateCount)(cVBoxUpdateCount);
     if (FAILED(rc))
         return rc; // ISystemProperties::setVBoxUpdateCount calls setError() on failure
@@ -316 +316 @@
     // Update the query URL and the VBoxUpdate settings (if necessary) with the 'Target' information.
     VBoxUpdateTarget_T enmTarget = VBoxUpdateTarget_Stable; // default branch is 'stable'
-    rc = pSystemProperties->COMGETTER(VBoxUpdateTarget)(&enmTarget);
+    rc = ptrSystemProperties->COMGETTER(VBoxUpdateTarget)(&enmTarget);
     if (FAILED(rc))
         return setErrorVrc(rc, tr("%s: retrieving ISystemProperties::Target failed: %Rrc"), __FUNCTION__, rc);
@@ -334 +334 @@
     }
 
-    rc = pSystemProperties->COMSETTER(VBoxUpdateTarget)(enmTarget);
+    rc = ptrSystemProperties->COMSETTER(VBoxUpdateTarget)(enmTarget);
     if (FAILED(rc))
         return rc; // ISystemProperties::setTarget calls setError() on failure
@@ -341 +341 @@
 
     /*
-     * Setup the User-Agent headers for the GET request
+     * Compose the User-Agent header for the GET request.
      */
     Bstr version;
@@ -351 +351 @@
     LogRelFunc(("userAgent = %s\n", strUserAgent.c_str()));
 
+    /*
+     * Create the HTTP client instance and pass it to a inner worker method to
+     * ensure proper cleanup.
+     */
     RTHTTP hHttp = NIL_RTHTTP;
     int vrc = RTHttpCreate(&hHttp);
-    if (RT_FAILURE(vrc))
-        return setErrorVrc(vrc, tr("%s: RTHttpCreate() failed: %Rrc"), __FUNCTION__, vrc);
-
+    if (RT_SUCCESS(vrc))
+    {
+        try
+        {
+            rc = i_checkForVBoxUpdateInner(hHttp, strUrl, strUserAgent, ptrSystemProperties);
+        }
+        catch (...)
+        {
+            AssertFailed();
+            rc = E_UNEXPECTED;
+        }
+        RTHttpDestroy(hHttp);
+    }
+    else
+        rc = setErrorVrc(vrc, tr("%s: RTHttpCreate() failed: %Rrc"), __FUNCTION__, vrc);
+    return S_OK;
+}
+
+HRESULT HostUpdate::i_checkForVBoxUpdateInner(RTHTTP hHttp, Utf8Str const &strUrl, Utf8Str const &strUserAgent,
+                                              ComPtr<ISystemProperties> const &ptrSystemProperties)
+{
     /// @todo Are there any other headers needed to be added first via RTHttpSetHeaders()?
-    vrc = RTHttpAddHeader(hHttp, "User-Agent", strUserAgent.c_str(), strUserAgent.length(), RTHTTPADDHDR_F_BACK);
+    int vrc = RTHttpAddHeader(hHttp, "User-Agent", strUserAgent.c_str(), strUserAgent.length(), RTHTTPADDHDR_F_BACK);
     if (RT_FAILURE(vrc))
         return setErrorVrc(vrc, tr("%s: RTHttpAddHeader() failed: %Rrc (on User-Agent)"), __FUNCTION__, vrc);
@@ -365 +387 @@
      */
     ProxyMode_T enmProxyMode;
-    rc = pSystemProperties->COMGETTER(ProxyMode)(&enmProxyMode);
-    if (FAILED(rc))
-        return setErrorVrc(rc, tr("%s: ISystemProperties::proxyMode() failed: %Rrc"), __FUNCTION__, rc);
+    HRESULT rc = ptrSystemProperties->COMGETTER(ProxyMode)(&enmProxyMode);
+    if (FAILED(rc))
+        return setError(rc, tr("%s: ISystemProperties::proxyMode() failed: %Rrc"), __FUNCTION__, rc);
 
     if (enmProxyMode == ProxyMode_Manual)
     {
         Bstr strProxyURL;
-        rc = pSystemProperties->COMGETTER(ProxyURL)(strProxyURL.asOutParam());
+        rc = ptrSystemProperties->COMGETTER(ProxyURL)(strProxyURL.asOutParam());
         if (FAILED(rc))
-            return setErrorVrc(rc, tr("%s: ISystemProperties::proxyURL() failed: %Rrc"), __FUNCTION__, rc);
+            return setError(rc, tr("%s: ISystemProperties::proxyURL() failed: %Rrc"), __FUNCTION__, rc);
         vrc = RTHttpSetProxyByUrl(hHttp, Utf8Str(strProxyURL).c_str());
         if (RT_FAILURE(vrc))
@@ -385 +407 @@
             return setErrorVrc(vrc, tr("%s: RTHttpUseSystemProxySettings() failed: %Rrc"), __FUNCTION__, vrc);
     }
+    else
+        Assert(enmProxyMode == ProxyMode_NoProxy);
 
     /*
@@ -395 +419 @@
         return setErrorVrc(vrc, tr("%s: RTHttpGetBinary() failed: %Rrc"), __FUNCTION__, vrc);
 
+    /** @todo this may throw and leak.   */
     RTCList<RTCString> lstHttpReply = RTCString((char *)pvResponse, (size_t)cbResponse).split(" ", RTCString::RemoveEmptyParts);
     RTHttpFreeResponse(pvResponse);
@@ -414 +439 @@
         LogRelFunc(("HTTP server reply = %s %s\n", lstHttpReply.at(0).c_str(), lstHttpReply.at(1).c_str()));
     }
-
-    // clean-up HTTP request paperwork
-    /** @todo r=bird: There is no chance that this would be NIL here unless
-     *        you've got stack corruption.  Besides, RTHttpDestruct ignores NIL. */
-    if (hHttp != NIL_RTHTTP)
-        RTHttpDestroy(hHttp);
 
     return S_OK;