From 9d8aeef4235f28bb8f7255727c05bed51ddeb4a0 Mon Sep 17 00:00:00 2001 From: 20dka Date: Mon, 7 Nov 2022 22:04:17 +0100 Subject: [PATCH] refactor of vehicle packet handling --- include/TServer.h | 4 +- src/TConsole.cpp | 36 +++++------- src/TServer.cpp | 147 +++++++++++++++++++++++++++------------------- 3 files changed, 103 insertions(+), 84 deletions(-) diff --git a/include/TServer.h b/include/TServer.h index f161412..a63fd9e 100644 --- a/include/TServer.h +++ b/include/TServer.h @@ -43,8 +43,8 @@ private: static bool ShouldSpawn(TClient& c, const std::string& CarJson, int ID); static bool IsUnicycle(TClient& c, const std::string& CarJson); static void Apply(TClient& c, int VID, const std::string& pckt); - static bool HandlePosition(TClient& c, const std::string& Packet); - static bool HandleVehicleUpdate(TClient& c, const std::string& Packet); + static bool HandlePosition(TClient& c, const std::string& PacketStr); + static bool HandleVehicleUpdate(const std::string& PacketStr, const int playerID); }; struct BufferView { diff --git a/src/TConsole.cpp b/src/TConsole.cpp index 80ebd13..9a897e9 100644 --- a/src/TConsole.cpp +++ b/src/TConsole.cpp @@ -24,6 +24,17 @@ static inline bool StringStartsWith(const std::string& What, const std::string& return What.size() >= StartsWith.size() && What.substr(0, StartsWith.size()) == StartsWith; } +static inline bool StringStartsWithLower(const std::string& Name1, const std::string& Name2) { + std::string Name1Lower = boost::algorithm::to_lower_copy(Name1); + return StringStartsWith(Name1Lower, Name2) || StringStartsWith(Name2, Name1Lower); +}; + +static inline bool StringStartsWithLowerBoth(const std::string& Name1, const std::string& Name2) { + std::string Name1Lower = boost::algorithm::to_lower_copy(Name1); + std::string Name2Lower = boost::algorithm::to_lower_copy(Name2); + return StringStartsWith(Name1Lower, Name2Lower) || StringStartsWith(Name2Lower, Name1Lower); +}; + TEST_CASE("StringStartsWith") { CHECK(StringStartsWith("Hello, World", "Hello")); CHECK(StringStartsWith("Hello, World", "H")); @@ -363,23 +374,18 @@ void TConsole::Command_Kick(const std::string&, const std::vector& if (!EnsureArgsCount(args, 1, size_t(-1))) { return; } - auto Name = args.at(0); + auto Name = boost::algorithm::to_lower_copy(args.at(0)); std::string Reason = "Kicked by server console"; if (args.size() > 1) { Reason = ConcatArgs({ args.begin() + 1, args.end() }); } beammp_trace("attempt to kick '" + Name + "' for '" + Reason + "'"); bool Kicked = false; - // TODO: this sucks, tolower is locale-dependent. - auto NameCompare = [](std::string Name1, std::string Name2) -> bool { - std::string Name1Lower = boost::algorithm::to_lower_copy(Name1); - std::string Name2Lower = boost::algorithm::to_lower_copy(Name2); - return StringStartsWith(Name1Lower, Name2Lower) || StringStartsWith(Name2Lower, Name1Lower); - }; + mLuaEngine->Server().ForEachClient([&](std::weak_ptr Client) -> bool { auto Locked = Client.lock(); if (Locked) { - if (NameCompare(Locked->GetName(), Name)) { + if (StringStartsWithLower(Locked->GetName(), Name)) { mLuaEngine->Network().ClientKick(*Locked, Reason); Kicked = true; return false; @@ -666,15 +672,10 @@ void TConsole::Autocomplete_Lua(const std::string& stub, std::vector& suggestions) { std::string stub_lower = boost::algorithm::to_lower_copy(stub); - auto LowercaseCompare = [](std::string Name1, std::string Name2) -> bool { - std::string NameLower = boost::algorithm::to_lower_copy(Name1); - return StringStartsWith(NameLower, Name2); - }; - mLuaEngine->Server().ForEachClient([&](std::weak_ptr Client) -> bool { auto Locked = Client.lock(); if (Locked) { - if (LowercaseCompare(Locked->GetName(), stub_lower)) { + if (StringStartsWithLower(Locked->GetName(), stub_lower)) { suggestions.push_back("kick " + Locked->GetName()); } } @@ -691,16 +692,11 @@ void TConsole::Autocomplete_Settings(const std::string& stub, std::vector bool { - std::string NameLower = boost::algorithm::to_lower_copy(Name1); - return StringStartsWith(NameLower, Name2); - }; - // suggest setting names if (command == "set" || command == "get") { for (const auto& [k, v] : Application::mSettings) { std::string key = std::string(k); - if (LowercaseCompare(key, arg)) { + if (StringStartsWithLower(key, arg)) { suggestions.push_back("settings " + command + " " + key); } } diff --git a/src/TServer.cpp b/src/TServer.cpp index 770e67f..56e6f74 100644 --- a/src/TServer.cpp +++ b/src/TServer.cpp @@ -17,6 +17,12 @@ #include "Json.h" +struct VehiclePacket { + std::string Data; + int Pid; + int Vid; +}; + static std::optional> GetPidVid(const std::string& str) { auto IDSep = str.find('-'); std::string pid = str.substr(0, IDSep); @@ -34,6 +40,42 @@ static std::optional> GetPidVid(const std::string& str) { return std::nullopt; } +static std::optional ParseVehiclePacket(const std::string& Packet, const int playerID) { + if (Packet.size() < 8) { //2 for code, 3<= for pidvid, 1<= for data, 2 for dividers + // invalid packet + return std::nullopt; + } + // Zp:serverVehicleID:data + // 0-0:data + std::string withoutCode = Packet.substr(3); + + auto NameDataSep = withoutCode.find(':', 3); + if (NameDataSep == std::string::npos) { + // invalid packet + return std::nullopt; + } + if (NameDataSep + 1 > withoutCode.size()) { + // invalid packet + return std::nullopt; + } + + std::string ServerVehicleID = withoutCode.substr(0, NameDataSep); + + std::string Data = withoutCode.substr(NameDataSep + 1); + + // parse veh ID + auto MaybePidVid = GetPidVid(ServerVehicleID); + if (MaybePidVid) { + int PID, VID; + std::tie(PID, VID) = MaybePidVid.value(); + + if (PID == playerID) { + return { {Data, PID, VID} }; //std::vector(Data.begin(), Data.end()) + } + } + return std::nullopt; +} + TEST_CASE("GetPidVid") { SUBCASE("Valid singledigit") { const auto MaybePidVid = GetPidVid("0-1"); @@ -77,6 +119,39 @@ TEST_CASE("GetPidVid") { } } +TEST_CASE("ParseVehiclePacket") { + SUBCASE("Valid packet") { + const auto valid = ParseVehiclePacket("Zp:0-0:{jsonstring}", 0); + CHECK(valid.has_value()); + } + SUBCASE("Valid packet 2") { + const auto packet = ParseVehiclePacket("Zp:12-3:{jsonstring}", 12); + CHECK(packet.has_value()); + CHECK_EQ(packet.value().Pid, 12); + CHECK_EQ(packet.value().Vid, 3); + } + SUBCASE("Missing packet") { + const auto valid = ParseVehiclePacket("", 0); + CHECK(!valid.has_value()); + } + SUBCASE("Missing ServerVehicleID") { + const auto valid = ParseVehiclePacket("Zp:{jsonstring}", 0); + CHECK(!valid.has_value()); + } + SUBCASE("Missing data") { + const auto valid = ParseVehiclePacket("Zp:0-0:", 0); + CHECK(!valid.has_value()); + } + SUBCASE("Incorrect Pid") { + const auto valid = ParseVehiclePacket("Zp:0-0:{jsonstring}", 1); + CHECK(!valid.has_value()); + } + SUBCASE("Incorrect Pid 2") { + const auto valid = ParseVehiclePacket("Zp:12-0:{jsonstring}", 13); + CHECK(!valid.has_value()); + } +} + TServer::TServer(const std::vector& Arguments) { beammp_infof("BeamMP Server v{} ({})", Application::ServerVersionString(), BEAMMP_GIT_HASH); Application::SetSubsystemStatus("Server", Application::Status::Starting); @@ -152,7 +227,7 @@ void TServer::GlobalParser(const std::weak_ptr& Client, std::vector= 86) { - if (HandleVehicleUpdate(*LockedClient, StringPacket)){ + if (HandleVehicleUpdate(StringPacket, LockedClient->GetID())){ Network.SendToAll(LockedClient.get(), Packet, false, false); } else { beammp_debugf("Invalid vehicle update packet received from '{}' ({}), ignoring it", LockedClient->GetName(), LockedClient->GetID()); @@ -438,71 +513,19 @@ void TServer::InsertClient(const std::shared_ptr& NewClient) { (void)mClients.insert(NewClient); } -bool TServer::HandlePosition(TClient& c, const std::string& Packet) { - if (Packet.size() < 3) { - // invalid packet - return false; - } - // Zp:serverVehicleID:data - // Zp:0:data - std::string withoutCode = Packet.substr(3); - auto NameDataSep = withoutCode.find(':', 2); - if (NameDataSep == std::string::npos || NameDataSep < 2) { - // invalid packet - return false; - } - // FIXME: ensure that -2 does what it should... it seems weird. - std::string ServerVehicleID = withoutCode.substr(2, NameDataSep - 2); - if (NameDataSep + 1 > withoutCode.size()) { - // invalid packet - return false; - } - std::string Data = withoutCode.substr(NameDataSep + 1); +bool TServer::HandlePosition(TClient& c, const std::string& PacketStr) { + auto MaybePacket = ParseVehiclePacket(PacketStr, c.GetID()); - // parse veh ID - auto MaybePidVid = GetPidVid(ServerVehicleID); - if (MaybePidVid) { - int PID = -1; - int VID = -1; - std::tie(PID, VID) = MaybePidVid.value(); - if (PID == c.GetID()) { - c.SetCarPosition(VID, Data); - return true; - } + if (MaybePacket) { + auto packet = MaybePacket.value(); + c.SetCarPosition(packet.Vid, packet.Data); + return true; } return false; } -bool TServer::HandleVehicleUpdate(TClient& c, const std::string& Packet) { - if (Packet.size() < 3) { - // invalid packet - return false; - } - // (Vi/We/Yl):serverVehicleID:data - // (Vi/We/Yl):serverVehicleID:data - std::string withoutCode = Packet.substr(3); - auto NameDataSep = withoutCode.find(':', 2); - if (NameDataSep == std::string::npos || NameDataSep < 2) { - // invalid packet - return false; - } - // FIXME: ensure that -2 does what it should... it seems weird. - std::string ServerVehicleID = withoutCode.substr(2, NameDataSep - 2); - if (NameDataSep + 1 > withoutCode.size()) { - // invalid packet - return false; - } - std::string Data = withoutCode.substr(NameDataSep + 1); +bool TServer::HandleVehicleUpdate(const std::string& PacketStr, const int playerID) { + auto MaybePacket = ParseVehiclePacket(PacketStr, playerID); - // parse veh ID - auto MaybePidVid = GetPidVid(ServerVehicleID); - if (MaybePidVid) { - int PID = -1; - int VID = -1; - std::tie(PID, VID) = MaybePidVid.value(); - if (PID == c.GetID()) { - return true; - } - } - return false; + return MaybePacket.has_value(); }