LLProxy code review fixes.

* Removed check_curl_code and check_curl_multi_code from the global namespace.
 * Added comments documenting which thread the public methods of LLProxy should be called from.
 * Corrected grammar in LLSingleton.h
 * Fixed a buffer scope problem in llpacketring.cpp.
master
Logan Dethrow 2011-09-06 16:54:53 -04:00
parent 9a1e8d78fe
commit a8d49f7cf5
7 changed files with 118 additions and 112 deletions

View File

@ -144,7 +144,7 @@ public:
* method to destroy the singleton early can prevent these crashes.
*
* An example where this is needed is for a LLSingleton that has an APR
* object as a member and make APR calls on destruction. The APR system is
* object as a member that makes APR calls on destruction. The APR system is
* shut down explicitly before main() exits. This causes a crash on exit.
* Using this method before the call to apr_terminate() and NOT calling
* getInstance() again will prevent the crash.

View File

@ -1213,3 +1213,13 @@ void LLCurl::cleanupClass()
}
const unsigned int LLCurl::MAX_REDIRECTS = 5;
// Provide access to LLCurl free functions outside of llcurl.cpp without polluting the global namespace.
void LLCurlFF::check_curl_code(CURLcode code)
{
check_curl_code(code);
}
void LLCurlFF::check_curl_multi_code(CURLMcode code)
{
check_curl_multi_code(code);
}

View File

@ -371,7 +371,12 @@ private:
bool mResultReturned;
};
void check_curl_code(CURLcode code);
void check_curl_multi_code(CURLMcode code);
// Provide access to LLCurl free functions outside of llcurl.cpp without polluting the global namespace.
namespace LLCurlFF
{
void check_curl_code(CURLcode code);
void check_curl_multi_code(CURLMcode code);
}
#endif // LL_LLCURL_H

View File

@ -228,13 +228,13 @@ S32 LLPacketRing::receivePacket (S32 socket, char *datap)
if (LLProxy::isSOCKSProxyEnabled())
{
U8 buffer[NET_BUFFER_SIZE + SOCKS_HEADER_SIZE];
packet_size = receive_packet(socket, reinterpret_cast<char *>(buffer));
packet_size = receive_packet(socket, static_cast<char*>(static_cast<void*>(buffer)));
if (packet_size > SOCKS_HEADER_SIZE)
{
// *FIX We are assuming ATYP is 0x01 (IPv4), not 0x03 (hostname) or 0x04 (IPv6)
memcpy(datap, buffer + SOCKS_HEADER_SIZE, packet_size - SOCKS_HEADER_SIZE);
proxywrap_t * header = reinterpret_cast<proxywrap_t *>(buffer);
proxywrap_t * header = static_cast<proxywrap_t*>(static_cast<void*>(buffer));
mLastSender.setAddress(header->addr);
mLastSender.setPort(ntohs(header->port));
@ -353,14 +353,20 @@ BOOL LLPacketRing::sendPacketImpl(int h_socket, const char * send_buffer, S32 bu
return send_packet(h_socket, send_buffer, buf_size, host.getAddress(), host.getPort());
}
proxywrap_t *socks_header = reinterpret_cast<proxywrap_t *>(&mProxyWrappedSendBuffer);
char headered_send_buffer[NET_BUFFER_SIZE + SOCKS_HEADER_SIZE];
proxywrap_t *socks_header = static_cast<proxywrap_t*>(static_cast<void*>(&headered_send_buffer));
socks_header->rsv = 0;
socks_header->addr = host.getAddress();
socks_header->port = htons(host.getPort());
socks_header->atype = ADDRESS_IPV4;
socks_header->frag = 0;
memcpy(mProxyWrappedSendBuffer + SOCKS_HEADER_SIZE, send_buffer, buf_size);
memcpy(headered_send_buffer + SOCKS_HEADER_SIZE, send_buffer, buf_size);
return send_packet(h_socket, (const char*) mProxyWrappedSendBuffer, buf_size + 10, LLProxy::getInstance()->getUDPProxy().getAddress(), LLProxy::getInstance()->getUDPProxy().getPort());
return send_packet( h_socket,
headered_send_buffer,
buf_size + SOCKS_HEADER_SIZE,
LLProxy::getInstance()->getUDPProxy().getAddress(),
LLProxy::getInstance()->getUDPProxy().getPort());
}

View File

@ -83,9 +83,6 @@ protected:
LLHost mLastSender;
LLHost mLastReceivingIF;
U8 mProxyWrappedSendBuffer[NET_BUFFER_SIZE + SOCKS_HEADER_SIZE];
private:
BOOL sendPacketImpl(int h_socket, const char * send_buffer, S32 buf_size, LLHost host);
};

