10 files changed, 163 insertions, 28 deletions
diff --git a/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp b/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp
index 241f375..5a35410 100644
--- a/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp
+++ b/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2016 Isode Limited.
+ * Copyright (c) 2014-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -91,6 +91,7 @@ class AvatarManagerImplTest : public CppUnit::TestFixture {
             /* send new presence to notify of blank avatar */
 
             vcardUpdate = std::make_shared<VCardUpdate>();
+            vcardUpdate->setPhotoHash("da39a3ee5e6b4b0d3255bfef95601890afd80709");
             presence = std::make_shared<Presence>();
             presence->setTo(ownerJID);
             presence->setFrom(personJID);
diff --git a/Swiften/Avatars/VCardUpdateAvatarManager.cpp b/Swiften/Avatars/VCardUpdateAvatarManager.cpp
index 3e8d87b..349af2f 100644
--- a/Swiften/Avatars/VCardUpdateAvatarManager.cpp
+++ b/Swiften/Avatars/VCardUpdateAvatarManager.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -32,6 +32,10 @@ void VCardUpdateAvatarManager::handlePresenceReceived(std::shared_ptr<Presence>
         return;
     }
     JID from = getAvatarJID(presence->getFrom());
+    if (update->getPhotoHash().size() != 40) {
+        SWIFT_LOG(debug) << "Invalid vCard avatar photo hash length. Must be hex-encoded SHA-1, i.e. 40 characters." << std::endl;
+        return;
+    }
     if (getAvatarHash(from) == update->getPhotoHash()) {
         return;
     }
diff --git a/Swiften/Base/Platform.h b/Swiften/Base/Platform.h
index 4deba2b..22dff30 100644
--- a/Swiften/Base/Platform.h
+++ b/Swiften/Base/Platform.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2019 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -43,9 +43,9 @@
 #endif
 
 // Endianness
-#include <boost/detail/endian.hpp>
-#if defined(BOOST_LITTLE_ENDIAN)
+#include <boost/predef/other/endian.h>
+#if defined(BOOST_ENDIAN_LITTLE_BYTE)
 #define SWIFTEN_LITTLE_ENDIAN
-#elif defined(BOOST_BIG_ENDIAN)
+#elif defined(BOOST_ENDIAN_BIG_BYTE)
 #define SWIFTEN_BIG_ENDIAN
 #endif
diff --git a/Swiften/ChangeLog.md b/Swiften/ChangeLog.md
index d823954..23d5185 100644
--- a/Swiften/ChangeLog.md
+++ b/Swiften/ChangeLog.md
@@ -1,3 +1,24 @@
+4.0.1 (2018-03-28)
+------------------
+- Fix handling errors when fetching own vCard
+
+4.0 (2018-03-20)
+----------------
+- Moved code-base to C++11
+    - Use C++11 threading instead of Boost.Thread library
+    - Use C++11 smart pointers instead of Boost's
+- Migrated from Boost.Signals to Boost.Signals2
+- Build without warnings on our CI platforms
+- General cleanup like remove of superflous files and #include statements. This means header files that previously were included implictly need to be explicitly included now
+- Support IPv6 addresses in URLs
+- Handle sessions being closed by the server
+- Verify certificates when using HTTPS in BOSH connections
+- In memory caching of latest entity capabilites lookups
+- Changed source code style to use soft tabs (4 spaces wide) instead of hard tabs. Custom patches for Swiften will need to be reformatted accordingly
+- Require a TLS backend for building
+- Update 3rdParty/lcov to version 1.12
+- Fix several possible race conditions, smaller leaks, and other small bugs
+
 4.0-rc1 ( 2017-05-17 )
 ----------------------
 - Handle sessions being closed by the server
diff --git a/Swiften/MUC/MUCBookmarkManager.cpp b/Swiften/MUC/MUCBookmarkManager.cpp
index 9f8ae77..511c88a 100644
--- a/Swiften/MUC/MUCBookmarkManager.cpp
+++ b/Swiften/MUC/MUCBookmarkManager.cpp
@@ -25,7 +25,7 @@ MUCBookmarkManager::MUCBookmarkManager(IQRouter* iqRouter) {
 }
 
 void MUCBookmarkManager::handleBookmarksReceived(std::shared_ptr<Storage> payload, ErrorPayload::ref error) {
-    if (error) {
+    if (error || !payload) {
         return;
     }
 
diff --git a/Swiften/Network/Connector.cpp b/Swiften/Network/Connector.cpp
index 457d8a9..5eddaba 100644
--- a/Swiften/Network/Connector.cpp
+++ b/Swiften/Network/Connector.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -148,6 +148,13 @@ void Connector::handleConnectionConnectFinished(bool error) {
             timer->stop();
             timer.reset();
     }
+    if (!currentConnection) {
+        // We've hit a race condition where multiple finisheds were on the eventloop queue at once.
+        // This is particularly likely on macOS where the hourly momentary wakeup while asleep
+        // can cause both a timeout and an onConnectFinished to be queued sequentially (SWIFT-232).
+        // Let the first one process as normal, but ignore the second.
+        return;
+    }
     currentConnection->onConnectFinished.disconnect(boost::bind(&Connector::handleConnectionConnectFinished, shared_from_this(), _1));
     if (error) {
         currentConnection.reset();
@@ -189,6 +196,7 @@ void Connector::finish(std::shared_ptr<Connection> connection) {
 
 void Connector::handleTimeout() {
     SWIFT_LOG(debug) << "Timeout" << std::endl;
+    SWIFT_LOG_ASSERT(currentConnection, error) << "Connection not valid but triggered a timeout" <<std::endl;
     handleConnectionConnectFinished(true);
 }
 
diff --git a/Swiften/Network/PlatformNATTraversalWorker.cpp b/Swiften/Network/PlatformNATTraversalWorker.cpp
index f56de0b..eaa13b3 100644
--- a/Swiften/Network/PlatformNATTraversalWorker.cpp
+++ b/Swiften/Network/PlatformNATTraversalWorker.cpp
@@ -157,7 +157,7 @@ NATTraversalInterface* PlatformNATTraversalWorker::getNATTraversalInterface() co
         miniUPnPInterface = new MiniUPnPInterface();
         miniUPnPSupported = miniUPnPInterface->isAvailable();
     }
-    SWIFT_LOG(debug) << "UPnP NAT traversal supported: " << miniUPnPSupported << std::endl;
+    SWIFT_LOG(debug) << "UPnP NAT traversal supported: " << static_cast<bool>(miniUPnPSupported) << std::endl;
     if (miniUPnPSupported) {
         return miniUPnPInterface;
     }
@@ -168,7 +168,7 @@ NATTraversalInterface* PlatformNATTraversalWorker::getNATTraversalInterface() co
         natPMPInterface = new NATPMPInterface();
         natPMPSupported = natPMPInterface->isAvailable();
     }
-    SWIFT_LOG(debug) << "NAT-PMP NAT traversal supported: " << natPMPSupported << std::endl;
+    SWIFT_LOG(debug) << "NAT-PMP NAT traversal supported: " << static_cast<bool>(natPMPSupported) << std::endl;
     if (natPMPSupported) {
         return natPMPInterface;
     }
diff --git a/Swiften/SConscript b/Swiften/SConscript
index e0f87ce..f52637b 100644
--- a/Swiften/SConscript
+++ b/Swiften/SConscript
@@ -618,8 +618,9 @@ if env["SCONS_STAGE"] == "build" :
 
     # Install swiften
     if swiften_env.get("SWIFTEN_INSTALLDIR", "") :
-        swiften_env.Install(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], "lib"), swiften_lib)
+        swiften_libdir = swiften_env.get("SWIFTEN_LIBDIR", "lib")
+        swiften_env.Install(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], swiften_libdir), swiften_lib)
         for alias in myenv["SWIFTEN_LIBRARY_ALIASES"] :
