Skip to content

Commit ba2f195

Browse files
laanwjlaanwj
committed
Merge #11363: net: Split socket create/connect
3830b6e net: use CreateSocket for binds (Cory Fields) df3bcf8 net: pass socket closing responsibility up to caller for outgoing connections (Cory Fields) 9e3b2f5 net: Move IsSelectableSocket check into socket creation (Cory Fields) 1729c29 net: split socket creation out of connection (Cory Fields) Pull request description: Requirement for #11227. We'll need to create sockets and perform the actual connect in separate steps, so break them up. #11227 adds an RAII wrapper around connection attempts, as a belt-and-suspenders in case a CloseSocket is missed. Tree-SHA512: de675bb718cc56d68893c303b8057ca062c7431eaa17ae7c4829caed119fa3f15b404d8f52aca22a6bca6e73a26fb79e898b335d090ab015bf6456cf417fc694
2 parents ef8ba7d + 3830b6e commit ba2f195

File tree

3 files changed

+63
-82
lines changed

3 files changed

+63
-82
lines changed

‎src/net.cpp

Lines changed: 28 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -415,39 +415,48 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
415415
if (addrConnect.IsValid()) {
416416
bool proxyConnectionFailed = false;
417417

418-
if (GetProxy(addrConnect.GetNetwork(), proxy))
418+
if (GetProxy(addrConnect.GetNetwork(), proxy)) {
419+
hSocket = CreateSocket(proxy.proxy);
420+
if (hSocket == INVALID_SOCKET) {
421+
return nullptr;
422+
}
419423
connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, &proxyConnectionFailed);
420-
else // no proxy needed (none set for target network)
424+
} else {
425+
// no proxy needed (none set for target network)
426+
hSocket = CreateSocket(addrConnect);
427+
if (hSocket == INVALID_SOCKET) {
428+
return nullptr;
429+
}
421430
connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout);
431+
}
422432
if (!proxyConnectionFailed) {
423433
// If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to
424434
// the proxy, mark this as an attempt.
425435
addrman.Attempt(addrConnect, fCountFailure);
426436
}
427437
} else if (pszDest && GetNameProxy(proxy)) {
438+
hSocket = CreateSocket(proxy.proxy);
439+
if (hSocket == INVALID_SOCKET) {
440+
return nullptr;
441+
}
428442
std::string host;
429443
int port = default_port;
430444
SplitHostPort(std::string(pszDest), port, host);
431445
connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, nullptr);
432446
}
433-
if (connected) {
434-
if (!IsSelectableSocket(hSocket)) {
435-
LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
436-
CloseSocket(hSocket);
437-
return nullptr;
438-
}
439-
440-
// Add node
441-
NodeId id = GetNewNodeId();
442-
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
443-
CAddress addr_bind = GetBindAddress(hSocket);
444-
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false);
445-
pnode->AddRef();
446-
447-
return pnode;
447+
if (!connected) {
448+
CloseSocket(hSocket);
449+
return nullptr;
448450
}
449451

450-
return nullptr;
452+
// Add node
453+
NodeId id = GetNewNodeId();
454+
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
455+
CAddress addr_bind = GetBindAddress(hSocket);
456+
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false);
457+
pnode->AddRef();
458+
459+
return pnode;
451460
}
452461

453462
void CConnman::DumpBanlist()
@@ -2056,44 +2065,21 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b
20562065
return false;
20572066
}
20582067

