From 2416dd7cc94e765efbbf9069dbf0f6fb71404ebb Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 22 Jun 2017 14:01:04 -0400 Subject: [PATCH 1/3] net: separate resolving and conecting ConnectSocketByName handled resolves as necessary, obscuring the connection process. With them separated, each can be handled asynchronously. Also, since proxies must be considered now anyway, go ahead and eliminate the ConnectSocket wrapper and use ConnectSocketDirectly... directly. --- src/net.cpp | 58 +++++++++++++++++++++++++++++++++---------------- src/netbase.cpp | 4 ++-- src/netbase.h | 3 +++ 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 587c9e511..a9f26fb56 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -380,19 +380,16 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo pszDest ? pszDest : addrConnect.ToString(), pszDest ? 0.0 : (double)(GetAdjustedTime() - addrConnect.nTime)/3600.0); - // Connect - SOCKET hSocket; - bool proxyConnectionFailed = false; - if (pszDest ? ConnectSocketByName(addrConnect, hSocket, pszDest, Params().GetDefaultPort(), nConnectTimeout, &proxyConnectionFailed) : - ConnectSocket(addrConnect, hSocket, nConnectTimeout, &proxyConnectionFailed)) - { - if (!IsSelectableSocket(hSocket)) { - LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n"); - CloseSocket(hSocket); - return nullptr; - } - - if (pszDest && addrConnect.IsValid()) { + // Resolve + const int default_port = Params().GetDefaultPort(); + if (pszDest) { + std::vector resolved; + if (Lookup(pszDest, resolved, default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) { + addrConnect = CAddress(resolved[GetRand(resolved.size())], NODE_NONE); + if (!addrConnect.IsValid()) { + LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s", addrConnect.ToString(), pszDest); + return nullptr; + } // It is possible that we already have a connection to the IP/port pszDest resolved to. // In that case, drop the connection that was just created, and return the existing CNode instead. // Also store the name we used to connect in that CNode, so that future FindNode() calls to that @@ -402,13 +399,40 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo if (pnode) { pnode->MaybeSetAddrName(std::string(pszDest)); - CloseSocket(hSocket); LogPrintf("Failed to open new connection, already connected\n"); return nullptr; } } + } - addrman.Attempt(addrConnect, fCountFailure); + // Connect + bool connected = false; + SOCKET hSocket; + proxyType proxy; + if (addrConnect.IsValid()) { + bool proxyConnectionFailed = false; + + if (GetProxy(addrConnect.GetNetwork(), proxy)) + connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, &proxyConnectionFailed); + else // no proxy needed (none set for target network) + connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout); + if (!proxyConnectionFailed) { + // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to + // the proxy, mark this as an attempt. + addrman.Attempt(addrConnect, fCountFailure); + } + } else if (pszDest && GetNameProxy(proxy)) { + std::string host; + int port = default_port; + SplitHostPort(std::string(pszDest), port, host); + connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, nullptr); + } + if (connected) { + if (!IsSelectableSocket(hSocket)) { + LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n"); + CloseSocket(hSocket); + return nullptr; + } // Add node NodeId id = GetNewNodeId(); @@ -419,10 +443,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo pnode->AddRef(); return pnode; - } else if (!proxyConnectionFailed) { - // If connecting to the node failed, and failure is not caused by a problem connecting to - // the proxy, mark this as an attempt. - addrman.Attempt(addrConnect, fCountFailure); } return nullptr; diff --git a/src/netbase.cpp b/src/netbase.cpp index 05f9f6961..d4a91da1f 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -399,7 +399,7 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials return true; } -bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout) +bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout) { hSocketRet = INVALID_SOCKET; @@ -534,7 +534,7 @@ bool IsProxy(const CNetAddr &addr) { return false; } -static bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed) +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed) { SOCKET hSocket = INVALID_SOCKET; // first connect to proxy server diff --git a/src/netbase.h b/src/netbase.h index 6572f0a12..50425f0f8 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -44,6 +44,7 @@ bool GetProxy(enum Network net, proxyType &proxyInfoOut); bool IsProxy(const CNetAddr &addr); bool SetNameProxy(const proxyType &addrProxy); bool HaveNameProxy(); +bool GetNameProxy(proxyType &nameProxyOut); bool LookupHost(const char *pszName, std::vector& vIP, unsigned int nMaxSolutions, bool fAllowLookup); bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup); bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup); @@ -52,6 +53,8 @@ CService LookupNumeric(const char *pszName, int portDefault = 0); bool LookupSubNet(const char *pszName, CSubNet& subnet); bool ConnectSocket(const CService &addr, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed = nullptr); bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault, int nTimeout, bool *outProxyConnectionFailed = nullptr); +bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout); +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed); /** Return readable error string for a network error code */ std::string NetworkErrorString(int err); /** Close socket and set hSocket to INVALID_SOCKET */ From 45fd75453e59868471572907db237630ca2b7afd Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Fri, 23 Jun 2017 12:29:50 -0400 Subject: [PATCH 2/3] net: remove now-superfluous numeric resolve This was added in order to help OpenNetworkConnection avoid creating a connection that it would end up aborting. It was necessary because resolving was done as part of the connection process. Now that resolving is separated from connecting, this case is detected before the connection is attempted. --- src/net.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index a9f26fb56..64b8fe726 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1934,11 +1934,9 @@ void CConnman::ThreadOpenAddedConnections() // the addednodeinfo state might change. break; } - // If strAddedNode is an IP/port, decode it immediately, so - // OpenNetworkConnection can detect existing connections to that IP/port. tried = true; - CService service(LookupNumeric(info.strAddedNode.c_str(), Params().GetDefaultPort())); - OpenNetworkConnection(CAddress(service, NODE_NONE), false, &grant, info.strAddedNode.c_str(), false, false, true); + CAddress addr(CService(), NODE_NONE); + OpenNetworkConnection(addr, false, &grant, info.strAddedNode.c_str(), false, false, true); if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) return; } From b887676e1b86ce03181b5876cf6203d617750d0a Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Fri, 23 Jun 2017 11:53:45 -0400 Subject: [PATCH 3/3] net: remove now-unused functions --- src/netbase.cpp | 41 ----------------------------------------- src/netbase.h | 2 -- 2 files changed, 43 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index d4a91da1f..9212be981 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -558,47 +558,6 @@ bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int hSocketRet = hSocket; return true; } - -bool ConnectSocket(const CService &addrDest, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed) -{ - proxyType proxy; - if (outProxyConnectionFailed) - *outProxyConnectionFailed = false; - - if (GetProxy(addrDest.GetNetwork(), proxy)) - return ConnectThroughProxy(proxy, addrDest.ToStringIP(), addrDest.GetPort(), hSocketRet, nTimeout, outProxyConnectionFailed); - else // no proxy needed (none set for target network) - return ConnectSocketDirectly(addrDest, hSocketRet, nTimeout); -} - -bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault, int nTimeout, bool *outProxyConnectionFailed) -{ - std::string strDest; - int port = portDefault; - - if (outProxyConnectionFailed) - *outProxyConnectionFailed = false; - - SplitHostPort(std::string(pszDest), port, strDest); - - proxyType proxy; - GetNameProxy(proxy); - - std::vector addrResolved; - if (Lookup(strDest.c_str(), addrResolved, port, fNameLookup && !HaveNameProxy(), 256)) { - if (addrResolved.size() > 0) { - addr = addrResolved[GetRand(addrResolved.size())]; - return ConnectSocket(addr, hSocketRet, nTimeout); - } - } - - addr = CService(); - - if (!HaveNameProxy()) - return false; - return ConnectThroughProxy(proxy, strDest, port, hSocketRet, nTimeout, outProxyConnectionFailed); -} - bool LookupSubNet(const char* pszName, CSubNet& ret) { std::string strSubnet(pszName); diff --git a/src/netbase.h b/src/netbase.h index 50425f0f8..e7d7bcb37 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -51,8 +51,6 @@ bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLoo bool Lookup(const char *pszName, std::vector& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); CService LookupNumeric(const char *pszName, int portDefault = 0); bool LookupSubNet(const char *pszName, CSubNet& subnet); -bool ConnectSocket(const CService &addr, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed = nullptr); -bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault, int nTimeout, bool *outProxyConnectionFailed = nullptr); bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout); bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed); /** Return readable error string for a network error code */