From c6d782ee1c904330b82e3e1ce2b90baf49ead4de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Remko=20Tron=C3=A7on?= <git@el-tramo.be>
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<JID, std::string> photoHashes;
+				
+			private:
+				typedef std::map<JID, VCard::ref> 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<VCardAvatarManager> testling = createManager();
+			vcardStorage->photoHashes[user1.toBare()] = avatar1Hash;
+			
+			std::string result = testling->getAvatarHash(user1);
+			
+			CPPUNIT_ASSERT_EQUAL(std::string(), result);
+		} 
+
+
 		void testVCardUpdateTriggersUpdate() {
 			std::auto_ptr<VCardAvatarManager> 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<JID> 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