STORM-1112 Protected LLProxy members during cross-thread calls to LLProxy::applyProxySettings()

3 files changed, 62 insertions, 34 deletions

diff --git a/indra/llmessage/llproxy.cpp b/indra/llmessage/llproxy.cpp
index 6040472cba..d34ad1a811 100644
--- a/indra/llmessage/llproxy.cpp
+++ b/indra/llmessage/llproxy.cpp
@@ -59,7 +59,10 @@ LLProxy::LLProxy():
 		mAuthMethodSelected(METHOD_NOAUTH),
 		mSocksUsername(),
 		mSocksPassword(),
-		mPool(gAPRPoolp)
+		mPool(gAPRPoolp),
+		mSOCKSAuthStrings(),
+		mHTTPProxyAddrStrings(),
+		mProxyMutex(0)
 {
 }
 
@@ -71,7 +74,7 @@ LLProxy::~LLProxy()
 
 	// Delete c_str versions of the addresses and credentials.
 	for_each(mSOCKSAuthStrings.begin(), mSOCKSAuthStrings.end(), DeletePointerArray());
-	for_each(mSOCKSAddrStrings.begin(), mSOCKSAddrStrings.end(), DeletePointerArray());
+	for_each(mHTTPProxyAddrStrings.begin(), mHTTPProxyAddrStrings.end(), DeletePointerArray());
 }
 
 // Perform a SOCKS 5 authentication and UDP association to the proxy
@@ -211,7 +214,7 @@ void LLProxy::stopProxy()
 
 	if (LLPROXY_SOCKS == mProxyType)
 	{
-		sHTTPProxyEnabled = false;
+		void disableHTTPProxy();
 	}
 
 	if (mProxyControlChannel)
@@ -234,42 +237,61 @@ void LLProxy::setAuthPassword(const std::string &username, const std::string &pa
 	U32 size = username.length() + password.length() + 2;
 	char* curl_auth_string  = new char[size];
 	snprintf(curl_auth_string, size, "%s:%s", username.c_str(), password.c_str());
+
+	LLMutexLock lock(&mProxyMutex);
 	mSOCKSAuthStrings.push_back(curl_auth_string);
 }
 
 void LLProxy::enableHTTPProxy(LLHost httpHost, LLHttpProxyType type)
 { 
+	LLMutexLock lock(&mProxyMutex);
+
 	sHTTPProxyEnabled = true;
 	mHTTPProxy        = httpHost;
 	mProxyType        = type;
 
 	U32 size = httpHost.getIPString().length() + 1;
-	char* socks_addr_string = new char[size];
-	strncpy(socks_addr_string, httpHost.getIPString().c_str(), size);
-	mSOCKSAddrStrings.push_back(socks_addr_string);
+	char* http_addr_string = new char[size];
+	strncpy(http_addr_string, httpHost.getIPString().c_str(), size);
+	mHTTPProxyAddrStrings.push_back(http_addr_string);
+}
+
+void LLProxy::enableHTTPProxy()
+{
+	LLMutexLock lock(&mProxyMutex);
+
+	sHTTPProxyEnabled = true;
+}
+
+void LLProxy::disableHTTPProxy()
+{
+	LLMutexLock lock(&mProxyMutex);
+
+	sHTTPProxyEnabled = false;
 }
 
 //static
 void LLProxy::cleanupClass()
 {
-	LLProxy::getInstance()->stopProxy();
+	getInstance()->stopProxy();
+	deleteSingleton();
 }
 
 // Apply proxy settings to CuRL request if either type of HTTP proxy is enabled.
 void LLProxy::applyProxySettings(LLCurl::Easy* handle)
 {
-	if (LLProxy::getInstance()->isHTTPProxyEnabled())
+	if (sHTTPProxyEnabled)
 	{
-		std::string address = LLProxy::getInstance()->getHTTPProxy().getIPString();
-		U16 port = LLProxy::getInstance()->getHTTPProxy().getPort();
+		std::string address = mHTTPProxy.getIPString();
+		U16 port = mHTTPProxy.getPort();
 		handle->setoptString(CURLOPT_PROXY, address.c_str());
 		handle->setopt(CURLOPT_PROXYPORT, port);
-		if (LLProxy::getInstance()->getHTTPProxyType() == LLPROXY_SOCKS)
+		if (mProxyType == LLPROXY_SOCKS)
 		{
 			handle->setopt(CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
-			if (LLProxy::getInstance()->getSelectedAuthMethod() == METHOD_PASSWORD)
+			if (mAuthMethodSelected == METHOD_PASSWORD)
 			{
-				handle->setoptString(CURLOPT_PROXYUSERPWD, LLProxy::getInstance()->getProxyUserPwdCURL());
+				handle->setoptString(CURLOPT_PROXYUSERPWD, getProxyUserPwdCURL());
 			}
 		}
 		else
@@ -281,18 +303,18 @@ void LLProxy::applyProxySettings(LLCurl::Easy* handle)
 
 void LLProxy::applyProxySettings(LLCurlEasyRequest* handle)
 {
-	if (LLProxy::getInstance()->isHTTPProxyEnabled())
+	if (sHTTPProxyEnabled)
 	{
-		std::string address = LLProxy::getInstance()->getHTTPProxy().getIPString();
-		U16 port = LLProxy::getInstance()->getHTTPProxy().getPort();
+		std::string address = mHTTPProxy.getIPString();
+		U16 port = mHTTPProxy.getPort();
 		handle->setoptString(CURLOPT_PROXY, address.c_str());
 		handle->setopt(CURLOPT_PROXYPORT, port);
-		if (LLProxy::getInstance()->getHTTPProxyType() == LLPROXY_SOCKS)
+		if (mProxyType == LLPROXY_SOCKS)
 		{
 			handle->setopt(CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
-			if (LLProxy::getInstance()->getSelectedAuthMethod() == METHOD_PASSWORD)
+			if (mAuthMethodSelected == METHOD_PASSWORD)
 			{
-				handle->setoptString(CURLOPT_PROXYUSERPWD, LLProxy::getInstance()->getProxyUserPwdCURL());
+				handle->setoptString(CURLOPT_PROXYUSERPWD, getProxyUserPwdCURL());
 			}
 		}
 		else
@@ -304,17 +326,17 @@ void LLProxy::applyProxySettings(LLCurlEasyRequest* handle)
 
 void LLProxy::applyProxySettings(CURL* handle)
 {
-	if (LLProxy::getInstance()->isHTTPProxyEnabled())
+	LLMutexLock lock(&mProxyMutex);
+	if (sHTTPProxyEnabled)
 	{
-		check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXY, mSOCKSAddrStrings.back()));
-
-		U16 port = LLProxy::getInstance()->getHTTPProxy().getPort();
+		check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXY, mHTTPProxyAddrStrings.back()));
+		U16 port = mHTTPProxy.getPort();
 		check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYPORT, port));
 
-		if (LLProxy::getInstance()->getHTTPProxyType() == LLPROXY_SOCKS)
+		if (mProxyType == LLPROXY_SOCKS)
 		{
 			check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5));
-			if (LLProxy::getInstance()->getSelectedAuthMethod() == METHOD_PASSWORD)
+			if (mAuthMethodSelected == METHOD_PASSWORD)
 			{
 				check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYUSERPWD, mSOCKSAuthStrings.back()));
 			}
