From 32f68f35cf5eabd4223ef4901486d43bb806867e Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sun, 10 Mar 2024 13:24:35 +0100 Subject: [PATCH] Avoid packets getting sent to disconnected players (#14444) Many functions expect RemotePlayer to have a valid peer ID, this however is not the case immediately after disconnecting where the object is still alive and pending for removal. ServerEnvironment::getPlayer(const char *, bool) now only returns players that are connected unless forced to. --- src/network/serverpackethandler.cpp | 2 +- src/script/lua_api/l_env.cpp | 4 +--- src/server.cpp | 33 +++++++++++------------------ src/server/serverinventorymgr.cpp | 4 ++-- src/serverenvironment.cpp | 10 ++++++--- src/serverenvironment.h | 2 +- 6 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index c3888c110..f4b5a4859 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -179,7 +179,7 @@ void Server::handleCommand_Init(NetworkPacket* pkt) return; } - RemotePlayer *player = m_env->getPlayer(playername); + RemotePlayer *player = m_env->getPlayer(playername, true); // If player is already connected, cancel if (player && player->getPeerId() != PEER_ID_INEXISTENT) { diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp index 1fc640d23..ccf2cd653 100644 --- a/src/script/lua_api/l_env.cpp +++ b/src/script/lua_api/l_env.cpp @@ -695,8 +695,6 @@ int ModApiEnv::l_get_connected_players(lua_State *L) lua_createtable(L, env->getPlayerCount(), 0); u32 i = 0; for (RemotePlayer *player : env->getPlayers()) { - if (player->getPeerId() == PEER_ID_INEXISTENT) - continue; PlayerSAO *sao = player->getPlayerSAO(); if (sao && !sao->isGone()) { getScriptApiBase(L)->objectrefGetOrCreate(L, sao); @@ -714,7 +712,7 @@ int ModApiEnv::l_get_player_by_name(lua_State *L) // Do it const char *name = luaL_checkstring(L, 1); RemotePlayer *player = env->getPlayer(name); - if (!player || player->getPeerId() == PEER_ID_INEXISTENT) + if (!player) return 0; PlayerSAO *sao = player->getPlayerSAO(); if (!sao || sao->isGone()) diff --git a/src/server.cpp b/src/server.cpp index eebd3ba56..19d92316c 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1107,7 +1107,7 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id) } } - RemotePlayer *player = m_env->getPlayer(playername.c_str()); + RemotePlayer *player = m_env->getPlayer(playername.c_str(), true); // If failed, cancel if (!playersao || !player) { @@ -1948,9 +1948,13 @@ void Server::SendMovePlayerRel(session_t peer_id, const v3f &added_pos) void Server::SendPlayerFov(session_t peer_id) { + RemotePlayer *player = m_env->getPlayer(peer_id); + if (!player) + return; + NetworkPacket pkt(TOCLIENT_FOV, 4 + 1 + 4, peer_id); - PlayerFovSpec fov_spec = m_env->getPlayer(peer_id)->getFov(); + PlayerFovSpec fov_spec = player->getFov(); pkt << fov_spec.fov << fov_spec.is_multiplier << fov_spec.transition_time; Send(&pkt); @@ -1978,8 +1982,7 @@ void Server::SendEyeOffset(session_t peer_id, v3f first, v3f third, v3f third_fr void Server::SendPlayerPrivileges(session_t peer_id) { RemotePlayer *player = m_env->getPlayer(peer_id); - assert(player); - if(player->getPeerId() == PEER_ID_INEXISTENT) + if (!player) return; std::set privs; @@ -1998,8 +2001,7 @@ void Server::SendPlayerPrivileges(session_t peer_id) void Server::SendPlayerInventoryFormspec(session_t peer_id) { RemotePlayer *player = m_env->getPlayer(peer_id); - assert(player); - if (player->getPeerId() == PEER_ID_INEXISTENT) + if (!player) return; NetworkPacket pkt(TOCLIENT_INVENTORY_FORMSPEC, 0, peer_id); @@ -2011,8 +2013,7 @@ void Server::SendPlayerInventoryFormspec(session_t peer_id) void Server::SendPlayerFormspecPrepend(session_t peer_id) { RemotePlayer *player = m_env->getPlayer(peer_id); - assert(player); - if (player->getPeerId() == PEER_ID_INEXISTENT) + if (!player) return; NetworkPacket pkt(TOCLIENT_FORMSPEC_PREPEND, 0, peer_id); @@ -2190,11 +2191,6 @@ s32 Server::playSound(ServerPlayingSound ¶ms, bool ephemeral) <<"\" not found"<getPeerId() == PEER_ID_INEXISTENT) { - infostream<<"Server::playSound: Player \""<getPeerId()); } else { std::vector clients = m_clients.getClientIDs(); @@ -2817,8 +2813,7 @@ void Server::SendMinimapModes(session_t peer_id, std::vector &modes, size_t wanted_mode) { RemotePlayer *player = m_env->getPlayer(peer_id); - assert(player); - if (player->getPeerId() == PEER_ID_INEXISTENT) + if (!player) return; NetworkPacket pkt(TOCLIENT_MINIMAP_MODES, 0, peer_id); @@ -3363,11 +3358,7 @@ void Server::notifyPlayer(const char *name, const std::wstring &msg) } RemotePlayer *player = m_env->getPlayer(name); - if (!player) { - return; - } - - if (player->getPeerId() == PEER_ID_INEXISTENT) + if (!player) return; SendChatMessage(player->getPeerId(), ChatMessage(msg)); @@ -4039,7 +4030,7 @@ PlayerSAO* Server::emergePlayer(const char *name, session_t peer_id, u16 proto_v RemotePlayer *player = m_env->getPlayer(name); // If player is already connected, cancel - if (player && player->getPeerId() != PEER_ID_INEXISTENT) { + if (player) { infostream<<"emergePlayer(): Player already connected"<getPlayer(name.c_str()); // if player is connected, send him the inventory - if (p && p->getPeerId() != PEER_ID_INEXISTENT) { + if (p) { m_env->getGameDef()->sendDetachedInventory( inv, name, p->getPeerId()); } @@ -152,7 +152,7 @@ bool ServerInventoryManager::removeDetachedInventory(const std::string &name) if (m_env) { RemotePlayer *player = m_env->getPlayer(owner.c_str()); - if (player && player->getPeerId() != PEER_ID_INEXISTENT) + if (player) m_env->getGameDef()->sendDetachedInventory( nullptr, name, player->getPeerId()); } diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index 6d394309c..df5fb5e12 100644 --- a/src/serverenvironment.cpp +++ b/src/serverenvironment.cpp @@ -584,13 +584,17 @@ RemotePlayer *ServerEnvironment::getPlayer(const session_t peer_id) return NULL; } -RemotePlayer *ServerEnvironment::getPlayer(const char* name) +RemotePlayer *ServerEnvironment::getPlayer(const char* name, bool match_invalid_peer) { for (RemotePlayer *player : m_players) { - if (strcmp(player->getName(), name) == 0) + if (strcmp(player->getName(), name) != 0) + continue; + + if (match_invalid_peer || player->getPeerId() != PEER_ID_INEXISTENT) return player; + break; } - return NULL; + return nullptr; } void ServerEnvironment::addPlayer(RemotePlayer *player) diff --git a/src/serverenvironment.h b/src/serverenvironment.h index 4a4872a8b..000f5f81f 100644 --- a/src/serverenvironment.h +++ b/src/serverenvironment.h @@ -388,7 +388,7 @@ public: bool static_exists, v3s16 static_block=v3s16(0,0,0)); RemotePlayer *getPlayer(const session_t peer_id); - RemotePlayer *getPlayer(const char* name); + RemotePlayer *getPlayer(const char* name, bool match_invalid_peer = false); const std::vector getPlayers() const { return m_players; } u32 getPlayerCount() const { return m_players.size(); }