From 6d791a80c28ebd22cd9f596647fed282164fd161 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Fri, 4 Jan 2019 19:04:01 -0800 Subject: [PATCH] Bump protocol version for BulkAvatarTraits and add some guard code around packet parsing. --- interface/src/Application.cpp | 2 +- libraries/avatars/src/AvatarHashMap.cpp | 29 ++++++++++++------- .../networking/src/udt/PacketHeaders.cpp | 1 + 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 6d870b58d6..04adc376e6 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2057,7 +2057,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo properties["avatar_ping"] = avatarMixerNode ? avatarMixerNode->getPingMs() : -1; properties["asset_ping"] = assetServerNode ? assetServerNode->getPingMs() : -1; properties["messages_ping"] = messagesMixerNode ? messagesMixerNode->getPingMs() : -1; - properties["atp_in_kbps"] = messagesMixerNode ? assetServerNode->getInboundKbps() : 0.0f; + properties["atp_in_kbps"] = assetServerNode ? assetServerNode->getInboundKbps() : 0.0f; auto loadingRequests = ResourceCache::getLoadingRequests(); diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 6a67ef6638..b3add74f9c 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -330,19 +330,25 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer void AvatarHashMap::processBulkAvatarTraits(QSharedPointer message, SharedNodePointer sendingNode) { AvatarTraits::TraitMessageSequence seq; - message->readPrimitive(&seq); + if (message->getBytesLeftToRead() > sizeof(AvatarTraits::TraitMessageSequence)) { + message->readPrimitive(&seq); - auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); - traitsAckPacket->writePrimitive(seq); - auto nodeList = DependencyManager::get(); - SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); - if (!avatarMixer.isNull()) { - // we have a mixer to send to, acknowledge that we received these - // traits. - nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); + auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); + traitsAckPacket->writePrimitive(seq); + auto nodeList = DependencyManager::get(); + SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); + if (!avatarMixer.isNull()) { + // we have a mixer to send to, acknowledge that we received these + // traits. + nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); + } + } + else { + qWarning() << "No BulkAvatarTraits packet sequence number."; + return; } - while (message->getBytesLeftToRead()) { + while (message->getBytesLeftToRead() >= NUM_BYTES_RFC4122_UUID + sizeof(AvatarTraits::TraitType)) { // read the avatar ID to figure out which avatar this is for auto avatarID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); @@ -407,6 +413,9 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess message->readPrimitive(&traitType); } } + if (message->getBytesLeftToRead() > 0) { + qWarning() << "Leftover bytes in BulkAvatarTraits message"; + } } void AvatarHashMap::processKillAvatar(QSharedPointer message, SharedNodePointer sendingNode) { diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 45aa0c2b22..a94d45efc9 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -97,6 +97,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::EntityQueryInitialResultsComplete: return static_cast(EntityVersion::ParticleSpin); case PacketType::BulkAvatarTraitsAck: + case PacketType::BulkAvatarTraits: return static_cast(AvatarMixerPacketVersion::AvatarTraitsAck); default: return 22;