2059-
SOCKET hListenSocket = socket(((struct sockaddr*)&sockaddr)->sa_family, SOCK_STREAM, IPPROTO_TCP);
2068+
SOCKET hListenSocket = CreateSocket(addrBind);
20602069
if (hListenSocket == INVALID_SOCKET)
20612070
{
20622071
strError = strprintf("Error: Couldn't open socket for incoming connections (socket returned error %s)", NetworkErrorString(WSAGetLastError()));
20632072
LogPrintf("%s\n", strError);
20642073
return false;
20652074
}
2066-
if (!IsSelectableSocket(hListenSocket))
2067-
{
2068-
strError = "Error: Couldn't create a listenable socket for incoming connections";
2069-
LogPrintf("%s\n", strError);
2070-
return false;
2071-
}
2072-
2073-
20742075
#ifndef WIN32
2075-
#ifdef SO_NOSIGPIPE
2076-
// Different way of disabling SIGPIPE on BSD
2077-
setsockopt(hListenSocket, SOL_SOCKET, SO_NOSIGPIPE, (void*)&nOne, sizeof(int));
2078-
#endif
20792076
// Allow binding if the port is still in TIME_WAIT state after
20802077
// the program was closed and restarted.
20812078
setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (void*)&nOne, sizeof(int));
2082-
// Disable Nagle's algorithm
2083-
setsockopt(hListenSocket, IPPROTO_TCP, TCP_NODELAY, (void*)&nOne, sizeof(int));
20842079
#else
20852080
setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (const char*)&nOne, sizeof(int));
2086-
setsockopt(hListenSocket, IPPROTO_TCP, TCP_NODELAY, (const char*)&nOne, sizeof(int));
20872081
#endif
20882082

