Fixed problems with loading XML files containing Windows newlines

By using xmlParseFile instead of xmlParseMemory, the system-dependent
newlines should be handled automatically.

The .tmx.gz files should still be supported, but instead of manually
decompressing them the xmlParseFile function should take care of that.

It also fixes the leaking of XML documents in both the SkillManager as
well as the PermissionManager, since they now rely on XML::Document,
which cleans up automatically.

Reviewed-by: Stefan Dombrowski

5 files changed, 37 insertions, 154 deletions

diff --git a/src/common/permissionmanager.cpp b/src/common/permissionmanager.cpp
index a2c67cd4..573e1d26 100644
--- a/src/common/permissionmanager.cpp
+++ b/src/common/permissionmanager.cpp
@@ -20,7 +20,6 @@
 
 #include "common/permissionmanager.h"
 
-#include "common/resourcemanager.h"
 #include "game-server/character.h"
 #include "utils/logger.h"
 #include "utils/xml.h"
@@ -52,37 +51,18 @@ void PermissionManager::initialize(const std::string & file)
 
 void PermissionManager::reload()
 {
-    int size;
-    char *data = ResourceManager::loadFile(permissionFile, size);
+    XML::Document doc(permissionFile);
+    xmlNodePtr rootNode = doc.rootNode();
 
-    if (!data) {
-        LOG_ERROR("Permission Manager: Could not find "
-                  << permissionFile << "!");
-        free(data);
-        return;
-    }
-
-    xmlDocPtr doc = xmlParseMemory(data, size);
-    free(data);
-
-    if (!doc)
-    {
-        LOG_ERROR("Permission Manager: Error while parsing permission database ("
-                  << permissionFile << ")!");
-        return;
-    }
-
-    xmlNodePtr node = xmlDocGetRootElement(doc);
-    if (!node || !xmlStrEqual(node->name, BAD_CAST "permissions"))
+    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "permissions"))
     {
         LOG_ERROR("Permission Manager: " << permissionFile
                   << " is not a valid database file!");
-        xmlFreeDoc(doc);
         return;
     }
 
     LOG_INFO("Loading permission reference...");
-    for (node = node->xmlChildrenNode; node != NULL; node = node->next)
+    for_each_xml_child_node(node, rootNode)
     {
         unsigned char classmask = 0x01;
         if (!xmlStrEqual(node->name, BAD_CAST "class"))
diff --git a/src/game-server/mapreader.cpp b/src/game-server/mapreader.cpp
index 80d0a107..462105f2 100644
--- a/src/game-server/mapreader.cpp
+++ b/src/game-server/mapreader.cpp
@@ -20,7 +20,6 @@
 
 #include "game-server/mapreader.h"
 
-#include "common/resourcemanager.h"
 #include "game-server/map.h"
 #include "game-server/mapcomposite.h"
 #include "game-server/mapmanager.h"
@@ -40,58 +39,18 @@ static std::vector< int > tilesetFirstGids;
 
 bool MapReader::readMap(const std::string &filename, MapComposite *composite)
 {
-    int fileSize;
-    char *buffer = ResourceManager::loadFile(filename, fileSize);
-
-    if (buffer == NULL)
-    {
-        LOG_ERROR("Error: Map file not found (" << filename.c_str() << ")");
-        return false;
-    }
-
-    xmlDocPtr doc = NULL;
-
-    int l = filename.length();
-    if (l > 3 && filename.substr(l - 3) == ".gz")
-    {
-        // Inflate the gzipped map data.
-        char *inflated;
-        unsigned inflatedSize = 0;
-        bool ret = inflateMemory(buffer, fileSize, inflated, inflatedSize);
-        free(buffer);
-        buffer = ret ? inflated : NULL;
-        fileSize = inflatedSize;
-    }
-
-    if (buffer)
-    {
-        // Parse the XML document.
-        doc = xmlParseMemory(buffer, fileSize);
-        free(buffer);
-    }
-
-    if (!doc)
-    {
-        LOG_ERROR("Error while parsing map file '" << filename << "'!");
-        return false;
-    }
-
-    Map *map = NULL;
-    xmlNodePtr node = xmlDocGetRootElement(doc);
-
-    std::vector<Thing *> things;
+    XML::Document doc(filename);
+    xmlNodePtr rootNode = doc.rootNode();
 
     // Parse the inflated map data.
-    if (node && xmlStrEqual(node->name, BAD_CAST "map"))
-    {
-        map = MapReader::readMap(node, filename, composite, things);
-    }
-    else
+    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "map"))
     {
         LOG_ERROR("Error: Not a map file (" << filename << ")!");
+        return false;
     }
 
-    xmlFreeDoc(doc);
+    std::vector<Thing *> things;
+    Map *map = readMap(rootNode, filename, composite, things);
 
     if (map)
     {
diff --git a/src/game-server/skillmanager.cpp b/src/game-server/skillmanager.cpp
index d15219a4..75f5f53c 100644
--- a/src/game-server/skillmanager.cpp
+++ b/src/game-server/skillmanager.cpp
@@ -20,7 +20,6 @@
 
 #include "game-server/skillmanager.h"
 
-#include "common/resourcemanager.h"
 #include "utils/string.h"   // for the toUpper function
 #include "utils/logger.h"
 #include "utils/xml.h"
@@ -45,42 +44,23 @@ void SkillManager::reload()
     skillMap["KNIFE"] = 101;
     */
 
-    int size;
-    char *data = ResourceManager::loadFile(skillReferenceFile, size);
+    XML::Document doc(skillReferenceFile);
+    xmlNodePtr rootNode = doc.rootNode();
 
-    std::string absPathFile = ResourceManager::resolve(skillReferenceFile);
-
-    if (!data)
-    {
-        LOG_ERROR("Skill Manager: Could not find " << skillReferenceFile << "!");
-        free(data);
-        return;
-    }
-
-    xmlDocPtr doc = xmlParseMemory(data, size);
-    free(data);
-
-    if (!doc)
+    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "skills"))
     {
-        LOG_ERROR("Skill Manager: Error while parsing skill database ("
-                  << absPathFile << ")!");
-        return;
-    }
-
-    xmlNodePtr node = xmlDocGetRootElement(doc);
-    if (!node || !xmlStrEqual(node->name, BAD_CAST "skills"))
-    {
-        LOG_ERROR("Skill Manager: " << absPathFile
+        LOG_ERROR("Skill Manager: " << skillReferenceFile
                   << " is not a valid database file!");
-        xmlFreeDoc(doc);
         return;
     }
 
-    LOG_INFO("Loading skill reference: " << absPathFile);
+    LOG_INFO("Loading skill reference: " << skillReferenceFile);
 
-    for_each_xml_child_node(setnode, node)
+    for_each_xml_child_node(setnode, rootNode)
     {
-        if (xmlStrEqual(setnode->name, BAD_CAST "set"))
+        if (!xmlStrEqual(setnode->name, BAD_CAST "set"))
+            continue;
+
         // we don't care about sets server-sided (yet?)
         for_each_xml_child_node(skillnode, setnode)
         {
@@ -158,7 +138,7 @@ void SkillManager::reload()
                  "Skill map loading. Defaults will fall back to id 0.");
 
     LOG_INFO("Loaded " << skillMap.size() << " skill references from "
-             << absPathFile);
+             << skillReferenceFile);
 }
 
 int SkillManager::getIdFromString(const std::string &name)
diff --git a/src/utils/xml.cpp b/src/utils/xml.cpp
index a4a5360e..5d579ddc 100644
--- a/src/utils/xml.cpp
+++ b/src/utils/xml.cpp
@@ -30,62 +30,31 @@
 
 namespace XML
 {
-    Document::Document(const std::string &filename, bool useResman):
+    Document::Document(const std::string &fileName, bool useResman):
         mDoc(0)
     {
-        int size;
-        char *data = NULL;
+        std::string resolvedFileName = fileName;
         if (useResman)
         {
-            data = ResourceManager::loadFile(filename, size);
-        }
-        else
-        {
-            std::ifstream file;
-            file.open(filename.c_str(), std::ios::in);
+            resolvedFileName = ResourceManager::resolve(fileName);
 
-            if (file.is_open())
+            if (resolvedFileName.empty())
             {
-                // Get length of file
-                file.seekg(0, std::ios::end);
-                size = file.tellg();
-                file.seekg(0, std::ios::beg);
-
-                data = (char*) malloc(size);
-
-                file.read(data, size);
-                file.close();
-            }
-            else
-            {
-                LOG_ERROR("(XML::Document) Error loading XML file: "
-                          << filename);
+                LOG_ERROR("(XML::Document) File not found in search path: "
+                          << fileName);
+                return;
             }
         }
 
-        if (data)
-        {
-            mDoc = xmlParseMemory(data, size);
-            free(data);
+        mDoc = xmlParseFile(resolvedFileName.c_str());
 
-            if (!mDoc)
-            {
-                LOG_ERROR("(XML::Document) Error parsing XML file: "
-                          << filename);
-            }
-        }
-        else
+        if (!mDoc)
         {
-            LOG_ERROR("(XML::Document) Error loading XML file: "
-                      << filename);
+            LOG_ERROR("(XML::Document) Error parsing XML file: "
+                      << resolvedFileName);
         }
     }
 
-    Document::Document(const char *data, int size)
-    {
-        mDoc = xmlParseMemory(data, size);
-    }
-
     Document::~Document()
     {
         if (mDoc)
diff --git a/src/utils/xml.h b/src/utils/xml.h
index eba88e56..2278ce46 100644
--- a/src/utils/xml.h
+++ b/src/utils/xml.h
@@ -40,19 +40,14 @@ namespace XML
     {
         public:
             /**
-             * Constructor that attempts to load the given file through the
-             * resource manager. Logs errors.
-             */
-            Document(const std::string &filename, bool useResman = true);
-
-            /**
-             * Constructor that attempts to load an XML document from memory.
-             * Does not log errors.
+             * Attempts to load an XML document from the given file. Logs an
+             * error when something went wrong.
              *
-             * @param data the string to parse as XML
-             * @param size the length of the string in bytes
+             * @param fileName  the file name of the XML document
+             * @param useResman whether to resolve the full path to the file
+             *                  using the resource manager (true by default).
              */
-            Document(const char *data, int size);
+            Document(const std::string &fileName, bool useResman = true);
 
             /**
              * Destructor. Frees the loaded XML file.