LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #15654: net: Remove unused unsanitized user agent string CNode::strSubVer

Browse source 
fa8548c5d1 net: Remove unused unsanitized user agent string CNode::strSubVer (MarcoFalke)

Pull request description:

  I fail to see a use case for this unsanitized byte array. In fact this can easily be confused with `cleanSubVer` and be displayed to the user (or logged) by a simple typo that is hard to find in review.

  Further reading: https://btcinformation.org/en/developer-reference#version

ACKs for commit fa8548:
  promag:
    utACK fa8548c, good catch.
  practicalswift:
    utACK fa8548c5d1
  sipa:
    utACK fa8548c5d1

Tree-SHA512: 3c3ff1504d1583ad099df9a6aa761458a82ec48a58ef7aaa9b5679a5281dd1b59036ba2932ed708488951a565b669a3083ef70be5a58472ff8677b971162ae2f
This commit is contained in:
MarcoFalke 2019-04-04 16:45:12 -04:00
commit c83442e174
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
3 changed files with 8 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/net.cpp
View file

@ -2627,7 +2627,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
{
hSocket = hSocketIn;
addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
strSubVer = "";
hashContinue = uint256();
filterInventoryKnown.reset();
pfilter = MakeUnique<CBloomFilter>();

14
src/net.h
View file

@ -53,7 +53,7 @@ static const unsigned int MAX_LOCATOR_SZ = 101;
static const unsigned int MAX_ADDR_TO_SEND = 1000;
/** Maximum length of incoming protocol messages (no message over 4 MB is currently acceptable). */
static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000;
/** Maximum length of strSubVer in `version` message */
/** Maximum length of the user agent string in `version` message */
static const unsigned int MAX_SUBVERSION_LENGTH = 256;
/** Maximum number of automatic outgoing nodes */
static const int MAX_OUTBOUND_CONNECTIONS = 8;
@ -650,12 +650,12 @@ public:
// Bind address of our side of the connection
const CAddress addrBind;
std::atomic<int> nVersion{0};
// strSubVer is whatever byte array we read from the wire. However, this field is intended
// to be printed out, displayed to humans in various forms and so on. So we sanitize it and
// store the sanitized version in cleanSubVer. The original should be used when dealing with
// the network or wire types and the cleaned string used when displayed or logged.
std::string strSubVer GUARDED_BY(cs_SubVer), cleanSubVer GUARDED_BY(cs_SubVer);
CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer
RecursiveMutex cs_SubVer;
/**
* cleanSubVer is a sanitized string of the user agent byte array we read
* from the wire. This cleaned string can safely be logged or displayed.
*/
std::string cleanSubVer GUARDED_BY(cs_SubVer){};
bool m_prefer_evict{false}; // This peer is preferred for eviction.
bool fWhitelisted{false}; // This peer can bypass DoS banning.
bool fFeeler{false}; // If true this node is being used as a short lived feeler.

3
src/net_processing.cpp
View file

@ -1843,7 +1843,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
ServiceFlags nServices;
int nVersion;
int nSendVersion;
std::string strSubVer;
std::string cleanSubVer;
int nStartingHeight = -1;
bool fRelay = true;
@ -1880,6 +1879,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
if (!vRecv.empty())
vRecv >> addrFrom >> nNonce;
if (!vRecv.empty()) {
std::string strSubVer;
vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH);
cleanSubVer = SanitizeString(strSubVer);
}
@ -1911,7 +1911,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
pfrom->SetAddrLocal(addrMe);
{
LOCK(pfrom->cs_SubVer);
pfrom->strSubVer = strSubVer;
pfrom->cleanSubVer = cleanSubVer;
}
pfrom->nStartingHeight = nStartingHeight;