From 82b68fce8d61627a223c7a79359b64c9f756dff1 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 15 Mar 2018 18:32:48 -0700 Subject: [PATCH 01/56] Quick attempt at using openssl HMAC in NLPacket --- libraries/networking/src/NLPacket.cpp | 12 ++++++ libraries/shared/src/HmacAuth.cpp | 55 +++++++++++++++++++++++++++ libraries/shared/src/HmacAuth.h | 32 ++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 libraries/shared/src/HmacAuth.cpp create mode 100644 libraries/shared/src/HmacAuth.h diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 5c5077691b..9df84e6abc 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -11,6 +11,8 @@ #include "NLPacket.h" +#include "HmacAuth.h" + int NLPacket::localHeaderSize(PacketType type) { bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type); bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type); @@ -150,6 +152,14 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { } QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret) { + HmacAuth hash; + int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; + hash.setKey(connectionSecret); + hash.addData(packet.getData() + offset, packet.getDataSize() - offset); + auto hashResult(hash.result()); + return QByteArray((const char*) hashResult.data(), (int) hashResult.size()); + /* QCryptographicHash hash(QCryptographicHash::Md5); int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) @@ -161,6 +171,8 @@ QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUu // return the hash return hash.result(); + */ + } void NLPacket::writeTypeAndVersion() { diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp new file mode 100644 index 0000000000..6cc6835329 --- /dev/null +++ b/libraries/shared/src/HmacAuth.cpp @@ -0,0 +1,55 @@ +// +// HmacAuth.cpp + +#include + +#include "HmacAuth.h" + +#include + +HmacAuth::HmacAuth(AuthMethod authMethod) + : _hmacContext(new(HMAC_CTX)) + , _authMethod(authMethod) { + HMAC_CTX_init(_hmacContext.get()); +} + +HmacAuth::~HmacAuth() { + HMAC_CTX_cleanup(_hmacContext.get()); +} + +bool HmacAuth::setKey(const char * keyValue, int keyLen) { + const EVP_MD * sslStruct = nullptr; + + switch (_authMethod) + { + case SHA1: + sslStruct = EVP_sha1(); + break; + + case RIPEMD160: + sslStruct = EVP_ripemd160(); + break; + + default: + return false; + } + + return (bool) HMAC_Init(_hmacContext.get(), keyValue, keyLen, sslStruct); +} + +bool HmacAuth::setKey(const QUuid& uidKey) { + const QByteArray rfcBytes(uidKey.toRfc4122()); + return setKey(rfcBytes.constData(), rfcBytes.length()); +} + +bool HmacAuth::addData(const char * data, int dataLen) { + return (bool) HMAC_Update(_hmacContext.get(), reinterpret_cast(data), dataLen); +} + +HmacAuth::HmacHash HmacAuth::result() { + HmacHash hashValue(EVP_MAX_MD_SIZE); + unsigned int hashLen; + HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen); + hashValue.resize((size_t) hashLen); + return hashValue; +} diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h new file mode 100644 index 0000000000..9d90f5fb4d --- /dev/null +++ b/libraries/shared/src/HmacAuth.h @@ -0,0 +1,32 @@ +// +// HmacAuth.h +// libraries/shared/src + +#ifndef hifi_HmacAuth_h +#define hifi_HmacAuth_h + +#include +#include + +struct hmac_ctx_st; +class QUuid; + +class HmacAuth { +public: + enum AuthMethod { SHA1, RIPEMD160 }; + typedef std::vector HmacHash; + + HmacAuth(AuthMethod authMethod = SHA1); + ~HmacAuth(); + + bool setKey(const char * keyValue, int keyLen); + bool setKey(const QUuid& uidKey); + bool addData(const char * data, int dataLen); + HmacHash result(); + +private: + std::unique_ptr _hmacContext; + AuthMethod _authMethod { SHA1 }; +}; + +#endif // hifi_HmacAuth_h From 480f76c21aaa92f358691fa9fc296bd65f308523 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 16 Mar 2018 11:50:03 -0700 Subject: [PATCH 02/56] Quick trial of HMAC-MD5 auth + timings --- libraries/networking/src/NLPacket.cpp | 18 ++++++++++++++++++ libraries/shared/src/HmacAuth.cpp | 12 ++++++++++++ libraries/shared/src/HmacAuth.h | 6 +++--- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 9df84e6abc..988e86afc2 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -13,6 +13,12 @@ #include "HmacAuth.h" +#define HIFI_HASH_TIMINGS +#ifdef HIFI_HASH_TIMINGS +#include "NetworkLogging.h" +#include "SharedUtil.h" +#endif + int NLPacket::localHeaderSize(PacketType type) { bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type); bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type); @@ -230,7 +236,19 @@ void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) c auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID; +#ifdef HIFI_HASH_TIMINGS + static quint64 totalTime = 0; + static int timedHashes = 0; + quint64 startTime = usecTimestampNow(); +#endif QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret); +#ifdef HIFI_HASH_TIMINGS + quint64 endTime = usecTimestampNow(); + totalTime += endTime - startTime; + if ((++timedHashes % 20) == 0) { + qCDebug(networking) << "Average packet hash time " << (totalTime / timedHashes / 1000.0f) << " ms"; + } +#endif memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size()); } diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp index 6cc6835329..469d77c624 100644 --- a/libraries/shared/src/HmacAuth.cpp +++ b/libraries/shared/src/HmacAuth.cpp @@ -22,10 +22,22 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) { switch (_authMethod) { + case MD5: + sslStruct = EVP_md5(); + break; + case SHA1: sslStruct = EVP_sha1(); break; + case SHA224: + sslStruct = EVP_sha224(); + break; + + case SHA256: + sslStruct = EVP_sha256(); + break; + case RIPEMD160: sslStruct = EVP_ripemd160(); break; diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h index 9d90f5fb4d..1ed6be0eb0 100644 --- a/libraries/shared/src/HmacAuth.h +++ b/libraries/shared/src/HmacAuth.h @@ -13,10 +13,10 @@ class QUuid; class HmacAuth { public: - enum AuthMethod { SHA1, RIPEMD160 }; + enum AuthMethod { MD5, SHA1, SHA224, SHA256, RIPEMD160 }; typedef std::vector HmacHash; - HmacAuth(AuthMethod authMethod = SHA1); + explicit HmacAuth(AuthMethod authMethod = MD5); ~HmacAuth(); bool setKey(const char * keyValue, int keyLen); @@ -26,7 +26,7 @@ public: private: std::unique_ptr _hmacContext; - AuthMethod _authMethod { SHA1 }; + AuthMethod _authMethod { MD5 }; }; #endif // hifi_HmacAuth_h From db8a1ccb3e16a8f561d1eae4daa29f12d0b2878a Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 16 Mar 2018 18:03:13 -0700 Subject: [PATCH 03/56] Set HMAC key once and reuse OpenSSL context Store the HMAC wrapper in Node. Unfortunately requires a lot of plumbing down to NLPacket. Added a mutex to the wrapper since suspicious crashes occurred. Authentication times seem to be comparable to existing MD5. --- libraries/networking/src/LimitedNodeList.cpp | 38 +++++++++++--------- libraries/networking/src/LimitedNodeList.h | 8 ++--- libraries/networking/src/NLPacket.cpp | 15 ++++---- libraries/networking/src/NLPacket.h | 6 ++-- libraries/networking/src/Node.cpp | 7 +++- libraries/networking/src/Node.h | 7 +++- libraries/shared/src/HmacAuth.cpp | 5 +++ libraries/shared/src/HmacAuth.h | 2 ++ 8 files changed, 56 insertions(+), 32 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 0803e380f2..861629fd72 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -36,6 +36,7 @@ #include "HifiSockAddr.h" #include "NetworkLogging.h" #include "udt/Packet.h" +#include "HmacAuth.h" static Setting::Handle LIMITED_NODELIST_LOCAL_PORT("LimitedNodeList.LocalPort", 0); @@ -319,7 +320,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe if (verifiedPacket && !ignoreVerification) { QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); - QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret()); + QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret(), sourceNode->getAuthenticateHash()); // check if the md5 hash in the header matches the hash we would expect if (packetHeaderHash != expectedHash) { @@ -363,7 +364,7 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) { _numCollectedBytes += packet.getDataSize(); } -void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& connectionSecret) { +void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret) { if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) { packet.writeSourceID(getSessionUUID()); } @@ -371,7 +372,7 @@ void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& conn if (!connectionSecret.isNull() && !PacketTypeEnum::getNonSourcedPackets().contains(packet.getType()) && !PacketTypeEnum::getNonVerifiedPackets().contains(packet.getType())) { - packet.writeVerificationHashGivenSecret(connectionSecret); + packet.writeVerificationHashGivenSecret(hmacAuth, connectionSecret); } } @@ -387,17 +388,18 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& emit dataSent(destinationNode.getType(), packet.getDataSize()); destinationNode.recordBytesSent(packet.getDataSize()); - return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getConnectionSecret()); + return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getAuthenticateHash(), + destinationNode.getConnectionSecret()); } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - const QUuid& connectionSecret) { + HmacAuth& hmacAuth, const QUuid& connectionSecret) { Q_ASSERT(!packet.isPartOfMessage()); Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); collectPacketStats(packet); - fillPacketHeader(packet, connectionSecret); + fillPacketHeader(packet, hmacAuth, connectionSecret); return _nodeSocket.writePacket(packet, sockAddr); } @@ -410,7 +412,8 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& emit dataSent(destinationNode.getType(), packet->getDataSize()); destinationNode.recordBytesSent(packet->getDataSize()); - return sendPacket(std::move(packet), *activeSocket, destinationNode.getConnectionSecret()); + return sendPacket(std::move(packet), *activeSocket, destinationNode.getAuthenticateHash(), + destinationNode.getConnectionSecret()); } else { qCDebug(networking) << "LimitedNodeList::sendPacket called without active socket for node" << destinationNode << "- not sending"; return ERROR_SENDING_PACKET_BYTES; @@ -418,18 +421,18 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& } qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - const QUuid& connectionSecret) { + HmacAuth& hmacAuth, const QUuid& connectionSecret) { Q_ASSERT(!packet->isPartOfMessage()); if (packet->isReliable()) { collectPacketStats(*packet); - fillPacketHeader(*packet, connectionSecret); + fillPacketHeader(*packet, hmacAuth, connectionSecret); auto size = packet->getDataSize(); _nodeSocket.writePacket(std::move(packet), sockAddr); return size; } else { - return sendUnreliablePacket(*packet, sockAddr, connectionSecret); + return sendUnreliablePacket(*packet, sockAddr, hmacAuth, connectionSecret); } } @@ -444,7 +447,8 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi packetList.closeCurrentPacket(); while (!packetList._packets.empty()) { - bytesSent += sendPacket(packetList.takeFront(), *activeSocket, connectionSecret); + bytesSent += sendPacket(packetList.takeFront(), *activeSocket, + destinationNode.getAuthenticateHash(), connectionSecret); } emit dataSent(destinationNode.getType(), bytesSent); @@ -457,14 +461,14 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi } qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - const QUuid& connectionSecret) { + HmacAuth& hmacAuth, const QUuid& connectionSecret) { qint64 bytesSent = 0; // close the last packet in the list packetList.closeCurrentPacket(); while (!packetList._packets.empty()) { - bytesSent += sendPacket(packetList.takeFront(), sockAddr, connectionSecret); + bytesSent += sendPacket(packetList.takeFront(), sockAddr, hmacAuth, connectionSecret); } return bytesSent; @@ -474,10 +478,11 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, // close the last packet in the list packetList->closeCurrentPacket(); + HmacAuth unusedHmac; for (std::unique_ptr& packet : packetList->_packets) { NLPacket* nlPacket = static_cast(packet.get()); collectPacketStats(*nlPacket); - fillPacketHeader(*nlPacket); + fillPacketHeader(*nlPacket, unusedHmac); } return _nodeSocket.writePacketList(std::move(packetList), sockAddr); @@ -492,7 +497,7 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, for (std::unique_ptr& packet : packetList->_packets) { NLPacket* nlPacket = static_cast(packet.get()); collectPacketStats(*nlPacket); - fillPacketHeader(*nlPacket, destinationNode.getConnectionSecret()); + fillPacketHeader(*nlPacket, destinationNode.getAuthenticateHash(), destinationNode.getConnectionSecret()); } return _nodeSocket.writePacketList(std::move(packetList), *activeSocket); @@ -515,7 +520,8 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& auto& destinationSockAddr = (overridenSockAddr.isNull()) ? *destinationNode.getActiveSocket() : overridenSockAddr; - return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getConnectionSecret()); + return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getAuthenticateHash(), + destinationNode.getConnectionSecret()); } int LimitedNodeList::updateNodeWithDataFromPacket(QSharedPointer message, SharedNodePointer sendingNode) { diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 7165b3dd63..8e73440f5b 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -132,18 +132,18 @@ public: // either to a node (via its active socket) or to a manual sockaddr qint64 sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode); qint64 sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - const QUuid& connectionSecret = QUuid()); + HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid()); // use sendPacket to send a moved unreliable or reliable NL packet to a node's active socket or manual sockaddr qint64 sendPacket(std::unique_ptr packet, const Node& destinationNode); qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - const QUuid& connectionSecret = QUuid()); + HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid()); // use sendUnreliableUnorderedPacketList to unreliably send separate packets from the packet list // either to a node's active socket or to a manual sockaddr qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const Node& destinationNode); qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - const QUuid& connectionSecret = QUuid()); + HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid()); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr @@ -364,7 +364,7 @@ protected: qint64 writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr, const QUuid& connectionSecret = QUuid()); void collectPacketStats(const NLPacket& packet); - void fillPacketHeader(const NLPacket& packet, const QUuid& connectionSecret = QUuid()); + void fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret = QUuid()); void setLocalSocket(const HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 988e86afc2..b32c1f1f7f 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -157,15 +157,15 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); } -QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret) { - HmacAuth hash; +QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash) { +#define HIFI_USE_HMAC +#ifdef HIFI_USE_HMAC int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; - hash.setKey(connectionSecret); hash.addData(packet.getData() + offset, packet.getDataSize() - offset); auto hashResult(hash.result()); return QByteArray((const char*) hashResult.data(), (int) hashResult.size()); - /* +#else QCryptographicHash hash(QCryptographicHash::Md5); int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) @@ -177,8 +177,7 @@ QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUu // return the hash return hash.result(); - */ - +#endif } void NLPacket::writeTypeAndVersion() { @@ -230,7 +229,7 @@ void NLPacket::writeSourceID(const QUuid& sourceID) const { _sourceID = sourceID; } -void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) const { +void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const { Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type) && !PacketTypeEnum::getNonVerifiedPackets().contains(_type)); @@ -241,7 +240,7 @@ void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) c static int timedHashes = 0; quint64 startTime = usecTimestampNow(); #endif - QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret); + QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret, hmacAuth); #ifdef HIFI_HASH_TIMINGS quint64 endTime = usecTimestampNow(); totalTime += endTime - startTime; diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index f49cc47645..f38f29ec36 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -18,6 +18,8 @@ #include "udt/Packet.h" +class HmacAuth; + class NLPacket : public udt::Packet { Q_OBJECT public: @@ -71,7 +73,7 @@ public: static QUuid sourceIDInHeader(const udt::Packet& packet); static QByteArray verificationHashInHeader(const udt::Packet& packet); - static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret); + static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash); PacketType getType() const { return _type; } void setType(PacketType type); @@ -82,7 +84,7 @@ public: const QUuid& getSourceID() const { return _sourceID; } void writeSourceID(const QUuid& sourceID) const; - void writeVerificationHashGivenSecret(const QUuid& connectionSecret) const; + void writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const; protected: diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index bd895c8ef1..6669c68a2e 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -86,9 +86,10 @@ NodeType_t NodeType::fromString(QString type) { Node::Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket, - const HifiSockAddr& localSocket, QObject* parent) : + const HifiSockAddr& localSocket, QObject* parent) : NetworkPeer(uuid, publicSocket, localSocket, parent), _type(type), + _authenticateHash(new HmacAuth), _pingMs(-1), // "Uninitialized" _clockSkewUsec(0), _mutex(), @@ -192,3 +193,7 @@ QDebug operator<<(QDebug debug, const Node& node) { debug.nospace() << node.getPublicSocket() << "/" << node.getLocalSocket(); return debug.nospace(); } + +void Node::_updateAuthenticateHash() { + _authenticateHash->setKey(_connectionSecret); +} diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h index 93b6a649d4..80d51202d5 100644 --- a/libraries/networking/src/Node.h +++ b/libraries/networking/src/Node.h @@ -33,6 +33,7 @@ #include "SimpleMovingAverage.h" #include "MovingPercentile.h" #include "NodePermissions.h" +#include "HmacAuth.h" class Node : public NetworkPeer { Q_OBJECT @@ -55,7 +56,8 @@ public: void setIsUpstream(bool isUpstream) { _isUpstream = isUpstream; } const QUuid& getConnectionSecret() const { return _connectionSecret; } - void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; } + void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; _updateAuthenticateHash(); } + HmacAuth& getAuthenticateHash() const { return *_authenticateHash; } NodeData* getLinkedData() const { return _linkedData.get(); } void setLinkedData(std::unique_ptr linkedData) { _linkedData = std::move(linkedData); } @@ -94,9 +96,12 @@ private: Node(const Node &otherNode); Node& operator=(Node otherNode); + void _updateAuthenticateHash(); + NodeType_t _type; QUuid _connectionSecret; + std::unique_ptr _authenticateHash; std::unique_ptr _linkedData; bool _isReplicated { false }; int _pingMs; diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp index 469d77c624..ca0ec39b94 100644 --- a/libraries/shared/src/HmacAuth.cpp +++ b/libraries/shared/src/HmacAuth.cpp @@ -46,6 +46,7 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) { return false; } + QMutexLocker lock(&_lock); return (bool) HMAC_Init(_hmacContext.get(), keyValue, keyLen, sslStruct); } @@ -55,13 +56,17 @@ bool HmacAuth::setKey(const QUuid& uidKey) { } bool HmacAuth::addData(const char * data, int dataLen) { + QMutexLocker lock(&_lock); return (bool) HMAC_Update(_hmacContext.get(), reinterpret_cast(data), dataLen); } HmacAuth::HmacHash HmacAuth::result() { HmacHash hashValue(EVP_MAX_MD_SIZE); unsigned int hashLen; + QMutexLocker lock(&_lock); HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen); hashValue.resize((size_t) hashLen); + // Clear state for possible reuse. + HMAC_Init(_hmacContext.get(), nullptr, 0, nullptr); return hashValue; } diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h index 1ed6be0eb0..305e1a36ed 100644 --- a/libraries/shared/src/HmacAuth.h +++ b/libraries/shared/src/HmacAuth.h @@ -7,6 +7,7 @@ #include #include +#include struct hmac_ctx_st; class QUuid; @@ -25,6 +26,7 @@ public: HmacHash result(); private: + QMutex _lock; std::unique_ptr _hmacContext; AuthMethod _authMethod { MD5 }; }; From d889384d946f8558f261084fc417cb6b2cc958da Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 19 Mar 2018 09:41:57 -0700 Subject: [PATCH 04/56] Use elaborated type-specifier for openssl internal class --- libraries/shared/src/HmacAuth.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h index 305e1a36ed..b39423de95 100644 --- a/libraries/shared/src/HmacAuth.h +++ b/libraries/shared/src/HmacAuth.h @@ -9,7 +9,6 @@ #include #include -struct hmac_ctx_st; class QUuid; class HmacAuth { @@ -27,7 +26,7 @@ public: private: QMutex _lock; - std::unique_ptr _hmacContext; + std::unique_ptr _hmacContext; AuthMethod _authMethod { MD5 }; }; From af21cac0c2a3587c2b3d6f12613d2b84ef6f0aa7 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 19 Mar 2018 12:53:16 -0700 Subject: [PATCH 05/56] Fixes for gcc --- libraries/networking/src/LimitedNodeList.h | 6 +++--- libraries/shared/src/HmacAuth.cpp | 2 ++ libraries/shared/src/HmacAuth.h | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 8e73440f5b..638f3efefc 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -132,18 +132,18 @@ public: // either to a node (via its active socket) or to a manual sockaddr qint64 sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode); qint64 sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid()); + HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); // use sendPacket to send a moved unreliable or reliable NL packet to a node's active socket or manual sockaddr qint64 sendPacket(std::unique_ptr packet, const Node& destinationNode); qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid()); + HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); // use sendUnreliableUnorderedPacketList to unreliably send separate packets from the packet list // either to a node's active socket or to a manual sockaddr qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const Node& destinationNode); qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid()); + HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp index ca0ec39b94..47f0e4d224 100644 --- a/libraries/shared/src/HmacAuth.cpp +++ b/libraries/shared/src/HmacAuth.cpp @@ -7,6 +7,8 @@ #include +HmacAuth HmacAuth::nullHmacAuth; + HmacAuth::HmacAuth(AuthMethod authMethod) : _hmacContext(new(HMAC_CTX)) , _authMethod(authMethod) { diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h index b39423de95..4970f08ca6 100644 --- a/libraries/shared/src/HmacAuth.h +++ b/libraries/shared/src/HmacAuth.h @@ -24,6 +24,8 @@ public: bool addData(const char * data, int dataLen); HmacHash result(); + static HmacAuth nullHmacAuth; + private: QMutex _lock; std::unique_ptr _hmacContext; From da7298b8bde004947d4a748d723f074b1f95d29b Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 19 Mar 2018 15:28:44 -0700 Subject: [PATCH 06/56] Support only HMAC - take out passing around of secret UUID Also other clean-up for production use. --- libraries/networking/src/LimitedNodeList.cpp | 38 +++++++++----------- libraries/networking/src/LimitedNodeList.h | 13 +++---- libraries/networking/src/NLPacket.cpp | 21 ++--------- libraries/networking/src/NLPacket.h | 4 +-- libraries/shared/src/HmacAuth.cpp | 10 ++++-- libraries/shared/src/HmacAuth.h | 11 ++++-- 6 files changed, 43 insertions(+), 54 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 861629fd72..d09e379909 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -320,7 +320,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe if (verifiedPacket && !ignoreVerification) { QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); - QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret(), sourceNode->getAuthenticateHash()); + QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getAuthenticateHash()); // check if the md5 hash in the header matches the hash we would expect if (packetHeaderHash != expectedHash) { @@ -364,15 +364,15 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) { _numCollectedBytes += packet.getDataSize(); } -void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret) { +void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAuth) { if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) { packet.writeSourceID(getSessionUUID()); } - if (!connectionSecret.isNull() + if (hmacAuth && !PacketTypeEnum::getNonSourcedPackets().contains(packet.getType()) && !PacketTypeEnum::getNonVerifiedPackets().contains(packet.getType())) { - packet.writeVerificationHashGivenSecret(hmacAuth, connectionSecret); + packet.writeVerificationHashGivenSecret(*hmacAuth); } } @@ -388,18 +388,17 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& emit dataSent(destinationNode.getType(), packet.getDataSize()); destinationNode.recordBytesSent(packet.getDataSize()); - return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getAuthenticateHash(), - destinationNode.getConnectionSecret()); + return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), &destinationNode.getAuthenticateHash()); } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth, const QUuid& connectionSecret) { + HmacAuth * hmacAuth) { Q_ASSERT(!packet.isPartOfMessage()); Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); collectPacketStats(packet); - fillPacketHeader(packet, hmacAuth, connectionSecret); + fillPacketHeader(packet, hmacAuth); return _nodeSocket.writePacket(packet, sockAddr); } @@ -412,8 +411,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& emit dataSent(destinationNode.getType(), packet->getDataSize()); destinationNode.recordBytesSent(packet->getDataSize()); - return sendPacket(std::move(packet), *activeSocket, destinationNode.getAuthenticateHash(), - destinationNode.getConnectionSecret()); + return sendPacket(std::move(packet), *activeSocket, &destinationNode.getAuthenticateHash()); } else { qCDebug(networking) << "LimitedNodeList::sendPacket called without active socket for node" << destinationNode << "- not sending"; return ERROR_SENDING_PACKET_BYTES; @@ -421,18 +419,18 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& } qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth, const QUuid& connectionSecret) { + HmacAuth * hmacAuth) { Q_ASSERT(!packet->isPartOfMessage()); if (packet->isReliable()) { collectPacketStats(*packet); - fillPacketHeader(*packet, hmacAuth, connectionSecret); + fillPacketHeader(*packet, hmacAuth); auto size = packet->getDataSize(); _nodeSocket.writePacket(std::move(packet), sockAddr); return size; } else { - return sendUnreliablePacket(*packet, sockAddr, hmacAuth, connectionSecret); + return sendUnreliablePacket(*packet, sockAddr, hmacAuth); } } @@ -448,7 +446,7 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi while (!packetList._packets.empty()) { bytesSent += sendPacket(packetList.takeFront(), *activeSocket, - destinationNode.getAuthenticateHash(), connectionSecret); + &destinationNode.getAuthenticateHash()); } emit dataSent(destinationNode.getType(), bytesSent); @@ -461,14 +459,14 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi } qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth, const QUuid& connectionSecret) { + HmacAuth * hmacAuth) { qint64 bytesSent = 0; // close the last packet in the list packetList.closeCurrentPacket(); while (!packetList._packets.empty()) { - bytesSent += sendPacket(packetList.takeFront(), sockAddr, hmacAuth, connectionSecret); + bytesSent += sendPacket(packetList.takeFront(), sockAddr, hmacAuth); } return bytesSent; @@ -478,11 +476,10 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, // close the last packet in the list packetList->closeCurrentPacket(); - HmacAuth unusedHmac; for (std::unique_ptr& packet : packetList->_packets) { NLPacket* nlPacket = static_cast(packet.get()); collectPacketStats(*nlPacket); - fillPacketHeader(*nlPacket, unusedHmac); + fillPacketHeader(*nlPacket, nullptr); } return _nodeSocket.writePacketList(std::move(packetList), sockAddr); @@ -497,7 +494,7 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, for (std::unique_ptr& packet : packetList->_packets) { NLPacket* nlPacket = static_cast(packet.get()); collectPacketStats(*nlPacket); - fillPacketHeader(*nlPacket, destinationNode.getAuthenticateHash(), destinationNode.getConnectionSecret()); + fillPacketHeader(*nlPacket, &destinationNode.getAuthenticateHash()); } return _nodeSocket.writePacketList(std::move(packetList), *activeSocket); @@ -520,8 +517,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& auto& destinationSockAddr = (overridenSockAddr.isNull()) ? *destinationNode.getActiveSocket() : overridenSockAddr; - return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getAuthenticateHash(), - destinationNode.getConnectionSecret()); + return sendPacket(std::move(packet), destinationSockAddr, &destinationNode.getAuthenticateHash()); } int LimitedNodeList::updateNodeWithDataFromPacket(QSharedPointer message, SharedNodePointer sendingNode) { diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 638f3efefc..6d546d4d65 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -128,22 +128,19 @@ public: virtual QUuid getDomainUUID() const { assert(false); return QUuid(); } virtual HifiSockAddr getDomainSockAddr() const { assert(false); return HifiSockAddr(); } - // use sendUnreliablePacket to send an unrelaible packet (that you do not need to move) + // use sendUnreliablePacket to send an unreliable packet (that you do not need to move) // either to a node (via its active socket) or to a manual sockaddr qint64 sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode); - qint64 sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); + qint64 sendUnreliablePacket(const NLPacket & packet, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth = nullptr); // use sendPacket to send a moved unreliable or reliable NL packet to a node's active socket or manual sockaddr qint64 sendPacket(std::unique_ptr packet, const Node& destinationNode); - qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); + qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth = nullptr); // use sendUnreliableUnorderedPacketList to unreliably send separate packets from the packet list // either to a node's active socket or to a manual sockaddr qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const Node& destinationNode); - qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); + qint64 sendUnreliableUnorderedPacketList(NLPacketList & packetList, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr @@ -364,7 +361,7 @@ protected: qint64 writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr, const QUuid& connectionSecret = QUuid()); void collectPacketStats(const NLPacket& packet); - void fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret = QUuid()); + void fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAuth); void setLocalSocket(const HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index b32c1f1f7f..99313247e9 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -157,27 +157,12 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); } -QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash) { -#define HIFI_USE_HMAC -#ifdef HIFI_USE_HMAC +QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, HmacAuth& hash) { int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; hash.addData(packet.getData() + offset, packet.getDataSize() - offset); auto hashResult(hash.result()); return QByteArray((const char*) hashResult.data(), (int) hashResult.size()); -#else - QCryptographicHash hash(QCryptographicHash::Md5); - - int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) - + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; - - // add the packet payload and the connection UUID - hash.addData(packet.getData() + offset, packet.getDataSize() - offset); - hash.addData(connectionSecret.toRfc4122()); - - // return the hash - return hash.result(); -#endif } void NLPacket::writeTypeAndVersion() { @@ -229,7 +214,7 @@ void NLPacket::writeSourceID(const QUuid& sourceID) const { _sourceID = sourceID; } -void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const { +void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const { Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type) && !PacketTypeEnum::getNonVerifiedPackets().contains(_type)); @@ -240,7 +225,7 @@ void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& static int timedHashes = 0; quint64 startTime = usecTimestampNow(); #endif - QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret, hmacAuth); + QByteArray verificationHash = hashForPacketAndSecret(*this, hmacAuth); #ifdef HIFI_HASH_TIMINGS quint64 endTime = usecTimestampNow(); totalTime += endTime - startTime; diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index f38f29ec36..8f73475530 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -73,7 +73,7 @@ public: static QUuid sourceIDInHeader(const udt::Packet& packet); static QByteArray verificationHashInHeader(const udt::Packet& packet); - static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash); + static QByteArray hashForPacketAndSecret(const udt::Packet & packet, HmacAuth & hash); PacketType getType() const { return _type; } void setType(PacketType type); @@ -84,7 +84,7 @@ public: const QUuid& getSourceID() const { return _sourceID; } void writeSourceID(const QUuid& sourceID) const; - void writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const; + void writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const; protected: diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp index 47f0e4d224..5d04bb96a4 100644 --- a/libraries/shared/src/HmacAuth.cpp +++ b/libraries/shared/src/HmacAuth.cpp @@ -1,5 +1,13 @@ // // HmacAuth.cpp +// libraries/shared/src +// +// Created by Simon Walton on 3/19/2018. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// #include @@ -7,8 +15,6 @@ #include -HmacAuth HmacAuth::nullHmacAuth; - HmacAuth::HmacAuth(AuthMethod authMethod) : _hmacContext(new(HMAC_CTX)) , _authMethod(authMethod) { diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h index 4970f08ca6..dfc79e8e47 100644 --- a/libraries/shared/src/HmacAuth.h +++ b/libraries/shared/src/HmacAuth.h @@ -1,6 +1,13 @@ // // HmacAuth.h // libraries/shared/src +// +// Created by Simon Walton on 3/19/2018. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// #ifndef hifi_HmacAuth_h #define hifi_HmacAuth_h @@ -14,7 +21,7 @@ class QUuid; class HmacAuth { public: enum AuthMethod { MD5, SHA1, SHA224, SHA256, RIPEMD160 }; - typedef std::vector HmacHash; + using HmacHash = std::vector; explicit HmacAuth(AuthMethod authMethod = MD5); ~HmacAuth(); @@ -24,8 +31,6 @@ public: bool addData(const char * data, int dataLen); HmacHash result(); - static HmacAuth nullHmacAuth; - private: QMutex _lock; std::unique_ptr _hmacContext; From 020a6a65852c7d56d4520199388895e9aa288248 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 19 Mar 2018 15:57:32 -0700 Subject: [PATCH 07/56] Take out hash timing code --- libraries/networking/src/NLPacket.cpp | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 99313247e9..37bb465ca9 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -13,12 +13,6 @@ #include "HmacAuth.h" -#define HIFI_HASH_TIMINGS -#ifdef HIFI_HASH_TIMINGS -#include "NetworkLogging.h" -#include "SharedUtil.h" -#endif - int NLPacket::localHeaderSize(PacketType type) { bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type); bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type); @@ -220,19 +214,8 @@ void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const { auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID; -#ifdef HIFI_HASH_TIMINGS - static quint64 totalTime = 0; - static int timedHashes = 0; - quint64 startTime = usecTimestampNow(); -#endif + QByteArray verificationHash = hashForPacketAndSecret(*this, hmacAuth); -#ifdef HIFI_HASH_TIMINGS - quint64 endTime = usecTimestampNow(); - totalTime += endTime - startTime; - if ((++timedHashes % 20) == 0) { - qCDebug(networking) << "Average packet hash time " << (totalTime / timedHashes / 1000.0f) << " ms"; - } -#endif memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size()); } From 2a486a4c1419f974c1a11b55fdb266d8c42e84c1 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 19 Mar 2018 17:48:11 -0700 Subject: [PATCH 08/56] Bump packet version numbers --- libraries/networking/src/LimitedNodeList.h | 3 ++- .../networking/src/udt/PacketHeaders.cpp | 21 ++++++++-------- libraries/networking/src/udt/PacketHeaders.h | 25 +++++++++++++------ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 6d546d4d65..612a6ce947 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -140,7 +140,8 @@ public: // use sendUnreliableUnorderedPacketList to unreliably send separate packets from the packet list // either to a node's active socket or to a manual sockaddr qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const Node& destinationNode); - qint64 sendUnreliableUnorderedPacketList(NLPacketList & packetList, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth); + qint64 sendUnreliableUnorderedPacketList(NLPacketList & packetList, const HifiSockAddr & sockAddr, + HmacAuth * hmacAuth = nullptr); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index a83924ee58..d6b59d59f9 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -25,30 +25,29 @@ int packetTypeMetaTypeId = qRegisterMetaType(); PacketVersion versionForPacketType(PacketType packetType) { switch (packetType) { case PacketType::DomainList: - return static_cast(DomainListVersion::GetMachineFingerprintFromUUIDSupport); + return static_cast(DomainListVersion::UseHmacAuthentication); case PacketType::EntityAdd: case PacketType::EntityEdit: case PacketType::EntityData: case PacketType::EntityPhysics: - return static_cast(EntityVersion::ShadowControl); + return static_cast(EntityVersion::UseHmacAuthentication); case PacketType::EntityQuery: - return static_cast(EntityQueryPacketVersion::RemovedJurisdictions); + return static_cast(EntityQueryPacketVersion::UseHmacAuthentication); case PacketType::AvatarIdentity: case PacketType::AvatarData: case PacketType::BulkAvatarData: case PacketType::KillAvatar: - return static_cast(AvatarMixerPacketVersion::FBXReaderNodeReparenting); + return static_cast(AvatarMixerPacketVersion::UseHmacAuthentication); case PacketType::MessagesData: - return static_cast(MessageDataVersion::TextOrBinaryData); + return static_cast(MessageDataVersion::UseHmacAuthentication); case PacketType::ICEServerHeartbeat: return 18; // ICE Server Heartbeat signing case PacketType::AssetMappingOperation: case PacketType::AssetMappingOperationReply: - return static_cast(AssetServerPacketVersion::RedirectedMappings); case PacketType::AssetGetInfo: case PacketType::AssetGet: case PacketType::AssetUpload: - return static_cast(AssetServerPacketVersion::RangeRequestSupport); + return static_cast(AssetServerPacketVersion::UseHmacAuthentication); case PacketType::NodeIgnoreRequest: return 18; // Introduction of node ignore request (which replaced an unused packet tpye) @@ -59,10 +58,10 @@ PacketVersion versionForPacketType(PacketType packetType) { return static_cast(DomainConnectRequestVersion::AlwaysHasMachineFingerprint); case PacketType::DomainServerAddedNode: - return static_cast(DomainServerAddedNodeVersion::PermissionsGrid); + return static_cast(DomainServerAddedNodeVersion::UseHmacAuthentication); case PacketType::EntityScriptCallMethod: - return static_cast(EntityScriptCallMethodVersion::ClientCallable); + return static_cast(EntityScriptCallMethodVersion::UseHmacAuthentication); case PacketType::MixedAudio: case PacketType::SilentAudioFrame: @@ -70,13 +69,13 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::MicrophoneAudioNoEcho: case PacketType::MicrophoneAudioWithEcho: case PacketType::AudioStreamStats: - return static_cast(AudioVersion::HighDynamicRangeVolume); + return static_cast(AudioVersion::UseHmacAuthentication); case PacketType::ICEPing: return static_cast(IcePingVersion::SendICEPeerID); case PacketType::DomainSettings: return 18; // replace min_avatar_scale and max_avatar_scale with min_avatar_height and max_avatar_height default: - return 17; + return 18; } } diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 98a9087d37..9dc3f2befd 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -231,25 +231,29 @@ enum class EntityVersion : PacketVersion { ZoneStageRemoved, SoftEntities, MaterialEntities, - ShadowControl + ShadowControl, + UseHmacAuthentication }; enum class EntityScriptCallMethodVersion : PacketVersion { ServerCallable = 18, - ClientCallable = 19 + ClientCallable = 19, + UseHmacAuthentication = 20 }; enum class EntityQueryPacketVersion: PacketVersion { JSONFilter = 18, JSONFilterWithFamilyTree = 19, ConnectionIdentifier = 20, - RemovedJurisdictions = 21 + RemovedJurisdictions = 21, + UseHmacAuthentication = 22 }; enum class AssetServerPacketVersion: PacketVersion { VegasCongestionControl = 19, RangeRequestSupport, - RedirectedMappings + RedirectedMappings, + UseHmacAuthentication }; enum class AvatarMixerPacketVersion : PacketVersion { @@ -274,7 +278,8 @@ enum class AvatarMixerPacketVersion : PacketVersion { AvatarIdentityLookAtSnapping, UpdatedMannequinDefaultAvatar, AvatarJointDefaultPoseFlags, - FBXReaderNodeReparenting + FBXReaderNodeReparenting, + UseHmacAuthentication }; enum class DomainConnectRequestVersion : PacketVersion { @@ -294,14 +299,16 @@ enum class DomainConnectionDeniedVersion : PacketVersion { enum class DomainServerAddedNodeVersion : PacketVersion { PrePermissionsGrid = 17, - PermissionsGrid + PermissionsGrid, + UseHmacAuthentication }; enum class DomainListVersion : PacketVersion { PrePermissionsGrid = 18, PermissionsGrid, GetUsernameFromUUIDSupport, - GetMachineFingerprintFromUUIDSupport + GetMachineFingerprintFromUUIDSupport, + UseHmacAuthentication }; enum class AudioVersion : PacketVersion { @@ -312,10 +319,12 @@ enum class AudioVersion : PacketVersion { SpaceBubbleChanges, HasPersonalMute, HighDynamicRangeVolume, + UseHmacAuthentication, }; enum class MessageDataVersion : PacketVersion { - TextOrBinaryData = 18 + TextOrBinaryData = 18, + UseHmacAuthentication }; enum class IcePingVersion : PacketVersion { From 8ce03d65b76493ddc4c8d8a92bd3eea6abfc938a Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 20 Mar 2018 18:16:45 -0700 Subject: [PATCH 09/56] Only update the connection secret UUID if it changes Rekeying the openssl HMAC context occasionally causes hash generation errors. It is not clear why. The Node secret never seems to change to check for this before rekeying. Also other clean-up for PR. --- libraries/networking/src/LimitedNodeList.cpp | 4 ++-- libraries/networking/src/NLPacket.h | 2 +- libraries/networking/src/Node.cpp | 8 +++++++- libraries/networking/src/Node.h | 4 +--- libraries/shared/src/HmacAuth.cpp | 2 +- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index d09e379909..7d4ac574da 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -439,14 +439,14 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi if (activeSocket) { qint64 bytesSent = 0; - auto connectionSecret = destinationNode.getConnectionSecret(); + auto& connectionHash = destinationNode.getAuthenticateHash(); // close the last packet in the list packetList.closeCurrentPacket(); while (!packetList._packets.empty()) { bytesSent += sendPacket(packetList.takeFront(), *activeSocket, - &destinationNode.getAuthenticateHash()); + &connectionHash); } emit dataSent(destinationNode.getType(), bytesSent); diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 8f73475530..88b856cfda 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -73,7 +73,7 @@ public: static QUuid sourceIDInHeader(const udt::Packet& packet); static QByteArray verificationHashInHeader(const udt::Packet& packet); - static QByteArray hashForPacketAndSecret(const udt::Packet & packet, HmacAuth & hash); + static QByteArray hashForPacketAndSecret(const udt::Packet& packet, HmacAuth& hash); PacketType getType() const { return _type; } void setType(PacketType type); diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index 6669c68a2e..5af5172580 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -109,6 +109,7 @@ void Node::setType(char type) { _symmetricSocket.setObjectName(typeString); } + void Node::updateClockSkewUsec(qint64 clockSkewSample) { _clockSkewMovingPercentile.updatePercentile(clockSkewSample); _clockSkewUsec = (quint64)_clockSkewMovingPercentile.getValueAtPercentile(); @@ -194,6 +195,11 @@ QDebug operator<<(QDebug debug, const Node& node) { return debug.nospace(); } -void Node::_updateAuthenticateHash() { +void Node::setConnectionSecret(const QUuid & connectionSecret) { + if (_connectionSecret == connectionSecret) { + return; + } + + _connectionSecret = connectionSecret; _authenticateHash->setKey(_connectionSecret); } diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h index 80d51202d5..fe99e9c1ca 100644 --- a/libraries/networking/src/Node.h +++ b/libraries/networking/src/Node.h @@ -56,7 +56,7 @@ public: void setIsUpstream(bool isUpstream) { _isUpstream = isUpstream; } const QUuid& getConnectionSecret() const { return _connectionSecret; } - void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; _updateAuthenticateHash(); } + void setConnectionSecret(const QUuid& connectionSecret); HmacAuth& getAuthenticateHash() const { return *_authenticateHash; } NodeData* getLinkedData() const { return _linkedData.get(); } @@ -96,8 +96,6 @@ private: Node(const Node &otherNode); Node& operator=(Node otherNode); - void _updateAuthenticateHash(); - NodeType_t _type; QUuid _connectionSecret; diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp index 5d04bb96a4..f3ffec2c05 100644 --- a/libraries/shared/src/HmacAuth.cpp +++ b/libraries/shared/src/HmacAuth.cpp @@ -55,7 +55,7 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) { } QMutexLocker lock(&_lock); - return (bool) HMAC_Init(_hmacContext.get(), keyValue, keyLen, sslStruct); + return (bool) HMAC_Init_ex(_hmacContext.get(), keyValue, keyLen, sslStruct, nullptr); } bool HmacAuth::setKey(const QUuid& uidKey) { From eb04f77c3dc71990ea6e4fb21a0b156c234dab98 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 14:04:34 -0700 Subject: [PATCH 10/56] HMAC Auth - code modifications requested by reviewer --- libraries/networking/src/LimitedNodeList.cpp | 16 ++++++------- libraries/networking/src/LimitedNodeList.h | 8 +++---- libraries/networking/src/NLPacket.cpp | 10 ++++---- libraries/networking/src/NLPacket.h | 6 ++--- libraries/networking/src/Node.cpp | 5 ++-- libraries/networking/src/Node.h | 6 ++--- .../shared/src/{HmacAuth.cpp => HMACAuth.cpp} | 23 +++++++++---------- .../shared/src/{HmacAuth.h => HMACAuth.h} | 16 ++++++------- 8 files changed, 44 insertions(+), 46 deletions(-) rename libraries/shared/src/{HmacAuth.cpp => HMACAuth.cpp} (78%) rename libraries/shared/src/{HmacAuth.h => HMACAuth.h} (74%) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 7d4ac574da..ab66b7ae92 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -36,7 +36,7 @@ #include "HifiSockAddr.h" #include "NetworkLogging.h" #include "udt/Packet.h" -#include "HmacAuth.h" +#include "HMACAuth.h" static Setting::Handle LIMITED_NODELIST_LOCAL_PORT("LimitedNodeList.LocalPort", 0); @@ -320,7 +320,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe if (verifiedPacket && !ignoreVerification) { QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); - QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getAuthenticateHash()); + QByteArray expectedHash = NLPacket::hashForPacketAndHMAC(packet, sourceNode->getAuthenticateHash()); // check if the md5 hash in the header matches the hash we would expect if (packetHeaderHash != expectedHash) { @@ -364,7 +364,7 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) { _numCollectedBytes += packet.getDataSize(); } -void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAuth) { +void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HMACAuth* hmacAuth) { if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) { packet.writeSourceID(getSessionUUID()); } @@ -372,7 +372,7 @@ void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAu if (hmacAuth && !PacketTypeEnum::getNonSourcedPackets().contains(packet.getType()) && !PacketTypeEnum::getNonVerifiedPackets().contains(packet.getType())) { - packet.writeVerificationHashGivenSecret(*hmacAuth); + packet.writeVerificationHash(*hmacAuth); } } @@ -392,7 +392,7 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HmacAuth * hmacAuth) { + HMACAuth * hmacAuth) { Q_ASSERT(!packet.isPartOfMessage()); Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); @@ -419,7 +419,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& } qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - HmacAuth * hmacAuth) { + HMACAuth* hmacAuth) { Q_ASSERT(!packet->isPartOfMessage()); if (packet->isReliable()) { collectPacketStats(*packet); @@ -459,7 +459,7 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi } qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - HmacAuth * hmacAuth) { + HMACAuth* hmacAuth) { qint64 bytesSent = 0; // close the last packet in the list @@ -479,7 +479,7 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, for (std::unique_ptr& packet : packetList->_packets) { NLPacket* nlPacket = static_cast(packet.get()); collectPacketStats(*nlPacket); - fillPacketHeader(*nlPacket, nullptr); + fillPacketHeader(*nlPacket); } return _nodeSocket.writePacketList(std::move(packetList), sockAddr); diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 612a6ce947..eb70fbcbdf 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -131,17 +131,17 @@ public: // use sendUnreliablePacket to send an unreliable packet (that you do not need to move) // either to a node (via its active socket) or to a manual sockaddr qint64 sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode); - qint64 sendUnreliablePacket(const NLPacket & packet, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth = nullptr); + qint64 sendUnreliablePacket(const NLPacket & packet, const HifiSockAddr & sockAddr, HMACAuth * hmacAuth = nullptr); // use sendPacket to send a moved unreliable or reliable NL packet to a node's active socket or manual sockaddr qint64 sendPacket(std::unique_ptr packet, const Node& destinationNode); - qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth = nullptr); + qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr & sockAddr, HMACAuth * hmacAuth = nullptr); // use sendUnreliableUnorderedPacketList to unreliably send separate packets from the packet list // either to a node's active socket or to a manual sockaddr qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const Node& destinationNode); qint64 sendUnreliableUnorderedPacketList(NLPacketList & packetList, const HifiSockAddr & sockAddr, - HmacAuth * hmacAuth = nullptr); + HMACAuth * hmacAuth = nullptr); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr @@ -362,7 +362,7 @@ protected: qint64 writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr, const QUuid& connectionSecret = QUuid()); void collectPacketStats(const NLPacket& packet); - void fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAuth); + void fillPacketHeader(const NLPacket& packet, HMACAuth* hmacAuth = nullptr); void setLocalSocket(const HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 37bb465ca9..93274843a6 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -11,7 +11,7 @@ #include "NLPacket.h" -#include "HmacAuth.h" +#include "HMACAuth.h" int NLPacket::localHeaderSize(PacketType type) { bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type); @@ -151,11 +151,11 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); } -QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, HmacAuth& hash) { +QByteArray NLPacket::hashForPacketAndHMAC(const udt::Packet& packet, HMACAuth& hash) { int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; hash.addData(packet.getData() + offset, packet.getDataSize() - offset); - auto hashResult(hash.result()); + auto hashResult { hash.result() }; return QByteArray((const char*) hashResult.data(), (int) hashResult.size()); } @@ -208,14 +208,14 @@ void NLPacket::writeSourceID(const QUuid& sourceID) const { _sourceID = sourceID; } -void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const { +void NLPacket::writeVerificationHash(HMACAuth& hmacAuth) const { Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type) && !PacketTypeEnum::getNonVerifiedPackets().contains(_type)); auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID; - QByteArray verificationHash = hashForPacketAndSecret(*this, hmacAuth); + QByteArray verificationHash = hashForPacketAndHMAC(*this, hmacAuth); memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size()); } diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 88b856cfda..302598f77c 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -18,7 +18,7 @@ #include "udt/Packet.h" -class HmacAuth; +class HMACAuth; class NLPacket : public udt::Packet { Q_OBJECT @@ -73,7 +73,7 @@ public: static QUuid sourceIDInHeader(const udt::Packet& packet); static QByteArray verificationHashInHeader(const udt::Packet& packet); - static QByteArray hashForPacketAndSecret(const udt::Packet& packet, HmacAuth& hash); + static QByteArray hashForPacketAndHMAC(const udt::Packet& packet, HMACAuth& hash); PacketType getType() const { return _type; } void setType(PacketType type); @@ -84,7 +84,7 @@ public: const QUuid& getSourceID() const { return _sourceID; } void writeSourceID(const QUuid& sourceID) const; - void writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const; + void writeVerificationHash(HMACAuth& hmacAuth) const; protected: diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index 5af5172580..132d27d311 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -89,8 +89,7 @@ Node::Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, QObject* parent) : NetworkPeer(uuid, publicSocket, localSocket, parent), _type(type), - _authenticateHash(new HmacAuth), - _pingMs(-1), // "Uninitialized" + _authenticateHash(new HMACAuth), _pingMs(-1), // "Uninitialized" _clockSkewUsec(0), _mutex(), _clockSkewMovingPercentile(30, 0.8f) // moving 80th percentile of 30 samples @@ -195,7 +194,7 @@ QDebug operator<<(QDebug debug, const Node& node) { return debug.nospace(); } -void Node::setConnectionSecret(const QUuid & connectionSecret) { +void Node::setConnectionSecret(const QUuid& connectionSecret) { if (_connectionSecret == connectionSecret) { return; } diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h index fe99e9c1ca..5b3b559582 100644 --- a/libraries/networking/src/Node.h +++ b/libraries/networking/src/Node.h @@ -33,7 +33,7 @@ #include "SimpleMovingAverage.h" #include "MovingPercentile.h" #include "NodePermissions.h" -#include "HmacAuth.h" +#include "HMACAuth.h" class Node : public NetworkPeer { Q_OBJECT @@ -57,7 +57,7 @@ public: const QUuid& getConnectionSecret() const { return _connectionSecret; } void setConnectionSecret(const QUuid& connectionSecret); - HmacAuth& getAuthenticateHash() const { return *_authenticateHash; } + HMACAuth& getAuthenticateHash() const { return *_authenticateHash; } NodeData* getLinkedData() const { return _linkedData.get(); } void setLinkedData(std::unique_ptr linkedData) { _linkedData = std::move(linkedData); } @@ -99,7 +99,7 @@ private: NodeType_t _type; QUuid _connectionSecret; - std::unique_ptr _authenticateHash; + std::unique_ptr _authenticateHash; std::unique_ptr _linkedData; bool _isReplicated { false }; int _pingMs; diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HMACAuth.cpp similarity index 78% rename from libraries/shared/src/HmacAuth.cpp rename to libraries/shared/src/HMACAuth.cpp index f3ffec2c05..9abce7b954 100644 --- a/libraries/shared/src/HmacAuth.cpp +++ b/libraries/shared/src/HMACAuth.cpp @@ -1,5 +1,5 @@ // -// HmacAuth.cpp +// HMACAuth.cpp // libraries/shared/src // // Created by Simon Walton on 3/19/2018. @@ -11,25 +11,24 @@ #include -#include "HmacAuth.h" +#include "HMACAuth.h" #include -HmacAuth::HmacAuth(AuthMethod authMethod) +HMACAuth::HMACAuth(AuthMethod authMethod) : _hmacContext(new(HMAC_CTX)) , _authMethod(authMethod) { HMAC_CTX_init(_hmacContext.get()); } -HmacAuth::~HmacAuth() { +HMACAuth::~HMACAuth() { HMAC_CTX_cleanup(_hmacContext.get()); } -bool HmacAuth::setKey(const char * keyValue, int keyLen) { - const EVP_MD * sslStruct = nullptr; +bool HMACAuth::setKey(const char * keyValue, int keyLen) { + const EVP_MD* sslStruct = nullptr; - switch (_authMethod) - { + switch (_authMethod) { case MD5: sslStruct = EVP_md5(); break; @@ -58,18 +57,18 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) { return (bool) HMAC_Init_ex(_hmacContext.get(), keyValue, keyLen, sslStruct, nullptr); } -bool HmacAuth::setKey(const QUuid& uidKey) { +bool HMACAuth::setKey(const QUuid& uidKey) { const QByteArray rfcBytes(uidKey.toRfc4122()); return setKey(rfcBytes.constData(), rfcBytes.length()); } -bool HmacAuth::addData(const char * data, int dataLen) { +bool HMACAuth::addData(const char * data, int dataLen) { QMutexLocker lock(&_lock); return (bool) HMAC_Update(_hmacContext.get(), reinterpret_cast(data), dataLen); } -HmacAuth::HmacHash HmacAuth::result() { - HmacHash hashValue(EVP_MAX_MD_SIZE); +HMACAuth::HMACHash HMACAuth::result() { + HMACHash hashValue(EVP_MAX_MD_SIZE); unsigned int hashLen; QMutexLocker lock(&_lock); HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen); diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HMACAuth.h similarity index 74% rename from libraries/shared/src/HmacAuth.h rename to libraries/shared/src/HMACAuth.h index dfc79e8e47..4bb20a6464 100644 --- a/libraries/shared/src/HmacAuth.h +++ b/libraries/shared/src/HMACAuth.h @@ -1,6 +1,6 @@ // -// HmacAuth.h -// libraries/shared/src +// HMACAuth.h +// libraries/shared/src // // Created by Simon Walton on 3/19/2018. // Copyright 2018 High Fidelity, Inc. @@ -18,23 +18,23 @@ class QUuid; -class HmacAuth { +class HMACAuth { public: enum AuthMethod { MD5, SHA1, SHA224, SHA256, RIPEMD160 }; - using HmacHash = std::vector; + using HMACHash = std::vector; - explicit HmacAuth(AuthMethod authMethod = MD5); - ~HmacAuth(); + explicit HMACAuth(AuthMethod authMethod = MD5); + ~HMACAuth(); bool setKey(const char * keyValue, int keyLen); bool setKey(const QUuid& uidKey); bool addData(const char * data, int dataLen); - HmacHash result(); + HMACHash result(); private: QMutex _lock; std::unique_ptr _hmacContext; - AuthMethod _authMethod { MD5 }; + AuthMethod _authMethod; }; #endif // hifi_HmacAuth_h From adbb2400ab5c01ca2da0a6b47e286f73b63b8eff Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 14:58:14 -0700 Subject: [PATCH 11/56] HMAC Auth - add openssl to cmake file for lib shared --- libraries/shared/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/shared/CMakeLists.txt b/libraries/shared/CMakeLists.txt index 713501aa77..bff842fdd8 100644 --- a/libraries/shared/CMakeLists.txt +++ b/libraries/shared/CMakeLists.txt @@ -2,6 +2,7 @@ set(TARGET_NAME shared) # TODO: there isn't really a good reason to have Script linked here - let's get what is requiring it out (RegisteredMetaTypes.cpp) setup_hifi_library(Gui Network Script) +include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") if (WIN32) target_link_libraries(${TARGET_NAME} Wbemuuid.lib) From fb16e772ba45f84b1b7dbdfdd3024caa13784e62 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 15:25:50 -0700 Subject: [PATCH 12/56] Move HMACAuth class to networking lib Also reverts addition of openssl headers to shared lib. Commit will require rerunning cmake. --- libraries/{shared => networking}/src/HMACAuth.cpp | 0 libraries/{shared => networking}/src/HMACAuth.h | 0 libraries/shared/CMakeLists.txt | 1 - 3 files changed, 1 deletion(-) rename libraries/{shared => networking}/src/HMACAuth.cpp (100%) rename libraries/{shared => networking}/src/HMACAuth.h (100%) diff --git a/libraries/shared/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp similarity index 100% rename from libraries/shared/src/HMACAuth.cpp rename to libraries/networking/src/HMACAuth.cpp diff --git a/libraries/shared/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h similarity index 100% rename from libraries/shared/src/HMACAuth.h rename to libraries/networking/src/HMACAuth.h diff --git a/libraries/shared/CMakeLists.txt b/libraries/shared/CMakeLists.txt index bff842fdd8..713501aa77 100644 --- a/libraries/shared/CMakeLists.txt +++ b/libraries/shared/CMakeLists.txt @@ -2,7 +2,6 @@ set(TARGET_NAME shared) # TODO: there isn't really a good reason to have Script linked here - let's get what is requiring it out (RegisteredMetaTypes.cpp) setup_hifi_library(Gui Network Script) -include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") if (WIN32) target_link_libraries(${TARGET_NAME} Wbemuuid.lib) From 64973aa334f5f6582ea183c5e026f6dd19b5db31 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 17:06:15 -0700 Subject: [PATCH 13/56] OpenSSL HMAC changes for Android Looks like Android uses OpenSSL 1.1.0, which provides an allocator for its HMAC context. --- libraries/networking/src/HMACAuth.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp index 9abce7b954..52c43fe574 100644 --- a/libraries/networking/src/HMACAuth.cpp +++ b/libraries/networking/src/HMACAuth.cpp @@ -9,6 +9,7 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // +#include #include #include "HMACAuth.h" @@ -16,7 +17,11 @@ #include HMACAuth::HMACAuth(AuthMethod authMethod) +#if OPENSSL_VERSION_NUMBER >= 0x10100000 + : _hmacContext(HMAC_CTX_new()) +#else : _hmacContext(new(HMAC_CTX)) +#endif , _authMethod(authMethod) { HMAC_CTX_init(_hmacContext.get()); } From 755d89464fb326e74311cd68ba648cc4a17e0dbe Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 17:15:49 -0700 Subject: [PATCH 14/56] HMAC Auth - reviewer-requested changes --- libraries/networking/src/HMACAuth.cpp | 2 +- libraries/networking/src/HMACAuth.h | 8 ++++---- libraries/networking/src/LimitedNodeList.cpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp index 52c43fe574..dc6d790425 100644 --- a/libraries/networking/src/HMACAuth.cpp +++ b/libraries/networking/src/HMACAuth.cpp @@ -30,7 +30,7 @@ HMACAuth::~HMACAuth() { HMAC_CTX_cleanup(_hmacContext.get()); } -bool HMACAuth::setKey(const char * keyValue, int keyLen) { +bool HMACAuth::setKey(const char* keyValue, int keyLen) { const EVP_MD* sslStruct = nullptr; switch (_authMethod) { diff --git a/libraries/networking/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h index 4bb20a6464..57f9dd64b8 100644 --- a/libraries/networking/src/HMACAuth.h +++ b/libraries/networking/src/HMACAuth.h @@ -1,6 +1,6 @@ // // HMACAuth.h -// libraries/shared/src +// libraries/networking/src // // Created by Simon Walton on 3/19/2018. // Copyright 2018 High Fidelity, Inc. @@ -9,8 +9,8 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#ifndef hifi_HmacAuth_h -#define hifi_HmacAuth_h +#ifndef hifi_HMACAuth_h +#define hifi_HMACAuth_h #include #include @@ -37,4 +37,4 @@ private: AuthMethod _authMethod; }; -#endif // hifi_HmacAuth_h +#endif // hifi_HMACAuth_h diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index ab66b7ae92..d2de034d0e 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -392,7 +392,7 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HMACAuth * hmacAuth) { + HMACAuth* hmacAuth) { Q_ASSERT(!packet.isPartOfMessage()); Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); From ef087702352784848b93bd60e6a2adcc76dd9121 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 17:17:24 -0700 Subject: [PATCH 15/56] Missed reviewer change --- libraries/networking/src/HMACAuth.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp index dc6d790425..1096098cdd 100644 --- a/libraries/networking/src/HMACAuth.cpp +++ b/libraries/networking/src/HMACAuth.cpp @@ -67,7 +67,7 @@ bool HMACAuth::setKey(const QUuid& uidKey) { return setKey(rfcBytes.constData(), rfcBytes.length()); } -bool HMACAuth::addData(const char * data, int dataLen) { +bool HMACAuth::addData(const char* data, int dataLen) { QMutexLocker lock(&_lock); return (bool) HMAC_Update(_hmacContext.get(), reinterpret_cast(data), dataLen); } From 3ced1c89237b5706b5365b06f4d2810e8d8aa565 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 17:31:54 -0700 Subject: [PATCH 16/56] More Openssl 1.1 (Android) fixes --- libraries/networking/src/HMACAuth.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp index 1096098cdd..77b4fb67b2 100644 --- a/libraries/networking/src/HMACAuth.cpp +++ b/libraries/networking/src/HMACAuth.cpp @@ -16,12 +16,17 @@ #include -HMACAuth::HMACAuth(AuthMethod authMethod) #if OPENSSL_VERSION_NUMBER >= 0x10100000 +HMACAuth::HMACAuth(AuthMethod authMethod) : _hmacContext(HMAC_CTX_new()) + , _authMethod(authMethod) { } + +HMACAuth::~HMACAuth() { } + #else + +HMACAuth::HMACAuth(AuthMethod authMethod) : _hmacContext(new(HMAC_CTX)) -#endif , _authMethod(authMethod) { HMAC_CTX_init(_hmacContext.get()); } @@ -29,6 +34,7 @@ HMACAuth::HMACAuth(AuthMethod authMethod) HMACAuth::~HMACAuth() { HMAC_CTX_cleanup(_hmacContext.get()); } +#endif bool HMACAuth::setKey(const char* keyValue, int keyLen) { const EVP_MD* sslStruct = nullptr; @@ -79,6 +85,6 @@ HMACAuth::HMACHash HMACAuth::result() { HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen); hashValue.resize((size_t) hashLen); // Clear state for possible reuse. - HMAC_Init(_hmacContext.get(), nullptr, 0, nullptr); + HMAC_Init_ex(_hmacContext.get(), nullptr, 0, nullptr, nullptr); return hashValue; } From d58b2acc8ce5c7ee8fd3211d41e8937656052e79 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 18:25:26 -0700 Subject: [PATCH 17/56] Use raw pointer for possibly-opaque openssl context type --- libraries/networking/src/HMACAuth.cpp | 18 +++++++++++------- libraries/networking/src/HMACAuth.h | 6 +++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp index 77b4fb67b2..baeffeadeb 100644 --- a/libraries/networking/src/HMACAuth.cpp +++ b/libraries/networking/src/HMACAuth.cpp @@ -21,18 +21,22 @@ HMACAuth::HMACAuth(AuthMethod authMethod) : _hmacContext(HMAC_CTX_new()) , _authMethod(authMethod) { } -HMACAuth::~HMACAuth() { } +HMACAuth::~HMACAuth() +{ + HMAC_CTX_free(_hmacContext); +} #else HMACAuth::HMACAuth(AuthMethod authMethod) : _hmacContext(new(HMAC_CTX)) , _authMethod(authMethod) { - HMAC_CTX_init(_hmacContext.get()); + HMAC_CTX_init(_hmacContext); } HMACAuth::~HMACAuth() { - HMAC_CTX_cleanup(_hmacContext.get()); + HMAC_CTX_cleanup(_hmacContext); + delete _hmacContext; } #endif @@ -65,7 +69,7 @@ bool HMACAuth::setKey(const char* keyValue, int keyLen) { } QMutexLocker lock(&_lock); - return (bool) HMAC_Init_ex(_hmacContext.get(), keyValue, keyLen, sslStruct, nullptr); + return (bool) HMAC_Init_ex(_hmacContext, keyValue, keyLen, sslStruct, nullptr); } bool HMACAuth::setKey(const QUuid& uidKey) { @@ -75,16 +79,16 @@ bool HMACAuth::setKey(const QUuid& uidKey) { bool HMACAuth::addData(const char* data, int dataLen) { QMutexLocker lock(&_lock); - return (bool) HMAC_Update(_hmacContext.get(), reinterpret_cast(data), dataLen); + return (bool) HMAC_Update(_hmacContext, reinterpret_cast(data), dataLen); } HMACAuth::HMACHash HMACAuth::result() { HMACHash hashValue(EVP_MAX_MD_SIZE); unsigned int hashLen; QMutexLocker lock(&_lock); - HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen); + HMAC_Final(_hmacContext, &hashValue[0], &hashLen); hashValue.resize((size_t) hashLen); // Clear state for possible reuse. - HMAC_Init_ex(_hmacContext.get(), nullptr, 0, nullptr, nullptr); + HMAC_Init_ex(_hmacContext, nullptr, 0, nullptr, nullptr); return hashValue; } diff --git a/libraries/networking/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h index 57f9dd64b8..89c20a3906 100644 --- a/libraries/networking/src/HMACAuth.h +++ b/libraries/networking/src/HMACAuth.h @@ -26,14 +26,14 @@ public: explicit HMACAuth(AuthMethod authMethod = MD5); ~HMACAuth(); - bool setKey(const char * keyValue, int keyLen); + bool setKey(const char* keyValue, int keyLen); bool setKey(const QUuid& uidKey); - bool addData(const char * data, int dataLen); + bool addData(const char* data, int dataLen); HMACHash result(); private: QMutex _lock; - std::unique_ptr _hmacContext; + struct hmac_ctx_st * _hmacContext; AuthMethod _authMethod; }; From 29b4353397bdfa168d0d212738d333c8091e7392 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 18:26:32 -0700 Subject: [PATCH 18/56] Spacing clean-up --- libraries/networking/src/HMACAuth.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h index 89c20a3906..0bf7a86ec1 100644 --- a/libraries/networking/src/HMACAuth.h +++ b/libraries/networking/src/HMACAuth.h @@ -33,7 +33,7 @@ public: private: QMutex _lock; - struct hmac_ctx_st * _hmacContext; + struct hmac_ctx_st* _hmacContext; AuthMethod _authMethod; }; From 16b0c48b73bb5d5afb31c2efc904328497dc84b8 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 18:32:25 -0700 Subject: [PATCH 19/56] HMACAuth - improved syntax for new --- libraries/networking/src/HMACAuth.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp index baeffeadeb..fdc2588f62 100644 --- a/libraries/networking/src/HMACAuth.cpp +++ b/libraries/networking/src/HMACAuth.cpp @@ -29,7 +29,7 @@ HMACAuth::~HMACAuth() #else HMACAuth::HMACAuth(AuthMethod authMethod) - : _hmacContext(new(HMAC_CTX)) + : _hmacContext(new HMAC_CTX()) , _authMethod(authMethod) { HMAC_CTX_init(_hmacContext); } From 3e1a33377615fa097605125d6eea0ac1c22d3a2a Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 21 Mar 2018 18:40:54 -0700 Subject: [PATCH 20/56] Just bump default packet version --- .../networking/src/udt/PacketHeaders.cpp | 19 +++++++------- libraries/networking/src/udt/PacketHeaders.h | 25 ++++++------------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index d6b59d59f9..f09a049fc4 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -25,29 +25,30 @@ int packetTypeMetaTypeId = qRegisterMetaType(); PacketVersion versionForPacketType(PacketType packetType) { switch (packetType) { case PacketType::DomainList: - return static_cast(DomainListVersion::UseHmacAuthentication); + return static_cast(DomainListVersion::GetMachineFingerprintFromUUIDSupport); case PacketType::EntityAdd: case PacketType::EntityEdit: case PacketType::EntityData: case PacketType::EntityPhysics: - return static_cast(EntityVersion::UseHmacAuthentication); + return static_cast(EntityVersion::ShadowControl); case PacketType::EntityQuery: - return static_cast(EntityQueryPacketVersion::UseHmacAuthentication); + return static_cast(EntityQueryPacketVersion::RemovedJurisdictions); case PacketType::AvatarIdentity: case PacketType::AvatarData: case PacketType::BulkAvatarData: case PacketType::KillAvatar: - return static_cast(AvatarMixerPacketVersion::UseHmacAuthentication); + return static_cast(AvatarMixerPacketVersion::FBXReaderNodeReparenting); case PacketType::MessagesData: - return static_cast(MessageDataVersion::UseHmacAuthentication); + return static_cast(MessageDataVersion::TextOrBinaryData); case PacketType::ICEServerHeartbeat: return 18; // ICE Server Heartbeat signing case PacketType::AssetMappingOperation: case PacketType::AssetMappingOperationReply: + return static_cast(AssetServerPacketVersion::RedirectedMappings); case PacketType::AssetGetInfo: case PacketType::AssetGet: case PacketType::AssetUpload: - return static_cast(AssetServerPacketVersion::UseHmacAuthentication); + return static_cast(AssetServerPacketVersion::RangeRequestSupport); case PacketType::NodeIgnoreRequest: return 18; // Introduction of node ignore request (which replaced an unused packet tpye) @@ -58,10 +59,10 @@ PacketVersion versionForPacketType(PacketType packetType) { return static_cast(DomainConnectRequestVersion::AlwaysHasMachineFingerprint); case PacketType::DomainServerAddedNode: - return static_cast(DomainServerAddedNodeVersion::UseHmacAuthentication); + return static_cast(DomainServerAddedNodeVersion::PermissionsGrid); case PacketType::EntityScriptCallMethod: - return static_cast(EntityScriptCallMethodVersion::UseHmacAuthentication); + return static_cast(EntityScriptCallMethodVersion::ClientCallable); case PacketType::MixedAudio: case PacketType::SilentAudioFrame: @@ -69,7 +70,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::MicrophoneAudioNoEcho: case PacketType::MicrophoneAudioWithEcho: case PacketType::AudioStreamStats: - return static_cast(AudioVersion::UseHmacAuthentication); + return static_cast(AudioVersion::HighDynamicRangeVolume); case PacketType::ICEPing: return static_cast(IcePingVersion::SendICEPeerID); case PacketType::DomainSettings: diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 9dc3f2befd..98a9087d37 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -231,29 +231,25 @@ enum class EntityVersion : PacketVersion { ZoneStageRemoved, SoftEntities, MaterialEntities, - ShadowControl, - UseHmacAuthentication + ShadowControl }; enum class EntityScriptCallMethodVersion : PacketVersion { ServerCallable = 18, - ClientCallable = 19, - UseHmacAuthentication = 20 + ClientCallable = 19 }; enum class EntityQueryPacketVersion: PacketVersion { JSONFilter = 18, JSONFilterWithFamilyTree = 19, ConnectionIdentifier = 20, - RemovedJurisdictions = 21, - UseHmacAuthentication = 22 + RemovedJurisdictions = 21 }; enum class AssetServerPacketVersion: PacketVersion { VegasCongestionControl = 19, RangeRequestSupport, - RedirectedMappings, - UseHmacAuthentication + RedirectedMappings }; enum class AvatarMixerPacketVersion : PacketVersion { @@ -278,8 +274,7 @@ enum class AvatarMixerPacketVersion : PacketVersion { AvatarIdentityLookAtSnapping, UpdatedMannequinDefaultAvatar, AvatarJointDefaultPoseFlags, - FBXReaderNodeReparenting, - UseHmacAuthentication + FBXReaderNodeReparenting }; enum class DomainConnectRequestVersion : PacketVersion { @@ -299,16 +294,14 @@ enum class DomainConnectionDeniedVersion : PacketVersion { enum class DomainServerAddedNodeVersion : PacketVersion { PrePermissionsGrid = 17, - PermissionsGrid, - UseHmacAuthentication + PermissionsGrid }; enum class DomainListVersion : PacketVersion { PrePermissionsGrid = 18, PermissionsGrid, GetUsernameFromUUIDSupport, - GetMachineFingerprintFromUUIDSupport, - UseHmacAuthentication + GetMachineFingerprintFromUUIDSupport }; enum class AudioVersion : PacketVersion { @@ -319,12 +312,10 @@ enum class AudioVersion : PacketVersion { SpaceBubbleChanges, HasPersonalMute, HighDynamicRangeVolume, - UseHmacAuthentication, }; enum class MessageDataVersion : PacketVersion { - TextOrBinaryData = 18, - UseHmacAuthentication + TextOrBinaryData = 18 }; enum class IcePingVersion : PacketVersion { From 68ab0eed68dfe8b8b2b76472e8c0891d52e477ee Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 22 Mar 2018 14:44:25 -0700 Subject: [PATCH 21/56] HMACAuth - fix some more spacing issue --- libraries/networking/src/HMACAuth.cpp | 4 ++-- libraries/networking/src/LimitedNodeList.h | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp index fdc2588f62..42b5c48d93 100644 --- a/libraries/networking/src/HMACAuth.cpp +++ b/libraries/networking/src/HMACAuth.cpp @@ -1,6 +1,6 @@ // -// HMACAuth.cpp -// libraries/shared/src +// HMACAuth.cpp +// libraries/networking/src // // Created by Simon Walton on 3/19/2018. // Copyright 2018 High Fidelity, Inc. diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index eb70fbcbdf..64969862ee 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -131,17 +131,17 @@ public: // use sendUnreliablePacket to send an unreliable packet (that you do not need to move) // either to a node (via its active socket) or to a manual sockaddr qint64 sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode); - qint64 sendUnreliablePacket(const NLPacket & packet, const HifiSockAddr & sockAddr, HMACAuth * hmacAuth = nullptr); + qint64 sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, HMACAuth* hmacAuth = nullptr); // use sendPacket to send a moved unreliable or reliable NL packet to a node's active socket or manual sockaddr qint64 sendPacket(std::unique_ptr packet, const Node& destinationNode); - qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr & sockAddr, HMACAuth * hmacAuth = nullptr); + qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, HMACAuth* hmacAuth = nullptr); // use sendUnreliableUnorderedPacketList to unreliably send separate packets from the packet list // either to a node's active socket or to a manual sockaddr qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const Node& destinationNode); - qint64 sendUnreliableUnorderedPacketList(NLPacketList & packetList, const HifiSockAddr & sockAddr, - HMACAuth * hmacAuth = nullptr); + qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, + HMACAuth* hmacAuth = nullptr); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr From b5f165d48176a4c55c66160ce2ec0136b96f6430 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 23 Mar 2018 16:55:19 -0700 Subject: [PATCH 22/56] Store a short ID with the Node on the domain-server side --- domain-server/src/DomainGatekeeper.cpp | 30 +++++++++++++++++++++++++- domain-server/src/DomainGatekeeper.h | 14 ++++++++++++ libraries/networking/src/NetworkPeer.h | 5 +++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 7d0b538f6e..748c089b21 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -26,7 +27,7 @@ using SharedAssignmentPointer = QSharedPointer; DomainGatekeeper::DomainGatekeeper(DomainServer* server) : _server(server) { - + initLocalIDManagement(); } void DomainGatekeeper::addPendingAssignedNode(const QUuid& nodeUUID, const QUuid& assignmentUUID, @@ -525,6 +526,7 @@ SharedNodePointer DomainGatekeeper::addVerifiedNodeFromConnectRequest(const Node SharedNodePointer newNode = limitedNodeList->addOrUpdateNode(nodeID, nodeConnection.nodeType, nodeConnection.publicSockAddr, nodeConnection.localSockAddr); + newNode->setLocalID(findOrCreateLocalID(nodeID)); // So that we can send messages to this node at will - we need to activate the correct socket on this node now newNode->activateMatchingOrNewSymmetricSocket(discoveredSocket); @@ -1014,3 +1016,29 @@ void DomainGatekeeper::refreshGroupsCache() { _server->_settingsManager.debugDumpGroupsState(); #endif } + +void DomainGatekeeper::initLocalIDManagement() { + std::uniform_int_distribution sixteenBitRand; + std::random_device randomDevice; + std::default_random_engine engine {randomDevice()}; + _currentLocalID = sixteenBitRand(engine); + // Ensure increment is odd. + _idIncrement = sixteenBitRand(engine) | 1; +} + +Node::LocalID DomainGatekeeper::findOrCreateLocalID(const QUuid& uuid) { + auto existingLocalIDIt = _uuidToLocalID.find(uuid); + if (existingLocalIDIt != _uuidToLocalID.end()) { + return existingLocalIDIt->second; + } + + Node::LocalID newLocalID; + do { + newLocalID = _currentLocalID; + _currentLocalID += _idIncrement; + } while (_localIDToUUID.find(newLocalID) != _localIDToUUID.end()); + + _uuidToLocalID.emplace(uuid, newLocalID); + _localIDToUUID.emplace(newLocalID, uuid); + return newLocalID; +} diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h index 09db075e07..896997a0e7 100644 --- a/domain-server/src/DomainGatekeeper.h +++ b/domain-server/src/DomainGatekeeper.h @@ -120,6 +120,20 @@ private: void getGroupMemberships(const QString& username); // void getIsGroupMember(const QString& username, const QUuid groupID); void getDomainOwnerFriendsList(); + + // Local ID management. + void initLocalIDManagement(); + Node::LocalID findOrCreateLocalID(const QUuid& uuid); + struct UuidHash { + size_t operator()(const QUuid& uuid) const { return qHash(uuid); } + }; + using UUIDToLocalID = std::unordered_map ; + using LocalIDToUUID = std::unordered_map; + UUIDToLocalID _uuidToLocalID; + LocalIDToUUID _localIDToUUID; + + Node::LocalID _currentLocalID; + quint16 _idIncrement; }; diff --git a/libraries/networking/src/NetworkPeer.h b/libraries/networking/src/NetworkPeer.h index 9842768b37..f36db402ce 100644 --- a/libraries/networking/src/NetworkPeer.h +++ b/libraries/networking/src/NetworkPeer.h @@ -39,6 +39,10 @@ public: const QUuid& getUUID() const { return _uuid; } void setUUID(const QUuid& uuid) { _uuid = uuid; } + using LocalID = quint16; + LocalID getLocalID() const { return _localID; } + void setLocalID(LocalID localID) { _localID = localID; } + void softReset(); void reset(); @@ -99,6 +103,7 @@ protected: void setActiveSocket(HifiSockAddr* discoveredSocket); QUuid _uuid; + LocalID _localID { 0 }; HifiSockAddr _publicSocket; HifiSockAddr _localSocket; From 4ec77e3af6f7752564e0cf8dc23d895e1079cc67 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 26 Mar 2018 14:57:59 -0700 Subject: [PATCH 23/56] WIP - assign short IDs to nodes and distribute them. --- domain-server/src/DomainGatekeeper.h | 3 ++- domain-server/src/DomainServer.cpp | 5 +++++ libraries/networking/src/LimitedNodeList.cpp | 9 +++++++++ libraries/networking/src/LimitedNodeList.h | 3 +++ libraries/networking/src/Node.cpp | 4 +++- libraries/networking/src/NodeList.cpp | 17 ++++++++++++----- 6 files changed, 34 insertions(+), 7 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h index 896997a0e7..afb848f271 100644 --- a/domain-server/src/DomainGatekeeper.h +++ b/domain-server/src/DomainGatekeeper.h @@ -41,6 +41,8 @@ public: void removeICEPeer(const QUuid& peerUUID) { _icePeers.remove(peerUUID); } + Node::LocalID findOrCreateLocalID(const QUuid& uuid); + static void sendProtocolMismatchConnectionDenial(const HifiSockAddr& senderSockAddr); public slots: void processConnectRequestPacket(QSharedPointer message); @@ -123,7 +125,6 @@ private: // Local ID management. void initLocalIDManagement(); - Node::LocalID findOrCreateLocalID(const QUuid& uuid); struct UuidHash { size_t operator()(const QUuid& uuid) const { return qHash(uuid); } }; diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 197ac7eac2..5ae2f8514c 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -691,6 +691,10 @@ void DomainServer::setupNodeListAndAssignments() { } } + // Create our own short session ID. + Node::LocalID serverSessionLocalID = _gatekeeper.findOrCreateLocalID(nodeList->getSessionUUID()); + nodeList->setSessionLocalID(serverSessionLocalID); + if (isMetaverseDomain) { // see if we think we're a temp domain (we have an API key) or a full domain const auto& temporaryDomainKey = DependencyManager::get()->getTemporaryDomainKey(getID()); @@ -1165,6 +1169,7 @@ void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const Hif extendedHeaderStream << limitedNodeList->getSessionUUID(); extendedHeaderStream << node->getUUID(); + extendedHeaderStream << node->getLocalID(); extendedHeaderStream << node->getPermissions(); auto domainListPackets = NLPacketList::create(PacketType::DomainList, extendedHeader); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 0803e380f2..0b2fb9475d 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -131,6 +131,15 @@ void LimitedNodeList::setSessionUUID(const QUuid& sessionUUID) { } } +Node::LocalID LimitedNodeList::getSessionLocalID() const { + return _sessionLocalID; +} + +void LimitedNodeList::setSessionLocalID(Node::LocalID sessionLocalID) { + QWriteLocker lock { &_sessionUUIDLock }; // Necessary? + _sessionLocalID = sessionLocalID; +} + void LimitedNodeList::setPermissions(const NodePermissions& newPermissions) { NodePermissions originalPermissions = _permissions; diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 7165b3dd63..c725e8abb7 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -106,6 +106,8 @@ public: Q_ENUM(ConnectionStep); QUuid getSessionUUID() const; void setSessionUUID(const QUuid& sessionUUID); + Node::LocalID getSessionLocalID() const; + void setSessionLocalID(Node::LocalID localID); void setPermissions(const NodePermissions& newPermissions); bool isAllowedEditor() const { return _permissions.can(NodePermissions::Permission::canAdjustLocks); } @@ -427,6 +429,7 @@ private slots: private: mutable QReadWriteLock _sessionUUIDLock; QUuid _sessionUUID; + Node::LocalID _sessionLocalID { 0 }; }; #endif // hifi_LimitedNodeList_h diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index bd895c8ef1..73b7c44e7e 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -168,6 +168,7 @@ QDataStream& operator<<(QDataStream& out, const Node& node) { out << node._localSocket; out << node._permissions; out << node._isReplicated; + out << node._localID; return out; } @@ -178,6 +179,7 @@ QDataStream& operator>>(QDataStream& in, Node& node) { in >> node._localSocket; in >> node._permissions; in >> node._isReplicated; + in >> node._localID; return in; } @@ -188,7 +190,7 @@ QDebug operator<<(QDebug debug, const Node& node) { } else { debug.nospace() << " (" << node.getType() << ")"; } - debug << " " << node.getUUID().toString().toLocal8Bit().constData() << " "; + debug << " " << node.getUUID().toString().toLocal8Bit().constData() << "(" << node.getLocalID() << ") "; debug.nospace() << node.getPublicSocket() << "/" << node.getLocalSocket(); return debug.nospace(); } diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index cb0d2e4cd5..752d420a4b 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -594,9 +594,13 @@ void NodeList::processDomainServerList(QSharedPointer message) return; } - // pull our owner UUID from the packet, it's always the first thing + // pull our owner (ie. session) UUID from the packet, it's always the first thing + // The short (16 bit) ID comes next. QUuid newUUID; + Node::LocalID newLocalID; packetStream >> newUUID; + packetStream >> newLocalID; + setSessionLocalID(newLocalID); setSessionUUID(newUUID); // if this was the first domain-server list from this domain, we've now connected @@ -638,12 +642,14 @@ void NodeList::processDomainServerRemovedNode(QSharedPointer me void NodeList::parseNodeFromPacketStream(QDataStream& packetStream) { // setup variables to read into from QDataStream qint8 nodeType; - QUuid nodeUUID, connectionUUID; + QUuid nodeUUID, connectionSecretUUID; HifiSockAddr nodePublicSocket, nodeLocalSocket; NodePermissions permissions; bool isReplicated; + Node::LocalID sessionLocalID; - packetStream >> nodeType >> nodeUUID >> nodePublicSocket >> nodeLocalSocket >> permissions >> isReplicated; + packetStream >> nodeType >> nodeUUID >> nodePublicSocket >> nodeLocalSocket >> permissions + >> isReplicated >> sessionLocalID; // if the public socket address is 0 then it's reachable at the same IP // as the domain server @@ -651,10 +657,11 @@ void NodeList::parseNodeFromPacketStream(QDataStream& packetStream) { nodePublicSocket.setAddress(_domainHandler.getIP()); } - packetStream >> connectionUUID; + packetStream >> connectionSecretUUID; SharedNodePointer node = addOrUpdateNode(nodeUUID, nodeType, nodePublicSocket, - nodeLocalSocket, isReplicated, false, connectionUUID, permissions); + nodeLocalSocket, isReplicated, false, connectionSecretUUID, permissions); + node->setLocalID(sessionLocalID); // nodes that are downstream or upstream of our own type are kept alive when we hear about them from the domain server // and always have their public socket as their active socket From d3464378b71e4a98a60cbaddda2be933008d7202 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 27 Mar 2018 13:46:58 -0700 Subject: [PATCH 24/56] Short local source IDs - checkpoint DS assigns 16-bit IDs as well as UUIDs; ACs track mappings; nodes use short IDs in packets. Initial setup works; then fails prob. due to DS UUID. --- assignment-client/src/assets/AssetServer.cpp | 2 +- assignment-client/src/audio/AudioMixer.cpp | 8 +++- assignment-client/src/avatars/AvatarMixer.cpp | 10 +++-- domain-server/src/DomainGatekeeper.cpp | 5 ++- domain-server/src/DomainServer.cpp | 20 +++++----- libraries/audio/src/InboundAudioStream.cpp | 6 +-- libraries/networking/src/LimitedNodeList.cpp | 39 ++++++++++++++----- libraries/networking/src/LimitedNodeList.h | 5 ++- libraries/networking/src/NLPacket.cpp | 18 +++++---- libraries/networking/src/NLPacket.h | 12 +++--- libraries/networking/src/NLPacketList.h | 4 +- libraries/networking/src/NodeList.cpp | 5 +-- libraries/networking/src/PacketReceiver.cpp | 5 +-- libraries/networking/src/ReceivedMessage.cpp | 2 +- libraries/networking/src/ReceivedMessage.h | 6 +-- libraries/octree/src/OctreeProcessor.cpp | 2 +- 16 files changed, 91 insertions(+), 58 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 87827a27d9..9d474c8c24 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -842,7 +842,7 @@ void AssetServer::handleAssetUpload(QSharedPointer message, Sha if (canWriteToAssetServer) { - qCDebug(asset_server) << "Starting an UploadAssetTask for upload from" << uuidStringWithoutCurlyBraces(message->getSourceID()); + qCDebug(asset_server) << "Starting an UploadAssetTask for upload from" << message->getSourceID(); auto task = new UploadAssetTask(message, senderNode, _filesDirectory, _filesizeLimit); _transferTaskPool.start(task); diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 8af4eec934..3b79eab06e 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -118,7 +118,11 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer mess // make sure we have a replicated node for the original sender of the packet auto nodeList = DependencyManager::get(); - QUuid nodeID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); + QUuid nodeID; + SharedNodePointer sourceNode = nodeList->nodeWithLocalID(message->getSourceID()); + if (sourceNode) { + nodeID = sourceNode->getUUID(); + } auto replicatedNode = nodeList->addOrUpdateNode(nodeID, NodeType::Agent, message->getSenderSockAddr(), message->getSenderSockAddr(), @@ -136,7 +140,7 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer mess auto replicatedMessage = QSharedPointer::create(audioData, rewrittenType, versionForPacketType(rewrittenType), - message->getSenderSockAddr(), nodeID); + message->getSenderSockAddr(), message->getSourceID()); getOrCreateClientData(replicatedNode.data())->queuePacket(replicatedMessage, replicatedNode); } diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 929941c05c..b0f1420472 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -112,8 +112,12 @@ void AvatarMixer::handleReplicatedPacket(QSharedPointer message void AvatarMixer::handleReplicatedBulkAvatarPacket(QSharedPointer message) { while (message->getBytesLeftToRead()) { // first, grab the node ID for this replicated avatar - auto nodeID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); - + QUuid nodeID; + auto nodeList = DependencyManager::get(); + SharedNodePointer sourceNode = nodeList->nodeWithLocalID(message->getSourceID()); + if (sourceNode) { + nodeID = sourceNode->getUUID(); + } // make sure we have an upstream replicated node that matches auto replicatedNode = addOrUpdateReplicatedNode(nodeID, message->getSenderSockAddr()); @@ -127,7 +131,7 @@ void AvatarMixer::handleReplicatedBulkAvatarPacket(QSharedPointer::create(avatarByteArray, PacketType::AvatarData, versionForPacketType(PacketType::AvatarData), - message->getSenderSockAddr(), nodeID); + message->getSenderSockAddr(), message->getSourceID()); // queue up the replicated avatar data with the client data for the replicated node auto start = usecTimestampNow(); diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 748c089b21..917d8a01c0 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -524,9 +524,10 @@ SharedNodePointer DomainGatekeeper::addVerifiedNodeFromConnectRequest(const Node auto limitedNodeList = DependencyManager::get(); + Node::LocalID newLocalID = findOrCreateLocalID(nodeID); SharedNodePointer newNode = limitedNodeList->addOrUpdateNode(nodeID, nodeConnection.nodeType, - nodeConnection.publicSockAddr, nodeConnection.localSockAddr); - newNode->setLocalID(findOrCreateLocalID(nodeID)); + nodeConnection.publicSockAddr, nodeConnection.localSockAddr, + newLocalID); // So that we can send messages to this node at will - we need to activate the correct socket on this node now newNode->activateMatchingOrNewSymmetricSocket(discoveredSocket); diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 5ae2f8514c..fee102d66a 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -593,8 +593,9 @@ bool DomainServer::isPacketVerified(const udt::Packet& packet) { if (!PacketTypeEnum::getNonSourcedPackets().contains(headerType)) { // this is a sourced packet - first check if we have a node that matches - QUuid sourceID = NLPacket::sourceIDInHeader(packet); - SharedNodePointer sourceNode = nodeList->nodeWithUUID(sourceID); + //QUuid sourceID = NLPacket::sourceIDInHeader(packet); + Node::LocalID localSourceID = NLPacket::sourceIDInHeader(packet); + SharedNodePointer sourceNode = nodeList->nodeWithLocalID(localSourceID); if (sourceNode) { // unverified DS packets (due to a lack of connection secret between DS + node) @@ -616,17 +617,17 @@ bool DomainServer::isPacketVerified(const udt::Packet& packet) { = LogHandler::getInstance().addRepeatedMessageRegex(UNKNOWN_REGEX); qDebug() << "Packet of type" << headerType - << "received from unmatched IP for UUID" << uuidStringWithoutCurlyBraces(sourceID); + << "received from unmatched IP for UUID" << uuidStringWithoutCurlyBraces(sourceNode->getUUID()); return false; } } else { - static const QString UNKNOWN_REGEX = "Packet of type \\d+ \\([\\sa-zA-Z:]+\\) received from unknown node with UUID"; + static const QString UNKNOWN_REGEX = "Packet of type \\d+ \\([\\sa-zA-Z:]+\\) received from unknown node with Local ID"; static QString repeatedMessage = LogHandler::getInstance().addRepeatedMessageRegex(UNKNOWN_REGEX); qDebug() << "Packet of type" << headerType - << "received from unknown node with UUID" << uuidStringWithoutCurlyBraces(sourceID); + << "received from unknown node with Local ID" << localSourceID; return false; } @@ -3203,13 +3204,12 @@ void DomainServer::processNodeDisconnectRequestPacket(QSharedPointer(); - const QUuid& nodeUUID = message->getSourceID(); - - qDebug() << "Received a disconnect request from node with UUID" << nodeUUID; + auto localID = message->getSourceID(); + qDebug() << "Received a disconnect request from node with local ID" << localID; // we want to check what type this node was before going to kill it so that we can avoid sending the RemovedNode // packet to nodes that don't care about this type - auto nodeToKill = limitedNodeList->nodeWithUUID(nodeUUID); + auto nodeToKill = limitedNodeList->nodeWithLocalID(localID); if (nodeToKill) { handleKillNode(nodeToKill); @@ -3477,7 +3477,7 @@ void DomainServer::handleDomainContentReplacementFromURLRequest(QSharedPointer message) { - auto node = DependencyManager::get()->nodeWithUUID(message->getSourceID()); + auto node = DependencyManager::get()->nodeWithLocalID(message->getSourceID()); if (node->getCanReplaceContent()) { handleOctreeFileReplacement(message->readAll()); } diff --git a/libraries/audio/src/InboundAudioStream.cpp b/libraries/audio/src/InboundAudioStream.cpp index 72acc7fcf6..983b5e1cb8 100644 --- a/libraries/audio/src/InboundAudioStream.cpp +++ b/libraries/audio/src/InboundAudioStream.cpp @@ -120,8 +120,8 @@ int InboundAudioStream::parseData(ReceivedMessage& message) { // parse sequence number and track it quint16 sequence; message.readPrimitive(&sequence); - SequenceNumberStats::ArrivalInfo arrivalInfo = _incomingSequenceNumberStats.sequenceNumberReceived(sequence, - message.getSourceID()); + SequenceNumberStats::ArrivalInfo arrivalInfo = _incomingSequenceNumberStats.sequenceNumberReceived(sequence, QUuid() // TBD + /*message.getSourceID()*/); QString codecInPacket = message.readString(); packetReceivedUpdateTimingStats(); @@ -186,7 +186,7 @@ int InboundAudioStream::parseData(ReceivedMessage& message) { _mismatchedAudioCodecCount = 0; // inform others of the mismatch - auto sendingNode = DependencyManager::get()->nodeWithUUID(message.getSourceID()); + auto sendingNode = DependencyManager::get()->nodeWithLocalID(message.getSourceID()); if (sendingNode) { emit mismatchedAudioCodec(sendingNode, _selectedCodecName, codecInPacket); qDebug(audio) << "Codec mismatch threshold exceeded, SelectedAudioFormat(" << _selectedCodecName << " ) sent"; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 0b2fb9475d..77daaa84ea 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -238,13 +238,16 @@ bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { senderString = QString("%1:%2").arg(senderSockAddr.getAddress().toString()).arg(senderSockAddr.getPort()); } } else { - sourceID = NLPacket::sourceIDInHeader(packet); + SharedNodePointer sourceNode = nodeWithLocalID(NLPacket::sourceIDInHeader(packet)); + if (sourceNode) { + sourceID = sourceNode->getUUID(); - hasBeenOutput = sourcedVersionDebugSuppressMap.contains(sourceID, headerType); + hasBeenOutput = sourcedVersionDebugSuppressMap.contains(sourceID, headerType); - if (!hasBeenOutput) { - sourcedVersionDebugSuppressMap.insert(sourceID, headerType); - senderString = uuidStringWithoutCurlyBraces(sourceID.toString()); + if (!hasBeenOutput) { + sourcedVersionDebugSuppressMap.insert(sourceID, headerType); + senderString = uuidStringWithoutCurlyBraces(sourceID.toString()); + } } } @@ -302,14 +305,17 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe return true; } } else { - QUuid sourceID = NLPacket::sourceIDInHeader(packet); // check if we were passed a sourceNode hint or if we need to look it up if (!sourceNode) { // figure out which node this is from - SharedNodePointer matchingNode = nodeWithUUID(sourceID); + NLPacket::LocalID sourceLocalID = NLPacket::sourceIDInHeader(packet); + + SharedNodePointer matchingNode = nodeWithLocalID(sourceLocalID); sourceNode = matchingNode.data(); } + + QUuid sourceID = sourceNode->getUUID(); if (!sourceNode && sourceID == getDomainUUID() && @@ -374,7 +380,7 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) { void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& connectionSecret) { if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) { - packet.writeSourceID(getSessionUUID()); + packet.writeSourceID(getSessionLocalID()); } if (!connectionSecret.isNull() @@ -557,6 +563,16 @@ SharedNodePointer LimitedNodeList::nodeWithUUID(const QUuid& nodeUUID) { return it == _nodeHash.cend() ? SharedNodePointer() : it->second; } +SharedNodePointer LimitedNodeList::nodeWithLocalID(Node::LocalID localID) const { + QReadLocker readLocker(&_nodeMutex); + + LocalIDMapping::const_iterator idIter = _localIDMap.find(localID); + if (idIter == _localIDMap.cend()) { + qCDebug(networking) << "No such Node with local ID " << localID; + } + return idIter == _localIDMap.cend() ? nullptr : idIter->second; +} + void LimitedNodeList::eraseAllNodes() { QSet killedNodes; @@ -565,6 +581,8 @@ void LimitedNodeList::eraseAllNodes() { // and then remove them from the hash QWriteLocker writeLocker(&_nodeMutex); + _localIDMap.erase(_localIDMap.begin(), _localIDMap.end()); + if (_nodeHash.size() > 0) { qCDebug(networking) << "LimitedNodeList::eraseAllNodes() removing all nodes from NodeList."; @@ -630,7 +648,7 @@ void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) { SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, - bool isReplicated, bool isUpstream, + Node::LocalID localID, bool isReplicated, bool isUpstream, const QUuid& connectionSecret, const NodePermissions& permissions) { QReadLocker readLocker(&_nodeMutex); NodeHash::const_iterator it = _nodeHash.find(uuid); @@ -644,6 +662,7 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t matchingNode->setConnectionSecret(connectionSecret); matchingNode->setIsReplicated(isReplicated); matchingNode->setIsUpstream(isUpstream || NodeType::isUpstream(nodeType)); + matchingNode->setLocalID(localID); return matchingNode; } else { @@ -653,6 +672,7 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t newNode->setIsUpstream(isUpstream || NodeType::isUpstream(nodeType)); newNode->setConnectionSecret(connectionSecret); newNode->setPermissions(permissions); + newNode->setLocalID(localID); // move the newly constructed node to the LNL thread newNode->moveToThread(thread()); @@ -693,6 +713,7 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t #else _nodeHash.emplace(newNode->getUUID(), newNodePointer); #endif + _localIDMap.emplace(localID, newNodePointer); readLocker.unlock(); qCDebug(networking) << "Added" << *newNode; diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index c725e8abb7..51c8831e2b 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -157,10 +157,11 @@ public: size_t size() const { QReadLocker readLock(&_nodeMutex); return _nodeHash.size(); } SharedNodePointer nodeWithUUID(const QUuid& nodeUUID); + SharedNodePointer nodeWithLocalID(Node::LocalID localID) const; SharedNodePointer addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, - bool isReplicated = false, bool isUpstream = false, + Node::LocalID localID = 0, bool isReplicated = false, bool isUpstream = false, const QUuid& connectionSecret = QUuid(), const NodePermissions& permissions = DEFAULT_AGENT_PERMISSIONS); @@ -429,6 +430,8 @@ private slots: private: mutable QReadWriteLock _sessionUUIDLock; QUuid _sessionUUID; + using LocalIDMapping = std::unordered_map; + LocalIDMapping _localIDMap; Node::LocalID _sessionLocalID { 0 }; }; diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 5c5077691b..66e74238aa 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -14,7 +14,7 @@ int NLPacket::localHeaderSize(PacketType type) { bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type); bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type); - qint64 optionalSize = (nonSourced ? 0 : NUM_BYTES_RFC4122_UUID) + ((nonSourced || nonVerified) ? 0 : NUM_BYTES_MD5_HASH); + qint64 optionalSize = (nonSourced ? 0 : NUM_BYTES_LOCALID) + ((nonSourced || nonVerified) ? 0 : NUM_BYTES_MD5_HASH); return sizeof(PacketType) + sizeof(PacketVersion) + optionalSize; } int NLPacket::totalHeaderSize(PacketType type, bool isPartOfMessage) { @@ -139,13 +139,14 @@ PacketVersion NLPacket::versionInHeader(const udt::Packet& packet) { return *reinterpret_cast(packet.getData() + headerOffset + sizeof(PacketType)); } -QUuid NLPacket::sourceIDInHeader(const udt::Packet& packet) { +NLPacket::LocalID NLPacket::sourceIDInHeader(const udt::Packet& packet) { int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion); - return QUuid::fromRfc4122(QByteArray::fromRawData(packet.getData() + offset, NUM_BYTES_RFC4122_UUID)); + return *reinterpret_cast(packet.getData() + offset); } QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { - int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID; + int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + + sizeof(PacketVersion) + NUM_BYTES_LOCALID; return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); } @@ -153,7 +154,7 @@ QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUu QCryptographicHash hash(QCryptographicHash::Md5); int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) - + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; + + NUM_BYTES_LOCALID + NUM_BYTES_MD5_HASH; // add the packet payload and the connection UUID hash.addData(packet.getData() + offset, packet.getDataSize() - offset); @@ -203,11 +204,12 @@ void NLPacket::readSourceID() { } } -void NLPacket::writeSourceID(const QUuid& sourceID) const { +void NLPacket::writeSourceID(LocalID sourceID) const { Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type)); auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion); - memcpy(_packet.get() + offset, sourceID.toRfc4122().constData(), NUM_BYTES_RFC4122_UUID); + + memcpy(_packet.get() + offset, &sourceID, sizeof(sourceID)); _sourceID = sourceID; } @@ -217,7 +219,7 @@ void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) c !PacketTypeEnum::getNonVerifiedPackets().contains(_type)); auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) - + NUM_BYTES_RFC4122_UUID; + + NUM_BYTES_LOCALID; QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret); memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size()); diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index f49cc47645..a4690a376c 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -43,10 +43,12 @@ public: // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | // NLPacket Header Format + using LocalID = qint16; + static const int NUM_BYTES_LOCALID = sizeof(LocalID); // this is used by the Octree classes - must be known at compile time static const int MAX_PACKET_HEADER_SIZE = sizeof(udt::Packet::SequenceNumberAndBitField) + sizeof(udt::Packet::MessageNumberAndBitField) + - sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_LOCALID + NUM_BYTES_MD5_HASH; static std::unique_ptr create(PacketType type, qint64 size = -1, bool isReliable = false, bool isPartOfMessage = false, PacketVersion version = 0); @@ -69,7 +71,7 @@ public: static PacketType typeInHeader(const udt::Packet& packet); static PacketVersion versionInHeader(const udt::Packet& packet); - static QUuid sourceIDInHeader(const udt::Packet& packet); + static LocalID sourceIDInHeader(const udt::Packet& packet); static QByteArray verificationHashInHeader(const udt::Packet& packet); static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret); @@ -79,9 +81,9 @@ public: PacketVersion getVersion() const { return _version; } void setVersion(PacketVersion version); - const QUuid& getSourceID() const { return _sourceID; } + LocalID getSourceID() const { return _sourceID; } - void writeSourceID(const QUuid& sourceID) const; + void writeSourceID(qint16 sourceID) const; void writeVerificationHashGivenSecret(const QUuid& connectionSecret) const; protected: @@ -106,7 +108,7 @@ protected: PacketType _type; PacketVersion _version; - mutable QUuid _sourceID; + mutable LocalID _sourceID; }; #endif // hifi_NLPacket_h diff --git a/libraries/networking/src/NLPacketList.h b/libraries/networking/src/NLPacketList.h index 910d39f71b..9c50033ca7 100644 --- a/libraries/networking/src/NLPacketList.h +++ b/libraries/networking/src/NLPacketList.h @@ -22,7 +22,7 @@ public: bool isReliable = false, bool isOrdered = false); PacketVersion getVersion() const { return _packetVersion; } - const QUuid& getSourceID() const { return _sourceID; } + NLPacket::LocalID getSourceID() const { return _sourceID; } qint64 getMaxSegmentSize() const override { return NLPacket::maxPayloadSize(_packetType, _isOrdered); } @@ -37,7 +37,7 @@ private: PacketVersion _packetVersion; - QUuid _sourceID; + NLPacket::LocalID _sourceID; }; Q_DECLARE_METATYPE(QSharedPointer) diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 752d420a4b..4556b441f2 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -659,9 +659,8 @@ void NodeList::parseNodeFromPacketStream(QDataStream& packetStream) { packetStream >> connectionSecretUUID; - SharedNodePointer node = addOrUpdateNode(nodeUUID, nodeType, nodePublicSocket, - nodeLocalSocket, isReplicated, false, connectionSecretUUID, permissions); - node->setLocalID(sessionLocalID); + SharedNodePointer node = addOrUpdateNode(nodeUUID, nodeType, nodePublicSocket, nodeLocalSocket, + sessionLocalID, isReplicated, false, connectionSecretUUID, permissions); // nodes that are downstream or upstream of our own type are kept alive when we hear about them from the domain server // and always have their public socket as their active socket diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 27b57ef26c..3d35424316 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -261,10 +261,7 @@ void PacketReceiver::handleVerifiedMessage(QSharedPointer recei SharedNodePointer matchingNode; - if (!receivedMessage->getSourceID().isNull()) { - matchingNode = nodeList->nodeWithUUID(receivedMessage->getSourceID()); - } - + matchingNode = nodeList->nodeWithLocalID(receivedMessage->getSourceID()); QMutexLocker packetListenerLocker(&_packetListenerLock); auto it = _messageListenerMap.find(receivedMessage->getType()); diff --git a/libraries/networking/src/ReceivedMessage.cpp b/libraries/networking/src/ReceivedMessage.cpp index 00b16908ce..e1a036b041 100644 --- a/libraries/networking/src/ReceivedMessage.cpp +++ b/libraries/networking/src/ReceivedMessage.cpp @@ -43,7 +43,7 @@ ReceivedMessage::ReceivedMessage(NLPacket& packet) } ReceivedMessage::ReceivedMessage(QByteArray byteArray, PacketType packetType, PacketVersion packetVersion, - const HifiSockAddr& senderSockAddr, QUuid sourceID) : + const HifiSockAddr& senderSockAddr, NLPacket::LocalID sourceID) : _data(byteArray), _headData(_data.mid(0, HEAD_DATA_SIZE)), _numPackets(1), diff --git a/libraries/networking/src/ReceivedMessage.h b/libraries/networking/src/ReceivedMessage.h index ae51e7592a..f94985b7d3 100644 --- a/libraries/networking/src/ReceivedMessage.h +++ b/libraries/networking/src/ReceivedMessage.h @@ -25,7 +25,7 @@ public: ReceivedMessage(const NLPacketList& packetList); ReceivedMessage(NLPacket& packet); ReceivedMessage(QByteArray byteArray, PacketType packetType, PacketVersion packetVersion, - const HifiSockAddr& senderSockAddr, QUuid sourceID = QUuid()); + const HifiSockAddr& senderSockAddr, NLPacket::LocalID sourceID = 0); QByteArray getMessage() const { return _data; } const char* getRawMessage() const { return _data.constData(); } @@ -40,7 +40,7 @@ public: bool failed() const { return _failed; } bool isComplete() const { return _isComplete; } - const QUuid& getSourceID() const { return _sourceID; } + NLPacket::LocalID getSourceID() const { return _sourceID; } const HifiSockAddr& getSenderSockAddr() { return _senderSockAddr; } qint64 getPosition() const { return _position; } @@ -93,7 +93,7 @@ private: std::atomic _position { 0 }; std::atomic _numPackets { 0 }; - QUuid _sourceID; + NLPacket::LocalID _sourceID; PacketType _packetType; PacketVersion _packetVersion; HifiSockAddr _senderSockAddr; diff --git a/libraries/octree/src/OctreeProcessor.cpp b/libraries/octree/src/OctreeProcessor.cpp index 65b30dd197..14c37d5116 100644 --- a/libraries/octree/src/OctreeProcessor.cpp +++ b/libraries/octree/src/OctreeProcessor.cpp @@ -96,7 +96,7 @@ void OctreeProcessor::processDatagram(ReceivedMessage& message, SharedNodePointe quint64 totalUncompress = 0; quint64 totalReadBitsteam = 0; - const QUuid& sourceUUID = message.getSourceID(); + const QUuid& sourceUUID = sourceNode->getUUID(); int subsection = 1; From bed403355461834e4768b3dfe459163df7826e6d Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 27 Mar 2018 18:18:14 -0700 Subject: [PATCH 25/56] Local node IDs now working correctly Move typedef to single location; fixes for replicated packets (probably still not correct); reserve zero as local ID; pass domain server's local ID in domain server list; other tweaks. --- assignment-client/src/audio/AudioMixer.cpp | 13 +++++-------- assignment-client/src/avatars/AvatarMixer.cpp | 10 +++------- domain-server/src/DomainGatekeeper.cpp | 2 +- domain-server/src/DomainServer.cpp | 4 +++- interface/src/octree/OctreePacketProcessor.cpp | 2 +- libraries/networking/src/LimitedNodeList.cpp | 4 ++-- libraries/networking/src/NLPacket.h | 4 ++-- libraries/networking/src/NetworkPeer.h | 3 ++- libraries/networking/src/NodeList.cpp | 3 +++ libraries/shared/src/UUID.h | 1 + 10 files changed, 23 insertions(+), 23 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 3b79eab06e..a50304e1d5 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -117,16 +117,13 @@ void AudioMixer::queueAudioPacket(QSharedPointer message, Share void AudioMixer::queueReplicatedAudioPacket(QSharedPointer message) { // make sure we have a replicated node for the original sender of the packet auto nodeList = DependencyManager::get(); - - QUuid nodeID; - SharedNodePointer sourceNode = nodeList->nodeWithLocalID(message->getSourceID()); - if (sourceNode) { - nodeID = sourceNode->getUUID(); - } + + // Node ID is now part of user data, since replicated audio packets are non-sourced. + QUuid nodeID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); auto replicatedNode = nodeList->addOrUpdateNode(nodeID, NodeType::Agent, message->getSenderSockAddr(), message->getSenderSockAddr(), - true, true); + 0, true, true); replicatedNode->setLastHeardMicrostamp(usecTimestampNow()); // construct a "fake" audio received message from the byte array and packet list information @@ -140,7 +137,7 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer mess auto replicatedMessage = QSharedPointer::create(audioData, rewrittenType, versionForPacketType(rewrittenType), - message->getSenderSockAddr(), message->getSourceID()); + message->getSenderSockAddr(), nodeList->getSessionLocalID()); getOrCreateClientData(replicatedNode.data())->queuePacket(replicatedMessage, replicatedNode); } diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index b0f1420472..f9f41822a5 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -74,7 +74,7 @@ SharedNodePointer addOrUpdateReplicatedNode(const QUuid& nodeID, const HifiSockA auto replicatedNode = DependencyManager::get()->addOrUpdateNode(nodeID, NodeType::Agent, senderSockAddr, senderSockAddr, - true, true); + 0, true, true); replicatedNode->setLastHeardMicrostamp(usecTimestampNow()); @@ -112,12 +112,8 @@ void AvatarMixer::handleReplicatedPacket(QSharedPointer message void AvatarMixer::handleReplicatedBulkAvatarPacket(QSharedPointer message) { while (message->getBytesLeftToRead()) { // first, grab the node ID for this replicated avatar - QUuid nodeID; - auto nodeList = DependencyManager::get(); - SharedNodePointer sourceNode = nodeList->nodeWithLocalID(message->getSourceID()); - if (sourceNode) { - nodeID = sourceNode->getUUID(); - } + // Node ID is now part of user data, since ReplicatedBulkAvatarPacket is non-sourced. + auto nodeID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); // make sure we have an upstream replicated node that matches auto replicatedNode = addOrUpdateReplicatedNode(nodeID, message->getSenderSockAddr()); diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 917d8a01c0..40130688fb 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -1037,7 +1037,7 @@ Node::LocalID DomainGatekeeper::findOrCreateLocalID(const QUuid& uuid) { do { newLocalID = _currentLocalID; _currentLocalID += _idIncrement; - } while (_localIDToUUID.find(newLocalID) != _localIDToUUID.end()); + } while (newLocalID == 0 || _localIDToUUID.find(newLocalID) != _localIDToUUID.end()); _uuidToLocalID.emplace(uuid, newLocalID); _localIDToUUID.emplace(newLocalID, uuid); diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index fee102d66a..4514ed6fd6 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -1159,7 +1159,8 @@ void DomainServer::handleConnectedNode(SharedNodePointer newNode) { } void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const HifiSockAddr &senderSockAddr) { - const int NUM_DOMAIN_LIST_EXTENDED_HEADER_BYTES = NUM_BYTES_RFC4122_UUID + NUM_BYTES_RFC4122_UUID + 2; + const int NUM_DOMAIN_LIST_EXTENDED_HEADER_BYTES = NUM_BYTES_RFC4122_UUID + NLPacket::NUM_BYTES_LOCALID + + NUM_BYTES_RFC4122_UUID + NLPacket::NUM_BYTES_LOCALID + 2; // setup the extended header for the domain list packets // this data is at the beginning of each of the domain list packets @@ -1169,6 +1170,7 @@ void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const Hif auto limitedNodeList = DependencyManager::get(); extendedHeaderStream << limitedNodeList->getSessionUUID(); + extendedHeaderStream << limitedNodeList->getSessionLocalID(); extendedHeaderStream << node->getUUID(); extendedHeaderStream << node->getLocalID(); extendedHeaderStream << node->getPermissions(); diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 122b58c057..0c2883a9a4 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -71,7 +71,7 @@ void OctreePacketProcessor::processPacket(QSharedPointer messag if (message->getVersion() != versionForPacketType(message->getType())) { static QMultiMap versionDebugSuppressMap; - const QUuid& senderUUID = message->getSourceID(); + const QUuid& senderUUID = sendingNode->getUUID(); if (!versionDebugSuppressMap.contains(senderUUID, packetType)) { qDebug() << "Was stats packet? " << wasStatsPacket; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 77daaa84ea..e1f472e59c 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -315,10 +315,10 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe sourceNode = matchingNode.data(); } - QUuid sourceID = sourceNode->getUUID(); + QUuid sourceID = sourceNode ? sourceNode->getUUID() : QUuid(); if (!sourceNode && - sourceID == getDomainUUID() && + /*sourceID == getDomainUUID() &&*/ packet.getSenderSockAddr() == getDomainSockAddr() && PacketTypeEnum::getDomainSourcedPackets().contains(headerType)) { // This is a packet sourced by the domain server diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index a4690a376c..e3107fae3f 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -43,7 +43,7 @@ public: // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | // NLPacket Header Format - using LocalID = qint16; + using LocalID = NetworkLocalID; static const int NUM_BYTES_LOCALID = sizeof(LocalID); // this is used by the Octree classes - must be known at compile time static const int MAX_PACKET_HEADER_SIZE = @@ -83,7 +83,7 @@ public: LocalID getSourceID() const { return _sourceID; } - void writeSourceID(qint16 sourceID) const; + void writeSourceID(LocalID sourceID) const; void writeVerificationHashGivenSecret(const QUuid& connectionSecret) const; protected: diff --git a/libraries/networking/src/NetworkPeer.h b/libraries/networking/src/NetworkPeer.h index f36db402ce..972d2cbdd7 100644 --- a/libraries/networking/src/NetworkPeer.h +++ b/libraries/networking/src/NetworkPeer.h @@ -18,6 +18,7 @@ #include #include +#include "UUID.h" #include "HifiSockAddr.h" const QString ICE_SERVER_HOSTNAME = "localhost"; @@ -39,7 +40,7 @@ public: const QUuid& getUUID() const { return _uuid; } void setUUID(const QUuid& uuid) { _uuid = uuid; } - using LocalID = quint16; + using LocalID = NetworkLocalID; LocalID getLocalID() const { return _localID; } void setLocalID(LocalID localID) { _localID = localID; } diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 4556b441f2..86e3694669 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -594,6 +594,9 @@ void NodeList::processDomainServerList(QSharedPointer message) return; } + Node::LocalID domainLocalID; + packetStream >> domainLocalID; + // pull our owner (ie. session) UUID from the packet, it's always the first thing // The short (16 bit) ID comes next. QUuid newUUID; diff --git a/libraries/shared/src/UUID.h b/libraries/shared/src/UUID.h index 7e7048486f..55ac0866ee 100644 --- a/libraries/shared/src/UUID.h +++ b/libraries/shared/src/UUID.h @@ -15,6 +15,7 @@ #include const int NUM_BYTES_RFC4122_UUID = 16; +using NetworkLocalID = quint16; QString uuidStringWithoutCurlyBraces(const QUuid& uuid); From f823f632115c0d9db4a5a89ac5f82883342b7c91 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 28 Mar 2018 13:27:43 -0700 Subject: [PATCH 26/56] Add domain local ID to DomainServerList Also handle domain local ID for 'domain sourced' packets; bump packet version. --- domain-server/src/DomainServer.cpp | 1 - libraries/networking/src/DomainHandler.h | 4 ++++ libraries/networking/src/LimitedNodeList.cpp | 9 +++------ libraries/networking/src/LimitedNodeList.h | 1 + libraries/networking/src/NodeList.cpp | 1 + libraries/networking/src/NodeList.h | 1 + libraries/networking/src/udt/PacketHeaders.cpp | 2 +- 7 files changed, 11 insertions(+), 8 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 4514ed6fd6..4077becc57 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -593,7 +593,6 @@ bool DomainServer::isPacketVerified(const udt::Packet& packet) { if (!PacketTypeEnum::getNonSourcedPackets().contains(headerType)) { // this is a sourced packet - first check if we have a node that matches - //QUuid sourceID = NLPacket::sourceIDInHeader(packet); Node::LocalID localSourceID = NLPacket::sourceIDInHeader(packet); SharedNodePointer sourceNode = nodeList->nodeWithLocalID(localSourceID); diff --git a/libraries/networking/src/DomainHandler.h b/libraries/networking/src/DomainHandler.h index fbc60e2492..ef5b3116da 100644 --- a/libraries/networking/src/DomainHandler.h +++ b/libraries/networking/src/DomainHandler.h @@ -45,6 +45,9 @@ public: const QUuid& getUUID() const { return _uuid; } void setUUID(const QUuid& uuid); + const Node::LocalID getLocalID() const { return _localID; } + void setLocalID(Node::LocalID localID) { _localID = localID; } + QString getHostname() const { return _domainURL.host(); } const QHostAddress& getIP() const { return _sockAddr.getAddress(); } @@ -181,6 +184,7 @@ private: void hardReset(); QUuid _uuid; + Node::LocalID _localID; QUrl _domainURL; HifiSockAddr _sockAddr; QUuid _assignmentUUID; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index e1f472e59c..31345c3c14 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -305,11 +305,11 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe return true; } } else { - + NLPacket::LocalID sourceLocalID = 0; // check if we were passed a sourceNode hint or if we need to look it up if (!sourceNode) { // figure out which node this is from - NLPacket::LocalID sourceLocalID = NLPacket::sourceIDInHeader(packet); + sourceLocalID = NLPacket::sourceIDInHeader(packet); SharedNodePointer matchingNode = nodeWithLocalID(sourceLocalID); sourceNode = matchingNode.data(); @@ -318,7 +318,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe QUuid sourceID = sourceNode ? sourceNode->getUUID() : QUuid(); if (!sourceNode && - /*sourceID == getDomainUUID() &&*/ + sourceLocalID == getDomainLocalID() && packet.getSenderSockAddr() == getDomainSockAddr() && PacketTypeEnum::getDomainSourcedPackets().contains(headerType)) { // This is a packet sourced by the domain server @@ -567,9 +567,6 @@ SharedNodePointer LimitedNodeList::nodeWithLocalID(Node::LocalID localID) const QReadLocker readLocker(&_nodeMutex); LocalIDMapping::const_iterator idIter = _localIDMap.find(localID); - if (idIter == _localIDMap.cend()) { - qCDebug(networking) << "No such Node with local ID " << localID; - } return idIter == _localIDMap.cend() ? nullptr : idIter->second; } diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 51c8831e2b..95d0e8b559 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -128,6 +128,7 @@ public: virtual bool isDomainServer() const { return true; } virtual QUuid getDomainUUID() const { assert(false); return QUuid(); } + virtual Node::LocalID getDomainLocalID() const { assert(false); return 0; } virtual HifiSockAddr getDomainSockAddr() const { assert(false); return HifiSockAddr(); } // use sendUnreliablePacket to send an unrelaible packet (that you do not need to move) diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 86e3694669..4b3595b279 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -608,6 +608,7 @@ void NodeList::processDomainServerList(QSharedPointer message) // if this was the first domain-server list from this domain, we've now connected if (!_domainHandler.isConnected()) { + _domainHandler.setLocalID(newLocalID); _domainHandler.setUUID(domainUUID); _domainHandler.setIsConnected(true); diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 7e3a7c2bd7..9595c5da84 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -94,6 +94,7 @@ public: virtual bool isDomainServer() const override { return false; } virtual QUuid getDomainUUID() const override { return _domainHandler.getUUID(); } + virtual Node::LocalID getDomainLocalID() const override { return _domainHandler.getLocalID(); } virtual HifiSockAddr getDomainSockAddr() const override { return _domainHandler.getSockAddr(); } public slots: diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index a83924ee58..8a3592de45 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -76,7 +76,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::DomainSettings: return 18; // replace min_avatar_scale and max_avatar_scale with min_avatar_height and max_avatar_height default: - return 17; + return 19; } } From b409e04734d4240968fe221981ceac70fd659630 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 28 Mar 2018 17:42:23 -0700 Subject: [PATCH 27/56] Handle all case of nodes being deleted from nodelist Delete erased nodes from local ID mapping, as this uses SharedPointer. Don't keep the mapping from local IDs to UUIDs in GateKeeper as it isn't used. --- domain-server/src/DomainGatekeeper.cpp | 6 ++++-- domain-server/src/DomainGatekeeper.h | 5 +++-- domain-server/src/DomainServer.cpp | 2 +- libraries/networking/src/LimitedNodeList.cpp | 4 ++++ libraries/networking/src/NLPacket.h | 18 +++++++++++++++++- 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 40130688fb..fb3c305b84 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -1033,13 +1033,15 @@ Node::LocalID DomainGatekeeper::findOrCreateLocalID(const QUuid& uuid) { return existingLocalIDIt->second; } + assert(_localIDs.size() < std::numeric_limits::max() - 2); + Node::LocalID newLocalID; do { newLocalID = _currentLocalID; _currentLocalID += _idIncrement; - } while (newLocalID == 0 || _localIDToUUID.find(newLocalID) != _localIDToUUID.end()); + } while (newLocalID == 0 || _localIDs.find(newLocalID) != _localIDs.end()); _uuidToLocalID.emplace(uuid, newLocalID); - _localIDToUUID.emplace(newLocalID, uuid); + _localIDs.insert(newLocalID); return newLocalID; } diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h index afb848f271..edd976a77b 100644 --- a/domain-server/src/DomainGatekeeper.h +++ b/domain-server/src/DomainGatekeeper.h @@ -15,6 +15,7 @@ #define hifi_DomainGatekeeper_h #include +#include #include #include @@ -129,9 +130,9 @@ private: size_t operator()(const QUuid& uuid) const { return qHash(uuid); } }; using UUIDToLocalID = std::unordered_map ; - using LocalIDToUUID = std::unordered_map; + using LocalIDs = std::unordered_set; + LocalIDs _localIDs; UUIDToLocalID _uuidToLocalID; - LocalIDToUUID _localIDToUUID; Node::LocalID _currentLocalID; quint16 _idIncrement; diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 4077becc57..77f25d4c7e 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -2905,7 +2905,7 @@ void DomainServer::updateReplicationNodes(ReplicationServerDirection direction) // manually add the replication node to our node list auto node = nodeList->addOrUpdateNode(QUuid::createUuid(), replicationServer.nodeType, replicationServer.sockAddr, replicationServer.sockAddr, - false, direction == Upstream); + 0, false, direction == Upstream); node->setIsForcedNeverSilent(true); qDebug() << "Adding" << (direction == Upstream ? "upstream" : "downstream") diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 31345c3c14..6ef119bf3b 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -695,6 +695,7 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t auto oldSoloNode = previousSoloIt->second; + _localIDMap.erase(oldSoloNode->getLocalID()); _nodeHash.unsafe_erase(previousSoloIt); handleNodeKill(oldSoloNode); @@ -850,6 +851,9 @@ void LimitedNodeList::removeSilentNodes() { }); foreach(const SharedNodePointer& killedNode, killedNodes) { + _nodeMutex.lockForWrite(); + _localIDMap.erase(killedNode->getLocalID()); + _nodeMutex.unlock(); handleNodeKill(killedNode); } } diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index e3107fae3f..a994b7bde0 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -21,6 +21,22 @@ class NLPacket : public udt::Packet { Q_OBJECT public: + // + // Current NLPacket format: + // + // | BYTE | BYTE | BYTE | BYTE | + // 0 1 2 3 + // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | Packet Type | Version | Local Node ID - sourced only | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | | + // | MD5 Verification - 16 bytes | + // | (ONLY FOR VERIFIED PACKETS) | + // | | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + // Old NLPacket format: // // | BYTE | BYTE | BYTE | BYTE | // @@ -41,7 +57,7 @@ public: // | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // | | | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | - // NLPacket Header Format + // using LocalID = NetworkLocalID; static const int NUM_BYTES_LOCALID = sizeof(LocalID); From 1b0b280f3a17d55f5a55e043066c768dcd56d9eb Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 29 Mar 2018 10:47:11 -0700 Subject: [PATCH 28/56] Fix SequenceNumberStats, misc. clean-up --- domain-server/src/DomainGatekeeper.cpp | 2 +- domain-server/src/DomainGatekeeper.h | 2 +- domain-server/src/DomainServer.cpp | 2 +- libraries/audio/src/InboundAudioStream.cpp | 4 ++-- libraries/networking/src/LimitedNodeList.cpp | 2 +- libraries/networking/src/SequenceNumberStats.cpp | 12 ++++++------ libraries/networking/src/SequenceNumberStats.h | 6 +++--- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index fb3c305b84..b4ce467e0a 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -1021,7 +1021,7 @@ void DomainGatekeeper::refreshGroupsCache() { void DomainGatekeeper::initLocalIDManagement() { std::uniform_int_distribution sixteenBitRand; std::random_device randomDevice; - std::default_random_engine engine {randomDevice()}; + std::default_random_engine engine { randomDevice() }; _currentLocalID = sixteenBitRand(engine); // Ensure increment is odd. _idIncrement = sixteenBitRand(engine) | 1; diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h index edd976a77b..ec7ec5701f 100644 --- a/domain-server/src/DomainGatekeeper.h +++ b/domain-server/src/DomainGatekeeper.h @@ -135,7 +135,7 @@ private: UUIDToLocalID _uuidToLocalID; Node::LocalID _currentLocalID; - quint16 _idIncrement; + Node::LocalID _idIncrement; }; diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 77f25d4c7e..7265dff15c 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -1159,7 +1159,7 @@ void DomainServer::handleConnectedNode(SharedNodePointer newNode) { void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const HifiSockAddr &senderSockAddr) { const int NUM_DOMAIN_LIST_EXTENDED_HEADER_BYTES = NUM_BYTES_RFC4122_UUID + NLPacket::NUM_BYTES_LOCALID + - NUM_BYTES_RFC4122_UUID + NLPacket::NUM_BYTES_LOCALID + 2; + NUM_BYTES_RFC4122_UUID + NLPacket::NUM_BYTES_LOCALID + 4; // setup the extended header for the domain list packets // this data is at the beginning of each of the domain list packets diff --git a/libraries/audio/src/InboundAudioStream.cpp b/libraries/audio/src/InboundAudioStream.cpp index 983b5e1cb8..172ec9411a 100644 --- a/libraries/audio/src/InboundAudioStream.cpp +++ b/libraries/audio/src/InboundAudioStream.cpp @@ -120,8 +120,8 @@ int InboundAudioStream::parseData(ReceivedMessage& message) { // parse sequence number and track it quint16 sequence; message.readPrimitive(&sequence); - SequenceNumberStats::ArrivalInfo arrivalInfo = _incomingSequenceNumberStats.sequenceNumberReceived(sequence, QUuid() // TBD - /*message.getSourceID()*/); + SequenceNumberStats::ArrivalInfo arrivalInfo = + _incomingSequenceNumberStats.sequenceNumberReceived(sequence, message.getSourceID()); QString codecInPacket = message.readString(); packetReceivedUpdateTimingStats(); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 6ef119bf3b..177bc729cd 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -136,7 +136,7 @@ Node::LocalID LimitedNodeList::getSessionLocalID() const { } void LimitedNodeList::setSessionLocalID(Node::LocalID sessionLocalID) { - QWriteLocker lock { &_sessionUUIDLock }; // Necessary? + QWriteLocker lock { &_sessionUUIDLock }; _sessionLocalID = sessionLocalID; } diff --git a/libraries/networking/src/SequenceNumberStats.cpp b/libraries/networking/src/SequenceNumberStats.cpp index 6d7b271606..052cdf1a4e 100644 --- a/libraries/networking/src/SequenceNumberStats.cpp +++ b/libraries/networking/src/SequenceNumberStats.cpp @@ -25,7 +25,7 @@ SequenceNumberStats::SequenceNumberStats(int statsHistoryLength, bool canDetectO : _lastReceivedSequence(0), _missingSet(), _stats(), - _lastSenderUUID(), + _lastSenderID(0), _statsHistory(statsHistoryLength), _lastUnreasonableSequence(0), _consecutiveUnreasonableOnTime(0) @@ -35,7 +35,7 @@ SequenceNumberStats::SequenceNumberStats(int statsHistoryLength, bool canDetectO void SequenceNumberStats::reset() { _missingSet.clear(); _stats = PacketStreamStats(); - _lastSenderUUID = QUuid(); + _lastSenderID = 0; _statsHistory.clear(); _lastUnreasonableSequence = 0; _consecutiveUnreasonableOnTime = 0; @@ -43,18 +43,18 @@ void SequenceNumberStats::reset() { static const int UINT16_RANGE = std::numeric_limits::max() + 1; -SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(quint16 incoming, QUuid senderUUID, const bool wantExtraDebugging) { +SequenceNumberStats::ArrivalInfo SequenceNumberStats::sequenceNumberReceived(quint16 incoming, NetworkLocalID senderID, const bool wantExtraDebugging) { SequenceNumberStats::ArrivalInfo arrivalInfo; // if the sender node has changed, reset all stats - if (senderUUID != _lastSenderUUID) { + if (senderID != _lastSenderID) { if (_stats._received > 0) { qCDebug(networking) << "sequence number stats was reset due to new sender node"; - qCDebug(networking) << "previous:" << _lastSenderUUID << "current:" << senderUUID; + qCDebug(networking) << "previous:" << _lastSenderID << "current:" << senderID; reset(); } - _lastSenderUUID = senderUUID; + _lastSenderID = senderID; } // determine our expected sequence number... handle rollover appropriately diff --git a/libraries/networking/src/SequenceNumberStats.h b/libraries/networking/src/SequenceNumberStats.h index 46149d4307..ba9db2a225 100644 --- a/libraries/networking/src/SequenceNumberStats.h +++ b/libraries/networking/src/SequenceNumberStats.h @@ -14,7 +14,7 @@ #include "SharedUtil.h" #include "RingBufferHistory.h" -#include +#include "UUID.h" const int MAX_REASONABLE_SEQUENCE_GAP = 1000; @@ -73,7 +73,7 @@ public: SequenceNumberStats(int statsHistoryLength = 0, bool canDetectOutOfSync = true); void reset(); - ArrivalInfo sequenceNumberReceived(quint16 incoming, QUuid senderUUID = QUuid(), const bool wantExtraDebugging = false); + ArrivalInfo sequenceNumberReceived(quint16 incoming, NetworkLocalID senderID = 0, const bool wantExtraDebugging = false); void pruneMissingSet(const bool wantExtraDebugging = false); void pushStatsToHistory() { _statsHistory.insert(_stats); } @@ -100,7 +100,7 @@ private: PacketStreamStats _stats; - QUuid _lastSenderUUID; + NetworkLocalID _lastSenderID; RingBufferHistory _statsHistory; From e9ec4df36f75ff54577c9ddcb76a5032077a18fe Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 29 Mar 2018 14:10:31 -0700 Subject: [PATCH 29/56] Gcc complained about superfluous const qualifier --- libraries/networking/src/DomainHandler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/DomainHandler.h b/libraries/networking/src/DomainHandler.h index ef5b3116da..c447e9a79b 100644 --- a/libraries/networking/src/DomainHandler.h +++ b/libraries/networking/src/DomainHandler.h @@ -45,7 +45,7 @@ public: const QUuid& getUUID() const { return _uuid; } void setUUID(const QUuid& uuid); - const Node::LocalID getLocalID() const { return _localID; } + Node::LocalID getLocalID() const { return _localID; } void setLocalID(Node::LocalID localID) { _localID = localID; } QString getHostname() const { return _domainURL.host(); } From 734b48eee0f02ad8a0b30f4ad6fd5908d2feaa44 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 30 Mar 2018 17:27:52 -0700 Subject: [PATCH 30/56] Local IDs - Reviewer's suggested improvements --- assignment-client/src/audio/AudioMixer.cpp | 2 +- libraries/networking/src/LimitedNodeList.cpp | 10 ++++---- libraries/networking/src/NLPacket.h | 24 +------------------- libraries/networking/src/PacketReceiver.cpp | 4 +++- 4 files changed, 11 insertions(+), 29 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index a50304e1d5..a18df68f90 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -137,7 +137,7 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer mess auto replicatedMessage = QSharedPointer::create(audioData, rewrittenType, versionForPacketType(rewrittenType), - message->getSenderSockAddr(), nodeList->getSessionLocalID()); + message->getSenderSockAddr(), message->getSourceID()); getOrCreateClientData(replicatedNode.data())->queuePacket(replicatedMessage, replicatedNode); } diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 177bc729cd..c30952c344 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -132,6 +132,7 @@ void LimitedNodeList::setSessionUUID(const QUuid& sessionUUID) { } Node::LocalID LimitedNodeList::getSessionLocalID() const { + QReadLocker readLock { &_sessionUUIDLock }; return _sessionLocalID; } @@ -578,7 +579,7 @@ void LimitedNodeList::eraseAllNodes() { // and then remove them from the hash QWriteLocker writeLocker(&_nodeMutex); - _localIDMap.erase(_localIDMap.begin(), _localIDMap.end()); + _localIDMap.clear(); if (_nodeHash.size() > 0) { qCDebug(networking) << "LimitedNodeList::eraseAllNodes() removing all nodes from NodeList."; @@ -851,9 +852,10 @@ void LimitedNodeList::removeSilentNodes() { }); foreach(const SharedNodePointer& killedNode, killedNodes) { - _nodeMutex.lockForWrite(); - _localIDMap.erase(killedNode->getLocalID()); - _nodeMutex.unlock(); + { + QWriteLocker writeLock { &_nodeMutex }; + _localIDMap.erase(killedNode->getLocalID()); + } handleNodeKill(killedNode); } } diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index a994b7bde0..b9c598500b 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -22,7 +22,7 @@ class NLPacket : public udt::Packet { Q_OBJECT public: // - // Current NLPacket format: + // NLPacket format: // // | BYTE | BYTE | BYTE | BYTE | // 0 1 2 3 @@ -35,28 +35,6 @@ public: // | (ONLY FOR VERIFIED PACKETS) | // | | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - - // Old NLPacket format: - // - // | BYTE | BYTE | BYTE | BYTE | - // - // 0 1 2 3 - // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | Packet Type | Version | | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | - // | | - // | Node UUID - 16 bytes | - // | (ONLY FOR SOURCED PACKETS) | - // | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | | | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | - // | | - // | MD5 Verification - 16 bytes | - // | (ONLY FOR VERIFIED PACKETS) | - // | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | | | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | // using LocalID = NetworkLocalID; diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 3d35424316..39a3800c4b 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -261,7 +261,9 @@ void PacketReceiver::handleVerifiedMessage(QSharedPointer recei SharedNodePointer matchingNode; - matchingNode = nodeList->nodeWithLocalID(receivedMessage->getSourceID()); + if (receivedMessage->getSourceID() != 0) { + matchingNode = nodeList->nodeWithLocalID(receivedMessage->getSourceID()); + } QMutexLocker packetListenerLocker(&_packetListenerLock); auto it = _messageListenerMap.find(receivedMessage->getType()); From efb1fdbc0d2597ff96e60292eff93cac454d9bf6 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 30 Mar 2018 18:29:26 -0700 Subject: [PATCH 31/56] Local IDs - add an explicit null value, use it for replicated packets --- assignment-client/src/audio/AudioMixer.cpp | 2 +- assignment-client/src/avatars/AvatarMixer.cpp | 2 +- domain-server/src/DomainGatekeeper.cpp | 2 +- libraries/networking/src/NLPacket.h | 2 ++ libraries/networking/src/NetworkPeer.h | 2 ++ libraries/networking/src/ReceivedMessage.h | 4 ++-- 6 files changed, 9 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index a18df68f90..d47172f62e 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -137,7 +137,7 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer mess auto replicatedMessage = QSharedPointer::create(audioData, rewrittenType, versionForPacketType(rewrittenType), - message->getSenderSockAddr(), message->getSourceID()); + message->getSenderSockAddr(), Node::NULL_LOCAL_ID); getOrCreateClientData(replicatedNode.data())->queuePacket(replicatedMessage, replicatedNode); } diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index f9f41822a5..25bbc4cd0a 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -127,7 +127,7 @@ void AvatarMixer::handleReplicatedBulkAvatarPacket(QSharedPointer::create(avatarByteArray, PacketType::AvatarData, versionForPacketType(PacketType::AvatarData), - message->getSenderSockAddr(), message->getSourceID()); + message->getSenderSockAddr(), Node::NULL_LOCAL_ID); // queue up the replicated avatar data with the client data for the replicated node auto start = usecTimestampNow(); diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index b4ce467e0a..25317bf44b 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -1039,7 +1039,7 @@ Node::LocalID DomainGatekeeper::findOrCreateLocalID(const QUuid& uuid) { do { newLocalID = _currentLocalID; _currentLocalID += _idIncrement; - } while (newLocalID == 0 || _localIDs.find(newLocalID) != _localIDs.end()); + } while (newLocalID == Node::NULL_LOCAL_ID || _localIDs.find(newLocalID) != _localIDs.end()); _uuidToLocalID.emplace(uuid, newLocalID); _localIDs.insert(newLocalID); diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index b9c598500b..edd03627c3 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -38,6 +38,8 @@ public: // using LocalID = NetworkLocalID; + static const LocalID NULL_LOCAL_ID = 0; + static const int NUM_BYTES_LOCALID = sizeof(LocalID); // this is used by the Octree classes - must be known at compile time static const int MAX_PACKET_HEADER_SIZE = diff --git a/libraries/networking/src/NetworkPeer.h b/libraries/networking/src/NetworkPeer.h index 972d2cbdd7..462daa1ed2 100644 --- a/libraries/networking/src/NetworkPeer.h +++ b/libraries/networking/src/NetworkPeer.h @@ -41,6 +41,8 @@ public: void setUUID(const QUuid& uuid) { _uuid = uuid; } using LocalID = NetworkLocalID; + static const LocalID NULL_LOCAL_ID = 0; + LocalID getLocalID() const { return _localID; } void setLocalID(LocalID localID) { _localID = localID; } diff --git a/libraries/networking/src/ReceivedMessage.h b/libraries/networking/src/ReceivedMessage.h index f94985b7d3..af87ef75af 100644 --- a/libraries/networking/src/ReceivedMessage.h +++ b/libraries/networking/src/ReceivedMessage.h @@ -25,7 +25,7 @@ public: ReceivedMessage(const NLPacketList& packetList); ReceivedMessage(NLPacket& packet); ReceivedMessage(QByteArray byteArray, PacketType packetType, PacketVersion packetVersion, - const HifiSockAddr& senderSockAddr, NLPacket::LocalID sourceID = 0); + const HifiSockAddr& senderSockAddr, NLPacket::LocalID sourceID = NLPacket::NULL_LOCAL_ID); QByteArray getMessage() const { return _data; } const char* getRawMessage() const { return _data.constData(); } @@ -93,7 +93,7 @@ private: std::atomic _position { 0 }; std::atomic _numPackets { 0 }; - NLPacket::LocalID _sourceID; + NLPacket::LocalID _sourceID { NLPacket::NULL_LOCAL_ID }; PacketType _packetType; PacketVersion _packetVersion; HifiSockAddr _senderSockAddr; From f4bd1f5ed7130d3272a45f5f6dfbc8253ae542be Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 2 Apr 2018 09:45:46 -0700 Subject: [PATCH 32/56] Add definition for NetworkPeer::NULL_LOCAL_ID to (hopefully) satisfy MacOS --- libraries/networking/src/NetworkPeer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/networking/src/NetworkPeer.cpp b/libraries/networking/src/NetworkPeer.cpp index b52c54c468..6d384ba558 100644 --- a/libraries/networking/src/NetworkPeer.cpp +++ b/libraries/networking/src/NetworkPeer.cpp @@ -23,6 +23,7 @@ #include #include "NodeType.h" +const NetworkPeer::LocalID NetworkPeer::NULL_LOCAL_ID; NetworkPeer::NetworkPeer(QObject* parent) : QObject(parent), From 2ba64e111509e2d3c77c2b860120856806f7648e Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 2 Apr 2018 10:46:38 -0700 Subject: [PATCH 33/56] LocalID - Use tbb hash map instead of std one for better concurrency --- libraries/networking/src/LimitedNodeList.cpp | 7 ++----- libraries/networking/src/LimitedNodeList.h | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index c30952c344..ec2e107567 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -696,7 +696,7 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t auto oldSoloNode = previousSoloIt->second; - _localIDMap.erase(oldSoloNode->getLocalID()); + _localIDMap.unsafe_erase(oldSoloNode->getLocalID()); _nodeHash.unsafe_erase(previousSoloIt); handleNodeKill(oldSoloNode); @@ -840,6 +840,7 @@ void LimitedNodeList::removeSilentNodes() { if (!node->isForcedNeverSilent() && (usecTimestampNow() - node->getLastHeardMicrostamp()) > (NODE_SILENCE_THRESHOLD_MSECS * USECS_PER_MSEC)) { // call the NodeHash erase to get rid of this node + _localIDMap.unsafe_erase(node->getLocalID()); it = _nodeHash.unsafe_erase(it); killedNodes.insert(node); @@ -852,10 +853,6 @@ void LimitedNodeList::removeSilentNodes() { }); foreach(const SharedNodePointer& killedNode, killedNodes) { - { - QWriteLocker writeLock { &_nodeMutex }; - _localIDMap.erase(killedNode->getLocalID()); - } handleNodeKill(killedNode); } } diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 95d0e8b559..6c61fbc43c 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -431,7 +431,7 @@ private slots: private: mutable QReadWriteLock _sessionUUIDLock; QUuid _sessionUUID; - using LocalIDMapping = std::unordered_map; + using LocalIDMapping = tbb::concurrent_unordered_map; LocalIDMapping _localIDMap; Node::LocalID _sessionLocalID { 0 }; }; From 765688d4dafb3724df35b501f87b54a480ba6251 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 3 Apr 2018 17:40:07 -0700 Subject: [PATCH 34/56] add JSDoc package lock json to gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index df91e0ca7b..8d92fe770b 100644 --- a/.gitignore +++ b/.gitignore @@ -91,3 +91,6 @@ interface/compiledResources # GPUCache interface/resources/GPUCache/* + +# package lock file for JSDoc tool +tools/jsdoc/package-lock.json From 22ac40040200f255ff647bd972df6fb4de9af603 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 3 Apr 2018 17:48:17 -0700 Subject: [PATCH 35/56] update electron version to 1.8.4 --- server-console/package-lock.json | 454 ++++++++++++++++--------------- server-console/package.json | 8 +- server-console/src/main.js | 6 +- 3 files changed, 243 insertions(+), 225 deletions(-) diff --git a/server-console/package-lock.json b/server-console/package-lock.json index e25fd3cded..4311fde51a 100644 --- a/server-console/package-lock.json +++ b/server-console/package-lock.json @@ -4,6 +4,12 @@ "lockfileVersion": 1, "requires": true, "dependencies": { + "@types/node": { + "version": "8.10.2", + "resolved": "https://registry.npmjs.org/@types/node/-/node-8.10.2.tgz", + "integrity": "sha512-A6Uv1anbsCvrRDtaUXS2xZ5tlzD+Kg7yMRlSLFDy3z0r7KlGXDzL14vELXIAgpk2aJbU3XeZZQRcEkLkowT92g==", + "dev": true + }, "abbrev": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", @@ -40,12 +46,6 @@ "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-2.2.1.tgz", "integrity": "sha1-tDLdM1i2NM914eRmQ2gkBTPB3b4=" }, - "any-promise": { - "version": "1.3.0", - "resolved": "https://registry.npmjs.org/any-promise/-/any-promise-1.3.0.tgz", - "integrity": "sha1-q8av7tzqUugJzcA3au0845Y10X8=", - "dev": true - }, "array-find-index": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/array-find-index/-/array-find-index-1.0.1.tgz", @@ -148,12 +148,6 @@ "lru-cache": "4.0.1" } }, - "balanced-match": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-0.3.0.tgz", - "integrity": "sha1-qRzdHr7xqGZZ5w/03vAWJfwtZ1Y=", - "dev": true - }, "base64-js": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.2.0.tgz", @@ -217,16 +211,6 @@ "hoek": "2.16.3" } }, - "brace-expansion": { - "version": "1.1.3", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.3.tgz", - "integrity": "sha1-Rr/1ARXUf8mriYVKu4fZgHihCZE=", - "dev": true, - "requires": { - "balanced-match": "0.3.0", - "concat-map": "0.0.1" - } - }, "buffers": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/buffers/-/buffers-0.1.1.tgz", @@ -515,29 +499,70 @@ "jsbn": "0.1.0" } }, - "electron-download": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/electron-download/-/electron-download-2.1.1.tgz", - "integrity": "sha1-AH07HyrTco0nzP5PhJayY/kTijE=", + "electron": { + "version": "1.8.4", + "resolved": "https://registry.npmjs.org/electron/-/electron-1.8.4.tgz", + "integrity": "sha512-2f1cx0G3riMFODXFftF5AHXy+oHfhpntZHTDN66Hxtl09gmEr42B3piNEod9MEmw72f75LX2JfeYceqq1PF8cA==", "dev": true, "requires": { - "debug": "2.2.0", - "home-path": "1.0.3", + "@types/node": "8.10.2", + "electron-download": "3.3.0", + "extract-zip": "1.5.0" + } + }, + "electron-download": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/electron-download/-/electron-download-3.3.0.tgz", + "integrity": "sha1-LP1U1pZsAZxNSa1l++Zcyc3vaMg=", + "dev": true, + "requires": { + "debug": "2.6.9", + "fs-extra": "0.30.0", + "home-path": "1.0.5", "minimist": "1.2.0", - "mkdirp": "0.5.1", - "mv": "2.1.1", - "nugget": "1.6.2", - "path-exists": "1.0.0", - "rc": "1.1.6" + "nugget": "2.0.1", + "path-exists": "2.1.0", + "rc": "1.1.6", + "semver": "5.5.0", + "sumchecker": "1.3.1" }, "dependencies": { "debug": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/debug/-/debug-2.2.0.tgz", - "integrity": "sha1-+HBX6ZWxofauaklgZkE3vFbwOdo=", + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "dev": true, "requires": { - "ms": "0.7.1" + "ms": "2.0.0" + } + }, + "fs-extra": { + "version": "0.30.0", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-0.30.0.tgz", + "integrity": "sha1-8jP/zAjU2n1DLapEl3aYnbHfk/A=", + "dev": true, + "requires": { + "graceful-fs": "4.1.3", + "jsonfile": "2.2.3", + "klaw": "1.3.1", + "path-is-absolute": "1.0.0", + "rimraf": "2.6.2" + } + }, + "semver": { + "version": "5.5.0", + "resolved": "https://registry.npmjs.org/semver/-/semver-5.5.0.tgz", + "integrity": "sha512-4SJ3dm0WAwWy/NVeioZh5AntkdJoWKxHxcmyP622fOkgHa4z3R0TdBJICINyaSDE6uNwVc8gZr+ZinwZAH4xIA==", + "dev": true + }, + "sumchecker": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/sumchecker/-/sumchecker-1.3.1.tgz", + "integrity": "sha1-ebs7RFbdBPGOvbwNcDodHa7FEF0=", + "dev": true, + "requires": { + "debug": "2.6.9", + "es6-promise": "4.2.4" } } } @@ -579,9 +604,9 @@ } }, "electron-packager": { - "version": "11.0.0", - "resolved": "https://registry.npmjs.org/electron-packager/-/electron-packager-11.0.0.tgz", - "integrity": "sha512-ufyYMe3Gt6IEZm9RuG+KK3Nh+V2jZHWg9gihp8wylUNtleQihECIXtQdpPJxH9740XFERVPraNEaa7cZvDzpyw==", + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/electron-packager/-/electron-packager-12.0.0.tgz", + "integrity": "sha1-uC0k14ovIUA7v9FmpbFWmJTVzQw=", "dev": true, "requires": { "asar": "0.14.2", @@ -590,20 +615,25 @@ "electron-osx-sign": "0.4.8", "extract-zip": "1.5.0", "fs-extra": "5.0.0", + "galactus": "0.2.0", "get-package-info": "1.0.0", - "mz": "2.7.0", "nodeify": "1.0.1", "parse-author": "2.0.0", "pify": "3.0.0", "plist": "2.1.0", - "pruner": "0.0.7", - "rcedit": "0.9.0", + "rcedit": "1.0.0", "resolve": "1.5.0", "sanitize-filename": "1.6.1", "semver": "5.5.0", - "yargs-parser": "8.1.0" + "yargs-parser": "9.0.2" }, "dependencies": { + "camelcase": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-4.1.0.tgz", + "integrity": "sha1-1UVjW+HjPFQmScaRc+Xeas+uNN0=", + "dev": true + }, "debug": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", @@ -726,6 +756,12 @@ "integrity": "sha1-5aSs0sEB/fPZpNB/DbxNtJ3SgXY=", "dev": true }, + "rcedit": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/rcedit/-/rcedit-1.0.0.tgz", + "integrity": "sha512-W7DNa34x/3OgWyDHsI172AG/Lr/lZ+PkavFkHj0QhhkBRcV9QTmRJE1tDKrWkx8XHPSBsmZkNv9OKue6pncLFQ==", + "dev": true + }, "semver": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/semver/-/semver-5.5.0.tgz", @@ -746,19 +782,18 @@ "resolved": "https://registry.npmjs.org/throttleit/-/throttleit-0.0.2.tgz", "integrity": "sha1-z+34jmDADdlpe2H90qg0OptoDq8=", "dev": true + }, + "yargs-parser": { + "version": "9.0.2", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-9.0.2.tgz", + "integrity": "sha1-nM9qQ0YP5O1Aqbto9I1DuKaMwHc=", + "dev": true, + "requires": { + "camelcase": "4.1.0" + } } } }, - "electron-prebuilt": { - "version": "0.37.5", - "resolved": "https://registry.npmjs.org/electron-prebuilt/-/electron-prebuilt-0.37.5.tgz", - "integrity": "sha1-OkGJgod4FdOnrB+bLi9KcPQg/3A=", - "dev": true, - "requires": { - "electron-download": "2.1.1", - "extract-zip": "1.5.0" - } - }, "end-of-stream": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/end-of-stream/-/end-of-stream-1.1.0.tgz", @@ -787,6 +822,12 @@ "is-arrayish": "0.2.1" } }, + "es6-promise": { + "version": "4.2.4", + "resolved": "https://registry.npmjs.org/es6-promise/-/es6-promise-4.2.4.tgz", + "integrity": "sha512-/NdNZVJg+uZgtm9eS3O6lrOLYmQag2DjdEXuPaHlZ6RuVqgqaVZfgYCepEIKsLqwdQArOPtC3XzRLqGGfT8KQQ==", + "dev": true + }, "escape-string-regexp": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz", @@ -873,6 +914,62 @@ } } }, + "flora-colossus": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/flora-colossus/-/flora-colossus-0.0.2.tgz", + "integrity": "sha1-fRvimh8X+k8isb1hSC+Gw04HuQE=", + "dev": true, + "requires": { + "debug": "3.1.0", + "fs-extra": "4.0.3" + }, + "dependencies": { + "debug": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", + "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", + "dev": true, + "requires": { + "ms": "2.0.0" + } + }, + "fs-extra": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-4.0.3.tgz", + "integrity": "sha512-q6rbdDd1o2mAnQreO7YADIxf/Whx4AHBiRf6d+/cVT8h44ss+lHgxf1FemcqDnQt9X3ct4McHr+JMGlYSsK7Cg==", + "dev": true, + "requires": { + "graceful-fs": "4.1.3", + "jsonfile": "4.0.0", + "universalify": "0.1.1" + } + }, + "jsonfile": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", + "integrity": "sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss=", + "dev": true, + "requires": { + "graceful-fs": "4.1.11" + }, + "dependencies": { + "graceful-fs": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.1.11.tgz", + "integrity": "sha1-Dovf5NHduIVNZOBOp8AOKgJuVlg=", + "dev": true, + "optional": true + } + } + }, + "ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=", + "dev": true + } + } + }, "forever-agent": { "version": "0.6.1", "resolved": "https://registry.npmjs.org/forever-agent/-/forever-agent-0.6.1.tgz", @@ -904,6 +1001,63 @@ "integrity": "sha1-FQStJSMVjKpA20onh8sBQRmU6k8=", "dev": true }, + "galactus": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/galactus/-/galactus-0.2.0.tgz", + "integrity": "sha1-w9Y7pVAkZv5A6mfMaJCFs90kqPw=", + "dev": true, + "requires": { + "debug": "3.1.0", + "flora-colossus": "0.0.2", + "fs-extra": "4.0.3" + }, + "dependencies": { + "debug": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", + "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", + "dev": true, + "requires": { + "ms": "2.0.0" + } + }, + "fs-extra": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-4.0.3.tgz", + "integrity": "sha512-q6rbdDd1o2mAnQreO7YADIxf/Whx4AHBiRf6d+/cVT8h44ss+lHgxf1FemcqDnQt9X3ct4McHr+JMGlYSsK7Cg==", + "dev": true, + "requires": { + "graceful-fs": "4.1.3", + "jsonfile": "4.0.0", + "universalify": "0.1.1" + } + }, + "jsonfile": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", + "integrity": "sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss=", + "dev": true, + "requires": { + "graceful-fs": "4.1.11" + }, + "dependencies": { + "graceful-fs": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.1.11.tgz", + "integrity": "sha1-Dovf5NHduIVNZOBOp8AOKgJuVlg=", + "dev": true, + "optional": true + } + } + }, + "ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=", + "dev": true + } + } + }, "generate-function": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/generate-function/-/generate-function-2.0.0.tgz", @@ -1107,9 +1261,9 @@ "integrity": "sha1-ILt0A9POo5jpHcRxCo/xuCdKJe0=" }, "home-path": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/home-path/-/home-path-1.0.3.tgz", - "integrity": "sha1-ns5Z/sPwMubRC1Q0/uJk30wt4y8=", + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/home-path/-/home-path-1.0.5.tgz", + "integrity": "sha1-eIspgVsS1Tus9XVkhHbm+QQdEz8=", "dev": true }, "hosted-git-info": { @@ -1461,15 +1615,6 @@ "mime-db": "1.22.0" } }, - "minimatch": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.0.tgz", - "integrity": "sha1-UjYVelHk8ATBd/s8Un/33Xjw74M=", - "dev": true, - "requires": { - "brace-expansion": "1.1.3" - } - }, "minimist": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", @@ -1723,61 +1868,9 @@ } }, "ms": { - "version": "0.7.1", - "resolved": "https://registry.npmjs.org/ms/-/ms-0.7.1.tgz", - "integrity": "sha1-nNE8A62/8ltl7/3nzoZO6VIBcJg=", - "dev": true - }, - "mv": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/mv/-/mv-2.1.1.tgz", - "integrity": "sha1-rmzg1vbV4KT32JN5jQPB6pVZtqI=", - "dev": true, - "requires": { - "mkdirp": "0.5.1", - "ncp": "2.0.0", - "rimraf": "2.4.5" - }, - "dependencies": { - "glob": { - "version": "6.0.4", - "resolved": "https://registry.npmjs.org/glob/-/glob-6.0.4.tgz", - "integrity": "sha1-DwiGD2oVUSey+t1PnOJLGqtuTSI=", - "dev": true, - "requires": { - "inflight": "1.0.4", - "inherits": "2.0.1", - "minimatch": "3.0.0", - "once": "1.3.3", - "path-is-absolute": "1.0.0" - } - }, - "rimraf": { - "version": "2.4.5", - "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.4.5.tgz", - "integrity": "sha1-7nEM5dk6j9uFb7Xqj/Di11k0sto=", - "dev": true, - "requires": { - "glob": "6.0.4" - } - } - } - }, - "mz": { - "version": "2.7.0", - "resolved": "https://registry.npmjs.org/mz/-/mz-2.7.0.tgz", - "integrity": "sha512-z81GNO7nnYMEhrGh9LeymoE4+Yr0Wn5McHIZMK5cfQCl+NDX08sCZgUc9/6MHni9IWuFLm1Z3HTCXu2z9fN62Q==", - "dev": true, - "requires": { - "any-promise": "1.3.0", - "object-assign": "4.0.1", - "thenify-all": "1.6.0" - } - }, - "ncp": { "version": "2.0.0", - "resolved": "https://registry.npmjs.org/ncp/-/ncp-2.0.0.tgz", - "integrity": "sha1-GVoh1sRuNh0vsSgbo4uR6d9727M=", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=", "dev": true }, "node-notifier": { @@ -1843,27 +1936,27 @@ } }, "nugget": { - "version": "1.6.2", - "resolved": "https://registry.npmjs.org/nugget/-/nugget-1.6.2.tgz", - "integrity": "sha1-iMpuA7pXBqmRc/XaCQJZPWvK4Qc=", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/nugget/-/nugget-2.0.1.tgz", + "integrity": "sha1-IBCVpIfhrTYIGzQy+jytpPjQcbA=", "dev": true, "requires": { - "debug": "2.2.0", + "debug": "2.6.9", "minimist": "1.2.0", "pretty-bytes": "1.0.4", "progress-stream": "1.2.0", "request": "2.71.0", - "single-line-log": "0.4.1", + "single-line-log": "1.1.2", "throttleit": "0.0.2" }, "dependencies": { "debug": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/debug/-/debug-2.2.0.tgz", - "integrity": "sha1-+HBX6ZWxofauaklgZkE3vFbwOdo=", + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "dev": true, "requires": { - "ms": "0.7.1" + "ms": "2.0.0" } }, "throttleit": { @@ -1966,10 +2059,13 @@ } }, "path-exists": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-1.0.0.tgz", - "integrity": "sha1-1aiZjrce83p0w06w2eum6HjuoIE=", - "dev": true + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-2.1.0.tgz", + "integrity": "sha1-D+tsZPD8UY2adU3V77YscCJ2H0s=", + "dev": true, + "requires": { + "pinkie-promise": "2.0.1" + } }, "path-is-absolute": { "version": "1.0.0", @@ -2070,46 +2166,6 @@ "is-promise": "1.0.1" } }, - "pruner": { - "version": "0.0.7", - "resolved": "https://registry.npmjs.org/pruner/-/pruner-0.0.7.tgz", - "integrity": "sha1-NF+8s+gHARY6HXrfVrrCKaWh5ME=", - "dev": true, - "requires": { - "fs-extra": "4.0.3" - }, - "dependencies": { - "fs-extra": { - "version": "4.0.3", - "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-4.0.3.tgz", - "integrity": "sha512-q6rbdDd1o2mAnQreO7YADIxf/Whx4AHBiRf6d+/cVT8h44ss+lHgxf1FemcqDnQt9X3ct4McHr+JMGlYSsK7Cg==", - "dev": true, - "requires": { - "graceful-fs": "4.1.3", - "jsonfile": "4.0.0", - "universalify": "0.1.1" - } - }, - "jsonfile": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", - "integrity": "sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss=", - "dev": true, - "requires": { - "graceful-fs": "4.1.11" - }, - "dependencies": { - "graceful-fs": { - "version": "4.1.11", - "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.1.11.tgz", - "integrity": "sha1-Dovf5NHduIVNZOBOp8AOKgJuVlg=", - "dev": true, - "optional": true - } - } - } - } - }, "pseudomap": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/pseudomap/-/pseudomap-1.0.2.tgz", @@ -2153,12 +2209,6 @@ "strip-json-comments": "1.0.4" } }, - "rcedit": { - "version": "0.9.0", - "resolved": "https://registry.npmjs.org/rcedit/-/rcedit-0.9.0.tgz", - "integrity": "sha1-ORDfVzRTmeKwMl9KUZAH+J5V7xw=", - "dev": true - }, "read-pkg": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/read-pkg/-/read-pkg-1.1.0.tgz", @@ -2288,10 +2338,13 @@ "dev": true }, "single-line-log": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/single-line-log/-/single-line-log-0.4.1.tgz", - "integrity": "sha1-h6VWSfdJ14PsDc2AToFA2Yc8fO4=", - "dev": true + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/single-line-log/-/single-line-log-1.1.2.tgz", + "integrity": "sha1-wvg/Jzo+GhbtsJlWYdoO1e8DM2Q=", + "dev": true, + "requires": { + "string-width": "1.0.1" + } }, "sntp": { "version": "1.0.9", @@ -2476,24 +2529,6 @@ } } }, - "thenify": { - "version": "3.3.0", - "resolved": "https://registry.npmjs.org/thenify/-/thenify-3.3.0.tgz", - "integrity": "sha1-5p44obq+lpsBCCB5eLn2K4hgSDk=", - "dev": true, - "requires": { - "any-promise": "1.3.0" - } - }, - "thenify-all": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/thenify-all/-/thenify-all-1.6.0.tgz", - "integrity": "sha1-GhkY1ALY/D+Y+/I02wvMjMEOlyY=", - "dev": true, - "requires": { - "thenify": "3.3.0" - } - }, "throttleit": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/throttleit/-/throttleit-1.0.0.tgz", @@ -2694,23 +2729,6 @@ "y18n": "3.2.1" } }, - "yargs-parser": { - "version": "8.1.0", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-8.1.0.tgz", - "integrity": "sha512-yP+6QqN8BmrgW2ggLtTbdrOyBNSI7zBa4IykmiV5R1wl1JWNxQvWhMfMdmzIYtKU7oP3OOInY/tl2ov3BDjnJQ==", - "dev": true, - "requires": { - "camelcase": "4.1.0" - }, - "dependencies": { - "camelcase": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-4.1.0.tgz", - "integrity": "sha1-1UVjW+HjPFQmScaRc+Xeas+uNN0=", - "dev": true - } - } - }, "yauzl": { "version": "2.4.1", "resolved": "https://registry.npmjs.org/yauzl/-/yauzl-2.4.1.tgz", diff --git a/server-console/package.json b/server-console/package.json index 0b13eeb2a8..2428d2574e 100644 --- a/server-console/package.json +++ b/server-console/package.json @@ -8,8 +8,8 @@ "" ], "devDependencies": { - "electron-packager": "^11.0.0", - "electron-prebuilt": "0.37.5" + "electron-packager": "^12.0.0", + "electron": "1.8.4" }, "repository": { "type": "git", @@ -25,6 +25,7 @@ "dependencies": { "always-tail": "0.2.0", "cheerio": "^0.19.0", + "electron-log": "1.1.1", "extend": "^3.0.0", "fs-extra": "^1.0.0", "node-notifier": "^5.2.1", @@ -32,7 +33,6 @@ "request": "^2.67.0", "request-progress": "1.0.2", "tar-fs": "^1.12.0", - "yargs": "^3.30.0", - "electron-log": "1.1.1" + "yargs": "^3.30.0" } } diff --git a/server-console/src/main.js b/server-console/src/main.js index efa04a8512..b08db6222f 100644 --- a/server-console/src/main.js +++ b/server-console/src/main.js @@ -8,9 +8,9 @@ const nativeImage = electron.nativeImage; const notifier = require('node-notifier'); const util = require('util'); const dialog = electron.dialog; -const Menu = require('menu'); -const Tray = require('tray'); -const shell = require('shell'); +const Menu = electron.Menu; +const Tray = electron.Tray; +const shell = electron.shell; const os = require('os'); const childProcess = require('child_process'); const path = require('path'); From c23d4df93237ba53373df2a7024849518ee93475 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 3 Apr 2018 17:54:26 -0700 Subject: [PATCH 36/56] use SIGTERM as default kill signal for child process bug --- server-console/src/modules/hf-process.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server-console/src/modules/hf-process.js b/server-console/src/modules/hf-process.js index 767befec7b..797ee38a0d 100644 --- a/server-console/src/modules/hf-process.js +++ b/server-console/src/modules/hf-process.js @@ -226,7 +226,7 @@ Process.prototype = extend(Process.prototype, { } }); } else { - var signal = force ? 'SIGKILL' : null; + var signal = force ? 'SIGKILL' : 'SIGTERM'; this.child.kill(signal); } From 7638dceee39e8c3134f8cfdb92d32d2ea8fca934 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 3 Apr 2018 18:06:18 -0700 Subject: [PATCH 37/56] fix remote require from downloader page --- server-console/src/downloader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server-console/src/downloader.js b/server-console/src/downloader.js index f7e67f03ce..7dc9223802 100644 --- a/server-console/src/downloader.js +++ b/server-console/src/downloader.js @@ -2,7 +2,7 @@ function ready() { console.log("Ready"); const electron = require('electron'); - const remote = require('remote'); + const remote = electron.remote; window.$ = require('./vendor/jquery/jquery-2.1.4.min.js'); $(".state").hide(); From 3a8c9de4fb9feca5f6a0a713230d1f397074a10e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 4 Apr 2018 10:38:32 -0700 Subject: [PATCH 38/56] fix some xcode warnings for unnecessary captured this --- assignment-client/src/Agent.cpp | 2 +- domain-server/src/DomainServer.cpp | 2 +- libraries/entities-renderer/src/RenderableWebEntityItem.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 10b8d44545..1df901dd98 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -380,7 +380,7 @@ void Agent::executeScript() { using namespace recording; static const FrameType AVATAR_FRAME_TYPE = Frame::registerFrameType(AvatarData::FRAME_NAME); - Frame::registerFrameHandler(AVATAR_FRAME_TYPE, [this, scriptedAvatar](Frame::ConstPointer frame) { + Frame::registerFrameHandler(AVATAR_FRAME_TYPE, [scriptedAvatar](Frame::ConstPointer frame) { auto recordingInterface = DependencyManager::get(); bool useFrameSkeleton = recordingInterface->getPlayerUseSkeletonModel(); diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index dbf2907cc0..5f19bbe46e 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -2926,7 +2926,7 @@ void DomainServer::updateReplicatedNodes() { } auto nodeList = DependencyManager::get(); - nodeList->eachMatchingNode([this](const SharedNodePointer& otherNode) -> bool { + nodeList->eachMatchingNode([](const SharedNodePointer& otherNode) -> bool { return otherNode->getType() == NodeType::Agent; }, [this](const SharedNodePointer& otherNode) { auto shouldReplicate = shouldReplicateNode(*otherNode); diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index bd00ded12d..4f072d40e3 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -266,7 +266,7 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { // FIXME, the max FPS could be better managed by being dynamic (based on the number of current surfaces // and the current rendering load) _webSurface->setMaxFps(DEFAULT_MAX_FPS); - QObject::connect(_webSurface.data(), &OffscreenQmlSurface::rootContextCreated, [this](QQmlContext* surfaceContext) { + QObject::connect(_webSurface.data(), &OffscreenQmlSurface::rootContextCreated, [](QQmlContext* surfaceContext) { // FIXME - Keyboard HMD only: Possibly add "HMDinfo" object to context for WebView.qml. surfaceContext->setContextProperty("desktop", QVariant()); // Let us interact with the keyboard From 2546ff91ca4427de972968794fdccb55924dc9d4 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 28 Mar 2018 15:05:51 -0700 Subject: [PATCH 39/56] Fix nodelist connections not resetting on both ends This change adds a connection ID to ping packets. Each node keeps a connection id for each other node that it has connected to. When a node is removed the connection id is incremented. If a node sees another node with a higher connection id, it will reset its connection with the new connection id, ensuring that local state is reset on both ends when nodes lose contact. --- domain-server/src/DomainGatekeeper.cpp | 12 ++++++--- libraries/networking/src/LimitedNodeList.cpp | 27 +++++++++++++++----- libraries/networking/src/LimitedNodeList.h | 11 +++++--- libraries/networking/src/NodeList.cpp | 24 ++++++++++++++--- 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 7d0b538f6e..9f24036e92 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -451,11 +451,12 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect return SharedNodePointer(); } - QUuid hintNodeID; + QUuid existingNodeID; // in case this is a node that's failing to connect // double check we don't have the same node whose sockets match exactly already in the list limitedNodeList->eachNodeBreakable([&](const SharedNodePointer& node){ + if (node->getPublicSocket() == nodeConnection.publicSockAddr && node->getLocalSocket() == nodeConnection.localSockAddr) { // we have a node that already has these exact sockets - this can occur if a node // is failing to connect to the domain @@ -465,15 +466,20 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect auto existingNodeData = static_cast(node->getLinkedData()); if (existingNodeData->getUsername() == username) { - hintNodeID = node->getUUID(); + qDebug() << "Deleting existing connection from same sockaddr: " << node->getUUID(); + existingNodeID = node->getUUID(); return false; } } return true; }); + if (!existingNodeID.isNull()) { + limitedNodeList->killNodeWithUUID(existingNodeID); + } + // add the connecting node (or re-use the matched one from eachNodeBreakable above) - SharedNodePointer newNode = addVerifiedNodeFromConnectRequest(nodeConnection, hintNodeID); + SharedNodePointer newNode = addVerifiedNodeFromConnectRequest(nodeConnection); // set the edit rights for this user newNode->setPermissions(userPerms); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 0803e380f2..e27e2d6d08 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -578,9 +578,10 @@ void LimitedNodeList::reset() { // we need to make sure any socket connections are gone so wait on that here _nodeSocket.clearConnections(); + _connectionIDs.clear(); } -bool LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID) { +bool LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID, ConnectionID newConnectionID) { QReadLocker readLocker(&_nodeMutex); NodeHash::iterator it = _nodeHash.find(nodeUUID); @@ -594,7 +595,7 @@ bool LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID) { _nodeHash.unsafe_erase(it); } - handleNodeKill(matchingNode); + handleNodeKill(matchingNode, newConnectionID); return true; } @@ -609,7 +610,7 @@ void LimitedNodeList::processKillNode(ReceivedMessage& message) { killNodeWithUUID(nodeUUID); } -void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) { +void LimitedNodeList::handleNodeKill(const SharedNodePointer& node, ConnectionID nextConnectionID) { qCDebug(networking) << "Killed" << *node; node->stopPingTimer(); emit nodeKilled(node); @@ -617,6 +618,15 @@ void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) { if (auto activeSocket = node->getActiveSocket()) { _nodeSocket.cleanupConnection(*activeSocket); } + + auto it = _connectionIDs.find(node->getUUID()); + if (it != _connectionIDs.end()) { + if (nextConnectionID == NULL_CONNECTION_ID) { + it->second++; + } else { + it->second = nextConnectionID; + } + } } SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType, @@ -638,6 +648,11 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t return matchingNode; } else { + auto it = _connectionIDs.find(uuid); + if (it == _connectionIDs.end()) { + _connectionIDs[uuid] = INITIAL_CONNECTION_ID; + } + // we didn't have this node, so add them Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket); newNode->setIsReplicated(isReplicated); @@ -712,13 +727,13 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t } } -std::unique_ptr LimitedNodeList::constructPingPacket(PingType_t pingType) { - int packetSize = sizeof(PingType_t) + sizeof(quint64); +std::unique_ptr LimitedNodeList::constructPingPacket(const QUuid& nodeId, PingType_t pingType) { + int packetSize = sizeof(PingType_t) + sizeof(quint64) + sizeof(int64_t); auto pingPacket = NLPacket::create(PacketType::Ping, packetSize); - pingPacket->writePrimitive(pingType); pingPacket->writePrimitive(usecTimestampNow()); + pingPacket->writePrimitive(_connectionIDs[nodeId]); return pingPacket; } diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 7165b3dd63..7ec3a41450 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -66,6 +66,10 @@ const QHostAddress DEFAULT_ASSIGNMENT_CLIENT_MONITOR_HOSTNAME = QHostAddress::Lo const QString USERNAME_UUID_REPLACEMENT_STATS_KEY = "$username"; +using ConnectionID = int64_t; +const ConnectionID NULL_CONNECTION_ID { -1 }; +const ConnectionID INITIAL_CONNECTION_ID { 0 }; + typedef std::pair UUIDNodePair; typedef tbb::concurrent_unordered_map NodeHash; @@ -180,7 +184,7 @@ public: void getPacketStats(float& packetsInPerSecond, float& bytesInPerSecond, float& packetsOutPerSecond, float& bytesOutPerSecond); void resetPacketStats(); - std::unique_ptr constructPingPacket(PingType_t pingType = PingType::Agnostic); + std::unique_ptr constructPingPacket(const QUuid& nodeId, PingType_t pingType = PingType::Agnostic); std::unique_ptr constructPingReplyPacket(ReceivedMessage& message); static std::unique_ptr constructICEPingPacket(PingType_t pingType, const QUuid& iceID); @@ -319,7 +323,7 @@ public slots: void startSTUNPublicSocketUpdate(); virtual void sendSTUNRequest(); - bool killNodeWithUUID(const QUuid& nodeUUID); + bool killNodeWithUUID(const QUuid& nodeUUID, ConnectionID newConnectionID = NULL_CONNECTION_ID); signals: void dataSent(quint8 channelType, int bytes); @@ -371,7 +375,7 @@ protected: bool packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet, Node* sourceNode = nullptr); void processSTUNResponse(std::unique_ptr packet); - void handleNodeKill(const SharedNodePointer& node); + void handleNodeKill(const SharedNodePointer& node, ConnectionID newConnectionID = NULL_CONNECTION_ID); void stopInitialSTUNUpdate(bool success); @@ -418,6 +422,7 @@ protected: } } + std::unordered_map _connectionIDs; private slots: void flagTimeForConnectionStep(ConnectionStep connectionStep, quint64 timestamp); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index cb0d2e4cd5..d33a81841a 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -214,6 +214,20 @@ void NodeList::processPingPacket(QSharedPointer message, Shared sendingNode->setSymmetricSocket(senderSockAddr); } } + + int64_t connectionId; + + message->readPrimitive(&connectionId); + + auto it = _connectionIDs.find(sendingNode->getUUID()); + if (it != _connectionIDs.end()) { + if (connectionId > it->second) { + qDebug() << "Received a ping packet with a larger connection id (" << connectionId << ">" << it->second << ") from " + << sendingNode->getUUID(); + killNodeWithUUID(sendingNode->getUUID(), connectionId); + } + } + } void NodeList::processPingReplyPacket(QSharedPointer message, SharedNodePointer sendingNode) { @@ -689,16 +703,18 @@ void NodeList::pingPunchForInactiveNode(const SharedNodePointer& node) { if (node->getConnectionAttempts() > 0 && node->getConnectionAttempts() % NUM_DEBUG_CONNECTION_ATTEMPTS == 0) { qCDebug(networking) << "No response to UDP hole punch pings for node" << node->getUUID() << "in last second."; } + + auto nodeId = node->getUUID(); // send the ping packet to the local and public sockets for this node - auto localPingPacket = constructPingPacket(PingType::Local); + auto localPingPacket = constructPingPacket(nodeId, PingType::Local); sendPacket(std::move(localPingPacket), *node, node->getLocalSocket()); - auto publicPingPacket = constructPingPacket(PingType::Public); + auto publicPingPacket = constructPingPacket(nodeId, PingType::Public); sendPacket(std::move(publicPingPacket), *node, node->getPublicSocket()); if (!node->getSymmetricSocket().isNull()) { - auto symmetricPingPacket = constructPingPacket(PingType::Symmetric); + auto symmetricPingPacket = constructPingPacket(nodeId, PingType::Symmetric); sendPacket(std::move(symmetricPingPacket), *node, node->getSymmetricSocket()); } @@ -768,7 +784,7 @@ void NodeList::sendKeepAlivePings() { auto type = node->getType(); return !node->isUpstream() && _nodeTypesOfInterest.contains(type) && !NodeType::isDownstream(type); }, [&](const SharedNodePointer& node) { - sendPacket(constructPingPacket(), *node); + sendPacket(constructPingPacket(node->getUUID()), *node); }); } From e37655ecdcbdaea5c186e823eaebde1e782e2673 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Wed, 11 Apr 2018 16:35:49 +1200 Subject: [PATCH 40/56] Update location.hostname JSDoc per serverless domains --- libraries/networking/src/AddressManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index b42aec2771..6609efcf30 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -39,7 +39,7 @@ const QString GET_PLACE = "/api/v1/places/%1"; * @property {Uuid} domainId - Synonym for domainId. Read-only. Deprecated: This property * is deprecated and will soon be removed. * @property {string} hostname - The name of the domain for your current metaverse address (e.g., "AvatarIsland", - * localhost, or an IP address). + * localhost, an IP address, or the file path to a serverless domain). * Read-only. * @property {string} href - Your current metaverse address (e.g., "hifi://avatarisland/15,-10,26/0,0,0,1") * regardless of whether or not you're connected to the domain. From c836014d213ad81db87e0274da4dd23cf68b13ad Mon Sep 17 00:00:00 2001 From: David Rowe Date: Thu, 12 Apr 2018 09:24:10 +1200 Subject: [PATCH 41/56] Fix JSDoc typo noticed in passing --- interface/src/scripting/WindowScriptingInterface.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/scripting/WindowScriptingInterface.h b/interface/src/scripting/WindowScriptingInterface.h index e3b092d011..2d92c945f2 100644 --- a/interface/src/scripting/WindowScriptingInterface.h +++ b/interface/src/scripting/WindowScriptingInterface.h @@ -528,8 +528,8 @@ private slots: signals: /**jsdoc - * Triggered when you change the domain you're visiting. Warning: Is not emitted if you go to domain that - * isn't running. + * Triggered when you change the domain you're visiting. Warning: Is not emitted if you go to a domain + * that isn't running. * @function Window.domainChanged * @param {string} domainURL - The domain's URL. * @returns {Signal} From bd59ff1573dc949f2937f68818a205a0263fd54e Mon Sep 17 00:00:00 2001 From: Clement Date: Fri, 13 Apr 2018 15:04:25 -0700 Subject: [PATCH 42/56] Fix threaded force crashes --- interface/src/Menu.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index df7546cd33..64ea9659ef 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -749,32 +749,32 @@ Menu::Menu() { action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashPureVirtualFunction); connect(action, &QAction::triggered, qApp, []() { crash::pureVirtualCall(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashPureVirtualFunctionThreaded); - connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::pureVirtualCall(); }); }); + connect(action, &QAction::triggered, qApp, []() { std::thread(crash::pureVirtualCall).join(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashDoubleFree); connect(action, &QAction::triggered, qApp, []() { crash::doubleFree(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashDoubleFreeThreaded); - connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::doubleFree(); }); }); + connect(action, &QAction::triggered, qApp, []() { std::thread(crash::doubleFree).join(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashAbort); connect(action, &QAction::triggered, qApp, []() { crash::doAbort(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashAbortThreaded); - connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::doAbort(); }); }); + connect(action, &QAction::triggered, qApp, []() { std::thread(crash::doAbort).join(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNullDereference); connect(action, &QAction::triggered, qApp, []() { crash::nullDeref(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNullDereferenceThreaded); - connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::nullDeref(); }); }); + connect(action, &QAction::triggered, qApp, []() { std::thread(crash::nullDeref).join(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashOutOfBoundsVectorAccess); connect(action, &QAction::triggered, qApp, []() { crash::outOfBoundsVectorCrash(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashOutOfBoundsVectorAccessThreaded); - connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::outOfBoundsVectorCrash(); }); }); + connect(action, &QAction::triggered, qApp, []() { std::thread(crash::outOfBoundsVectorCrash).join(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNewFault); connect(action, &QAction::triggered, qApp, []() { crash::newFault(); }); action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNewFaultThreaded); - connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::newFault(); }); }); + connect(action, &QAction::triggered, qApp, []() { std::thread(crash::newFault).join(); }); // Developer > Log... addActionToQMenuAndActionHash(developerMenu, MenuOption::Log, Qt::CTRL | Qt::SHIFT | Qt::Key_L, From e9d291257677c20944148faae66a22497c6000ff Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 16 Apr 2018 12:03:58 -0700 Subject: [PATCH 43/56] Update variable naming in NodeList from Id to ID --- libraries/networking/src/NodeList.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index d33a81841a..e099a2c527 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -215,16 +215,16 @@ void NodeList::processPingPacket(QSharedPointer message, Shared } } - int64_t connectionId; + int64_t connectionID; - message->readPrimitive(&connectionId); + message->readPrimitive(&connectionID); auto it = _connectionIDs.find(sendingNode->getUUID()); if (it != _connectionIDs.end()) { - if (connectionId > it->second) { - qDebug() << "Received a ping packet with a larger connection id (" << connectionId << ">" << it->second << ") from " + if (connectionID > it->second) { + qDebug() << "Received a ping packet with a larger connection id (" << connectionID << ">" << it->second << ") from " << sendingNode->getUUID(); - killNodeWithUUID(sendingNode->getUUID(), connectionId); + killNodeWithUUID(sendingNode->getUUID(), connectionID); } } @@ -704,17 +704,17 @@ void NodeList::pingPunchForInactiveNode(const SharedNodePointer& node) { qCDebug(networking) << "No response to UDP hole punch pings for node" << node->getUUID() << "in last second."; } - auto nodeId = node->getUUID(); + auto nodeID = node->getUUID(); // send the ping packet to the local and public sockets for this node - auto localPingPacket = constructPingPacket(nodeId, PingType::Local); + auto localPingPacket = constructPingPacket(nodeID, PingType::Local); sendPacket(std::move(localPingPacket), *node, node->getLocalSocket()); - auto publicPingPacket = constructPingPacket(nodeId, PingType::Public); + auto publicPingPacket = constructPingPacket(nodeID, PingType::Public); sendPacket(std::move(publicPingPacket), *node, node->getPublicSocket()); if (!node->getSymmetricSocket().isNull()) { - auto symmetricPingPacket = constructPingPacket(nodeId, PingType::Symmetric); + auto symmetricPingPacket = constructPingPacket(nodeID, PingType::Symmetric); sendPacket(std::move(symmetricPingPacket), *node, node->getSymmetricSocket()); } From a626e9f6f31f4bf1dd30c8bf9c5e4aa14889cdba Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 17 Apr 2018 15:09:10 -0700 Subject: [PATCH 44/56] Add special handler for heap corruption --- interface/src/Crashpad.cpp | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/interface/src/Crashpad.cpp b/interface/src/Crashpad.cpp index 8ed3fc23bd..c824aaa9e4 100644 --- a/interface/src/Crashpad.cpp +++ b/interface/src/Crashpad.cpp @@ -31,10 +31,40 @@ using namespace crashpad; static const std::string BACKTRACE_URL { CMAKE_BACKTRACE_URL }; static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN }; +static std::wstring gIPCPipe; + extern QString qAppFileName(); // crashpad::AnnotationList* crashpadAnnotations { nullptr }; +#include + +LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { + if (pExceptionInfo->ExceptionRecord->ExceptionCode == 0xe06d7363) { // external exception + return EXCEPTION_CONTINUE_SEARCH; + } + + if (pExceptionInfo->ExceptionRecord->ExceptionCode == 0xc0000374) { // heap corruption + qCritical() << "VectoredExceptionHandler"; + qCritical() << QString::number(pExceptionInfo->ExceptionRecord->ExceptionCode, 16); + qCritical() << "Heap corruption!"; + + CrashpadClient client; + if (gIPCPipe.length()) { + + bool rc = client.SetHandlerIPCPipe(gIPCPipe); + qCritical() << "SetHandlerIPCPipe = " << rc; + } else { + qCritical() << "No IPC Pipe was previously defined for crash handler."; + } + qCritical() << "Calling DumpAndCrash()"; + client.DumpAndCrash(pExceptionInfo); + return EXCEPTION_CONTINUE_SEARCH; + } + + return EXCEPTION_CONTINUE_SEARCH; +} + bool startCrashHandler() { if (BACKTRACE_URL.empty() || BACKTRACE_TOKEN.empty()) { return false; @@ -76,7 +106,12 @@ bool startCrashHandler() { // Enable automated uploads. database->GetSettings()->SetUploadsEnabled(true); - return client.StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true); + bool result = client.StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true); + gIPCPipe = client.GetHandlerIPCPipe(); + + AddVectoredExceptionHandler(0, vectoredExceptionHandler); + + return result; } void setCrashAnnotation(std::string name, std::string value) { From 7e6d45ead783a6acbb94ba9ccce03ad4344c4779 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 17 Apr 2018 16:22:42 -0700 Subject: [PATCH 45/56] CR --- interface/src/Crashpad.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/interface/src/Crashpad.cpp b/interface/src/Crashpad.cpp index c824aaa9e4..27da619af1 100644 --- a/interface/src/Crashpad.cpp +++ b/interface/src/Crashpad.cpp @@ -40,18 +40,19 @@ extern QString qAppFileName(); #include LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { - if (pExceptionInfo->ExceptionRecord->ExceptionCode == 0xe06d7363) { // external exception + static const DWORD EXTERNAL_EXCEPTION_CODE{ 0xe06d7363 }; + static const DWORD HEAP_CORRUPTION_CODE{ 0xc0000374 }; + + auto exceptionCode = pExceptionInfo->ExceptionRecord->ExceptionCode; + if (exceptionCode == EXTERNAL_EXCEPTION_CODE) { return EXCEPTION_CONTINUE_SEARCH; } - if (pExceptionInfo->ExceptionRecord->ExceptionCode == 0xc0000374) { // heap corruption - qCritical() << "VectoredExceptionHandler"; - qCritical() << QString::number(pExceptionInfo->ExceptionRecord->ExceptionCode, 16); - qCritical() << "Heap corruption!"; + if (exceptionCode == HEAP_CORRUPTION_CODE) { + qCritical() << "VectoredExceptionHandler: Heap corruption:" << QString::number(exceptionCode, 16); CrashpadClient client; if (gIPCPipe.length()) { - bool rc = client.SetHandlerIPCPipe(gIPCPipe); qCritical() << "SetHandlerIPCPipe = " << rc; } else { From 01c39d4310484d4b1d930de8af26240cf701d186 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 18 Apr 2018 11:53:47 -0700 Subject: [PATCH 46/56] Use NULL_LOCAL_ID in more places, as requested by review --- assignment-client/src/audio/AudioMixer.cpp | 2 +- domain-server/src/DomainGatekeeper.h | 5 +---- domain-server/src/DomainServer.cpp | 2 +- libraries/networking/src/LimitedNodeList.cpp | 2 +- libraries/networking/src/LimitedNodeList.h | 6 +++--- libraries/networking/src/PacketReceiver.cpp | 2 +- libraries/networking/src/SequenceNumberStats.cpp | 4 ++-- libraries/networking/src/SequenceNumberStats.h | 3 ++- 8 files changed, 12 insertions(+), 14 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index d47172f62e..34eb138697 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -123,7 +123,7 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer mess auto replicatedNode = nodeList->addOrUpdateNode(nodeID, NodeType::Agent, message->getSenderSockAddr(), message->getSenderSockAddr(), - 0, true, true); + Node::NULL_LOCAL_ID, true, true); replicatedNode->setLastHeardMicrostamp(usecTimestampNow()); // construct a "fake" audio received message from the byte array and packet list information diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h index ec7ec5701f..8402e58559 100644 --- a/domain-server/src/DomainGatekeeper.h +++ b/domain-server/src/DomainGatekeeper.h @@ -126,10 +126,7 @@ private: // Local ID management. void initLocalIDManagement(); - struct UuidHash { - size_t operator()(const QUuid& uuid) const { return qHash(uuid); } - }; - using UUIDToLocalID = std::unordered_map ; + using UUIDToLocalID = std::unordered_map ; using LocalIDs = std::unordered_set; LocalIDs _localIDs; UUIDToLocalID _uuidToLocalID; diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 7265dff15c..2fa86cc860 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -2905,7 +2905,7 @@ void DomainServer::updateReplicationNodes(ReplicationServerDirection direction) // manually add the replication node to our node list auto node = nodeList->addOrUpdateNode(QUuid::createUuid(), replicationServer.nodeType, replicationServer.sockAddr, replicationServer.sockAddr, - 0, false, direction == Upstream); + Node::NULL_LOCAL_ID, false, direction == Upstream); node->setIsForcedNeverSilent(true); qDebug() << "Adding" << (direction == Upstream ? "upstream" : "downstream") diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index ec2e107567..88bf45a419 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -306,7 +306,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe return true; } } else { - NLPacket::LocalID sourceLocalID = 0; + NLPacket::LocalID sourceLocalID = Node::NULL_LOCAL_ID; // check if we were passed a sourceNode hint or if we need to look it up if (!sourceNode) { // figure out which node this is from diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 6c61fbc43c..c91c012f27 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -128,7 +128,7 @@ public: virtual bool isDomainServer() const { return true; } virtual QUuid getDomainUUID() const { assert(false); return QUuid(); } - virtual Node::LocalID getDomainLocalID() const { assert(false); return 0; } + virtual Node::LocalID getDomainLocalID() const { assert(false); return Node::NULL_LOCAL_ID; } virtual HifiSockAddr getDomainSockAddr() const { assert(false); return HifiSockAddr(); } // use sendUnreliablePacket to send an unrelaible packet (that you do not need to move) @@ -162,8 +162,8 @@ public: SharedNodePointer addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, - Node::LocalID localID = 0, bool isReplicated = false, bool isUpstream = false, - const QUuid& connectionSecret = QUuid(), + Node::LocalID localID = Node::NULL_LOCAL_ID, bool isReplicated = false, + bool isUpstream = false, const QUuid& connectionSecret = QUuid(), const NodePermissions& permissions = DEFAULT_AGENT_PERMISSIONS); static bool parseSTUNResponse(udt::BasePacket* packet, QHostAddress& newPublicAddress, uint16_t& newPublicPort); diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 39a3800c4b..fe2a273d61 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -261,7 +261,7 @@ void PacketReceiver::handleVerifiedMessage(QSharedPointer recei SharedNodePointer matchingNode; - if (receivedMessage->getSourceID() != 0) { + if (receivedMessage->getSourceID() != Node::NULL_LOCAL_ID) { matchingNode = nodeList->nodeWithLocalID(receivedMessage->getSourceID()); } QMutexLocker packetListenerLocker(&_packetListenerLock); diff --git a/libraries/networking/src/SequenceNumberStats.cpp b/libraries/networking/src/SequenceNumberStats.cpp index 052cdf1a4e..03e149eb0e 100644 --- a/libraries/networking/src/SequenceNumberStats.cpp +++ b/libraries/networking/src/SequenceNumberStats.cpp @@ -25,7 +25,7 @@ SequenceNumberStats::SequenceNumberStats(int statsHistoryLength, bool canDetectO : _lastReceivedSequence(0), _missingSet(), _stats(), - _lastSenderID(0), + _lastSenderID(NULL_LOCAL_ID), _statsHistory(statsHistoryLength), _lastUnreasonableSequence(0), _consecutiveUnreasonableOnTime(0) @@ -35,7 +35,7 @@ SequenceNumberStats::SequenceNumberStats(int statsHistoryLength, bool canDetectO void SequenceNumberStats::reset() { _missingSet.clear(); _stats = PacketStreamStats(); - _lastSenderID = 0; + _lastSenderID = NULL_LOCAL_ID; _statsHistory.clear(); _lastUnreasonableSequence = 0; _consecutiveUnreasonableOnTime = 0; diff --git a/libraries/networking/src/SequenceNumberStats.h b/libraries/networking/src/SequenceNumberStats.h index ba9db2a225..8705f960b7 100644 --- a/libraries/networking/src/SequenceNumberStats.h +++ b/libraries/networking/src/SequenceNumberStats.h @@ -73,7 +73,7 @@ public: SequenceNumberStats(int statsHistoryLength = 0, bool canDetectOutOfSync = true); void reset(); - ArrivalInfo sequenceNumberReceived(quint16 incoming, NetworkLocalID senderID = 0, const bool wantExtraDebugging = false); + ArrivalInfo sequenceNumberReceived(quint16 incoming, NetworkLocalID senderID = NULL_LOCAL_ID, const bool wantExtraDebugging = false); void pruneMissingSet(const bool wantExtraDebugging = false); void pushStatsToHistory() { _statsHistory.insert(_stats); } @@ -101,6 +101,7 @@ private: PacketStreamStats _stats; NetworkLocalID _lastSenderID; + static const NetworkLocalID NULL_LOCAL_ID = (NetworkLocalID) 0; RingBufferHistory _statsHistory; From c964852e45436643e4ba43fa13bf93114b6eefde Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 18 Apr 2018 15:09:00 -0700 Subject: [PATCH 47/56] Another use of NULL_LOCAL_ID --- assignment-client/src/avatars/AvatarMixer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 25bbc4cd0a..29340f6474 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -74,7 +74,7 @@ SharedNodePointer addOrUpdateReplicatedNode(const QUuid& nodeID, const HifiSockA auto replicatedNode = DependencyManager::get()->addOrUpdateNode(nodeID, NodeType::Agent, senderSockAddr, senderSockAddr, - 0, true, true); + Node::NULL_LOCAL_ID, true, true); replicatedNode->setLastHeardMicrostamp(usecTimestampNow()); From bef4eb1d05c8a6b98dbb7fdcf9d1f20585bf0de3 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 19 Apr 2018 14:03:51 -0700 Subject: [PATCH 48/56] Increase packet version for Ping --- libraries/networking/src/udt/PacketHeaders.cpp | 2 ++ libraries/networking/src/udt/PacketHeaders.h | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index a83924ee58..11b2c516f8 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -75,6 +75,8 @@ PacketVersion versionForPacketType(PacketType packetType) { return static_cast(IcePingVersion::SendICEPeerID); case PacketType::DomainSettings: return 18; // replace min_avatar_scale and max_avatar_scale with min_avatar_height and max_avatar_height + case PacketType::Ping: + return static_cast(PingVersion::IncludeConnectionID); default: return 17; } diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 09fd31a41e..091fcb1091 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -322,4 +322,8 @@ enum class IcePingVersion : PacketVersion { SendICEPeerID = 18 }; +enum class PingVersion : PacketVersion { + IncludeConnectionID = 18 +}; + #endif // hifi_PacketHeaders_h From 0552a7242a7d066b9077c8ecb88bd2dceff2548d Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 20 Apr 2018 09:52:35 +1200 Subject: [PATCH 49/56] Further JSDoc updates for serverless domains --- libraries/networking/src/AddressManager.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index 6c6070914f..0009bef49a 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -34,15 +34,16 @@ const QString GET_PLACE = "/api/v1/places/%1"; * * @namespace location * @property {Uuid} domainID - A UUID uniquely identifying the domain you're visiting. Is {@link Uuid|Uuid.NULL} if you're not - * connected to the domain. + * connected to the domain or are at a serverless domain. * Read-only. * @property {Uuid} domainId - Synonym for domainId. Read-only. Deprecated: This property * is deprecated and will soon be removed. * @property {string} hostname - The name of the domain for your current metaverse address (e.g., "AvatarIsland", - * localhost, an IP address, or the file path to a serverless domain). + * localhost, or an IP address). Is blank if you're at a serverless domain. * Read-only. * @property {string} href - Your current metaverse address (e.g., "hifi://avatarisland/15,-10,26/0,0,0,1") - * regardless of whether or not you're connected to the domain. + * regardless of whether or not you're connected to the domain. Starts with "file:///" if at a serverless + * domain. * Read-only. * @property {boolean} isConnected - true if you're connected to the domain in your current href * metaverse address, otherwise false. From 8ade4f34f4c70fe6fdb00ad82c0ba9deab22617b Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 20 Apr 2018 10:11:01 +1200 Subject: [PATCH 50/56] Tweak wording --- libraries/networking/src/AddressManager.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index 0009bef49a..94eff46bda 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -34,16 +34,16 @@ const QString GET_PLACE = "/api/v1/places/%1"; * * @namespace location * @property {Uuid} domainID - A UUID uniquely identifying the domain you're visiting. Is {@link Uuid|Uuid.NULL} if you're not - * connected to the domain or are at a serverless domain. + * connected to the domain or are in a serverless domain. * Read-only. * @property {Uuid} domainId - Synonym for domainId. Read-only. Deprecated: This property * is deprecated and will soon be removed. * @property {string} hostname - The name of the domain for your current metaverse address (e.g., "AvatarIsland", - * localhost, or an IP address). Is blank if you're at a serverless domain. + * localhost, or an IP address). Is blank if you're in a serverless domain. * Read-only. * @property {string} href - Your current metaverse address (e.g., "hifi://avatarisland/15,-10,26/0,0,0,1") - * regardless of whether or not you're connected to the domain. Starts with "file:///" if at a serverless - * domain. + * regardless of whether or not you're connected to the domain. Starts with "file:///" if you're in a + * serverless domain. * Read-only. * @property {boolean} isConnected - true if you're connected to the domain in your current href * metaverse address, otherwise false. From 377fc6d6c6d8041550d4b2c9919a54d42f6865cb Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 19 Apr 2018 16:55:32 -0700 Subject: [PATCH 51/56] Revert to old packet version for ICE packets Some ICE (& STUN) packets were using the default version, which we bumped for the HMAC change. This commit breaks out the switch for them and reverts to 17. --- libraries/networking/src/udt/PacketHeaders.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index f09a049fc4..0dbeb1e92e 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -24,6 +24,8 @@ int packetTypeMetaTypeId = qRegisterMetaType(); PacketVersion versionForPacketType(PacketType packetType) { switch (packetType) { + case PacketType::StunResponse: + return 17; case PacketType::DomainList: return static_cast(DomainListVersion::GetMachineFingerprintFromUUIDSupport); case PacketType::EntityAdd: @@ -40,8 +42,21 @@ PacketVersion versionForPacketType(PacketType packetType) { return static_cast(AvatarMixerPacketVersion::FBXReaderNodeReparenting); case PacketType::MessagesData: return static_cast(MessageDataVersion::TextOrBinaryData); + // ICE packets + case PacketType::ICEServerPeerInformation: + return 17; + case PacketType::ICEServerHeartbeatACK: + return 17; + case PacketType::ICEServerQuery: + return 17; case PacketType::ICEServerHeartbeat: return 18; // ICE Server Heartbeat signing + case PacketType::ICEPing: + return static_cast(IcePingVersion::SendICEPeerID); + case PacketType::ICEPingReply: + return 17; + case PacketType::ICEServerHeartbeatDenied: + return 17; case PacketType::AssetMappingOperation: case PacketType::AssetMappingOperationReply: return static_cast(AssetServerPacketVersion::RedirectedMappings); @@ -71,8 +86,6 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::MicrophoneAudioWithEcho: case PacketType::AudioStreamStats: return static_cast(AudioVersion::HighDynamicRangeVolume); - case PacketType::ICEPing: - return static_cast(IcePingVersion::SendICEPeerID); case PacketType::DomainSettings: return 18; // replace min_avatar_scale and max_avatar_scale with min_avatar_height and max_avatar_height default: From 0a7cccc3f7fead14c3b48abf1bc459a3327d7dd1 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 20 Apr 2018 12:47:53 +1200 Subject: [PATCH 52/56] Move Vec2 and Vec4 JSDoc --- libraries/script-engine/src/Vec3.h | 18 ------------------ libraries/shared/src/RegisteredMetaTypes.h | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/libraries/script-engine/src/Vec3.h b/libraries/script-engine/src/Vec3.h index 7ed0fd9e8b..c419749c26 100644 --- a/libraries/script-engine/src/Vec3.h +++ b/libraries/script-engine/src/Vec3.h @@ -21,14 +21,6 @@ #include "GLMHelpers.h" -/**jsdoc - * A 2-dimensional vector. - * - * @typedef {object} Vec2 - * @property {number} x - X-coordinate of the vector. - * @property {number} y - Y-coordinate of the vector. - */ - /**jsdoc * A 3-dimensional vector. * @@ -38,16 +30,6 @@ * @property {number} z - Z-coordinate of the vector. */ -/**jsdoc - * A 4-dimensional vector. - * - * @typedef {object} Vec4 - * @property {number} x - X-coordinate of the vector. - * @property {number} y - Y-coordinate of the vector. - * @property {number} z - Z-coordinate of the vector. - * @property {number} w - W-coordinate of the vector. - */ - /**jsdoc * A color vector. * diff --git a/libraries/shared/src/RegisteredMetaTypes.h b/libraries/shared/src/RegisteredMetaTypes.h index 78e368748b..689d1a3f42 100644 --- a/libraries/shared/src/RegisteredMetaTypes.h +++ b/libraries/shared/src/RegisteredMetaTypes.h @@ -44,6 +44,15 @@ void registerMetaTypes(QScriptEngine* engine); QScriptValue mat4toScriptValue(QScriptEngine* engine, const glm::mat4& mat4); void mat4FromScriptValue(const QScriptValue& object, glm::mat4& mat4); +/**jsdoc + * A 4-dimensional vector. + * + * @typedef {object} Vec4 + * @property {number} x - X-coordinate of the vector. + * @property {number} y - Y-coordinate of the vector. + * @property {number} z - Z-coordinate of the vector. + * @property {number} w - W-coordinate of the vector. + */ // Vec4 QScriptValue vec4toScriptValue(QScriptEngine* engine, const glm::vec4& vec4); void vec4FromScriptValue(const QScriptValue& object, glm::vec4& vec4); @@ -59,6 +68,13 @@ QVariant vec3toVariant(const glm::vec3& vec3); glm::vec3 vec3FromVariant(const QVariant &object, bool& valid); glm::vec3 vec3FromVariant(const QVariant &object); +/**jsdoc + * A 2-dimensional vector. + * + * @typedef {object} Vec2 + * @property {number} x - X-coordinate of the vector. + * @property {number} y - Y-coordinate of the vector. + */ // Vec2 QScriptValue vec2toScriptValue(QScriptEngine* engine, const glm::vec2 &vec2); void vec2FromScriptValue(const QScriptValue &object, glm::vec2 &vec2); From b1e214de5a7baa52ecfd8c1efadbc88aa86f3fdf Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 20 Apr 2018 16:40:15 +1200 Subject: [PATCH 53/56] Vec3 API JSDoc --- libraries/script-engine/src/Vec3.h | 323 ++++++++++++++++++++++++++++- 1 file changed, 321 insertions(+), 2 deletions(-) diff --git a/libraries/script-engine/src/Vec3.h b/libraries/script-engine/src/Vec3.h index c419749c26..635f2a530c 100644 --- a/libraries/script-engine/src/Vec3.h +++ b/libraries/script-engine/src/Vec3.h @@ -22,7 +22,7 @@ #include "GLMHelpers.h" /**jsdoc - * A 3-dimensional vector. + * A 3-dimensional vector. See also the {@link Vec3(0)|Vec3} object. * * @typedef {object} Vec3 * @property {number} x - X-coordinate of the vector. @@ -31,7 +31,7 @@ */ /**jsdoc - * A color vector. + * A color vector. See also the {@link Vec3(0)|Vec3} object. * * @typedef {object} Vec3Color * @property {number} x - Red component value. Integer in the range 0 - 255. @@ -39,6 +39,51 @@ * @property {number} z - Blue component value. Integer in the range 0 - 255. */ +/**jsdoc + * The Vec3 API facilities for generating and manipulating 3-dimensional vectors. High Fidelity uses a right-handed + * Cartesian coordinate system where the y-axis is the "up" and the negative z-axis is the "front" direction. + * High Fidelity coordinate system + * + * @namespace Vec3 + * @variation 0 + * @property {Vec3} UNIT_X - { x: 1, y: 0, z: 0 } : Unit vector in the x-axis direction. Read-only. + * @property {Vec3} UNIT_Y - { x: 0, y: 1, z: 0 } : Unit vector in the y-axis direction. Read-only. + * @property {Vec3} UNIT_Z - { x: 0, y: 0, z: 1 } : Unit vector in the z-axis direction. Read-only. + * @property {Vec3} UNIT_NEG_X - { x: -1, y: 0, z: 0 } : Unit vector in the negative x-axis direction. + * Read-only. + * @property {Vec3} UNIT_NEG_Y - { x: 0, y: -1, z: 0 } : Unit vector in the negative y-axis direction. + * Read-only. + * @property {Vec3} UNIT_NEG_Z - { x: 0, y: 0, z: -1 } : Unit vector in the negative z-axis direction. + * Read-only. + * @property {Vec3} UNIT_XY - { x: 0.707107, y: 0.707107, z: 0 } : Unit vector in the direction of the diagonal + * between the x and y axes. Read-only. + * @property {Vec3} UNIT_XZ - { x: 0.707107, y: 0, z: 0.707107 } : Unit vector in the direction of the diagonal + * between the x and z axes. Read-only. + * @property {Vec3} UNIT_YZ - { x: 0, y: 0.707107, z: 0.707107 } : Unit vector in the direction of the diagonal + * between the y and z axes. Read-only. + * @property {Vec3} UNIT_XYZ - { x: 0.577350, y: 0.577350, z: 0.577350 } : Unit vector in the direction of the + * diagonal between the x, y, and z axes. Read-only. + * @property {Vec3} FLOAT_MAX - { x: 3.402823e+38, y: 3.402823e+38, z: 3.402823e+38 } : Vector with all axis + * values set to the maximum floating point value. Read-only. + * @property {Vec3} FLOAT_MIN - { x: -3.402823e+38, y: -3.402823e+38, z: -3.402823e+38 } : Vector with all axis + * values set to the negative of the maximum floating point value. Read-only. + * @property {Vec3} ZERO - { x: 0, y: 0, z: 0 } : Vector with all axis values set to 0. + * Read-only. + * @property {Vec3} ONE - { x: 1, y: 1, z: 1 } : Vector with all axis values set to 1. + * Read-only. + * @property {Vec3} TWO - { x: 2, y: 2, z: 2 } : Vector with all axis values set to 2. + * Read-only. + * @property {Vec3} HALF - { x: 0.5, y: 0.5, z: 0.5 } : Vector with all axis values set to 0.5. + * Read-only. + * @property {Vec3} RIGHT - { x: 1, y: 0, z: 0 } : Unit vector in the "right" direction. Synonym for + * UNIT_X. Read-only. + * @property {Vec3} UP - { x: 0, y: 1, z: 0 } : Unit vector in the "up" direction. Synonym for + * UNIT_Y. Read-only. + * @property {Vec3} FRONT - { x: 0, y: 0, z: -1 } : Unit vector in the "front" direction. Synonym for + * UNIT_NEG_Z. Read-only. + */ + /// Scriptable interface a Vec3ernion helper class object. Used exclusively in the JavaScript API class Vec3 : public QObject, protected QScriptable { Q_OBJECT @@ -63,27 +108,301 @@ class Vec3 : public QObject, protected QScriptable { Q_PROPERTY(glm::vec3 FRONT READ FRONT CONSTANT) public slots: + + /**jsdoc + * Calculate the reflection of a vector in a plane. + * @function Vec3(0).reflect + * @param {Vec3} v - The vector to reflect. + * @param {Vec3} normal - The normal of the plane. + * @returns {Vec3} The vector reflected in the plane given by the normal. + * @example Reflect a vector in the x-z plane. + * var v = { x: 1, y: 2, z: 3 }; + * var normal = Vec3.UNIT_Y; + * var reflected = Vec3.reflect(v, normal); + * print(JSON.stringify(reflected)); // {"x":1,"y":-2,"z":3} + */ glm::vec3 reflect(const glm::vec3& v1, const glm::vec3& v2) { return glm::reflect(v1, v2); } + + /**jsdoc + * Calculate the cross product of two vectors. + * @function Vec3(0).cross + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @returns {Vec3} The cross product of v1 and v2. + * @example The cross product of x and y unit vectors is the z unit vector. + * var v1 = { x: 1, y: 0, z: 0 }; + * var v2 = { x: 0, y: 1, z: 0 }; + * var crossProduct = Vec3.cross(v1, v2); + * print(JSON.stringify(crossProduct)); // {"x":0,"y":0,"z":1} + */ glm::vec3 cross(const glm::vec3& v1, const glm::vec3& v2) { return glm::cross(v1, v2); } + + /**jsdoc + * Calculate the dot product of two vectors. + * @function Vec3(0).dot + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @returns {number} The dot product of v1 and v2. + * @example The dot product of vectors at right angles is 0. + * var v1 = { x: 1, y: 0, z: 0 }; + * var v2 = { x: 0, y: 1, z: 0 }; + * var dotProduct = Vec3.dot(v1, v2); + * print(dotProduct); // 0 + */ float dot(const glm::vec3& v1, const glm::vec3& v2) { return glm::dot(v1, v2); } + + /**jsdoc + * Multiply a vector by a scale factor. + * @function Vec3(0).multiply + * @param {Vec3} v - The vector. + * @param {number} scale - The scale factor. + * @returns {Vec3} The vector with each ordinate value multiplied by the scale. + */ glm::vec3 multiply(const glm::vec3& v1, float f) { return v1 * f; } + + /**jsdoc + * Multiply a vector by a scale factor. + * @function Vec3(0).multiply + * @param {number} scale - The scale factor. + * @param {Vec3} v - The vector. + * @returns {Vec3} The vector with each ordinate value multiplied by the scale. + */ glm::vec3 multiply(float f, const glm::vec3& v1) { return v1 * f; } + + /**jsdoc + * Multiply two vectors. + * @function Vec3(0).multiplyVbyV + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @returns {Vec3} A vector formed by multiplying the ordinates of two vectors: { x: v1.x * v2.x, y: v1.y * v2.y, + * z: v1.z * v2.z }. + * @example Multiply two vectors. + * var v1 = { x: 1, y: 2, z: 3 }; + * var v2 = { x: 1, y: 2, z: 3 }; + * var multiplied = Vec3.multiplyVbyV(v1, v2); + * print(JSON.stringify(multiplied)); // {"x":1,"y":4,"z":9} + */ glm::vec3 multiplyVbyV(const glm::vec3& v1, const glm::vec3& v2) { return v1 * v2; } + + /**jsdoc + * Rotate a vector. + * @function Vec3(0).multiplyQbyV + * @param {Quat} q - The rotation to apply. + * @param {Vec3} v - The vector to rotate. + * @returns {Vec3} v rotated by q. + * @example Rotate the negative z-axis by 90 degrees about the x-axis. + * var v = Vec3.UNIT_NEG_Z; + * var q = Quat.fromPitchYawRollDegrees(90, 0, 0); + * var result = Vec3.multiplyQbyV(q, v); + * print(JSON.stringify(result)); // {"x":0,"y":1.000,"z":1.19e-7} + */ glm::vec3 multiplyQbyV(const glm::quat& q, const glm::vec3& v) { return q * v; } + + /**jsdoc + * Calculate the sum of two vectors. + * @function Vec3(0).sum + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @returns {Vec3} The sum of the two vectors. + */ glm::vec3 sum(const glm::vec3& v1, const glm::vec3& v2) { return v1 + v2; } + + /**jsdoc + * Calculate one vector subtracted from another. + * @function Vec3(0).subtract + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @returns {Vec3} The second vector subtracted from the first. + */ glm::vec3 subtract(const glm::vec3& v1, const glm::vec3& v2) { return v1 - v2; } + + /**jsdoc + * Calculate the length of a vector + * @function Vec3(0).length + * @param {Vec3} v - The vector. + * @returns {number} The length of the vector. + */ float length(const glm::vec3& v) { return glm::length(v); } + + /**jsdoc + * Calculate the distance between two points. + * @function Vec3(0).distance + * @param {Vec3} p1 - The first point. + * @param {Vec3} p2 - The second point. + * @returns {number} The distance between the two points. + * @example The distance between two points is aways positive. + * var p1 = { x: 0, y: 0, z: 0 }; + * var p2 = { x: 0, y: 0, z: 10 }; + * var distance = Vec3.distance(p1, p2); + * print(distance); // 10 + * + * p2 = { x: 0, y: 0, z: -10 }; + * distance = Vec3.distance(p1, p2); + * print(distance); // 10 + */ float distance(const glm::vec3& v1, const glm::vec3& v2) { return glm::distance(v1, v2); } + + /**jsdoc + * Calculate the angle of rotation from one vector onto another, with the sign depending on a reference vector. + * @function Vec3(0).orientedAngle + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @param {Vec3} ref - Reference vector. + * @returns {number} The angle of rotation from the first vector to the second, in degrees, with a positive sign if the + * rotation axis aligns with the reference vector (has a positive dot product) otherwise a negative sign. + * @example Compare Vec3.angle() and Vec3.orientedAngle(). + * var v1 = { x: 5, y: 0, z: 0 }; + * var v2 = { x: 5, y: 0, z: 5 }; + * var angle = Vec3.getAngle(v1, v2); + * print(angle * 180 / Math.PI); // 45 + * + * print(Vec3.orientedAngle(v1, v2, Vec3.UNIT_Y)); // -45 + * print(Vec3.orientedAngle(v1, v2, Vec3.UNIT_NEG_Y)); // 45 + * print(Vec3.orientedAngle(v1, v2, { x: 1, y: 2, z: -1 })); // -45 + * print(Vec3.orientedAngle(v1, v2, { x: 1, y: -2, z: -1 })); // 45 + */ float orientedAngle(const glm::vec3& v1, const glm::vec3& v2, const glm::vec3& v3); + + /**jsdoc + * Normalize a vector so that its length is 1. + * @function Vec3(0).normalize + * @param {Vec3} v - The vector to normalize. + * @returns {Vec3} The vector normalized to have a length of 1. + * @example Normalize a vector. + * var v = { x: 10, y: 10, z: 0 }; + * var normalized = Vec3.normalize(v); + * print(JSON.stringify(normalized)); // {"x":0.7071,"y":0.7071,"z":0} + * print(Vec3.length(normalized)); // 1 + */ glm::vec3 normalize(const glm::vec3& v) { return glm::normalize(v); }; + + /**jsdoc + * Calculate a linear interpolation between two vectors. + * @function Vec3(0).mix + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @param {number} factor - The interpolation factor in the range 0.0 to 1.0. + * @returns {Vec3} The linear interpolation between the two vectors: (1 - factor) * v1 + factor * v2. + * @example Linear interpolation between two vectors. + * var v1 = { x: 10, y: 0, z: 0 }; + * var v2 = { x: 0, y: 10, z: 0 }; + * var interpolated = Vec3.mix(v1, v2, 0.75); // 1/4 of v1 and 3/4 of v2. + * print(JSON.stringify(interpolated)); // {"x":2.5,"y":7.5","z":0} + */ glm::vec3 mix(const glm::vec3& v1, const glm::vec3& v2, float m) { return glm::mix(v1, v2, m); } + + /**jsdoc + * Print to the program log a text label followed by a vector value. + * @function Vec3(0).print + * @param {string} label - The label to print. + * @param {Vec3} v - The vector value to print. + * @example Two ways of printing a label and vector value. + * var v = { x: 1, y: 2, z: 3 }; + * Vec3.print("Vector: ", v); // dvec3(1.000000, 2.000000, 3.000000) + * print("Vector: " + JSON.stringify(v)); // {"x":1,"y":2,"z":3} + */ void print(const QString& label, const glm::vec3& v); + + /**jsdoc + * Test whether two vectors are equal. Note: The vectors must be exactly equal in order for + * true to be returned; it is often better to use {@link Vec3(0).withinEpsilon|withinEpsilon}. + * @function Vec3(0).equal + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @returns {boolean} true if the two vectors are exactly equal, otherwise false. + * @example Vectors are only equal if exactly the same. + * var v1 = { x: 10, y: 10, z: 10 }; + * var v2 = { x: 10, y: 10, z: 10 }; + * var equal = Vec3.equal(v1, v2); + * print(equal); // true + * + * v2 = { x: 10, y: 10, z: 10.0005 }; + * equal = Vec3.equal(v1, v2); + * print(equal); // false + */ bool equal(const glm::vec3& v1, const glm::vec3& v2) { return v1 == v2; } + + /**jsdoc + * Test whether two vectors are equal within a tolerance. Note: It is often better to use this function + * than {@link Vec3(0).equal|equal}. + * @function Vec3(0).withinEpsilon + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @param {number} epsilon - The maximum distance between the two vectors. + * @returns {boolean} true if the distance between the points represented by the vectors is less than or equal + * to the epsilon, otherwise false. + * @example Testing vectors for near equality. + * var v1 = { x: 10, y: 10, z: 10 }; + * var v2 = { x: 10, y: 10, z: 10.0005 }; + * var equal = Vec3.equal(v1, v2); + * print(equal); // false + * + * equal = Vec3.withinEpsilon(v1, v2, 0.001); + * print(equal); // true + */ bool withinEpsilon(const glm::vec3& v1, const glm::vec3& v2, float epsilon); + + /**jsdoc + * Calculate polar coordinates (elevation, azimuth, radius) that transform the unit z-axis vector onto a point. + * @function Vec3(0).toPolar + * @param {Vec3} p - The point to calculate the polar coordinates for. + * @returns {Vec3} Vector of polar coordinates for the point: x elevation rotation about the x-axis in + * radians, y azimuth rotation about the y-axis in radians, and z scale. + * @example Polar coordinates for a point. + * var v = { x: 5, y: 2.5, z: 5 }; + * var polar = Vec3.toPolar(v); + * print("Elevation: " + polar.x * 180 / Math.PI); // -19.471 + * print("Azimuth: " + polar.y * 180 / Math.PI); // 45 + * print("Radius: " + polar.z); // 7.5 + */ // FIXME misnamed, should be 'spherical' or 'euler' depending on the implementation glm::vec3 toPolar(const glm::vec3& v); + + /**jsdoc + * Calculate the coordinates of a point from polar coordinate transformation of the unit z-axis vector. + * @function Vec3(0).fromPolar + * @param {Vec3} polar - The polar coordinates of a point: x elevation rotation about the x-axis in radians, + * y azimuth rotation about the y-axis in radians, and z scale. + * @returns {Vec3} The coordinates of the point. + * @example Polar coordinates to Cartesian. + * var polar = { x: -19.471 * Math.PI / 180, y: 45 * Math.PI / 180, z: 7.5 }; + * var p = Vec3.fromPolar(polar); + * print(JSON.stringify(p)); // {"x":5,"y":2.5,"z":5} + */ + // FIXME misnamed, should be 'spherical' or 'euler' depending on the implementation glm::vec3 fromPolar(const glm::vec3& polar); + + /**jsdoc + * Calculate the unit vector corresponding to polar coordinates elevation and azimuth transformation of the unit z-axis + * vector. + * @function Vec3(0).fromPolar + * @param {number} elevation - Rotation about the x-axis, in radians. + * @param {number} azimuth - Rotation about the y-axis, in radians. + * @returns {Vec3} Unit vector for the direction specified by elevation and azimuth. + * @example Polar coordinates to Cartesian. + * var elevation = -19.471 * Math.PI / 180; + * var rotation = 45 * Math.PI / 180; + * var p = Vec3.fromPolar(elevation, rotation); + * print(JSON.stringify(p)); // {"x":0.667,"y":0.333,"z":0.667} + * + * p = Vec3.multiply(7.5, p); + * print(JSON.stringify(p)); // {"x":5,"y":2.5,"z":5} + */ + // FIXME misnamed, should be 'spherical' or 'euler' depending on the implementation glm::vec3 fromPolar(float elevation, float azimuth); + + /**jsdoc + * Calculate the angle between two vectors. + * @function Vec3(0).getAngle + * @param {Vec3} v1 - The first vector. + * @param {Vec3} v2 - The second vector. + * @returns {number} The angle between the two vectors, in radians. + * @example Calculate the angle between two vectors. + * var v1 = { x: 10, y: 0, z: 0 }; + * var v2 = { x: 0, y: 0, z: 10 }; + * var angle = Vec3.getAngle(v1, v2); + * print(angle * 180 / Math.PI); // 90 + */ float getAngle(const glm::vec3& v1, const glm::vec3& v2); private: From 8173ac87a864d69ff3fcde338aa1170af6d9198b Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 20 Apr 2018 16:41:10 +1200 Subject: [PATCH 54/56] Minor improvement to Quat.IDENTITY JSDoc --- libraries/script-engine/src/Quat.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/script-engine/src/Quat.h b/libraries/script-engine/src/Quat.h index 68b4dcd408..e6e395d9bf 100644 --- a/libraries/script-engine/src/Quat.h +++ b/libraries/script-engine/src/Quat.h @@ -35,7 +35,8 @@ * of gimbal lock. * @namespace Quat * @variation 0 - * @property IDENTITY {Quat} The identity rotation, i.e., no rotation. Its value is { x: 0, y: 0, z: 0, w: 1 }. + * @property IDENTITY {Quat} { x: 0, y: 0, z: 0, w: 1 } : The identity rotation, i.e., no rotation. + * Read-only. * @example Print the IDENTITY value. * print(JSON.stringify(Quat.IDENTITY)); // { x: 0, y: 0, z: 0, w: 1 } * print(JSON.stringify(Quat.safeEulerAngles(Quat.IDENTITY))); // { x: 0, y: 0, z: 0 } From 3b4b43cf66d8ea45f5386fdf9359a6674bd7f61e Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Wed, 18 Apr 2018 13:29:39 -0700 Subject: [PATCH 55/56] try to fix audio crash on startup --- interface/src/Application.cpp | 13 +- interface/src/scripting/Audio.cpp | 142 ++++++++++++++------- interface/src/scripting/Audio.h | 20 ++- libraries/audio-client/src/AudioClient.cpp | 24 ++-- libraries/audio-client/src/AudioClient.h | 10 +- 5 files changed, 134 insertions(+), 75 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index aec31f2de4..cfd1d4412c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1102,10 +1102,9 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo auto audioScriptingInterface = DependencyManager::get(); auto myAvatarPosition = DependencyManager::get()->getMyAvatar()->getWorldPosition(); float distance = glm::distance(myAvatarPosition, position); - bool shouldMute = !audioClient->isMuted() && (distance < radius); - if (shouldMute) { - audioClient->toggleMute(); + if (distance < radius) { + audioClient->setMuted(true); audioScriptingInterface->environmentMuted(); } }); @@ -1508,7 +1507,8 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo if (state) { if (action == controller::toInt(controller::Action::TOGGLE_MUTE)) { - DependencyManager::get()->toggleMute(); + auto audioClient = DependencyManager::get(); + audioClient->setMuted(!audioClient->isMuted()); } else if (action == controller::toInt(controller::Action::CYCLE_CAMERA)) { cycleCamera(); } else if (action == controller::toInt(controller::Action::CONTEXT_MENU)) { @@ -3461,7 +3461,8 @@ void Application::keyPressEvent(QKeyEvent* event) { case Qt::Key_M: if (isMeta) { - DependencyManager::get()->toggleMute(); + auto audioClient = DependencyManager::get(); + audioClient->setMuted(!audioClient->isMuted()); } break; @@ -5119,7 +5120,7 @@ void Application::update(float deltaTime) { if (menu->isOptionChecked(MenuOption::AutoMuteAudio) && !audioClient->isMuted()) { if (_lastFaceTrackerUpdate > 0 && ((usecTimestampNow() - _lastFaceTrackerUpdate) > MUTE_MICROPHONE_AFTER_USECS)) { - audioClient->toggleMute(); + audioClient->setMuted(true); _lastFaceTrackerUpdate = 0; } } else { diff --git a/interface/src/scripting/Audio.cpp b/interface/src/scripting/Audio.cpp index be9b4280f7..387900b2ae 100644 --- a/interface/src/scripting/Audio.cpp +++ b/interface/src/scripting/Audio.cpp @@ -50,110 +50,162 @@ float Audio::loudnessToLevel(float loudness) { Audio::Audio() : _devices(_contextIsHMD) { auto client = DependencyManager::get().data(); - connect(client, &AudioClient::muteToggled, this, &Audio::onMutedChanged); - connect(client, &AudioClient::noiseReductionChanged, this, &Audio::onNoiseReductionChanged); + connect(client, &AudioClient::muteToggled, this, &Audio::setMuted); + connect(client, &AudioClient::noiseReductionChanged, this, &Audio::enableNoiseReduction); connect(client, &AudioClient::inputLoudnessChanged, this, &Audio::onInputLoudnessChanged); - connect(client, &AudioClient::inputVolumeChanged, this, &Audio::onInputVolumeChanged); + connect(client, &AudioClient::inputVolumeChanged, this, &Audio::setInputVolume); connect(this, &Audio::contextChanged, &_devices, &AudioDevices::onContextChanged); enableNoiseReduction(enableNoiseReductionSetting.get()); } bool Audio::startRecording(const QString& filepath) { auto client = DependencyManager::get().data(); - return client->startRecording(filepath); + return resultWithWriteLock([&] { + return client->startRecording(filepath); + }); } bool Audio::getRecording() { auto client = DependencyManager::get().data(); - return client->getRecording(); + return resultWithReadLock([&] { + return client->getRecording(); + }); } void Audio::stopRecording() { auto client = DependencyManager::get().data(); - client->stopRecording(); + withWriteLock([&] { + client->stopRecording(); + }); +} + +bool Audio::isMuted() const { + return resultWithReadLock([&] { + return _isMuted; + }); } void Audio::setMuted(bool isMuted) { - if (_isMuted != isMuted) { - auto client = DependencyManager::get().data(); - QMetaObject::invokeMethod(client, "toggleMute"); + bool changed = false; + withWriteLock([&] { + if (_isMuted != isMuted) { + _isMuted = isMuted; + auto client = DependencyManager::get().data(); + QMetaObject::invokeMethod(client, "setMuted", Q_ARG(bool, isMuted), Q_ARG(bool, false)); + changed = true; + } + }); + if (changed) { + emit mutedChanged(isMuted); } } -void Audio::onMutedChanged() { - bool isMuted = DependencyManager::get()->isMuted(); - if (_isMuted != isMuted) { - _isMuted = isMuted; - emit mutedChanged(_isMuted); - } +bool Audio::noiseReductionEnabled() const { + return resultWithReadLock([&] { + return _enableNoiseReduction; + }); } void Audio::enableNoiseReduction(bool enable) { - if (_enableNoiseReduction != enable) { - auto client = DependencyManager::get().data(); - QMetaObject::invokeMethod(client, "setNoiseReduction", Q_ARG(bool, enable)); - enableNoiseReductionSetting.set(enable); + bool changed = false; + withWriteLock([&] { + if (_enableNoiseReduction != enable) { + _enableNoiseReduction = enable; + auto client = DependencyManager::get().data(); + QMetaObject::invokeMethod(client, "setNoiseReduction", Q_ARG(bool, enable), Q_ARG(bool, false)); + enableNoiseReductionSetting.set(enable); + changed = true; + } + }); + if (changed) { + emit noiseReductionChanged(enable); } } -void Audio::onNoiseReductionChanged() { - bool noiseReductionEnabled = DependencyManager::get()->isNoiseReductionEnabled(); - if (_enableNoiseReduction != noiseReductionEnabled) { - _enableNoiseReduction = noiseReductionEnabled; - emit noiseReductionChanged(_enableNoiseReduction); - } +float Audio::getInputVolume() const { + return resultWithReadLock([&] { + return _inputVolume; + }); } void Audio::setInputVolume(float volume) { // getInputVolume will not reflect changes synchronously, so clamp beforehand volume = glm::clamp(volume, 0.0f, 1.0f); - if (_inputVolume != volume) { - auto client = DependencyManager::get().data(); - QMetaObject::invokeMethod(client, "setInputVolume", Q_ARG(float, volume)); + bool changed = false; + withWriteLock([&] { + if (_inputVolume != volume) { + _inputVolume = volume; + auto client = DependencyManager::get().data(); + QMetaObject::invokeMethod(client, "setInputVolume", Q_ARG(float, volume), Q_ARG(bool, false)); + changed = true; + } + }); + if (changed) { + emit inputVolumeChanged(volume); } } -void Audio::onInputVolumeChanged(float volume) { - if (_inputVolume != volume) { - _inputVolume = volume; - emit inputVolumeChanged(_inputVolume); - } +float Audio::getInputLevel() const { + return resultWithReadLock([&] { + return _inputLevel; + }); } void Audio::onInputLoudnessChanged(float loudness) { float level = loudnessToLevel(loudness); - - if (_inputLevel != level) { - _inputLevel = level; - emit inputLevelChanged(_inputLevel); + bool changed = false; + withWriteLock([&] { + if (_inputLevel != level) { + _inputLevel = level; + changed = true; + } + }); + if (changed) { + emit inputLevelChanged(level); } } QString Audio::getContext() const { - return _contextIsHMD ? Audio::HMD : Audio::DESKTOP; + return resultWithReadLock([&] { + return _contextIsHMD ? Audio::HMD : Audio::DESKTOP; + }); } void Audio::onContextChanged() { + bool changed = false; bool isHMD = qApp->isHMDMode(); - if (_contextIsHMD != isHMD) { - _contextIsHMD = isHMD; - emit contextChanged(getContext()); + withWriteLock([&] { + if (_contextIsHMD != isHMD) { + _contextIsHMD = isHMD; + changed = true; + } + }); + if (changed) { + emit contextChanged(isHMD ? Audio::HMD : Audio::DESKTOP); } } void Audio::setReverb(bool enable) { - DependencyManager::get()->setReverb(enable); + withWriteLock([&] { + DependencyManager::get()->setReverb(enable); + }); } void Audio::setReverbOptions(const AudioEffectOptions* options) { - DependencyManager::get()->setReverbOptions(options); + withWriteLock([&] { + DependencyManager::get()->setReverbOptions(options); + }); } void Audio::setInputDevice(const QAudioDeviceInfo& device, bool isHMD) { - _devices.chooseInputDevice(device, isHMD); + withWriteLock([&] { + _devices.chooseInputDevice(device, isHMD); + }); } void Audio::setOutputDevice(const QAudioDeviceInfo& device, bool isHMD) { - _devices.chooseOutputDevice(device, isHMD); + withWriteLock([&] { + _devices.chooseOutputDevice(device, isHMD); + }); } diff --git a/interface/src/scripting/Audio.h b/interface/src/scripting/Audio.h index 0f0043510c..b4e63b80c5 100644 --- a/interface/src/scripting/Audio.h +++ b/interface/src/scripting/Audio.h @@ -17,10 +17,11 @@ #include "AudioEffectOptions.h" #include "SettingHandle.h" #include "AudioFileWav.h" +#include namespace scripting { -class Audio : public AudioScriptingInterface { +class Audio : public AudioScriptingInterface, protected ReadWriteLockable { Q_OBJECT SINGLETON_DEPENDENCY @@ -40,16 +41,13 @@ public: virtual ~Audio() {} - bool isMuted() const { return _isMuted; } - bool noiseReductionEnabled() const { return _enableNoiseReduction; } - float getInputVolume() const { return _inputVolume; } - float getInputLevel() const { return _inputLevel; } + bool isMuted() const; + bool noiseReductionEnabled() const; + float getInputVolume() const; + float getInputLevel() const; QString getContext() const; - void setMuted(bool muted); - void enableNoiseReduction(bool enable); void showMicMeter(bool show); - void setInputVolume(float volume); Q_INVOKABLE void setInputDevice(const QAudioDeviceInfo& device, bool isHMD); Q_INVOKABLE void setOutputDevice(const QAudioDeviceInfo& device, bool isHMD); @@ -72,9 +70,9 @@ public slots: void onContextChanged(); private slots: - void onMutedChanged(); - void onNoiseReductionChanged(); - void onInputVolumeChanged(float volume); + void setMuted(bool muted); + void enableNoiseReduction(bool enable); + void setInputVolume(float volume); void onInputLoudnessChanged(float loudness); protected: diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index e2dae92a94..f643719a2e 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -757,7 +757,7 @@ void AudioClient::Gate::flush() { void AudioClient::handleNoisyMutePacket(QSharedPointer message) { if (!_muted) { - toggleMute(); + setMuted(true); // have the audio scripting interface emit a signal to say we were muted by the mixer emit mutedByMixer(); @@ -1384,15 +1384,21 @@ void AudioClient::sendMuteEnvironmentPacket() { } } -void AudioClient::toggleMute() { - _muted = !_muted; - emit muteToggled(); +void AudioClient::setMuted(bool muted, bool emitSignal) { + if (_muted != muted) { + _muted = muted; + if (emitSignal) { + emit muteToggled(_muted); + } + } } -void AudioClient::setNoiseReduction(bool enable) { +void AudioClient::setNoiseReduction(bool enable, bool emitSignal) { if (_isNoiseGateEnabled != enable) { _isNoiseGateEnabled = enable; - emit noiseReductionChanged(); + if (emitSignal) { + emit noiseReductionChanged(_isNoiseGateEnabled); + } } } @@ -2018,9 +2024,11 @@ void AudioClient::startThread() { moveToNewNamedThread(this, "Audio Thread", [this] { start(); }); } -void AudioClient::setInputVolume(float volume) { +void AudioClient::setInputVolume(float volume, bool emitSignal) { if (_audioInput && volume != (float)_audioInput->volume()) { _audioInput->setVolume(volume); - emit inputVolumeChanged(_audioInput->volume()); + if (emitSignal) { + emit inputVolumeChanged(_audioInput->volume()); + } } } diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index 3bfbdb49ce..9ee7bcfeba 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -189,13 +189,13 @@ public slots: void reset(); void audioMixerKilled(); - void toggleMute(); + void setMuted(bool muted, bool emitSignal = true); bool isMuted() { return _muted; } virtual bool setIsStereoInput(bool stereo) override; virtual bool isStereoInput() override { return _isStereoInput; } - void setNoiseReduction(bool isNoiseGateEnabled); + void setNoiseReduction(bool isNoiseGateEnabled, bool emitSignal = true); bool isNoiseReductionEnabled() const { return _isNoiseGateEnabled; } bool getLocalEcho() { return _shouldEchoLocally; } @@ -218,7 +218,7 @@ public slots: bool switchAudioDevice(QAudio::Mode mode, const QString& deviceName); float getInputVolume() const { return (_audioInput) ? (float)_audioInput->volume() : 0.0f; } - void setInputVolume(float volume); + void setInputVolume(float volume, bool emitSignal = true); void setReverb(bool reverb); void setReverbOptions(const AudioEffectOptions* options); @@ -229,8 +229,8 @@ public slots: signals: void inputVolumeChanged(float volume); - void muteToggled(); - void noiseReductionChanged(); + void muteToggled(bool muted); + void noiseReductionChanged(bool noiseReductionEnabled); void mutedByMixer(); void inputReceived(const QByteArray& inputSamples); void inputLoudnessChanged(float loudness); From ed0fd8c4d21f626dd650190e523dedecfaff6eda Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 20 Apr 2018 11:13:15 -0700 Subject: [PATCH 56/56] Merge fix --- domain-server/src/DomainServer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index d5f3c99b38..96de8f7f12 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -612,13 +612,13 @@ bool DomainServer::isPacketVerified(const udt::Packet& packet) { return nodeList->isPacketVerifiedWithSource(packet, sourceNode.data()); } else { HIFI_FDEBUG("Packet of type" << headerType - << "received from unmatched IP for UUID" << uuidStringWithoutCurlyBraces(sourceID)); + << "received from unmatched IP for UUID" << uuidStringWithoutCurlyBraces(sourceNode->getUUID())); return false; } } else { HIFI_FDEBUG("Packet of type" << headerType - << "received from unknown node with UUID" << uuidStringWithoutCurlyBraces(sourceID)); + << "received from unknown node with UUID" << uuidStringWithoutCurlyBraces(sourceNode->getUUID())); return false; }