From d671d2663558a8bac144061ed6a4674671ddecc5 Mon Sep 17 00:00:00 2001 From: Aaron Kimbrell Date: Sat, 26 Sep 2026 18:42:13 -0500 Subject: [PATCH] fix: character xml load and save no longer crash on bad data Loading a character whose xml has a malformed flag threw out of std::stoul, and a character whose xml failed to parse (or has no obj tag) dereferenced null on load and again on every save. - Flags are parsed with TryParse and a malformed one is logged and skipped instead of aborting the load. - The quick parse checks for the obj and items tags before using them. - Saving refuses to run when the document has no obj tag, logging that the character was not saved, instead of crashing and taking every other player's unsaved progress with it. - The remaining obj-child lookups go through a helper that returns null when the root is missing. Component LoadFromXml/UpdateXml still assume a valid document; refusing to load a player with invalid xml belongs in the world server's load path, which is being converted separately. Refs #1332 Co-Authored-By: Claude Opus 5.5 --- dGame/Character.cpp | 55 ++++++++++++++++++++++++++++++--------------- dGame/Character.h | 8 +++++++ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/dGame/Character.cpp b/dGame/Character.cpp index f234fd3f5..763223e59 100644 --- a/dGame/Character.cpp +++ b/dGame/Character.cpp @@ -88,7 +88,13 @@ void Character::DoQuickXMLDataParse() { return; } - tinyxml2::XMLElement* mf = m_Doc.FirstChildElement("obj")->FirstChildElement("mf"); + auto* const obj = m_Doc.FirstChildElement("obj"); + if (!obj) { + LOG("Character %s (%llu) xml has no obj tag!", m_Name.c_str(), m_ID); + return; + } + + tinyxml2::XMLElement* mf = obj->FirstChildElement("mf"); if (!mf) { LOG("Failed to find mf tag!"); return; @@ -107,13 +113,14 @@ void Character::DoQuickXMLDataParse() { mf->QueryAttribute("ess", &m_Eyes); mf->QueryAttribute("ms", &m_Mouth); - tinyxml2::XMLElement* inv = m_Doc.FirstChildElement("obj")->FirstChildElement("inv"); + tinyxml2::XMLElement* inv = obj->FirstChildElement("inv"); if (!inv) { LOG("Char has no inv!"); return; } - tinyxml2::XMLElement* bag = inv->FirstChildElement("items")->FirstChildElement("in"); + auto* const items = inv->FirstChildElement("items"); + tinyxml2::XMLElement* bag = items ? items->FirstChildElement("in") : nullptr; if (!bag) { LOG("Couldn't find bag0!"); @@ -140,7 +147,7 @@ void Character::DoQuickXMLDataParse() { } - tinyxml2::XMLElement* character = m_Doc.FirstChildElement("obj")->FirstChildElement("char"); + tinyxml2::XMLElement* character = GetXmlObjChild("char"); if (character) { character->QueryAttribute("cc", &m_Coins); int32_t gm_level = 0; @@ -204,7 +211,7 @@ void Character::DoQuickXMLDataParse() { character->QueryAttribute("lzrw", &m_OriginalRotation.w); } - auto* flags = m_Doc.FirstChildElement("obj")->FirstChildElement("flag"); + auto* flags = GetXmlObjChild("flag"); if (flags) { auto* currentChild = flags->FirstChildElement(); while (currentChild) { @@ -212,13 +219,15 @@ void Character::DoQuickXMLDataParse() { const auto* id = currentChild->Attribute("id"); const auto* si = currentChild->Attribute("si"); if (temp && id) { - uint32_t index = 0; - uint64_t value = 0; + // A malformed flag in the save should skip that flag, not throw out of the whole load. + const auto index = GeneralUtils::TryParse(id); + const auto value = GeneralUtils::TryParse(temp); - index = std::stoul(id); - value = std::stoull(temp); - - m_PlayerFlags.insert(std::make_pair(index, value)); + if (index && value) { + m_PlayerFlags.insert(std::make_pair(index.value(), value.value())); + } else { + LOG("Skipping malformed flag (id %s, v %s) for character %llu", id, temp, m_ObjectID); + } } else if (si) { auto value = GeneralUtils::TryParse(si); if (value) m_SessionFlags.insert(value.value()); @@ -248,10 +257,17 @@ void Character::SaveXMLToDatabase() { return; } + // Without an obj tag the xml never loaded, so there is nothing valid to write back. + auto* const obj = m_Doc.FirstChildElement("obj"); + if (!obj) { + LOG("%llu:%s has no loaded xml while saving! CHARACTER WILL NOT BE SAVED!", this->GetID(), this->GetName().c_str()); + return; + } + //For metrics, we'll record the time it took to save: auto start = std::chrono::system_clock::now(); - tinyxml2::XMLElement* character = m_Doc.FirstChildElement("obj")->FirstChildElement("char"); + tinyxml2::XMLElement* character = GetXmlObjChild("char"); if (character) { character->SetAttribute("gm", static_cast(m_GMLevel)); character->SetAttribute("cc", m_Coins); @@ -287,10 +303,10 @@ void Character::SaveXMLToDatabase() { } //Export our flags: - auto* flags = m_Doc.FirstChildElement("obj")->FirstChildElement("flag"); + auto* flags = GetXmlObjChild("flag"); if (!flags) { flags = m_Doc.NewElement("flag"); //Create a flags tag if we don't have one - m_Doc.FirstChildElement("obj")->LinkEndChild(flags); //Link it to the obj tag so we can find next time + obj->LinkEndChild(flags); //Link it to the obj tag so we can find next time } flags->DeleteChildren(); //Clear it if we have anything, so that we can fill it up again without dupes @@ -319,7 +335,7 @@ void Character::SaveXMLToDatabase() { void Character::SetIsNewLogin() { // If we dont have a flag element, then we cannot have a s element as a child of flag. - auto* flags = m_Doc.FirstChildElement("obj")->FirstChildElement("flag"); + auto* flags = GetXmlObjChild("flag"); if (!flags) return; auto* currentChild = flags->FirstChildElement(); @@ -437,10 +453,13 @@ void Character::SetRetroactiveFlags() { void Character::SaveXmlRespawnCheckpoints() { //Export our respawn points: - auto* points = m_Doc.FirstChildElement("obj")->FirstChildElement("res"); + auto* const obj = m_Doc.FirstChildElement("obj"); + if (!obj) return; + + auto* points = obj->FirstChildElement("res"); if (!points) { points = m_Doc.NewElement("res"); - m_Doc.FirstChildElement("obj")->LinkEndChild(points); + obj->LinkEndChild(points); } points->DeleteChildren(); @@ -459,7 +478,7 @@ void Character::SaveXmlRespawnCheckpoints() { void Character::LoadXmlRespawnCheckpoints() { m_WorldRespawnCheckpoints.clear(); - auto* points = m_Doc.FirstChildElement("obj")->FirstChildElement("res"); + auto* points = GetXmlObjChild("res"); if (!points) { return; } diff --git a/dGame/Character.h b/dGame/Character.h index e066a0ad4..31e5eb7fd 100644 --- a/dGame/Character.h +++ b/dGame/Character.h @@ -488,6 +488,14 @@ private: */ LWOOBJID m_ObjectID{ LWOOBJID_EMPTY }; + /** + * Gets a child of the root obj tag of the character xml, or nullptr if either is missing. + */ + tinyxml2::XMLElement* GetXmlObjChild(const char* name) { + auto* const obj = m_Doc.FirstChildElement("obj"); + return obj ? obj->FirstChildElement(name) : nullptr; + } + /** * The user that owns this character. */