diff --git a/indra/llmessage/llproxy.h b/indra/llmessage/llproxy.h
index 7d1431a4b3..df1ec9121e 100644
--- a/indra/llmessage/llproxy.h
+++ b/indra/llmessage/llproxy.h
@@ -32,6 +32,7 @@
 #include "lliosocket.h"
 #include "llmemory.h"
 #include "llsingleton.h"
+#include "llthread.h"
 #include <string>
 
 // Error codes returned from the StartProxy method
@@ -190,9 +191,10 @@ public:
 	// Proxy HTTP packets via httpHost, which can be a SOCKS 5 or a HTTP proxy
 	// as specified in type
 	void enableHTTPProxy(LLHost httpHost, LLHttpProxyType type);
+	void enableHTTPProxy();
 
 	// Stop proxying HTTP packets
-	void disableHTTPProxy() { sHTTPProxyEnabled = false; }
+	void disableHTTPProxy();
 
 	// Get the UDP proxy address and port
 	LLHost getUDPProxy() const { return mUDPProxy; }
@@ -218,16 +220,17 @@ public:
 	void applyProxySettings(LLCurlEasyRequest* handle);
 
 private:
-
 	// Open a communication channel to the SOCKS 5 proxy proxy, at port messagePort
 	S32 proxyHandshake(LLHost proxy, U32 messagePort);
 
+private:
 	// socket handle to proxy TCP control channel
 	LLSocket::ptr_t mProxyControlChannel;
 
-	// is the UDP proxy enabled?
+	// Is the UDP proxy enabled?
 	static bool sUDPProxyEnabled;
-	// is the http proxy enabled?
+	// Is the HTTP proxy enabled? 
+	// Do not toggle directly, use enableHTTPProxy() and disableHTTPProxy()
 	static bool sHTTPProxyEnabled;
 
 	// currently selected http proxy type
@@ -235,7 +238,7 @@ private:
 
 	// UDP proxy address and port
 	LLHost mUDPProxy;
-	// TCP Proxy control channel address and port
+	// TCP proxy control channel address and port
 	LLHost mTCPProxy;
 	// HTTP proxy address and port
 	LLHost mHTTPProxy;
@@ -248,9 +251,13 @@ private:
 	// SOCKS 5 password
 	std::string mSocksPassword;
 
-	// Vectors to store valid pointers to string options that have been passed to CURL requests.
+	// Vectors to store valid pointers to string options that might have been set on CURL requests.
+	// This results in a behavior similar to LLCurl::Easy::setoptstring()
 	std::vector<char*> mSOCKSAuthStrings;
-	std::vector<char*> mSOCKSAddrStrings;
+	std::vector<char*> mHTTPProxyAddrStrings;
+
+	// Mutex to protect members in cross-thread calls to applyProxySettings()
+	LLMutex mProxyMutex;
 
 	// APR pool for the socket
 	apr_pool_t* mPool;
diff --git a/indra/llmessage/llurlrequest.cpp b/indra/llmessage/llurlrequest.cpp
index 6fe9dce7d3..528830dc56 100644
--- a/indra/llmessage/llurlrequest.cpp
+++ b/indra/llmessage/llurlrequest.cpp
@@ -228,8 +228,7 @@ void LLURLRequest::useProxy(bool use_proxy)
         }
     }
 
-
-    lldebugs << "use_proxy = " << (use_proxy?'Y':'N') << ", env_proxy = " << (env_proxy ? env_proxy : "(null)") << llendl;
+    LL_DEBUGS("Proxy") << "use_proxy = " << (use_proxy?'Y':'N') << ", env_proxy = " << (env_proxy ? env_proxy : "(null)") << LL_ENDL;
 
     if (env_proxy && use_proxy)
     {