2089-
// Set to non-blocking, incoming connections will also inherit this
2090-
if (!SetSocketNonBlocking(hListenSocket, true)) {
2091-
CloseSocket(hListenSocket);
2092-
strError = strprintf("BindListenPort: Setting listening socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
2093-
LogPrintf("%s\n", strError);
2094-
return false;
2095-
}
2096-
20972083
// some systems don't have IPV6_V6ONLY but are always v6only; others do have the option
20982084
// and enable it by default or not. Try to enable it, if possible.
20992085
if (addrBind.IsIPv6()) {

‎src/netbase.cpp

Lines changed: 32 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,11 @@ std::string Socks5ErrorString(uint8_t err)
313313
}
314314

315315
/** Connect using SOCKS5 (as described in RFC1928) */
316-
static bool Socks5(const std::string& strDest, int port, const ProxyCredentials *auth, SOCKET& hSocket)
316+
static bool Socks5(const std::string& strDest, int port, const ProxyCredentials *auth, const SOCKET& hSocket)
317317
{
318318
IntrRecvError recvr;
319319
LogPrint(BCLog::NET, "SOCKS5 connecting %s\n", strDest);
320320
if (strDest.size() > 255) {
321-
CloseSocket(hSocket);
322321
return error("Hostname too long");
323322
}
324323
// Accepted authentication methods
@@ -334,17 +333,14 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
334333
}
335334
ssize_t ret = send(hSocket, (const char*)vSocks5Init.data(), vSocks5Init.size(), MSG_NOSIGNAL);
336335
if (ret != (ssize_t)vSocks5Init.size()) {
337-
CloseSocket(hSocket);
338336
return error("Error sending to proxy");
339337
}
340338
uint8_t pchRet1[2];
341339
if ((recvr = InterruptibleRecv(pchRet1, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
342-
CloseSocket(hSocket);
343340
LogPrintf("Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure\n", strDest, port);
344341
return false;
345342
}
346343
if (pchRet1[0] != SOCKSVersion::SOCKS5) {
347-
CloseSocket(hSocket);
348344
return error("Proxy failed to initialize");
349345
}
350346
if (pchRet1[1] == SOCKS5Method::USER_PASS && auth) {
@@ -359,23 +355,19 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
359355
vAuth.insert(vAuth.end(), auth->password.begin(), auth->password.end());
360356
ret = send(hSocket, (const char*)vAuth.data(), vAuth.size(), MSG_NOSIGNAL);
361357
if (ret != (ssize_t)vAuth.size()) {
362-
CloseSocket(hSocket);
363358
return error("Error sending authentication to proxy");
364359
}
365360
LogPrint(BCLog::PROXY, "SOCKS5 sending proxy authentication %s:%s\n", auth->username, auth->password);
366361
uint8_t pchRetA[2];
367362
if ((recvr = InterruptibleRecv(pchRetA, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
368-
CloseSocket(hSocket);
369363
return error("Error reading proxy authentication response");
370364
}
371365
if (pchRetA[0] != 0x01 || pchRetA[1] != 0x00) {
372-
CloseSocket(hSocket);
373366
return error("Proxy authentication unsuccessful");
374367
}
375368
} else if (pchRet1[1] == SOCKS5Method::NOAUTH) {
376369
// Perform no authentication
377370
} else {
378-
CloseSocket(hSocket);
379371
return error("Proxy requested wrong authentication method %02x", pchRet1[1]);
380372
}
381373
std::vector<uint8_t> vSocks5;
@@ -389,12 +381,10 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
389381
vSocks5.push_back((port >> 0) & 0xFF);
390382
ret = send(hSocket, (const char*)vSocks5.data(), vSocks5.size(), MSG_NOSIGNAL);
391383
if (ret != (ssize_t)vSocks5.size()) {
392-
CloseSocket(hSocket);
393384
return error("Error sending to proxy");
394385
}
395386
uint8_t pchRet2[4];
396387
if ((recvr = InterruptibleRecv(pchRet2, 4, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
397-
CloseSocket(hSocket);
398388
if (recvr == IntrRecvError::Timeout) {
399389
/* If a timeout happens here, this effectively means we timed out while connecting
400390
* to the remote node. This is very common for Tor, so do not print an
@@ -405,17 +395,14 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
405395
}
406396
}
407397
if (pchRet2[0] != SOCKSVersion::SOCKS5) {
408-
CloseSocket(hSocket);
409398
return error("Proxy failed to accept request");
410399
}
411400
if (pchRet2[1] != SOCKS5Reply::SUCCEEDED) {
412401
// Failures to connect to a peer that are not proxy errors
413-
CloseSocket(hSocket);
414402
LogPrintf("Socks5() connect to %s:%d failed: %s\n", strDest, port, Socks5ErrorString(pchRet2[1]));
415403
return false;
416404
}
417405
if (pchRet2[2] != 0x00) { // Reserved field must be 0
418-
CloseSocket(hSocket);
419406
return error("Error: malformed proxy response");
420407
}
421408
uint8_t pchRet3[256];
@@ -427,41 +414,42 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
427414
{
428415
recvr = InterruptibleRecv(pchRet3, 1, SOCKS5_RECV_TIMEOUT, hSocket);
429416
if (recvr != IntrRecvError::OK) {
430-
CloseSocket(hSocket);
431417
return error("Error reading from proxy");
432418
}
433419
int nRecv = pchRet3[0];
434420
recvr = InterruptibleRecv(pchRet3, nRecv, SOCKS5_RECV_TIMEOUT, hSocket);
435421
break;
436422
}
437-
default: CloseSocket(hSocket); return error("Error: malformed proxy response");
423+
default: return error("Error: malformed proxy response");
438424
}
439425
if (recvr != IntrRecvError::OK) {
440-
CloseSocket(hSocket);
441426
return error("Error reading from proxy");
442427
}
443428
if ((recvr = InterruptibleRecv(pchRet3, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
444-
CloseSocket(hSocket);
445429
return error("Error reading from proxy");
446430
}
447431
LogPrint(BCLog::NET, "SOCKS5 connected %s\n", strDest);
448432
return true;
449433
}
450434

451-
bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout)
435+
SOCKET CreateSocket(const CService &addrConnect)
452436
{
453-
hSocketRet = INVALID_SOCKET;
454-
455437
struct sockaddr_storage sockaddr;
456438
socklen_t len = sizeof(sockaddr);
457439
if (!addrConnect.GetSockAddr((struct sockaddr*)&sockaddr, &len)) {
458-
LogPrintf("Cannot connect to %s: unsupported network\n", addrConnect.ToString());
459-
return false;
440+
LogPrintf("Cannot create socket for %s: unsupported network\n", addrConnect.ToString());
441+
return INVALID_SOCKET;
460442
}
461443

462444
SOCKET hSocket = socket(((struct sockaddr*)&sockaddr)->sa_family, SOCK_STREAM, IPPROTO_TCP);
463445
if (hSocket == INVALID_SOCKET)
464-
return false;
446+
return INVALID_SOCKET;
447+
448+
if (!IsSelectableSocket(hSocket)) {
449+
CloseSocket(hSocket);
450+
LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
451+
return INVALID_SOCKET;
452+
}
465453

466454
#ifdef SO_NOSIGPIPE
467455
int set = 1;
@@ -475,9 +463,23 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
475463
// Set to non-blocking
476464
if (!SetSocketNonBlocking(hSocket, true)) {
477465
CloseSocket(hSocket);
478-
return error("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
466+
LogPrintf("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
479467
}
468+
return hSocket;
469+
}
480470

471+
bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout)
472+
{
473+
struct sockaddr_storage sockaddr;
474+
socklen_t len = sizeof(sockaddr);
475+
if (hSocket == INVALID_SOCKET) {
476+
LogPrintf("Cannot connect to %s: invalid socket\n", addrConnect.ToString());
477+
return false;
478+
}
479+
if (!addrConnect.GetSockAddr((struct sockaddr*)&sockaddr, &len)) {
480+
LogPrintf("Cannot connect to %s: unsupported network\n", addrConnect.ToString());
481+
return false;
482+
}
481483
if (connect(hSocket, (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR)
482484
{
483485
int nErr = WSAGetLastError();
@@ -492,13 +494,11 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
492494
if (nRet == 0)
493495
{
494496
LogPrint(BCLog::NET, "connection to %s timeout\n", addrConnect.ToString());
495-
CloseSocket(hSocket);
496497
return false;
497498
}
498499
if (nRet == SOCKET_ERROR)
499500
{
500501
LogPrintf("select() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
501-
CloseSocket(hSocket);
502502
return false;
503503
}
504504
socklen_t nRetSize = sizeof(nRet);
@@ -509,13 +509,11 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
509509
#endif
510510
{
511511
LogPrintf("getsockopt() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
512-
CloseSocket(hSocket);
513512
return false;
514513
}
515514
if (nRet != 0)
516515
{
517516
LogPrintf("connect() to %s failed after select(): %s\n", addrConnect.ToString(), NetworkErrorString(nRet));
518-
CloseSocket(hSocket);
519517
return false;
520518
}
521519
}
@@ -526,12 +524,9 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
526524
#endif
527525
{
528526
LogPrintf("connect() to %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
529-
CloseSocket(hSocket);
530527
return false;
531528
}
532529
}
533-
534-
hSocketRet = hSocket;
535530
return true;
536531
}
537532

@@ -583,9 +578,8 @@ bool IsProxy(const CNetAddr &addr) {
583578
return false;
584579
}
585580

586-
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed)
581+
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocket, int nTimeout, bool *outProxyConnectionFailed)
587582
{
588-
SOCKET hSocket = INVALID_SOCKET;
589583
// first connect to proxy server
590584
if (!ConnectSocketDirectly(proxy.proxy, hSocket, nTimeout)) {
591585
if (outProxyConnectionFailed)
@@ -597,14 +591,14 @@ bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int
597591
ProxyCredentials random_auth;
598592
static std::atomic_int counter(0);
599593
random_auth.username = random_auth.password = strprintf("%i", counter++);
600-
if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket))
594+
if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket)) {
601595
return false;
596+
}
602597
} else {
603-
if (!Socks5(strDest, (unsigned short)port, 0, hSocket))
598+
if (!Socks5(strDest, (unsigned short)port, 0, hSocket)) {
604599
return false;
600+
}
605601
}
606-
607-
hSocketRet = hSocket;
608602
return true;
609603
}
610604
bool LookupSubNet(const char* pszName, CSubNet& ret)

‎src/netbase.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLoo
5151
bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
5252
CService LookupNumeric(const char *pszName, int portDefault = 0);
5353
bool LookupSubNet(const char *pszName, CSubNet& subnet);
54-
bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout);
55-
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
54+
SOCKET CreateSocket(const CService &addrConnect);
55+
bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocketRet, int nTimeout);
56+
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
5657
/** Return readable error string for a network error code */
5758
std::string NetworkErrorString(int err);
5859
/** Close socket and set hSocket to INVALID_SOCKET */

0 commit comments

Comments
 (0)