Only delete empty expeditions when the dz is empty

Zones are no longer able to delete expeditions. World now tracks empty
expeditions in cache and only deletes them when it detects an
expedition's dynamic zone instance has no more clients inside.

This fixes an exploit where lockouts couldn't be applied to expeditions
after all members were removed because zones were deleting the expedition
immediately. Clients still inside the dz were able to complete events
before being kicked from the instance while not having an expedition.

Expeditions are no longer purged from database in the world purge
instance timer to avoid a possible race with this new system

common/servertalk.h

@@ -159,7 +159,7 @@
 #define ServerOP_ExpeditionRequestInvite      0x040f
 #define ServerOP_ExpeditionReplayOnJoin       0x0410
 #define ServerOP_ExpeditionLockState          0x0411
-#define ServerOP_ExpeditionExpired            0x0412
+#define ServerOP_ExpeditionMembersRemoved     0x0412
 
 #define ServerOP_DzCharacterChange            0x0450
 #define ServerOP_DzRemoveAllCharacters        0x0451

world/expedition.cpp

@@ -45,10 +45,10 @@ Expedition::Expedition(
 	m_expire_time = std::chrono::system_clock::from_time_t(m_start_time + m_duration);
 }
 
-void Expedition::SendZonesExpeditionExpired()
+void Expedition::SendZonesExpeditionDeleted()
 {
 	uint32_t pack_size = sizeof(ServerExpeditionID_Struct);
-	auto pack = std::unique_ptr<ServerPacket>(new ServerPacket(ServerOP_ExpeditionExpired, pack_size));
+	auto pack = std::unique_ptr<ServerPacket>(new ServerPacket(ServerOP_ExpeditionDeleted, pack_size));
 	auto buf = reinterpret_cast<ServerExpeditionID_Struct*>(pack->pBuffer);
 	buf->expedition_id = GetID();
 	zoneserver_list.SendPacket(pack.get());
@@ -95,6 +95,34 @@ void ExpeditionCache::RemoveExpedition(uint32_t expedition_id)
 	), m_expeditions.end());
 }
 
+void ExpeditionCache::MemberChange(uint32_t expedition_id, uint32_t character_id, bool remove)
+{
+	auto it = std::find_if(m_expeditions.begin(), m_expeditions.end(), [&](const Expedition& expedition) {
+		return expedition.GetID() == expedition_id;
+	});
+
+	if (it != m_expeditions.end())
+	{
+		if (remove) {
+			it->RemoveMember(character_id);
+		} else {
+			it->AddMember(character_id);
+		}
+	}
+}
+
+void ExpeditionCache::RemoveAllMembers(uint32_t expedition_id)
+{
+	auto it = std::find_if(m_expeditions.begin(), m_expeditions.end(), [&](const Expedition& expedition) {
+		return expedition.GetID() == expedition_id;
+	});
+
+	if (it != m_expeditions.end())
+	{
+		it->RemoveAllMembers();
+	}
+}
+
 void ExpeditionCache::Process()
 {
 	if (!m_process_throttle_timer.Check())
@@ -104,25 +132,30 @@ void ExpeditionCache::Process()
 
 	std::vector<uint32_t> expedition_ids;
 
-	// check for expired expeditions (using the dz instance expiration time)
+	// check cache for expired or empty expeditions to delete and notify zones.
 	for (auto it = m_expeditions.begin(); it != m_expeditions.end();)
 	{
-		if (!it->IsExpired())
+		bool is_deleted = false;
+
+		if (it->IsEmpty() || it->IsExpired())
 		{
-			++it;
+			// don't delete expedition until its dz instance is empty. this prevents
-		}
+			// an exploit where all members leave expedition and complete an event
-		else
+			// before being kicked from removal timer. the lockout could never be
+			// applied because the zone expedition cache was already invalidated.
+			auto dz_zoneserver = zoneserver_list.FindByInstanceID(it->GetInstanceID());
+			if (!dz_zoneserver || dz_zoneserver->NumPlayers() == 0)
 			{
-			// we need to delete expired expeditions from the database now instead
+				LogExpeditions("Expedition [{}] expired or empty, notifying zones and deleting", it->GetID());
-			// of waiting for purge timer so members can request new expeditions.
-			// the dz should process this on its own to kick any clients inside it
-			LogExpeditions("Expedition [{}] expired, notifying zones and deleting", it->GetID());
 				expedition_ids.emplace_back(it->GetID());
-			it->SendZonesExpeditionExpired();
+				it->SendZonesExpeditionDeleted();
-			it = m_expeditions.erase(it);
+				is_deleted = true;
 			}
 		}
 
+		it = is_deleted ? m_expeditions.erase(it) : it + 1;
+	}
+
 	if (!expedition_ids.empty())
 	{
 		ExpeditionDatabase::DeleteExpeditions(expedition_ids);
@@ -177,9 +210,13 @@ std::vector<Expedition> ExpeditionDatabase::LoadExpeditions()
 			expedition_details.instance_id,
 			instance_list.zone,
 			instance_list.start_time,
-			instance_list.duration
+			instance_list.duration,
+			expedition_members.character_id
 		FROM expedition_details
 			INNER JOIN instance_list ON expedition_details.instance_id = instance_list.id
+			INNER JOIN expedition_members
+				ON expedition_members.expedition_id = expedition_details.id
+				AND expedition_members.is_current_member = TRUE
 		ORDER BY expedition_details.id;
 	);
 