-            myenv.Command(myenv.File(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], "lib", alias)), [env.Value(swiften_lib[0].name), swiften_lib[0]], symlink)
+            myenv.Command(myenv.File(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], swiften_libdir, alias)), [env.Value(swiften_lib[0].name), swiften_lib[0]], symlink)
         for include in swiften_includes :
             swiften_env.Install(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], "include", os.path.dirname(include)), "#/" + include)
diff --git a/Swiften/VCards/UnitTest/VCardManagerTest.cpp b/Swiften/VCards/UnitTest/VCardManagerTest.cpp
index 3d5338d..669c3ff 100644
--- a/Swiften/VCards/UnitTest/VCardManagerTest.cpp
+++ b/Swiften/VCards/UnitTest/VCardManagerTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -31,7 +31,17 @@ class VCardManagerTest : public CppUnit::TestFixture {
         CPPUNIT_TEST(testRequest_Error);
         CPPUNIT_TEST(testRequest_VCardAlreadyRequested);
         CPPUNIT_TEST(testRequest_AfterPreviousRequest);
-        CPPUNIT_TEST(testRequestOwnVCard);
+
+        CPPUNIT_TEST(testRequestVCard_ReturnFullVCard);
+        CPPUNIT_TEST(testRequestVCard_ReturnEmptyVCard);
+        CPPUNIT_TEST(testRequestVCard_ReturnItemNotFoundError);
+        CPPUNIT_TEST(testRequestVCard_ReturnFeatureNotImplementedError);
+
+        CPPUNIT_TEST(testRequestOwnVCard_ReturnFullVCard);
+        CPPUNIT_TEST(testRequestOwnVCard_ReturnEmptyVCard);
+        CPPUNIT_TEST(testRequestOwnVCard_ReturnItemNotFoundError);
+        CPPUNIT_TEST(testRequestOwnVCard_ReturnFeatureNotImplementedError);
+
         CPPUNIT_TEST(testCreateSetVCardRequest);
         CPPUNIT_TEST(testCreateSetVCardRequest_Error);
         CPPUNIT_TEST_SUITE_END();
@@ -54,7 +64,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testGet_NewVCardRequestsVCard() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             VCard::ref result = testling->getVCardAndRequestWhenNeeded(JID("foo@bar.com/baz"));
 
             CPPUNIT_ASSERT(!result);
@@ -63,7 +73,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testGet_ExistingVCard() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             VCard::ref vcard(new VCard());
             vcard->setFullName("Foo Bar");
             vcardStorage->setVCard(JID("foo@bar.com/baz"), vcard);
@@ -75,7 +85,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testRequest_RequestsVCard() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             testling->requestVCard(JID("foo@bar.com/baz"));
 
             CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
@@ -83,7 +93,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testRequest_ReceiveEmitsNotification() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             testling->requestVCard(JID("foo@bar.com/baz"));
             stanzaChannel->onIQReceived(createVCardResult());
 
@@ -96,7 +106,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testRequest_Error() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             testling->requestVCard(JID("foo@bar.com/baz"));
             stanzaChannel->onIQReceived(IQ::createError(JID("baz@fum.com/foo"), stanzaChannel->sentStanzas[0]->getTo(), stanzaChannel->sentStanzas[0]->getID()));
 
@@ -105,7 +115,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testRequest_VCardAlreadyRequested() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             testling->requestVCard(JID("foo@bar.com/baz"));
             VCard::ref result = testling->getVCardAndRequestWhenNeeded(JID("foo@bar.com/baz"));
 
@@ -114,7 +124,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testRequest_AfterPreviousRequest() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             testling->requestVCard(JID("foo@bar.com/baz"));
             stanzaChannel->onIQReceived(createVCardResult());
             testling->requestVCard(JID("foo@bar.com/baz"));
@@ -123,8 +133,60 @@ class VCardManagerTest : public CppUnit::TestFixture {
             CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(1, JID("foo@bar.com/baz"), IQ::Get));
         }
 
