From acef1b868c879277b912584cdf7eeb4bbd43503f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 3 Nov 2014 15:05:40 -0800 Subject: [PATCH 1/3] Revert "Revert "don't allow a HifiSockAddr lookup to block application"" This reverts commit fabc3aea5709960744a72f187c86573bb5351302. --- libraries/networking/src/HifiSockAddr.cpp | 39 ++++++++++++++------ libraries/networking/src/HifiSockAddr.h | 7 +++- libraries/networking/src/LimitedNodeList.cpp | 6 +-- libraries/networking/src/LimitedNodeList.h | 1 + 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/libraries/networking/src/HifiSockAddr.cpp b/libraries/networking/src/HifiSockAddr.cpp index 97e9721356..3a200fd392 100644 --- a/libraries/networking/src/HifiSockAddr.cpp +++ b/libraries/networking/src/HifiSockAddr.cpp @@ -9,9 +9,9 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include -#include -#include +#include +#include +#include #include "HifiSockAddr.h" @@ -36,14 +36,15 @@ HifiSockAddr::HifiSockAddr(const HifiSockAddr& otherSockAddr) { _port = otherSockAddr._port; } -HifiSockAddr::HifiSockAddr(const QString& hostname, quint16 hostOrderPort) { - // lookup the IP by the hostname - QHostInfo hostInfo = QHostInfo::fromName(hostname); - foreach(const QHostAddress& address, hostInfo.addresses()) { - if (address.protocol() == QAbstractSocket::IPv4Protocol) { - _address = address; - _port = hostOrderPort; - } +HifiSockAddr::HifiSockAddr(const QString& hostname, quint16 hostOrderPort) : + _address(hostname), + _port(hostOrderPort) +{ + // if we parsed an IPv4 address out of the hostname, don't look it up + if (_address.protocol() != QAbstractSocket::IPv4Protocol) { + // sync lookup the IP by the hostname + int lookupID = QHostInfo::lookupHost(hostname, this, SLOT(handleLookupResult(QHostInfo))); + qDebug() << "Looking up IP address for hostname" << hostname << "- lookup ID is" << lookupID; } } @@ -75,6 +76,22 @@ bool HifiSockAddr::operator==(const HifiSockAddr& rhsSockAddr) const { return _address == rhsSockAddr._address && _port == rhsSockAddr._port; } +void HifiSockAddr::handleLookupResult(const QHostInfo& hostInfo) { + if (hostInfo.error() != QHostInfo::NoError) { + qDebug() << "Lookup failed for" << hostInfo.lookupId() << ":" << hostInfo.errorString(); + } + + foreach(const QHostAddress& address, hostInfo.addresses()) { + // just take the first IPv4 address + if (address.protocol() == QAbstractSocket::IPv4Protocol) { + _address = address; + qDebug() << "QHostInfo lookup result for" + << hostInfo.hostName() << "with lookup ID" << hostInfo.lookupId() << "is" << address.toString(); + break; + } + } +} + QDebug operator<<(QDebug debug, const HifiSockAddr& sockAddr) { debug.nospace() << sockAddr._address.toString().toLocal8Bit().constData() << ":" << sockAddr._port; return debug.space(); diff --git a/libraries/networking/src/HifiSockAddr.h b/libraries/networking/src/HifiSockAddr.h index 42f815390a..5bbd27437b 100644 --- a/libraries/networking/src/HifiSockAddr.h +++ b/libraries/networking/src/HifiSockAddr.h @@ -19,9 +19,10 @@ #include #endif -#include +#include -class HifiSockAddr { +class HifiSockAddr : public QObject { + Q_OBJECT public: HifiSockAddr(); HifiSockAddr(const QHostAddress& address, quint16 port); @@ -51,6 +52,8 @@ public: friend QDebug operator<<(QDebug debug, const HifiSockAddr& sockAddr); friend QDataStream& operator<<(QDataStream& dataStream, const HifiSockAddr& sockAddr); friend QDataStream& operator>>(QDataStream& dataStream, HifiSockAddr& sockAddr); +private slots: + void handleLookupResult(const QHostInfo& hostInfo); private: QHostAddress _address; quint16 _port; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 5c4dc6cea2..043f0621bb 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -73,6 +73,7 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short _dtlsSocket(NULL), _localSockAddr(), _publicSockAddr(), + _stunSockAddr(STUN_SERVER_HOSTNAME, STUN_SERVER_PORT), _numCollectedPackets(0), _numCollectedBytes(0), _packetStatTimer() @@ -583,11 +584,8 @@ void LimitedNodeList::sendSTUNRequest() { QUuid randomUUID = QUuid::createUuid(); memcpy(stunRequestPacket + packetIndex, randomUUID.toRfc4122().data(), NUM_TRANSACTION_ID_BYTES); - // lookup the IP for the STUN server - HifiSockAddr stunSockAddr(STUN_SERVER_HOSTNAME, STUN_SERVER_PORT); - _nodeSocket.writeDatagram((char*) stunRequestPacket, sizeof(stunRequestPacket), - stunSockAddr.getAddress(), stunSockAddr.getPort()); + _stunSockAddr.getAddress(), _stunSockAddr.getPort()); } void LimitedNodeList::rebindNodeSocket() { diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 73381d01a5..64495fbd34 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -163,6 +163,7 @@ protected: QUdpSocket* _dtlsSocket; HifiSockAddr _localSockAddr; HifiSockAddr _publicSockAddr; + HifiSockAddr _stunSockAddr; int _numCollectedPackets; int _numCollectedBytes; QElapsedTimer _packetStatTimer; From 74753e5b8bc66c168cb8b432e7d1602dd5766931 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 3 Nov 2014 15:13:41 -0800 Subject: [PATCH 2/3] allow a blocking lookup of IP address in HifiSockAddr --- assignment-client/src/AssignmentClient.cpp | 7 ++----- libraries/networking/src/HifiSockAddr.cpp | 13 +++++++++---- libraries/networking/src/HifiSockAddr.h | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index e7ac7577b9..929d6c76c8 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -96,17 +96,14 @@ AssignmentClient::AssignmentClient(int &argc, char **argv) : assignmentServerPort = argumentVariantMap.value(CUSTOM_ASSIGNMENT_SERVER_PORT_OPTION).toString().toUInt(); } - - HifiSockAddr assignmentServerSocket(DEFAULT_ASSIGNMENT_SERVER_HOSTNAME, assignmentServerPort); // check for an overriden assignment server hostname if (argumentVariantMap.contains(CUSTOM_ASSIGNMENT_SERVER_HOSTNAME_OPTION)) { - _assignmentServerHostname = argumentVariantMap.value(CUSTOM_ASSIGNMENT_SERVER_HOSTNAME_OPTION).toString(); - // change the hostname for our assignment server - assignmentServerSocket = HifiSockAddr(_assignmentServerHostname, assignmentServerSocket.getPort()); + _assignmentServerHostname = argumentVariantMap.value(CUSTOM_ASSIGNMENT_SERVER_HOSTNAME_OPTION).toString(); } + HifiSockAddr assignmentServerSocket(_assignmentServerHostname, assignmentServerPort, true); nodeList->setAssignmentServerSocket(assignmentServerSocket); qDebug() << "Assignment server socket is" << assignmentServerSocket; diff --git a/libraries/networking/src/HifiSockAddr.cpp b/libraries/networking/src/HifiSockAddr.cpp index 3a200fd392..5e7ad3a2da 100644 --- a/libraries/networking/src/HifiSockAddr.cpp +++ b/libraries/networking/src/HifiSockAddr.cpp @@ -36,15 +36,20 @@ HifiSockAddr::HifiSockAddr(const HifiSockAddr& otherSockAddr) { _port = otherSockAddr._port; } -HifiSockAddr::HifiSockAddr(const QString& hostname, quint16 hostOrderPort) : +HifiSockAddr::HifiSockAddr(const QString& hostname, quint16 hostOrderPort, bool shouldBlockForLookup) : _address(hostname), _port(hostOrderPort) { // if we parsed an IPv4 address out of the hostname, don't look it up if (_address.protocol() != QAbstractSocket::IPv4Protocol) { - // sync lookup the IP by the hostname - int lookupID = QHostInfo::lookupHost(hostname, this, SLOT(handleLookupResult(QHostInfo))); - qDebug() << "Looking up IP address for hostname" << hostname << "- lookup ID is" << lookupID; + // lookup the IP by the hostname + if (shouldBlockForLookup) { + QHostInfo result = QHostInfo::fromName(hostname); + handleLookupResult(result); + } else { + int lookupID = QHostInfo::lookupHost(hostname, this, SLOT(handleLookupResult(QHostInfo))); + qDebug() << "Looking up IP address for hostname" << hostname << "- lookup ID is" << lookupID; + } } } diff --git a/libraries/networking/src/HifiSockAddr.h b/libraries/networking/src/HifiSockAddr.h index 5bbd27437b..064f8032ca 100644 --- a/libraries/networking/src/HifiSockAddr.h +++ b/libraries/networking/src/HifiSockAddr.h @@ -27,7 +27,7 @@ public: HifiSockAddr(); HifiSockAddr(const QHostAddress& address, quint16 port); HifiSockAddr(const HifiSockAddr& otherSockAddr); - HifiSockAddr(const QString& hostname, quint16 hostOrderPort); + HifiSockAddr(const QString& hostname, quint16 hostOrderPort, bool shouldBlockForLookup = false); HifiSockAddr(const sockaddr* sockaddr); bool isNull() const { return _address.isNull() && _port == 0; } From 88300f06ad6b775ef21f1e54415ae8c8a51caa30 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 3 Nov 2014 15:14:39 -0800 Subject: [PATCH 3/3] cleanup some debug in HifiSockAddr for hostname lookup --- libraries/networking/src/HifiSockAddr.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/HifiSockAddr.cpp b/libraries/networking/src/HifiSockAddr.cpp index 5e7ad3a2da..11674a948e 100644 --- a/libraries/networking/src/HifiSockAddr.cpp +++ b/libraries/networking/src/HifiSockAddr.cpp @@ -44,11 +44,12 @@ HifiSockAddr::HifiSockAddr(const QString& hostname, quint16 hostOrderPort, bool if (_address.protocol() != QAbstractSocket::IPv4Protocol) { // lookup the IP by the hostname if (shouldBlockForLookup) { + qDebug() << "Asynchronously looking up IP address for hostname" << hostname; QHostInfo result = QHostInfo::fromName(hostname); handleLookupResult(result); } else { int lookupID = QHostInfo::lookupHost(hostname, this, SLOT(handleLookupResult(QHostInfo))); - qDebug() << "Looking up IP address for hostname" << hostname << "- lookup ID is" << lookupID; + qDebug() << "Synchronously looking up IP address for hostname" << hostname << "- lookup ID is" << lookupID; } } }