From ba6a4ea344e28d61224dc54a8f816276afa2bfa3 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Thu, 21 Nov 2013 17:38:29 +0400 Subject: [PATCH 1/2] Add some additional logging to give extra network insight. --- src/main.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 705e94fab..a39dd9e8e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -797,9 +797,6 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa g_signals.EraseTransaction(ptxOld->GetHash()); g_signals.SyncTransaction(hash, tx, NULL); - LogPrint("mempool", "AcceptToMemoryPool: : accepted %s (poolsz %"PRIszu")\n", - hash.ToString().c_str(), - pool.mapTx.size()); return true; } @@ -3168,7 +3165,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) pfrom->fSuccessfullyConnected = true; - LogPrintf("receive version message: version %d, blocks=%d, us=%s, them=%s, peer=%s\n", pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString().c_str(), addrFrom.ToString().c_str(), pfrom->addr.ToString().c_str()); + LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, them=%s, peer=%s\n", pfrom->strSubVer.c_str(), pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString().c_str(), addrFrom.ToString().c_str(), pfrom->addr.ToString().c_str()); AddTimeData(pfrom->addr, nTime); @@ -3427,6 +3424,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) vWorkQueue.push_back(inv.hash); vEraseQueue.push_back(inv.hash); + + LogPrint("mempool", "AcceptToMemoryPool: %s %s : accepted %s (poolsz %"PRIszu")\n", + pfrom->addr.ToString().c_str(), pfrom->strSubVer.c_str(), + tx.GetHash().ToString().c_str(), + mempool.mapTx.size()); + // Recursively process any orphan transactions that depended on this one for (unsigned int i = 0; i < vWorkQueue.size(); i++) { @@ -3475,7 +3478,10 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) } int nDoS = 0; if (state.IsInvalid(nDoS)) - { + { + LogPrint("mempool", "%s from %s %s was not accepted into the memory pool: %s\n", tx.GetHash().ToString().c_str(), + pfrom->addr.ToString().c_str(), pfrom->strSubVer.c_str(), + state.GetRejectReason().c_str()); pfrom->PushMessage("reject", strCommand, state.GetRejectCode(), state.GetRejectReason(), inv.hash); if (nDoS > 0) From a946aa8d3ec7009ac670eeb65a525efe5eeb6e84 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Tue, 26 Nov 2013 12:52:21 +0100 Subject: [PATCH 2/2] Store and use a sanitized subVer --- src/main.cpp | 12 +++++++----- src/net.cpp | 2 +- src/net.h | 8 ++++++-- src/rpcnet.cpp | 5 ++++- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a39dd9e8e..b6888f975 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3097,8 +3097,10 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) pfrom->nVersion = 300; if (!vRecv.empty()) vRecv >> addrFrom >> nNonce; - if (!vRecv.empty()) + if (!vRecv.empty()) { vRecv >> pfrom->strSubVer; + pfrom->cleanSubVer = SanitizeString(pfrom->strSubVer); + } if (!vRecv.empty()) vRecv >> pfrom->nStartingHeight; if (!vRecv.empty()) @@ -3165,7 +3167,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) pfrom->fSuccessfullyConnected = true; - LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, them=%s, peer=%s\n", pfrom->strSubVer.c_str(), pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString().c_str(), addrFrom.ToString().c_str(), pfrom->addr.ToString().c_str()); + LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, them=%s, peer=%s\n", pfrom->cleanSubVer.c_str(), pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString().c_str(), addrFrom.ToString().c_str(), pfrom->addr.ToString().c_str()); AddTimeData(pfrom->addr, nTime); @@ -3426,7 +3428,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) LogPrint("mempool", "AcceptToMemoryPool: %s %s : accepted %s (poolsz %"PRIszu")\n", - pfrom->addr.ToString().c_str(), pfrom->strSubVer.c_str(), + pfrom->addr.ToString().c_str(), pfrom->cleanSubVer.c_str(), tx.GetHash().ToString().c_str(), mempool.mapTx.size()); @@ -3480,7 +3482,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) if (state.IsInvalid(nDoS)) { LogPrint("mempool", "%s from %s %s was not accepted into the memory pool: %s\n", tx.GetHash().ToString().c_str(), - pfrom->addr.ToString().c_str(), pfrom->strSubVer.c_str(), + pfrom->addr.ToString().c_str(), pfrom->cleanSubVer.c_str(), state.GetRejectReason().c_str()); pfrom->PushMessage("reject", strCommand, state.GetRejectCode(), state.GetRejectReason(), inv.hash); @@ -3618,7 +3620,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) if (!(sProblem.empty())) { LogPrint("net", "pong %s %s: %s, %"PRIx64" expected, %"PRIx64" received, %"PRIszu" bytes\n", pfrom->addr.ToString().c_str(), - pfrom->strSubVer.c_str(), + pfrom->cleanSubVer.c_str(), sProblem.c_str(), pfrom->nPingNonceSent, nonce, diff --git a/src/net.cpp b/src/net.cpp index c547cf333..fcef9feea 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -616,7 +616,7 @@ void CNode::copyStats(CNodeStats &stats) X(nTimeConnected); X(addrName); X(nVersion); - X(strSubVer); + X(cleanSubVer); X(fInbound); X(nStartingHeight); X(nMisbehavior); diff --git a/src/net.h b/src/net.h index 278462a0b..effce35dc 100644 --- a/src/net.h +++ b/src/net.h @@ -121,7 +121,7 @@ public: int64_t nTimeConnected; std::string addrName; int nVersion; - std::string strSubVer; + std::string cleanSubVer; bool fInbound; int nStartingHeight; int nMisbehavior; @@ -203,7 +203,11 @@ public: std::string addrName; CService addrLocal; int nVersion; - std::string strSubVer; + // 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, cleanSubVer; bool fOneShot; bool fClient; bool fInbound; diff --git a/src/rpcnet.cpp b/src/rpcnet.cpp index 9f8dea80b..8f0df798b 100644 --- a/src/rpcnet.cpp +++ b/src/rpcnet.cpp @@ -126,7 +126,10 @@ Value getpeerinfo(const Array& params, bool fHelp) if (stats.dPingWait > 0.0) obj.push_back(Pair("pingwait", stats.dPingWait)); obj.push_back(Pair("version", stats.nVersion)); - obj.push_back(Pair("subver", stats.strSubVer)); + // Use the sanitized form of subver here, to avoid tricksy remote peers from + // corrupting or modifiying the JSON output by putting special characters in + // their ver message. + obj.push_back(Pair("subver", stats.cleanSubVer)); obj.push_back(Pair("inbound", stats.fInbound)); obj.push_back(Pair("startingheight", stats.nStartingHeight)); obj.push_back(Pair("banscore", stats.nMisbehavior));