Merge #7696: Fix de-serialization bug where AddrMan is left corrupted
1475ecf
Fix de-serialization bug where AddrMan is corrupted after exception * CAddrDB modified so that when de-serialization code throws an exception Addrman is reset to a clean state * CAddrDB modified to make unit tests possible * Regression test created to ensure bug is fixed * StartNode modifed to clear adrman if CAddrDB::Read returns an error code. (EthanHeilman)
This commit is contained in:
commit
5c3f8ddcaa
4 changed files with 146 additions and 0 deletions
|
@ -59,6 +59,7 @@ BITCOIN_TESTS =\
|
|||
test/merkle_tests.cpp \
|
||||
test/miner_tests.cpp \
|
||||
test/multisig_tests.cpp \
|
||||
test/net_tests.cpp \
|
||||
test/netbase_tests.cpp \
|
||||
test/pmt_tests.cpp \
|
||||
test/policyestimator_tests.cpp \
|
||||
|
|
|
@ -1950,6 +1950,7 @@ void StartNode(boost::thread_group& threadGroup, CScheduler& scheduler)
|
|||
if (adb.Read(addrman))
|
||||
LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman.size(), GetTimeMillis() - nStart);
|
||||
else {
|
||||
addrman.Clear(); // Addrman can be in an inconsistent state after failure, reset it
|
||||
LogPrintf("Invalid or missing peers.dat; recreating\n");
|
||||
DumpAddresses();
|
||||
}
|
||||
|
@ -2320,6 +2321,11 @@ bool CAddrDB::Read(CAddrMan& addr)
|
|||
if (hashIn != hashTmp)
|
||||
return error("%s: Checksum mismatch, data corrupted", __func__);
|
||||
|
||||
return Read(addr, ssPeers);
|
||||
}
|
||||
|
||||
bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers)
|
||||
{
|
||||
unsigned char pchMsgTmp[4];
|
||||
try {
|
||||
// de-serialize file header (network specific magic number) and ..
|
||||
|
@ -2333,6 +2339,8 @@ bool CAddrDB::Read(CAddrMan& addr)
|
|||
ssPeers >> addr;
|
||||
}
|
||||
catch (const std::exception& e) {
|
||||
// de-serialization has failed, ensure addrman is left in a clean state
|
||||
addr.Clear();
|
||||
return error("%s: Deserialize or I/O error - %s", __func__, e.what());
|
||||
}
|
||||
|
||||
|
|
|
@ -794,6 +794,7 @@ public:
|
|||
CAddrDB();
|
||||
bool Write(const CAddrMan& addr);
|
||||
bool Read(CAddrMan& addr);
|
||||
bool Read(CAddrMan& addr, CDataStream& ssPeers);
|
||||
};
|
||||
|
||||
/** Access to the banlist database (banlist.dat) */
|
||||
|
|
136
src/test/net_tests.cpp
Normal file
136
src/test/net_tests.cpp
Normal file
|
@ -0,0 +1,136 @@
|
|||
// Copyright (c) 2012-2016 The Bitcoin Core developers
|
||||
// Distributed under the MIT software license, see the accompanying
|
||||
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
||||
#include "addrman.h"
|
||||
#include "test/test_bitcoin.h"
|
||||
#include <string>
|
||||
#include <boost/test/unit_test.hpp>
|
||||
#include "hash.h"
|
||||
#include "serialize.h"
|
||||
#include "streams.h"
|
||||
#include "net.h"
|
||||
#include "chainparams.h"
|
||||
|
||||
using namespace std;
|
||||
|
||||
class CAddrManSerializationMock : public CAddrMan
|
||||
{
|
||||
public:
|
||||
virtual void Serialize(CDataStream& s, int nType, int nVersionDummy) const = 0;
|
||||
};
|
||||
|
||||
class CAddrManUncorrupted : public CAddrManSerializationMock
|
||||
{
|
||||
public:
|
||||
void Serialize(CDataStream& s, int nType, int nVersionDummy) const
|
||||
{
|
||||
CAddrMan::Serialize(s, nType, nVersionDummy);
|
||||
}
|
||||
};
|
||||
|
||||
class CAddrManCorrupted : public CAddrManSerializationMock
|
||||
{
|
||||
public:
|
||||
void Serialize(CDataStream& s, int nType, int nVersionDummy) const
|
||||
{
|
||||
// Produces corrupt output that claims addrman has 20 addrs when it only has one addr.
|
||||
unsigned char nVersion = 1;
|
||||
s << nVersion;
|
||||
s << ((unsigned char)32);
|
||||
s << nKey;
|
||||
s << 10; // nNew
|
||||
s << 10; // nTried
|
||||
|
||||
int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
|
||||
s << nUBuckets;
|
||||
|
||||
CAddress addr = CAddress(CService("252.1.1.1", 7777));
|
||||
CAddrInfo info = CAddrInfo(addr, CNetAddr("252.2.2.2"));
|
||||
s << info;
|
||||
}
|
||||
};
|
||||
|
||||
CDataStream AddrmanToStream(CAddrManSerializationMock& addrman)
|
||||
{
|
||||
CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION);
|
||||
ssPeersIn << FLATDATA(Params().MessageStart());
|
||||
ssPeersIn << addrman;
|
||||
std::string str = ssPeersIn.str();
|
||||
vector<unsigned char> vchData(str.begin(), str.end());
|
||||
return CDataStream(vchData, SER_DISK, CLIENT_VERSION);
|
||||
}
|
||||
|
||||
BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup)
|
||||
|
||||
BOOST_AUTO_TEST_CASE(caddrdb_read)
|
||||
{
|
||||
CAddrManUncorrupted addrmanUncorrupted;
|
||||
|
||||
CService addr1 = CService("250.7.1.1", 8333);
|
||||
CService addr2 = CService("250.7.2.2", 9999);
|
||||
CService addr3 = CService("250.7.3.3", 9999);
|
||||
|
||||
// Add three addresses to new table.
|
||||
addrmanUncorrupted.Add(CAddress(addr1), CService("252.5.1.1", 8333));
|
||||
addrmanUncorrupted.Add(CAddress(addr2), CService("252.5.1.1", 8333));
|
||||
addrmanUncorrupted.Add(CAddress(addr3), CService("252.5.1.1", 8333));
|
||||
|
||||
// Test that the de-serialization does not throw an exception.
|
||||
CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted);
|
||||
bool exceptionThrown = false;
|
||||
CAddrMan addrman1;
|
||||
|
||||
BOOST_CHECK(addrman1.size() == 0);
|
||||
try {
|
||||
unsigned char pchMsgTmp[4];
|
||||
ssPeers1 >> FLATDATA(pchMsgTmp);
|
||||
ssPeers1 >> addrman1;
|
||||
} catch (const std::exception& e) {
|
||||
exceptionThrown = true;
|
||||
}
|
||||
|
||||
BOOST_CHECK(addrman1.size() == 3);
|
||||
BOOST_CHECK(exceptionThrown == false);
|
||||
|
||||
// Test that CAddrDB::Read creates an addrman with the correct number of addrs.
|
||||
CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted);
|
||||
|
||||
CAddrMan addrman2;
|
||||
CAddrDB adb;
|
||||
BOOST_CHECK(addrman2.size() == 0);
|
||||
adb.Read(addrman2, ssPeers2);
|
||||
BOOST_CHECK(addrman2.size() == 3);
|
||||
}
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
|
||||
{
|
||||
CAddrManCorrupted addrmanCorrupted;
|
||||
|
||||
// Test that the de-serialization of corrupted addrman throws an exception.
|
||||
CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted);
|
||||
bool exceptionThrown = false;
|
||||
CAddrMan addrman1;
|
||||
BOOST_CHECK(addrman1.size() == 0);
|
||||
try {
|
||||
unsigned char pchMsgTmp[4];
|
||||
ssPeers1 >> FLATDATA(pchMsgTmp);
|
||||
ssPeers1 >> addrman1;
|
||||
} catch (const std::exception& e) {
|
||||
exceptionThrown = true;
|
||||
}
|
||||
// Even through de-serialization failed adddrman is not left in a clean state.
|
||||
BOOST_CHECK(addrman1.size() == 1);
|
||||
BOOST_CHECK(exceptionThrown);
|
||||
|
||||
// Test that CAddrDB::Read leaves addrman in a clean state if de-serialization fails.
|
||||
CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted);
|
||||
|
||||
CAddrMan addrman2;
|
||||
CAddrDB adb;
|
||||
BOOST_CHECK(addrman2.size() == 0);
|
||||
adb.Read(addrman2, ssPeers2);
|
||||
BOOST_CHECK(addrman2.size() == 0);
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
Loading…
Reference in a new issue