-        void testRequestOwnVCard() {
-            std::shared_ptr<VCardManager> testling = createManager();
+        void testRequestVCard_ReturnFullVCard() {
+            auto testling = createManager();
+            testling->requestVCard(JID("foo@bar.com/baz"));
+            stanzaChannel->onIQReceived(createVCardResult());
+
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
+            CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(0, JID("foo@bar.com/baz"), IQ::Get));
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size()));
+            CPPUNIT_ASSERT_EQUAL(JID("foo@bar.com/baz"), changes[0].first);
+            CPPUNIT_ASSERT_EQUAL(std::string("Foo Bar"), changes[0].second->getFullName());
+            CPPUNIT_ASSERT_EQUAL(false, changes[0].second->isEmpty());
+        }
+
+        void testRequestVCard_ReturnEmptyVCard() {
+            auto testling = createManager();
+            testling->requestVCard(JID("foo@bar.com/baz"));
+                  stanzaChannel->onIQReceived([&](){
+                    auto vcard = std::make_shared<VCard>();
+                    return IQ::createResult(JID("foo@bar.com/baz"), stanzaChannel->sentStanzas[0]->getTo(), stanzaChannel->sentStanzas[0]->getID(), vcard);
+            }());
+
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
+            CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(0, JID("foo@bar.com/baz"), IQ::Get));
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size()));
+            CPPUNIT_ASSERT_EQUAL(true, changes[0].second->isEmpty());
+        }
+
+        void testRequestVCard_ReturnItemNotFoundError() {
+            auto testling = createManager();
+            testling->requestVCard(JID("foo@bar.com/baz"));
+                  stanzaChannel->onIQReceived([&](){
+                    return IQ::createError(JID("foo@bar.com/baz"), stanzaChannel->sentStanzas[0]->getTo(), stanzaChannel->sentStanzas[0]->getID(), ErrorPayload::ItemNotFound);
+            }());
+
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
+            CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(0, JID("foo@bar.com/baz"), IQ::Get));
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size()));
+            CPPUNIT_ASSERT_EQUAL(true, changes[0].second->isEmpty());
+        }
+
+        void testRequestVCard_ReturnFeatureNotImplementedError() {
+            auto testling = createManager();
+            testling->requestVCard(JID("foo@bar.com/baz"));
+                  stanzaChannel->onIQReceived([&](){
+                    return IQ::createError(JID("foo@bar.com/baz"), stanzaChannel->sentStanzas[0]->getTo(), stanzaChannel->sentStanzas[0]->getID(), ErrorPayload::FeatureNotImplemented);
+            }());
+
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
+            CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(0, JID("foo@bar.com/baz"), IQ::Get));
+            CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(changes.size()));
+        }
+
+        void testRequestOwnVCard_ReturnFullVCard() {
+            auto testling = createManager();
             testling->requestVCard(ownJID);
             stanzaChannel->onIQReceived(createOwnVCardResult());
 
@@ -139,8 +201,47 @@ class VCardManagerTest : public CppUnit::TestFixture {
             CPPUNIT_ASSERT_EQUAL(std::string("Myself"), ownChanges[0]->getFullName());
         }
 
+        void testRequestOwnVCard_ReturnEmptyVCard() {
+            auto testling = createManager();
+            testling->requestVCard(ownJID);
+            stanzaChannel->onIQReceived([&](){
+                    auto vcard = std::make_shared<VCard>();
+                    return IQ::createResult(JID(), stanzaChannel->sentStanzas[0]->getTo(), stanzaChannel->sentStanzas[0]->getID(), vcard);
+            }());
+
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
+            CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(0, JID(), IQ::Get));
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size()));
+            CPPUNIT_ASSERT_EQUAL(true, changes[0].second->isEmpty());
+        }
+
+        void testRequestOwnVCard_ReturnItemNotFoundError() {
+            auto testling = createManager();
+            testling->requestVCard(ownJID);
+            stanzaChannel->onIQReceived([&](){
+                    return IQ::createError(JID(), stanzaChannel->sentStanzas[0]->getTo(), stanzaChannel->sentStanzas[0]->getID(), ErrorPayload::ItemNotFound);
+            }());
+
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
+            CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(0, JID(), IQ::Get));
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size()));
+            CPPUNIT_ASSERT_EQUAL(true, changes[0].second->isEmpty());
+        }
+
+        void testRequestOwnVCard_ReturnFeatureNotImplementedError() {
+            auto testling = createManager();
+            testling->requestVCard(ownJID);
+            stanzaChannel->onIQReceived([&](){
+                    return IQ::createError(JID(), stanzaChannel->sentStanzas[0]->getTo(), stanzaChannel->sentStanzas[0]->getID(), ErrorPayload::FeatureNotImplemented);
+            }());
+
+            CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(stanzaChannel->sentStanzas.size()));
+            CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<VCard>(0, JID(), IQ::Get));
+            CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(changes.size()));
+        }
+
         void testCreateSetVCardRequest() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             VCard::ref vcard = std::make_shared<VCard>();
             vcard->setFullName("New Name");
             SetVCardRequest::ref request = testling->createSetVCardRequest(vcard);
@@ -154,7 +255,7 @@ class VCardManagerTest : public CppUnit::TestFixture {
         }
 
         void testCreateSetVCardRequest_Error() {
-            std::shared_ptr<VCardManager> testling = createManager();
+            auto testling = createManager();
             VCard::ref vcard = std::make_shared<VCard>();
             vcard->setFullName("New Name");
             SetVCardRequest::ref request = testling->createSetVCardRequest(vcard);
diff --git a/Swiften/VCards/VCardManager.cpp b/Swiften/VCards/VCardManager.cpp
index 95b96fa..9423702 100644
--- a/Swiften/VCards/VCardManager.cpp
+++ b/Swiften/VCards/VCardManager.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -50,10 +50,9 @@ void VCardManager::requestOwnVCard() {
     requestVCard(JID());
 }
 
-
 void VCardManager::handleVCardReceived(const JID& actualJID, VCard::ref vcard, ErrorPayload::ref error) {
     requestedVCards.erase(actualJID);
-    if (!error) {
+    if (!error || (error && error->getCondition() == ErrorPayload::ItemNotFound)) {
         if (!vcard) {
             vcard = VCard::ref(new VCard());
         }