@@ -190,7 +227,13 @@ std::vector<Expedition> ExpeditionDatabase::LoadExpeditions()
 	}
 	else
 	{
+		uint32_t last_expedition_id = 0;
+
 		for (auto row = results.begin(); row != results.end(); ++row)
+		{
+			uint32_t expedition_id = strtoul(row[0], nullptr, 10);
+
+			if (last_expedition_id != expedition_id)
 			{
 				expeditions.emplace_back(Expedition{
 					static_cast<uint32_t>(strtoul(row[0], nullptr, 10)), // expedition_id
@@ -200,6 +243,12 @@ std::vector<Expedition> ExpeditionDatabase::LoadExpeditions()
 					static_cast<uint32_t>(strtoul(row[4], nullptr, 10))  // duration
 				});
 			}
+
+			last_expedition_id = expedition_id;
+
+			uint32_t member_id = static_cast<uint32_t>(strtoul(row[5], nullptr, 10));
+			expeditions.back().AddMember(member_id);
+		}
 	}
 
 	return expeditions;
@@ -207,15 +256,21 @@ std::vector<Expedition> ExpeditionDatabase::LoadExpeditions()
 
 Expedition ExpeditionDatabase::LoadExpedition(uint32_t expedition_id)
 {
+	Expedition expedition;
+
 	std::string query = fmt::format(SQL(
 		SELECT
 			expedition_details.id,
 			expedition_details.instance_id,
 			instance_list.zone,
 			instance_list.start_time,
-			instance_list.duration
+			instance_list.duration,
+			expedition_members.character_id
 		FROM expedition_details
 			INNER JOIN instance_list ON expedition_details.instance_id = instance_list.id
+			INNER JOIN expedition_members
+				ON expedition_members.expedition_id = expedition_details.id
+				AND expedition_members.is_current_member = TRUE
 		WHERE expedition_details.id = {};
 	), expedition_id);
 
@@ -224,19 +279,29 @@ Expedition ExpeditionDatabase::LoadExpedition(uint32_t expedition_id)
 	{
 		LogExpeditions("Failed to load expedition [{}] for world cache", expedition_id);
 	}
-	else if (results.RowCount() > 0)
+	else
 	{
-		auto row = results.begin();
+		bool created = false;
-		return Expedition{
+		for (auto row = results.begin(); row != results.end(); ++row)
+		{
+			if (!created)
+			{
+				expedition = Expedition{
 					static_cast<uint32_t>(strtoul(row[0], nullptr, 10)), // expedition_id
 					static_cast<uint32_t>(strtoul(row[1], nullptr, 10)), // dz_instance_id
 					static_cast<uint32_t>(strtoul(row[2], nullptr, 10)), // dz_zone_id
 					static_cast<uint32_t>(strtoul(row[3], nullptr, 10)), // start_time
 					static_cast<uint32_t>(strtoul(row[4], nullptr, 10))  // duration
 				};
+				created = true;
 			}
 
-	return Expedition{};
+			auto member_id = static_cast<uint32_t>(strtoul(row[5], nullptr, 10));
+			expedition.AddMember(member_id);
+		}
+	}
+
+	return expedition;
 }
 
 void ExpeditionDatabase::DeleteExpeditions(const std::vector<uint32_t>& expedition_ids)
@@ -280,10 +345,25 @@ void ExpeditionMessage::HandleZoneMessage(ServerPacket* pack)
 		zoneserver_list.SendPacket(pack);
 		break;
 	}
