2008-01-10 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>

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

@@ -1,3 +1,13 @@
+2008-01-10  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	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  <tujikawa at rednoah dot com>
 
 	Fixed the bug that DefaultPeerStorage::returnPeer() may delete wrong

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<uint32_t*>(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;

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_