From bf6e38872829f87ff408f294184da14c8c24bfd4 Mon Sep 17 00:00:00 2001 From: Yohann Ferreira Date: Fri, 22 Oct 2010 13:09:25 +0200 Subject: Made the servers check for positive id in xml db loading. Also fixed a memleak when loading an invalid monster attack. Resolves: Mana-Mantis #215. Reviewed-by: Thorbjorn. --- src/game-server/attributemanager.cpp | 64 +++++++++++++++++++++--------------- src/game-server/attributemanager.hpp | 11 +++++-- src/game-server/itemmanager.cpp | 6 ++-- src/game-server/mapmanager.cpp | 14 +++++++- src/game-server/monstermanager.cpp | 37 ++++++++++++++++----- src/game-server/skillmanager.cpp | 2 +- src/game-server/statusmanager.cpp | 7 ++-- 7 files changed, 96 insertions(+), 45 deletions(-) (limited to 'src') diff --git a/src/game-server/attributemanager.cpp b/src/game-server/attributemanager.cpp index 7e32d4dc..3c9fab23 100644 --- a/src/game-server/attributemanager.cpp +++ b/src/game-server/attributemanager.cpp @@ -61,23 +61,29 @@ void AttributeManager::reload() { if (xmlStrEqual(attributenode->name, BAD_CAST "attribute")) { - unsigned int id = XML::getProperty(attributenode, "id", 0); + int id = XML::getProperty(attributenode, "id", 0); - mAttributeMap[id] = std::pair< bool , - std::vector > - (false , std::vector()); + if (id <= 0) + { + LOG_WARN("Attribute manager: attribute '" << id + << "' is invalid and will be ignored."); + continue; + } + + mAttributeMap[id] = AttributeInfoMap(false, + std::vector()); unsigned int layerCount = 0; for_each_xml_child_node(subnode, attributenode) { if (xmlStrEqual(subnode->name, BAD_CAST "modifier")) { - std::string sType = utils::toUpper(XML::getProperty(subnode, - "stacktype", "")); - std::string eType = utils::toUpper(XML::getProperty(subnode, - "modtype", "")); - std::string tag = utils::toUpper(XML::getProperty(subnode, - "tag", "")); + std::string sType = utils::toUpper( + XML::getProperty(subnode, "stacktype", "")); + std::string eType = utils::toUpper( + XML::getProperty(subnode, "modtype", "")); + std::string tag = utils::toUpper( + XML::getProperty(subnode, "tag", "")); AT_TY pSType; AME_TY pEType; if (!sType.empty()) @@ -93,11 +99,12 @@ void AttributeManager::reload() pSType = TY_NSTB; else { - LOG_WARN("Attribute manager: attribute '" << id - << "' has unknown stack type '" << sType - << "', skipping modifier!"); + LOG_WARN("Attribute manager: attribute '" + << id << "' has unknown stack type '" + << sType << "', skipping modifier!"); fail = true; } + if (!fail) { if (eType == "ADDITIVE") @@ -106,36 +113,41 @@ void AttributeManager::reload() pEType = AME_MULT; else { - LOG_WARN("Attribute manager: attribute '" << id - << "' has unknown modification type '" << sType - << "', skipping modifier!"); + LOG_WARN( + "Attribute manager: attribute '" << id + << "' has unknown modification type '" + << sType << "', skipping modifier!"); fail = true; } if (!fail) { - mAttributeMap[id].second.push_back(AttributeInfoType(pSType, pEType)); - std::string tag = XML::getProperty(subnode, "tag", ""); + mAttributeMap[id].second.push_back( + AttributeInfoType(pSType, pEType)); + std::string tag = XML::getProperty( + subnode, "tag", ""); if (!tag.empty()) - mTagMap.insert(std::make_pair(tag, - std::make_pair(id, layerCount))); + mTagMap.insert( + std::make_pair(tag, + std::make_pair(id, layerCount))); ++layerCount; } } } else - LOG_WARN("Attribute manager: attribute '" << id << - "' has undefined modification type, skipping modifier!"); + LOG_WARN("Attribute manager: attribute '" << id + << "' has undefined modification type, " + << "skipping modifier!"); } else { LOG_WARN("Attribute manager: attribute '" << id << - "' has undefined stack type, skipping modifier!"); + "' has undefined stack type, skipping modifier!"); } } } - std::string scope = utils::toUpper(XML::getProperty(attributenode, - "scope", std::string())); + std::string scope = utils::toUpper( + XML::getProperty(attributenode, "scope", std::string())); if (scope.empty()) { // Give a warning unless scope has been explicitly set to "NONE" @@ -165,7 +177,7 @@ void AttributeManager::reload() LOG_DEBUG("Attribute manager: attribute '" << id << "' set to have no default scope."); } - } + } // End 'attribute' } LOG_DEBUG("attribute map:"); diff --git a/src/game-server/attributemanager.hpp b/src/game-server/attributemanager.hpp index 0afac022..c5f1d870 100644 --- a/src/game-server/attributemanager.hpp +++ b/src/game-server/attributemanager.hpp @@ -60,10 +60,15 @@ class AttributeManager const std::string *getTagFromInfo(unsigned int, unsigned int) const; private: - // attribute id -> { modifiable, { stackable type, effect type }[] } - typedef std::map< int, std::pair< bool, std::vector > > AttributeMap; + // modifiable, { stackable type, effect type }[] + typedef std::pair< bool, + std::vector > AttributeInfoMap; + + // Attribute id -> { modifiable, { stackable type, effect type }[] } + typedef std::map< int, AttributeInfoMap > AttributeMap; // tag name -> { attribute id, layer } - typedef std::map< std::string, std::pair > TagMap; + typedef std::map< std::string, + std::pair > TagMap; // being type id -> (*{ stackable type, effect type })[] AttributeScopes mAttributeScopes[ATTR_MAX]; diff --git a/src/game-server/itemmanager.cpp b/src/game-server/itemmanager.cpp index 802197d6..34a6170a 100644 --- a/src/game-server/itemmanager.cpp +++ b/src/game-server/itemmanager.cpp @@ -126,10 +126,10 @@ void ItemManager::reload() continue; int id = XML::getProperty(node, "id", 0); - if (!id) + if (id < 1) { - LOG_WARN("Item Manager: An item has no ID in " - << mItemReferenceFile << ", and so has been ignored!"); + LOG_WARN("Item Manager: Item ID: " << id << " is invalid in " + << mItemReferenceFile << ", and will be ignored."); continue; } diff --git a/src/game-server/mapmanager.cpp b/src/game-server/mapmanager.cpp index bee43585..ab9b2dc5 100644 --- a/src/game-server/mapmanager.cpp +++ b/src/game-server/mapmanager.cpp @@ -70,7 +70,7 @@ int MapManager::initialize(const std::string &mapReferenceFile) std::string name = XML::getProperty(node, "name", std::string()); // Test id and map name - if (id != 0 && !name.empty()) + if (id > 0 && !name.empty()) { // Testing if the file is actually in the maps folder std::string file = std::string("maps/") + name + ".tmx"; @@ -89,6 +89,18 @@ int MapManager::initialize(const std::string &mapReferenceFile) ++loadedMaps; } } + else + { + if (name.empty()) + { + LOG_WARN("Invalid unnamed map Id: " << id << '.'); + } + else + { + LOG_WARN("Invalid map Id: " << id << " for map: " + << name << '.'); + } + } } if (loadedMaps > 0) diff --git a/src/game-server/monstermanager.cpp b/src/game-server/monstermanager.cpp index f59799c3..1444e17b 100644 --- a/src/game-server/monstermanager.cpp +++ b/src/game-server/monstermanager.cpp @@ -79,10 +79,10 @@ void MonsterManager::reload() if (!xmlStrEqual(node->name, BAD_CAST "monster")) continue; - int id = XML::getProperty(node, "id", -1); + int id = XML::getProperty(node, "id", 0); std::string name = XML::getProperty(node, "name", "unnamed"); - if (id == -1) + if (id < 1) { LOG_WARN("Monster Manager: There is a monster (" << name << ") without ID in " @@ -210,19 +210,32 @@ void MonsterManager::reload() std::string sElement = XML::getProperty(subnode, "element", "neutral"); att->element = elementFromString(sElement); std::string sType = XML::getProperty(subnode, "type", "physical"); - if (sType == "physical") {att->type = DAMAGE_PHYSICAL; } - else if (sType == "magical" || sType == "magic") {att->type = DAMAGE_MAGICAL; } - else if (sType == "other") {att->type = DAMAGE_OTHER; } - else { + + bool validMonsterAttack = true; + if (sType == "physical") + { + att->type = DAMAGE_PHYSICAL; + } + else if (sType == "magical" || sType == "magic") + { + att->type = DAMAGE_MAGICAL; + } + else if (sType == "other") + { + att->type = DAMAGE_OTHER; + } + else + { LOG_WARN("Monster manager " << mMonsterReferenceFile << ": unknown damage type '" << sType << "'."); } - if (att->id == 0) + if (att->id < 1) { LOG_WARN(mMonsterReferenceFile << ": Attack without ID for monster #" << id << " (" << name << ") - attack ignored"); + validMonsterAttack = false; } else if (att->element == ELEMENT_ILLEGAL) { @@ -230,17 +243,25 @@ void MonsterManager::reload() << ": Attack with unknown element \"" << sElement << "\" for monster #" << id << " (" << name << ") - attack ignored"); + validMonsterAttack = false; } else if (att->type == -1) { LOG_WARN(mMonsterReferenceFile << ": Attack with unknown type \"" << sType << "\"" << " for monster #" << id << " (" << name << ")"); + validMonsterAttack = false; } - else + + if (validMonsterAttack) { monster->addAttack(att); } + else + { + delete att; + att = 0; + } } else if (xmlStrEqual(subnode->name, BAD_CAST "script")) diff --git a/src/game-server/skillmanager.cpp b/src/game-server/skillmanager.cpp index 99c03f0e..2cfcba58 100644 --- a/src/game-server/skillmanager.cpp +++ b/src/game-server/skillmanager.cpp @@ -90,7 +90,7 @@ void SkillManager::reload() std::string()); name = utils::toUpper(name); int id = XML::getProperty(skillnode, "id", 0); - if (id && !name.empty()) + if (id > 0 && !name.empty()) { bool duplicateKey = false; for (SkillMap::iterator i = skillMap.begin(); diff --git a/src/game-server/statusmanager.cpp b/src/game-server/statusmanager.cpp index 21c396e7..008e9827 100644 --- a/src/game-server/statusmanager.cpp +++ b/src/game-server/statusmanager.cpp @@ -65,10 +65,11 @@ void StatusManager::reload() continue; int id = XML::getProperty(node, "id", 0); - if (id == 0) + if (id < 1) { - LOG_WARN("Status Manager: An (ignored) Status has no ID in " - << statusReferenceFile << "!"); + LOG_WARN("Status Manager: The status ID: " << id << " in " + << statusReferenceFile + << " is invalid and will be ignored."); continue; } -- cgit v1.2.3-60-g2f50