-	case ServerOP_ExpeditionDeleted:
+	case ServerOP_ExpeditionMemberChange:
+	{
+		auto buf = reinterpret_cast<ServerExpeditionMemberChange_Struct*>(pack->pBuffer);
+		expedition_cache.MemberChange(buf->expedition_id, buf->char_id, buf->removed);
+		zoneserver_list.SendPacket(pack);
+		break;
+	}
+	case ServerOP_ExpeditionMemberSwap:
+	{
+		auto buf = reinterpret_cast<ServerExpeditionMemberSwap_Struct*>(pack->pBuffer);
+		expedition_cache.MemberChange(buf->expedition_id, buf->remove_char_id, true);
+		expedition_cache.MemberChange(buf->expedition_id, buf->add_char_id, false);
+		zoneserver_list.SendPacket(pack);
+		break;
+	}
+	case ServerOP_ExpeditionMembersRemoved:
 	{
 		auto buf = reinterpret_cast<ServerExpeditionID_Struct*>(pack->pBuffer);
-		expedition_cache.RemoveExpedition(buf->expedition_id);
+		expedition_cache.RemoveAllMembers(buf->expedition_id);
 		zoneserver_list.SendPacket(pack);
 		break;
 	}

world/expedition.h

@@ -24,6 +24,7 @@
 #include "../common/rulesys.h"
 #include "../common/timer.h"
 #include <chrono>
+#include <unordered_set>
 #include <vector>
 
 extern class ExpeditionCache expedition_cache;
@@ -56,6 +57,8 @@ public:
 	void AddExpedition(uint32_t expedition_id);
 	void RemoveExpedition(uint32_t expedition_id);
 	void LoadActiveExpeditions();
+	void MemberChange(uint32_t expedition_id, uint32_t character_id, bool remove);
+	void RemoveAllMembers(uint32_t expedition_id);
 	void Process();
 
 private:
@@ -71,9 +74,15 @@ public:
 		uint32_t expedition_id, uint32_t instance_id, uint32_t dz_zone_id,
 		uint32_t expire_time, uint32_t duration);
 
+	void AddMember(uint32_t character_id) { m_member_ids.emplace(character_id); }
+	void RemoveMember(uint32_t character_id) { m_member_ids.erase(character_id); }
+	void RemoveAllMembers() { m_member_ids.clear(); }
 	uint32_t GetID() const { return m_expedition_id; }
+	uint16_t GetInstanceID() const { return static_cast<uint16_t>(m_dz_instance_id); }
+	uint16_t GetZoneID() const { return static_cast<uint16_t>(m_dz_zone_id); }
+	bool IsEmpty() const { return m_member_ids.empty(); }
 	bool IsExpired() const { return m_expire_time < std::chrono::system_clock::now(); }
-	void SendZonesExpeditionExpired();
+	void SendZonesExpeditionDeleted();
 
 private:
 	uint32_t m_expedition_id  = 0;
@@ -81,6 +90,7 @@ private:
 	uint32_t m_dz_zone_id     = 0;
 	uint32_t m_start_time     = 0;
 	uint32_t m_duration       = 0;
+	std::unordered_set<uint32_t> m_member_ids;
 	std::chrono::time_point<std::chrono::system_clock> m_expire_time;
 };
 

world/main.cpp

