From c6d782ee1c904330b82e3e1ce2b90baf49ead4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Remko=20Tron=C3=A7on?= Date: Sun, 28 Aug 2011 16:28:49 +0200 Subject: Fix assertion on inconsistent VCard photohash cache. Release-Notes: Don't crash when the VCard cache is inconsistent. diff --git a/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp b/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp index e91e402..7db9c95 100644 --- a/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp +++ b/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp @@ -13,8 +13,8 @@ #include "Swiften/Elements/VCard.h" #include "Swiften/Avatars/VCardAvatarManager.h" #include "Swiften/Avatars/AvatarMemoryStorage.h" -#include "Swiften/VCards/VCardMemoryStorage.h" #include "Swiften/VCards/VCardManager.h" +#include "Swiften/VCards/VCardStorage.h" #include "Swiften/MUC/MUCRegistry.h" #include "Swiften/Queries/IQRouter.h" #include "Swiften/Client/DummyStanzaChannel.h" @@ -30,9 +30,46 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture { CPPUNIT_TEST(testGetAvatarHashUnknownAvatarKnownVCardStoresAvatar); CPPUNIT_TEST(testGetAvatarHashUnknownAvatarUnknownVCard); CPPUNIT_TEST(testVCardUpdateTriggersUpdate); + CPPUNIT_TEST(testGetAvatarHashKnownAvatarUnknownVCard); CPPUNIT_TEST_SUITE_END(); public: + class TestVCardStorage : public VCardStorage { + public: + virtual VCard::ref getVCard(const JID& jid) const { + VCardMap::const_iterator i = vcards.find(jid); + if (i != vcards.end()) { + return i->second; + } + else { + return VCard::ref(); + } + } + + virtual void setVCard(const JID& jid, VCard::ref v) { + vcards[jid] = v; + } + + std::string getPhotoHash(const JID& jid) const { + if (photoHashes.find(jid) != photoHashes.end()) { + return photoHashes.find(jid)->second; + } + VCard::ref vCard = getVCard(jid); + if (vCard && !vCard->getPhoto().isEmpty()) { + return Hexify::hexify(SHA1::getHash(vCard->getPhoto())); + } + else { + return ""; + } + } + + std::map photoHashes; + + private: + typedef std::map VCardMap; + VCardMap vcards; + }; + void setUp() { ownJID = JID("foo@fum.com/bum"); stanzaChannel = new DummyStanzaChannel(); @@ -40,7 +77,7 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture { iqRouter = new IQRouter(stanzaChannel); mucRegistry = new DummyMUCRegistry(); avatarStorage = new AvatarMemoryStorage(); - vcardStorage = new VCardMemoryStorage(); + vcardStorage = new TestVCardStorage(); vcardManager = new VCardManager(ownJID, iqRouter, vcardStorage); avatar1 = ByteArray("abcdefg"); avatar1Hash = Hexify::hexify(SHA1::getHash(avatar1)); @@ -95,6 +132,16 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(std::string(), result); } + void testGetAvatarHashKnownAvatarUnknownVCard() { + std::auto_ptr testling = createManager(); + vcardStorage->photoHashes[user1.toBare()] = avatar1Hash; + + std::string result = testling->getAvatarHash(user1); + + CPPUNIT_ASSERT_EQUAL(std::string(), result); + } + + void testVCardUpdateTriggersUpdate() { std::auto_ptr testling = createManager(); vcardManager->requestVCard(user1); @@ -143,7 +190,7 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture { DummyMUCRegistry* mucRegistry; AvatarMemoryStorage* avatarStorage; VCardManager* vcardManager; - VCardMemoryStorage* vcardStorage; + TestVCardStorage* vcardStorage; ByteArray avatar1; std::string avatar1Hash; std::vector changes; diff --git a/Swiften/Avatars/VCardAvatarManager.cpp b/Swiften/Avatars/VCardAvatarManager.cpp index 046e6f3..a7e6fea 100644 --- a/Swiften/Avatars/VCardAvatarManager.cpp +++ b/Swiften/Avatars/VCardAvatarManager.cpp @@ -34,8 +34,13 @@ std::string VCardAvatarManager::getAvatarHash(const JID& jid) const { if (!hash.empty()) { if (!avatarStorage_->hasAvatar(hash)) { VCard::ref vCard = vcardManager_->getVCard(avatarJID); - assert(vCard); - avatarStorage_->addAvatar(hash, vCard->getPhoto()); + if (vCard) { + avatarStorage_->addAvatar(hash, vCard->getPhoto()); + } + else { + // Can happen if the cache is inconsistent. + hash = ""; + } } } return hash; -- cgit v0.10.2-6-g49f6