From 9a7fe58c57999e1fff6887473048a96a8ede1736 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 10 Jan 2008 15:12:32 +0000 Subject: [PATCH] 2008-01-10 Tatsuhiro Tsujikawa Fixed the bug that EX_TOO_LONG_PAYLOAD exception is thrown if just payload length(4bytes) are received. This happens because lenbufLength is not updated in this particular case and successive call of receiveMessage() overwrites payload length with bytes recieved which are payload body. * src/PeerConnection.cc * src/message.h --- ChangeLog | 10 +++++++ src/PeerConnection.cc | 63 ++++++++++++++++++++----------------------- src/message.h | 2 +- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/ChangeLog b/ChangeLog index 81e17f99..50b506ce 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2008-01-10 Tatsuhiro Tsujikawa + + Fixed the bug that EX_TOO_LONG_PAYLOAD exception is thrown if just + payload length(4bytes) are received. This happens because lenbufLength + is not updated in this particular case and successive call of + receiveMessage() overwrites payload length with bytes recieved which + are payload body. + * src/PeerConnection.cc + * src/message.h + 2008-01-10 Tatsuhiro Tsujikawa Fixed the bug that DefaultPeerStorage::returnPeer() may delete wrong diff --git a/src/PeerConnection.cc b/src/PeerConnection.cc index db5e0298..da65572b 100644 --- a/src/PeerConnection.cc +++ b/src/PeerConnection.cc @@ -71,31 +71,28 @@ int32_t PeerConnection::sendMessage(const unsigned char* data, int32_t dataLengt } bool PeerConnection::receiveMessage(unsigned char* data, int32_t& dataLength) { - if(resbufLength == 0 && lenbufLength != 4) { + if(resbufLength == 0 && 4 > lenbufLength) { if(!socket->isReadable(0)) { return false; } - // read payload size, 4-byte integer - int32_t remain = 4-lenbufLength; - int32_t temp = remain; - // TODO fix this - socket->readData((char*)lenbuf+lenbufLength, temp); - if(temp == 0) { + // read payload size, 32bit unsigned integer + int32_t remaining = 4-lenbufLength; + int32_t temp = remaining; + socket->readData(lenbuf+lenbufLength, remaining); + if(remaining == 0) { // we got EOF logger->debug("CUID#%d - In PeerConnection::receiveMessage(), remain=%d", - cuid, remain); + cuid, temp); throw new DlAbortEx(EX_EOF_FROM_PEER); } - if(remain != temp) { - // still 4-temp bytes to go - lenbufLength += temp; + lenbufLength += remaining; + if(4 > lenbufLength) { + // still 4-lenbufLength bytes to go return false; } - //payloadLen = ntohl(nPayloadLen); - int32_t payloadLength = ntohl(*((int32_t*)lenbuf)); - if(payloadLength > MAX_PAYLOAD_LEN || payloadLength < 0) { - throw new DlAbortEx(EX_TOO_LONG_PAYLOAD, - payloadLength); + uint32_t payloadLength = ntohl(*(reinterpret_cast(lenbuf))); + if(payloadLength > MAX_PAYLOAD_LEN) { + throw new DlAbortEx(EX_TOO_LONG_PAYLOAD, payloadLength); } currentPayloadLength = payloadLength; } @@ -104,16 +101,17 @@ bool PeerConnection::receiveMessage(unsigned char* data, int32_t& dataLength) { } // we have currentPayloadLen-resbufLen bytes to read int32_t remaining = currentPayloadLength-resbufLength; + int32_t temp = remaining; if(remaining > 0) { - socket->readData((char*)resbuf+resbufLength, remaining); + socket->readData(resbuf+resbufLength, remaining); if(remaining == 0) { // we got EOF logger->debug("CUID#%d - In PeerConnection::receiveMessage(), payloadlen=%d, remaining=%d", - cuid, currentPayloadLength, remaining); + cuid, currentPayloadLength, temp); throw new DlAbortEx(EX_EOF_FROM_PEER); } resbufLength += remaining; - if(currentPayloadLength != resbufLength) { + if(currentPayloadLength > resbufLength) { return false; } } @@ -128,29 +126,26 @@ bool PeerConnection::receiveMessage(unsigned char* data, int32_t& dataLength) { bool PeerConnection::receiveHandshake(unsigned char* data, int32_t& dataLength, bool peek) { - int32_t remain = BtHandshakeMessage::MESSAGE_LENGTH-resbufLength; - if(remain != 0 && !socket->isReadable(0)) { + int32_t remaining = BtHandshakeMessage::MESSAGE_LENGTH-resbufLength; + if(remaining > 0 && !socket->isReadable(0)) { dataLength = 0; return false; } - int32_t temp = remain; - if(remain != 0) { - socket->readData((char*)resbuf+resbufLength, temp); - if(temp == 0) { + bool retval = true; + if(remaining > 0) { + int32_t temp = remaining; + socket->readData(resbuf+resbufLength, remaining); + if(remaining == 0) { // we got EOF logger->debug("CUID#%d - In PeerConnection::receiveHandshake(), remain=%d", - cuid, remain); + cuid, temp); throw new DlAbortEx(EX_EOF_FROM_PEER); } + resbufLength += remaining; + if(BtHandshakeMessage::MESSAGE_LENGTH > resbufLength) { + retval = false; + } } - bool retval; - if(remain != temp) { - retval = false; - } else { - retval = true; - } - resbufLength += temp; - int32_t writeLength = resbufLength > dataLength ? dataLength : resbufLength; memcpy(data, resbuf, writeLength); dataLength = writeLength; diff --git a/src/message.h b/src/message.h index 0bea00c6..bbf70b7b 100644 --- a/src/message.h +++ b/src/message.h @@ -210,6 +210,6 @@ #define EX_INVALID_RANGE_HEADER _("Invalid range header. Request: %s-%s/%s, Response: %s-%s/%s") #define EX_NO_RESULT_WITH_YOUR_PREFS _("No file matched with your preference.") #define EX_EXCEPTION_CAUGHT _("Exception caught") -#define EX_TOO_LONG_PAYLOAD _("Max payload length exceeded or invalid. length = %d") +#define EX_TOO_LONG_PAYLOAD _("Max payload length exceeded or invalid. length = %u") #define EX_FILE_LENGTH_MISMATCH_BETWEEN_LOCAL_AND_REMOTE _("Invalid file length. Cannot continue download %s: local %s, remote %s") #endif // _D_MESSAGE_H_