[net] Don't return an optional from TransportDeserializer::GetMessage()

Also, access mapRecvBytesPerMsgCmd with `at()` not `find()`. This
throws an error if COMMAND_OTHER doesn't exist, which should never
happen. `find()` instead just accessed the last element, which could make
debugging more difficult.

Resolves review comments from PR19107:

- https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478718436
- https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478714497
This commit is contained in:
Troy Giorshev
2020-11-05 05:05:32 -05:00
committed by John Newbery
parent e35c4a3d80
commit 8c96008ab1
3 changed files with 34 additions and 35 deletions

View File

@@ -641,25 +641,26 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete)
if (m_deserializer->Complete()) {
// decompose a transport agnostic CNetMessage from the deserializer
uint32_t out_err_raw_size{0};
std::optional<CNetMessage> result{m_deserializer->GetMessage(time, out_err_raw_size)};
if (!result) {
bool reject_message{false};
CNetMessage msg = m_deserializer->GetMessage(time, reject_message);
if (reject_message) {
// Message deserialization failed. Drop the message but don't disconnect the peer.
// store the size of the corrupt message
mapRecvBytesPerMsgCmd.find(NET_MESSAGE_COMMAND_OTHER)->second += out_err_raw_size;
mapRecvBytesPerMsgCmd.at(NET_MESSAGE_COMMAND_OTHER) += msg.m_raw_message_size;
continue;
}
//store received bytes per message command
//to prevent a memory DOS, only allow valid commands
mapMsgCmdSize::iterator i = mapRecvBytesPerMsgCmd.find(result->m_command);
if (i == mapRecvBytesPerMsgCmd.end())
// Store received bytes per message command
// to prevent a memory DOS, only allow valid commands
auto i = mapRecvBytesPerMsgCmd.find(msg.m_command);
if (i == mapRecvBytesPerMsgCmd.end()) {
i = mapRecvBytesPerMsgCmd.find(NET_MESSAGE_COMMAND_OTHER);
}
assert(i != mapRecvBytesPerMsgCmd.end());
i->second += result->m_raw_message_size;
i->second += msg.m_raw_message_size;
// push the message to the process queue,
vRecvMsg.push_back(std::move(*result));
vRecvMsg.push_back(std::move(msg));
complete = true;
}
@@ -733,16 +734,18 @@ const uint256& V1TransportDeserializer::GetMessageHash() const
return data_hash;
}
std::optional<CNetMessage> V1TransportDeserializer::GetMessage(const std::chrono::microseconds time, uint32_t& out_err_raw_size)
CNetMessage V1TransportDeserializer::GetMessage(const std::chrono::microseconds time, bool& reject_message)
{
// Initialize out parameter
reject_message = false;
// decompose a single CNetMessage from the TransportDeserializer
std::optional<CNetMessage> msg(std::move(vRecv));
CNetMessage msg(std::move(vRecv));
// store command string, time, and sizes
msg->m_command = hdr.GetCommand();
msg->m_time = time;
msg->m_message_size = hdr.nMessageSize;
msg->m_raw_message_size = hdr.nMessageSize + CMessageHeader::HEADER_SIZE;
msg.m_command = hdr.GetCommand();
msg.m_time = time;
msg.m_message_size = hdr.nMessageSize;
msg.m_raw_message_size = hdr.nMessageSize + CMessageHeader::HEADER_SIZE;
uint256 hash = GetMessageHash();
@@ -752,17 +755,15 @@ std::optional<CNetMessage> V1TransportDeserializer::GetMessage(const std::chrono
// Check checksum and header command string
if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) {
LogPrint(BCLog::NET, "Header error: Wrong checksum (%s, %u bytes), expected %s was %s, peer=%d\n",
SanitizeString(msg->m_command), msg->m_message_size,
SanitizeString(msg.m_command), msg.m_message_size,
HexStr(Span<uint8_t>(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)),
HexStr(hdr.pchChecksum),
m_node_id);
out_err_raw_size = msg->m_raw_message_size;
msg = std::nullopt;
reject_message = true;
} else if (!hdr.IsCommandValid()) {
LogPrint(BCLog::NET, "Header error: Invalid message type (%s, %u bytes), peer=%d\n",
SanitizeString(hdr.GetCommand()), msg->m_message_size, m_node_id);
out_err_raw_size = msg->m_raw_message_size;
msg.reset();
SanitizeString(hdr.GetCommand()), msg.m_message_size, m_node_id);
reject_message = true;
}
// Always reset the network deserializer (prepare for the next message)