View File

@ -43,7 +43,7 @@
bool LLProxy::sUDPProxyEnabled = false;
// Some helpful TCP static functions.
static S32 tcp_blocking_handshake(LLSocket::ptr_t handle, char * dataout, apr_size_t outlen, char * datain, apr_size_t maxinlen); // Do a TCP data handshake
static apr_status_t tcp_blocking_handshake(LLSocket::ptr_t handle, char * dataout, apr_size_t outlen, char * datain, apr_size_t maxinlen); // Do a TCP data handshake
static LLSocket::ptr_t tcp_open_channel(apr_pool_t* pool, LLHost host); // Open a TCP channel to a given host
static void tcp_close_channel(LLSocket::ptr_t* handle_ptr); // Close an open TCP channel
@ -52,7 +52,6 @@ LLProxy::LLProxy():
mProxyMutex(0),
mUDPProxy(),
mTCPProxy(),
mPool(gAPRPoolp),
mHTTPProxy(),
mProxyType(LLPROXY_SOCKS),
mAuthMethodSelected(METHOD_NOAUTH),
@ -64,14 +63,13 @@ LLProxy::LLProxy():
LLProxy::~LLProxy()
{
stopSOCKSProxy();
sUDPProxyEnabled = false;
mHTTPProxyEnabled = false;
disableHTTPProxy();
}
/**
* @brief Open the SOCKS 5 TCP control channel.
*
* Perform a SOCKS 5 authentication and UDP association to the proxy server.
* Perform a SOCKS 5 authentication and UDP association with the proxy server.
*
* @param proxy The SOCKS 5 server to connect to.
* @return SOCKS_OK if successful, otherwise a socks error code from llproxy.h.
@ -84,11 +82,15 @@ S32 LLProxy::proxyHandshake(LLHost proxy)
socks_auth_request_t socks_auth_request;
socks_auth_response_t socks_auth_response;
socks_auth_request.version = SOCKS_VERSION; // SOCKS version 5
socks_auth_request.num_methods = 1; // Sending 1 method.
socks_auth_request.methods = getSelectedAuthMethod(); // Send only the selected method.
socks_auth_request.version = SOCKS_VERSION; // SOCKS version 5
socks_auth_request.num_methods = 1; // Sending 1 method.
socks_auth_request.methods = getSelectedAuthMethod(); // Send only the selected method.
result = tcp_blocking_handshake(mProxyControlChannel, (char*)&socks_auth_request, sizeof(socks_auth_request), (char*)&socks_auth_response, sizeof(socks_auth_response));
result = tcp_blocking_handshake(mProxyControlChannel,
static_cast<char*>(static_cast<void*>(&socks_auth_request)),
sizeof(socks_auth_request),
static_cast<char*>(static_cast<void*>(&socks_auth_response)),
sizeof(socks_auth_response));
if (result != APR_SUCCESS)
{
LL_WARNS("Proxy") << "SOCKS authentication request failed, error on TCP control channel : " << result << LL_ENDL;
@ -103,7 +105,7 @@ S32 LLProxy::proxyHandshake(LLHost proxy)
return SOCKS_NOT_ACCEPTABLE;
}
// SOCKS 5 USERNAME/PASSWORD authentication
/* SOCKS 5 USERNAME/PASSWORD authentication */
if (socks_auth_response.method == METHOD_PASSWORD)
{
// The server has requested a username/password combination
@ -115,11 +117,15 @@ S32 LLProxy::proxyHandshake(LLHost proxy)
password_auth[1] = socks_username.size();
memcpy(&password_auth[2], socks_username.c_str(), socks_username.size());
password_auth[socks_username.size() + 2] = socks_password.size();
memcpy(&password_auth[socks_username.size()+3], socks_password.c_str(), socks_password.size());
memcpy(&password_auth[socks_username.size() + 3], socks_password.c_str(), socks_password.size());
authmethod_password_reply_t password_reply;
result = tcp_blocking_handshake(mProxyControlChannel, password_auth, request_size, (char*)&password_reply, sizeof(password_reply));
result = tcp_blocking_handshake(mProxyControlChannel,
password_auth,
request_size,
static_cast<char*>(static_cast<void*>(&password_reply)),
sizeof(password_reply));
delete[] password_auth;
if (result != APR_SUCCESS)
@ -151,7 +157,11 @@ S32 LLProxy::proxyHandshake(LLHost proxy)
// "If the client is not in possession of the information at the time of the UDP ASSOCIATE,
// the client MUST use a port number and address of all zeros. RFC 1928"
result = tcp_blocking_handshake(mProxyControlChannel, (char*)&connect_request, sizeof(connect_request), (char*)&connect_reply, sizeof(connect_reply));
result = tcp_blocking_handshake(mProxyControlChannel,
static_cast<char*>(static_cast<void*>(&connect_request)),
sizeof(connect_request),
static_cast<char*>(static_cast<void*>(&connect_reply)),
sizeof(connect_reply));
if (result != APR_SUCCESS)
{
LL_WARNS("Proxy") << "SOCKS connect request failed, error on TCP control channel : " << result << LL_ENDL;
@ -170,6 +180,8 @@ S32 LLProxy::proxyHandshake(LLHost proxy)
mUDPProxy.setAddress(proxy.getAddress());
// The connection was successful. We now have the UDP port to send requests that need forwarding to.
LL_INFOS("Proxy") << "SOCKS 5 UDP proxy connected on " << mUDPProxy << LL_ENDL;
sUDPProxyEnabled = true;
return SOCKS_OK;
}
@ -177,7 +189,8 @@ S32 LLProxy::proxyHandshake(LLHost proxy)
* @brief Initiates a SOCKS 5 proxy session.
*
* Performs basic checks on host to verify that it is a valid address. Opens the control channel
* and then negotiates the proxy connection with the server.
* and then negotiates the proxy connection with the server. Closes any existing SOCKS
* connection before proceeding. Also disables an HTTP proxy if it is using SOCKS as the proxy.
*
*
* @param host Socks server to connect to.
@ -185,45 +198,32 @@ S32 LLProxy::proxyHandshake(LLHost proxy)
*/
S32 LLProxy::startSOCKSProxy(LLHost host)
{
S32 status = SOCKS_OK;
if (host.isOk())
{
mTCPProxy = host;
}
else
{
status = SOCKS_INVALID_HOST;
return SOCKS_INVALID_HOST;
}
if (mProxyControlChannel && status == SOCKS_OK)
// Close any running SOCKS connection.
stopSOCKSProxy();
mProxyControlChannel = tcp_open_channel(gAPRPoolp, mTCPProxy);
if (!mProxyControlChannel)
{
tcp_close_channel(&mProxyControlChannel);
return SOCKS_HOST_CONNECT_FAILED;
}
if (status == SOCKS_OK)
{
mProxyControlChannel = tcp_open_channel(mPool, mTCPProxy);
if (!mProxyControlChannel)
{
status = SOCKS_HOST_CONNECT_FAILED;
}
}
S32 status = proxyHandshake(mTCPProxy);
if (status == SOCKS_OK)
{
status = proxyHandshake(mTCPProxy);
}
if (status == SOCKS_OK)
{
sUDPProxyEnabled = true;
}
else
if (status != SOCKS_OK)
{
// Shut down the proxy if any of the above steps failed.
stopSOCKSProxy();
}
return status;
}
@ -244,7 +244,7 @@ void LLProxy::stopSOCKSProxy()
if (LLPROXY_SOCKS == getHTTPProxyType())
{
void disableHTTPProxy();
disableHTTPProxy();
}
if (mProxyControlChannel)
@ -352,16 +352,6 @@ void LLProxy::disableHTTPProxy()
mHTTPProxyEnabled = false;
}
/**
* @brief Get the HTTP proxy address and port
*/
//
LLHost LLProxy::getHTTPProxy() const
{
LLMutexLock lock(&mProxyMutex);
return mHTTPProxy;
}
/**
* @brief Get the currently selected HTTP proxy type
*/
@ -443,21 +433,21 @@ void LLProxy::applyProxySettings(CURL* handle)
// Now test again to verify that the proxy wasn't disabled between the first check and the lock.
if (mHTTPProxyEnabled)
{
check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXY, mHTTPProxy.getIPString().c_str()));
check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYPORT, mHTTPProxy.getPort()));
LLCurlFF::check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXY, mHTTPProxy.getIPString().c_str()));
LLCurlFF::check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYPORT, mHTTPProxy.getPort()));
if (mProxyType == LLPROXY_SOCKS)
{
check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5));
LLCurlFF::check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5));
if (mAuthMethodSelected == METHOD_PASSWORD)
{
std::string auth_string = mSocksUsername + ":" + mSocksPassword;
check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYUSERPWD, auth_string.c_str()));
LLCurlFF::check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYUSERPWD, auth_string.c_str()));
}
}
else
{
check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYTYPE, CURLPROXY_HTTP));
LLCurlFF::check_curl_code(curl_easy_setopt(handle, CURLOPT_PROXYTYPE, CURLPROXY_HTTP));
}
}
}
@ -476,7 +466,7 @@ void LLProxy::applyProxySettings(CURL* handle)
* @param maxinlen Maximum possible length of received data. Short reads are allowed.
* @return Indicates APR status code of exchange. APR_SUCCESS if exchange was successful, -1 if invalid data length was received.
*/
static S32 tcp_blocking_handshake(LLSocket::ptr_t handle, char * dataout, apr_size_t outlen, char * datain, apr_size_t maxinlen)
static apr_status_t tcp_blocking_handshake(LLSocket::ptr_t handle, char * dataout, apr_size_t outlen, char * datain, apr_size_t maxinlen)
{
apr_socket_t* apr_socket = handle->getSocket();
apr_status_t rv = APR_SUCCESS;
@ -544,7 +534,7 @@ static LLSocket::ptr_t tcp_open_channel(apr_pool_t* pool, LLHost host)
/**
* @brief Close the socket.
*
* @param handle_ptr A pointer-to-pointer to avoid increasing the use count.
* @param handle_ptr The handle of the socket being closed. A pointer-to-pointer to avoid increasing the use count.
*/
static void tcp_close_channel(LLSocket::ptr_t* handle_ptr)
{

View File

@ -226,63 +226,68 @@ public:
/*###########################################################################################
METHODS THAT DO NOT LOCK mProxyMutex!
###########################################################################################*/
// Constructor, cannot have parameters due to LLSingleton parent class. Call from main thread only.
LLProxy();
// static check for enabled status for UDP packets
// Static check for enabled status for UDP packets. Call from main thread only.
static bool isSOCKSProxyEnabled() { return sUDPProxyEnabled; }
// check for enabled status for HTTP packets
// mHTTPProxyEnabled is atomic, so no locking is required for thread safety.
bool isHTTPProxyEnabled() const { return mHTTPProxyEnabled; }
// Get the UDP proxy address and port
// Get the UDP proxy address and port. Call from main thread only.
LLHost getUDPProxy() const { return mUDPProxy; }
// Get the SOCKS 5 TCP control channel address and port
LLHost getTCPProxy() const { return mTCPProxy; }
/*###########################################################################################
END OF NON-LOCKING METHODS
###########################################################################################*/
/*###########################################################################################
METHODS THAT DO LOCK mProxyMutex! DO NOT CALL WHILE mProxyMutex IS LOCKED!
METHODS THAT LOCK mProxyMutex! DO NOT CALL WHILE mProxyMutex IS LOCKED!
###########################################################################################*/
// Destructor, closes open connections. Do not call directly, use cleanupClass().
~LLProxy();
// Start a connection to the SOCKS 5 proxy
S32 startSOCKSProxy(LLHost host);
// Disconnect and clean up any connection to the SOCKS 5 proxy
void stopSOCKSProxy();
// Delete LLProxy singleton, destroying the APR pool used by the control channel.
// Delete LLProxy singleton. Allows the apr_socket used in the SOCKS 5 control channel to be
// destroyed before the call to apr_terminate. Call from main thread only.
static void cleanupClass();
// Set up to use Password auth when connecting to the SOCKS proxy
bool setAuthPassword(const std::string &username, const std::string &password);
// Set up to use No Auth when connecting to the SOCKS proxy
void setAuthNone();
// Get the currently selected auth method.
LLSocks5AuthType getSelectedAuthMethod() const;
// Proxy HTTP packets via httpHost, which can be a SOCKS 5 or a HTTP proxy
// as specified in type
bool enableHTTPProxy(LLHost httpHost, LLHttpProxyType type);
bool enableHTTPProxy();
// Stop proxying HTTP packets
void disableHTTPProxy();
// Apply the current proxy settings to a curl request. Doesn't do anything if mHTTPProxyEnabled is false.
// Safe to call from any thread.
void applyProxySettings(CURL* handle);
void applyProxySettings(LLCurl::Easy* handle);
void applyProxySettings(LLCurlEasyRequest* handle);
// Get the HTTP proxy address and port
LLHost getHTTPProxy() const;
// Start a connection to the SOCKS 5 proxy. Call from main thread only.
S32 startSOCKSProxy(LLHost host);
// Disconnect and clean up any connection to the SOCKS 5 proxy. Call from main thread only.
void stopSOCKSProxy();
// Use Password auth when connecting to the SOCKS proxy. Call from main thread only.
bool setAuthPassword(const std::string &username, const std::string &password);
// Disable authentication when connecting to the SOCKS proxy. Call from main thread only.
void setAuthNone();
// Proxy HTTP packets via httpHost, which can be a SOCKS 5 or a HTTP proxy.
// as specified in type. Call from main thread only.
bool enableHTTPProxy(LLHost httpHost, LLHttpProxyType type);
bool enableHTTPProxy();
// Stop proxying HTTP packets. Call from main thread only.
void disableHTTPProxy();
/*###########################################################################################
END OF LOCKING METHODS
###########################################################################################*/
private:
/*###########################################################################################
METHODS THAT LOCK mProxyMutex! DO NOT CALL WHILE mProxyMutex IS LOCKED!
###########################################################################################*/
// Perform a SOCKS 5 authentication and UDP association with the proxy server.
S32 proxyHandshake(LLHost proxy);
// Get the currently selected auth method.
LLSocks5AuthType getSelectedAuthMethod() const;
// Get the currently selected HTTP proxy type
LLHttpProxyType getHTTPProxyType() const;
@ -293,17 +298,13 @@ public:
/*###########################################################################################
END OF LOCKING METHODS
###########################################################################################*/
private:
// Open a communication channel to the SOCKS 5 proxy proxy, at port messagePort.
S32 proxyHandshake(LLHost proxy);
private:
// Is the HTTP proxy enabled?
// Safe to read in any thread, do not write directly,
// use enableHTTPProxy() and disableHTTPProxy() instead.
// Is the HTTP proxy enabled? Safe to read in any thread, but do not write directly.
// Instead use enableHTTPProxy() and disableHTTPProxy() instead.
mutable LLAtomic32<bool> mHTTPProxyEnabled;
// Mutex to protect shared members in non-main thread calls to applyProxySettings()
// Mutex to protect shared members in non-main thread calls to applyProxySettings().
mutable LLMutex mProxyMutex;
/*###########################################################################################
@ -321,9 +322,6 @@ private:
// socket handle to proxy TCP control channel
LLSocket::ptr_t mProxyControlChannel;
// APR pool for the socket
apr_pool_t* mPool;
/*###########################################################################################
END OF UNSHARED MEMBERS
###########################################################################################*/