@@ -607,7 +607,6 @@ int main(int argc, char** argv) {
 		if (PurgeInstanceTimer.Check()) {
 			database.PurgeExpiredInstances();
 			database.PurgeAllDeletedDataBuckets();
-			ExpeditionDatabase::PurgeExpiredExpeditions();
 			ExpeditionDatabase::PurgeExpiredCharacterLockouts();
 		}
 

world/zoneserver.cpp

@@ -1371,8 +1371,6 @@ void ZoneServer::HandleMessage(uint16 opcode, const EQ::Net::Packet &p) {
 	case ServerOP_ExpeditionLeaderChanged:
 	case ServerOP_ExpeditionLockout:
 	case ServerOP_ExpeditionLockState:
-	case ServerOP_ExpeditionMemberChange:
-	case ServerOP_ExpeditionMemberSwap:
 	case ServerOP_ExpeditionMemberStatus:
 	case ServerOP_ExpeditionReplayOnJoin:
 	case ServerOP_ExpeditionDzCompass:
@@ -1383,8 +1381,10 @@ void ZoneServer::HandleMessage(uint16 opcode, const EQ::Net::Packet &p) {
 		break;
 	}
 	case ServerOP_ExpeditionCreate:
-	case ServerOP_ExpeditionDeleted:
 	case ServerOP_ExpeditionGetOnlineMembers:
+	case ServerOP_ExpeditionMemberChange:
+	case ServerOP_ExpeditionMemberSwap:
+	case ServerOP_ExpeditionMembersRemoved:
 	case ServerOP_ExpeditionDzAddPlayer:
 	case ServerOP_ExpeditionDzMakeLeader:
 	case ServerOP_ExpeditionRemoveCharLockouts:

zone/expedition.cpp

@@ -132,7 +132,7 @@ Expedition* Expedition::TryCreate(
 		auto inserted = zone->expedition_cache.emplace(expedition_id, std::move(expedition));
 
 		inserted.first->second->SendUpdatesToZoneMembers();
-		inserted.first->second->SendWorldExpeditionUpdate(); // cache in other zones
+		inserted.first->second->SendWorldExpeditionUpdate(ServerOP_ExpeditionCreate); // cache in other zones
 
 		Client* leader_client = request.GetLeaderClient();
 
@@ -486,11 +486,10 @@ void Expedition::RemoveAllMembers(bool enable_removal_timers, bool update_dz_exp
 		m_dynamiczone.UpdateExpireTime(RuleI(Expedition, EmptyDzShutdownDelaySeconds));
 	}
 
-	ExpeditionDatabase::DeleteAllMembers(m_id);
+	ExpeditionDatabase::UpdateAllMembersRemoved(m_id);
-	ExpeditionDatabase::DeleteExpedition(m_id);
 
 	SendUpdatesToZoneMembers(true);
-	SendWorldExpeditionUpdate(true);
+	SendWorldExpeditionUpdate(ServerOP_ExpeditionMembersRemoved);
 }
 
 bool Expedition::RemoveMember(const std::string& remove_char_name)
@@ -519,8 +518,6 @@ bool Expedition::RemoveMember(const std::string& remove_char_name)
 	uint32_t member_count = ExpeditionDatabase::GetExpeditionMemberCount(m_id);
 	if (member_count == 0)
 	{
-		// zone cache removal will occur in world message handler
-		ExpeditionDatabase::DeleteExpedition(m_id);
 		if (RuleB(Expedition, EmptyDzShutdownEnabled))
 		{
 			m_dynamiczone.UpdateExpireTime(RuleI(Expedition, EmptyDzShutdownDelaySeconds));
@@ -1412,11 +1409,10 @@ std::unique_ptr<EQApplicationPacket> Expedition::CreateLeaderNamePacket()
 	return outapp;
 }
 
-void Expedition::SendWorldExpeditionUpdate(bool destroyed)
+void Expedition::SendWorldExpeditionUpdate(uint16_t server_opcode)
 {
-	uint16_t opcode = destroyed ? ServerOP_ExpeditionDeleted : ServerOP_ExpeditionCreate;
 	uint32_t pack_size = sizeof(ServerExpeditionID_Struct);
-	auto pack = std::unique_ptr<ServerPacket>(new ServerPacket(opcode, pack_size));
+	auto pack = std::unique_ptr<ServerPacket>(new ServerPacket(server_opcode, pack_size));
 	auto buf = reinterpret_cast<ServerExpeditionID_Struct*>(pack->pBuffer);
 	buf->expedition_id = GetID();
 	buf->sender_zone_id = zone ? zone->GetZoneID() : 0;
@@ -1605,24 +1601,32 @@ void Expedition::HandleWorldMessage(ServerPacket* pack)
 		break;
 	}
 	case ServerOP_ExpeditionDeleted:
-	case ServerOP_ExpeditionExpired:
 	{
+		// sent by world when it deletes expired or empty expeditions
 		auto buf = reinterpret_cast<ServerExpeditionID_Struct*>(pack->pBuffer);
 		auto expedition = Expedition::FindCachedExpeditionByID(buf->expedition_id);
 		if (zone && expedition)
 		{
-			if (!zone->IsZone(buf->sender_zone_id, buf->sender_instance_id))
+			expedition->SendUpdatesToZoneMembers(true, false); // any members silently removed
-			{
-				// expired deletions should be silent
-				bool notify_members = (pack->opcode == ServerOP_ExpeditionDeleted);
-				expedition->SendUpdatesToZoneMembers(true, notify_members);
-			}
 
-			// remove even from sender zone
+			LogExpeditionsModerate("Deleting expedition [{}] from zone cache", buf->expedition_id);
 			zone->expedition_cache.erase(buf->expedition_id);
 		}
 		break;
 	}
+	case ServerOP_ExpeditionMembersRemoved:
+	{
+		auto buf = reinterpret_cast<ServerExpeditionID_Struct*>(pack->pBuffer);
+		if (zone && !zone->IsZone(buf->sender_zone_id, buf->sender_instance_id))
+		{
+			auto expedition = Expedition::FindCachedExpeditionByID(buf->expedition_id);
+			if (expedition)
+			{
+				expedition->SendUpdatesToZoneMembers(true);
+			}
+		}
+		break;
+	}
 	case ServerOP_ExpeditionLeaderChanged:
 	{
 		auto buf = reinterpret_cast<ServerExpeditionMemberChange_Struct*>(pack->pBuffer);
@@ -1656,11 +1660,6 @@ void Expedition::HandleWorldMessage(ServerPacket* pack)
 		auto expedition = Expedition::FindCachedExpeditionByID(buf->expedition_id);
 		if (expedition && zone)
 		{
-			LogExpeditionsDetail(
-				"World member change message -- remove: [{}] name: [{}] zone: [{}]:[{}] sender: [{}]:[{}]",
-				buf->removed, buf->char_name, zone->GetZoneID(), zone->GetInstanceID(), buf->sender_zone_id, buf->sender_instance_id
-			);
-
 			if (!zone->IsZone(buf->sender_zone_id, buf->sender_instance_id))
 			{
 				if (buf->removed)
@@ -1672,12 +1671,6 @@ void Expedition::HandleWorldMessage(ServerPacket* pack)
 					expedition->ProcessMemberAdded(buf->char_name, buf->char_id);
 				}
 			}
-
-			// remove this expedition from zone cache if last member was removed
-			if (buf->removed && expedition->GetMemberCount() == 0)
-			{
-				zone->expedition_cache.erase(buf->expedition_id);
-			}
 		}
 		break;
 	}

zone/expedition.h

@@ -156,7 +156,7 @@ private:
 	void SendLeaderMessage(Client* leader_client, uint16_t chat_type, uint32_t string_id, const std::initializer_list<std::string>& parameters = {});
 	void SendUpdatesToZoneMembers(bool clear = false, bool message_on_clear = true);
 	void SendWorldDzLocationUpdate(uint16_t server_opcode, const DynamicZoneLocation& location);
-	void SendWorldExpeditionUpdate(bool destroyed = false);
+	void SendWorldExpeditionUpdate(uint16_t server_opcode);
 	void SendWorldGetOnlineMembers();
 	void SendWorldAddPlayerInvite(const std::string& inviter_name, const std::string& swap_remove_name, const std::string& add_name, bool pending = false);
 	void SendWorldLeaderChanged();

zone/expedition_database.cpp

@@ -320,18 +320,6 @@ void ExpeditionDatabase::DeletePendingLockouts(uint32_t character_id)
 	database.QueryDatabase(query);
 }
 
-void ExpeditionDatabase::DeleteExpedition(uint32_t expedition_id)
-{
-	LogExpeditionsDetail("Deleting expedition [{}]", expedition_id);
-
-	auto query = fmt::format("DELETE FROM expedition_details WHERE id = {}", expedition_id);
-	auto results = database.QueryDatabase(query);
-	if (!results.Success())
-	{
-		LogExpeditions("Failed to delete expedition [{}]", expedition_id);
-	}
-}
-
 void ExpeditionDatabase::DeleteLockout(uint32_t expedition_id, const std::string& event_name)
 {
 	LogExpeditionsDetail("Deleting expedition [{}] lockout event [{}]", expedition_id, event_name);
@@ -348,21 +336,6 @@ void ExpeditionDatabase::DeleteLockout(uint32_t expedition_id, const std::string
 	}
 }
 
-void ExpeditionDatabase::DeleteAllMembers(uint32_t expedition_id)
-{
-	LogExpeditionsDetail("Deleting all members of expedition [{}]", expedition_id);
-
-	auto query = fmt::format(SQL(
-		DELETE FROM expedition_members WHERE expedition_id = {};
-	), expedition_id);
-
-	auto results = database.QueryDatabase(query);
-	if (!results.Success())
-	{
-		LogExpeditions("Failed to delete all members of expedition [{}]", expedition_id);
-	}
-}
-
 uint32_t ExpeditionDatabase::GetExpeditionIDFromCharacterID(uint32_t character_id)
 {
 	LogExpeditionsDetail("Getting expedition id for character [{}]", character_id);
@@ -675,6 +648,18 @@ void ExpeditionDatabase::UpdateMemberRemoved(uint32_t expedition_id, uint32_t ch
 	}
 }
 
+void ExpeditionDatabase::UpdateAllMembersRemoved(uint32_t expedition_id)
+{
+	LogExpeditionsDetail("Updating all members of expedition [{}] as removed", expedition_id);
+
+	auto query = fmt::format(SQL(
+		UPDATE expedition_members SET is_current_member = FALSE
+		WHERE expedition_id = {};
+	), expedition_id);
+
+	database.QueryDatabase(query);
+}
+
 void ExpeditionDatabase::UpdateReplayLockoutOnJoin(uint32_t expedition_id, bool add_on_join)
 {
 	LogExpeditionsDetail("Updating replay lockout on join [{}] for expedition [{}]", add_on_join, expedition_id);

zone/expedition_database.h

@@ -44,11 +44,9 @@ namespace ExpeditionDatabase
 	MySQLRequestResult LoadCharacterLockouts(uint32_t character_id, const std::string& expedition_name);
 	MySQLRequestResult LoadExpeditionMembers(uint32_t expedition_id);
 	MySQLRequestResult LoadValidationData(const std::string& character_names_query, const std::string& expedition_name);
-	void DeleteAllMembers(uint32_t expedition_id);
 	void DeleteAllCharacterLockouts(uint32_t character_id);
 	void DeleteAllCharacterLockouts(uint32_t character_id, const std::string& expedition_name);
 	void DeleteCharacterLockout(uint32_t character_id, const std::string& expedition_name, const std::string& event_name);
-	void DeleteExpedition(uint32_t expedition_id);
 	void DeleteLockout(uint32_t expedition_id, const std::string& event_name);
 	void DeleteMembersLockout(
 		const std::vector<ExpeditionMember>& members, const std::string& expedition_name, const std::string& event_name);
@@ -69,6 +67,7 @@ namespace ExpeditionDatabase
 	void UpdateLeaderID(uint32_t expedition_id, uint32_t leader_id);
 	void UpdateLockState(uint32_t expedition_id, bool is_locked);
 	void UpdateMemberRemoved(uint32_t expedition_id, uint32_t character_id);
+	void UpdateAllMembersRemoved(uint32_t expedition_id);
 	void UpdateReplayLockoutOnJoin(uint32_t expedition_id, bool add_on_join);
 };
 

zone/worldserver.cpp

@@ -2901,13 +2901,13 @@ void WorldServer::HandleMessage(uint16 opcode, const EQ::Net::Packet &p)
 	}
 	case ServerOP_ExpeditionCreate:
 	case ServerOP_ExpeditionDeleted:
-	case ServerOP_ExpeditionExpired:
 	case ServerOP_ExpeditionLeaderChanged:
 	case ServerOP_ExpeditionLockout:
 	case ServerOP_ExpeditionLockState:
 	case ServerOP_ExpeditionMemberChange:
 	case ServerOP_ExpeditionMemberSwap:
 	case ServerOP_ExpeditionMemberStatus:
+	case ServerOP_ExpeditionMembersRemoved:
 	case ServerOP_ExpeditionReplayOnJoin:
 	case ServerOP_ExpeditionGetOnlineMembers:
 	case ServerOP_ExpeditionDzAddPlayer: