From dc4f987a2eba41ba560301aa138ab88db28d2287 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 8 Oct 2015 15:29:42 -0700 Subject: [PATCH 1/4] fix segmented write logic in PacketList --- libraries/networking/src/udt/PacketList.cpp | 44 ++++++++++++++------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index ffe2b3eeba..c6b554f237 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -176,26 +176,42 @@ qint64 PacketList::writeData(const char* data, qint64 maxSize) { // We need to try and pull the first part of the segment out to our new packet // check now to see if this is an unsupported write - int numBytesToEnd = _currentPacket->bytesAvailableForWrite(); - - if ((newPacket->size() - numBytesToEnd) < sizeRemaining) { + int segmentSize = _currentPacket->pos() - _segmentStartIndex; + + if (segmentSize + sizeRemaining > newPacket->getPayloadCapacity()) { // this is an unsupported case - the segment is bigger than the size of an individual packet // but the PacketList is not going to be sent ordered qDebug() << "Error in PacketList::writeData - attempted to write a segment to an unordered packet that is" << "larger than the payload size."; Q_ASSERT(false); + + // we won't be writing this new data to the packet go back before the current segment + // and return -1 to indicate error + _currentPacket->seek(_segmentStartIndex); + _currentPacket->setPayloadSize(_segmentStartIndex); + + return -1; + } else { + // copy from currentPacket where the segment started to the beginning of the newPacket + newPacket->write(_currentPacket->getPayload() + _segmentStartIndex, segmentSize); + + // the current segment now starts at the beginning of the new packet + _segmentStartIndex = _extendedHeader.size(); + + // shrink the current payload to the actual size of the packet + _currentPacket->setPayloadSize(_segmentStartIndex); } - - int segmentSize = _currentPacket->pos() - _segmentStartIndex; - - // copy from currentPacket where the segment started to the beginning of the newPacket - newPacket->write(_currentPacket->getPayload() + _segmentStartIndex, segmentSize); - - // the current segment now starts at the beginning of the new packet - _segmentStartIndex = _extendedHeader.size(); - - // shrink the current payload to the actual size of the packet - _currentPacket->setPayloadSize(_segmentStartIndex); + } + + if (sizeRemaining > newPacket->getPayloadCapacity()) { + // this is an unsupported case - attempting to write a block of data larger + // than the capacity of a new packet in an unordered PacketList + qDebug() << "Error in PacketList::writeData - attempted to write data to an unordered packet that is" + << "larger than the payload size."; + Q_ASSERT(false); + + // return -1 to indicate error + return -1; } // move the current packet to our list of packets From 519df1565a519f5c5d09c03796e3f9e56f60f76c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 8 Oct 2015 15:33:21 -0700 Subject: [PATCH 2/4] fix comment format in PacketList --- libraries/networking/src/udt/PacketList.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index c6b554f237..1ef27410cd 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -185,8 +185,8 @@ qint64 PacketList::writeData(const char* data, qint64 maxSize) { << "larger than the payload size."; Q_ASSERT(false); - // we won't be writing this new data to the packet go back before the current segment - // and return -1 to indicate error + // we won't be writing this new data to the packet + // go back before the current segment and return -1 to indicate error _currentPacket->seek(_segmentStartIndex); _currentPacket->setPayloadSize(_segmentStartIndex); From 0d9421a65e6ba66ae5cdb4ceb1e48068c8028ffd Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 8 Oct 2015 15:40:49 -0700 Subject: [PATCH 3/4] constantize the PacketList write error --- libraries/networking/src/udt/PacketList.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index 1ef27410cd..28c0a034c7 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -150,6 +150,8 @@ void PacketList::preparePackets(MessageNumber messageNumber) { } } +const qint64 PACKET_LIST_WRITE_ERROR = -1; + qint64 PacketList::writeData(const char* data, qint64 maxSize) { auto sizeRemaining = maxSize; @@ -190,7 +192,7 @@ qint64 PacketList::writeData(const char* data, qint64 maxSize) { _currentPacket->seek(_segmentStartIndex); _currentPacket->setPayloadSize(_segmentStartIndex); - return -1; + return PACKET_LIST_WRITE_ERROR; } else { // copy from currentPacket where the segment started to the beginning of the newPacket newPacket->write(_currentPacket->getPayload() + _segmentStartIndex, segmentSize); @@ -210,8 +212,7 @@ qint64 PacketList::writeData(const char* data, qint64 maxSize) { << "larger than the payload size."; Q_ASSERT(false); - // return -1 to indicate error - return -1; + return PACKET_LIST_WRITE_ERROR; } // move the current packet to our list of packets From aa2a8edc6f332154aafa68c6eff5002a6fd348a0 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 8 Oct 2015 15:44:18 -0700 Subject: [PATCH 4/4] use _segmentStartIndex before changing its value --- libraries/networking/src/udt/PacketList.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index 28c0a034c7..09d8628a1c 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -197,11 +197,11 @@ qint64 PacketList::writeData(const char* data, qint64 maxSize) { // copy from currentPacket where the segment started to the beginning of the newPacket newPacket->write(_currentPacket->getPayload() + _segmentStartIndex, segmentSize); - // the current segment now starts at the beginning of the new packet - _segmentStartIndex = _extendedHeader.size(); - // shrink the current payload to the actual size of the packet _currentPacket->setPayloadSize(_segmentStartIndex); + + // the current segment now starts at the beginning of the new packet + _segmentStartIndex = _extendedHeader.size(); } }