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 From eba9a50f10082430c38b849b6af95f92f3de6ef8 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:39:42 +0200 Subject: Keep VCard photo hash cache consistent. diff --git a/Swiften/VCards/VCardFileStorage.cpp b/Swiften/VCards/VCardFileStorage.cpp index cd5cc9c..476fde1 100644 --- a/Swiften/VCards/VCardFileStorage.cpp +++ b/Swiften/VCards/VCardFileStorage.cpp @@ -48,6 +48,7 @@ VCardFileStorage::VCardFileStorage(boost::filesystem::path dir) : vcardsPath(dir } boost::shared_ptr<VCard> VCardFileStorage::getVCard(const JID& jid) const { + boost::shared_ptr<VCard> result; try { boost::filesystem::path vcardPath(getVCardPath(jid)); if (boost::filesystem::exists(vcardPath)) { @@ -57,16 +58,14 @@ boost::shared_ptr<VCard> VCardFileStorage::getVCard(const JID& jid) const { VCardParser parser; PayloadParserTester tester(&parser); tester.parse(data.toString()); - return boost::dynamic_pointer_cast<VCard>(parser.getPayload()); - } - else { - return boost::shared_ptr<VCard>(); + result = boost::dynamic_pointer_cast<VCard>(parser.getPayload()); } } catch (const boost::filesystem::filesystem_error& e) { std::cerr << "ERROR: " << e.what() << std::endl; - return boost::shared_ptr<VCard>(); } + getAndUpdatePhotoHash(jid, result); + return result; } void VCardFileStorage::setVCard(const JID& jid, VCard::ref v) { -- cgit v0.10.2-6-g49f6