Merge pull request mapnik#2880 from zerebubuth/fix/rapidxml-loader-trim-whitespace

Fix rapidxml XML loader to trim whitespace

Author: Dane Springmeyer

include/mapnik/xml_loader.hpp

@@ -23,14 +23,17 @@
 #ifndef MAPNIK_LIBXML2_LOADER_HPP
 #define MAPNIK_LIBXML2_LOADER_HPP
 
+// mapnik
+#include <mapnik/config.hpp> // for MAPNIK_DECL
+
 // stl
 #include <string>
 
 namespace mapnik
 {
-class xml_node;
-void read_xml(std::string const & filename, xml_node &node);
-void read_xml_string(std::string const & str, xml_node &node, std::string const & base_path="");
+class MAPNIK_DECL xml_node;
+MAPNIK_DECL void read_xml(std::string const & filename, xml_node &node);
+MAPNIK_DECL void read_xml_string(std::string const & str, xml_node &node, std::string const & base_path="");
 }
 
 #endif // MAPNIK_LIBXML2_LOADER_HPP

include/mapnik/xml_node.hpp

@@ -23,6 +23,9 @@
 #ifndef MAPNIK_XML_NODE_H
 #define MAPNIK_XML_NODE_H
 
+//mapnik
+#include <mapnik/config.hpp> // for MAPNIK_DECL
+
 //boost
 #include <boost/optional.hpp>
 
@@ -34,17 +37,17 @@
 
 namespace mapnik
 {
-class xml_tree;
+class MAPNIK_DECL xml_tree;
 
-class xml_attribute
+class MAPNIK_DECL xml_attribute
 {
 public:
     xml_attribute(const char * value_);
     std::string value;
     mutable bool processed;
 };
 
-class node_not_found: public std::exception
+class MAPNIK_DECL node_not_found: public std::exception
 {
 public:
     node_not_found(std::string const& node_name);
@@ -56,7 +59,7 @@ class node_not_found: public std::exception
     mutable std::string msg_;
 };
 
-class attribute_not_found: public std::exception
+class MAPNIK_DECL attribute_not_found: public std::exception
 {
 public:
     attribute_not_found(std::string const& node_name, std::string const& attribute_name);
@@ -69,7 +72,7 @@ class attribute_not_found: public std::exception
     mutable std::string msg_;
 };
 
-class more_than_one_child: public std::exception
+class MAPNIK_DECL more_than_one_child: public std::exception
 {
 public:
     more_than_one_child(std::string const& node_name);
@@ -81,7 +84,7 @@ class more_than_one_child: public std::exception
     mutable std::string msg_;
 };
 
-class xml_node
+class MAPNIK_DECL xml_node
 {
 public:
     using const_iterator = std::list<xml_node>::const_iterator;

include/mapnik/xml_tree.hpp

@@ -33,7 +33,7 @@
 namespace mapnik
 {
 
-class xml_tree
+class MAPNIK_DECL xml_tree
 {
 public:
     xml_tree(std::string const& encoding="utf8");

src/rapidxml_loader.cpp

@@ -138,7 +138,11 @@ class rapidxml_loader : util::noncopyable
         {
             if (cur_node->value_size() > 0) // Don't add empty text nodes
             {
-                node.add_child(cur_node->value(), 0, true);
+                // parsed text values should have leading and trailing
+                // whitespace trimmed.
+                std::string trimmed = cur_node->value();
+                mapnik::util::trim(trimmed);
+                node.add_child(trimmed.c_str(), 0, true);
             }
         }
         break;

test/unit/serialization/xml_parser_trim.cpp

@@ -0,0 +1,44 @@
+#include "catch.hpp"
+
+#include <mapnik/debug.hpp>
+#include <mapnik/value.hpp>
+#include <mapnik/xml_tree.hpp>
+#include <mapnik/xml_loader.hpp>
+
+TEST_CASE("xml parser") {
+
+  SECTION("trims whitespace") {
+
+    // simple and non-valid mapnik XML reduced from the empty_parameter2.xml
+    // test case. this is to check that the xml parsing routine is trimming
+    // whitespace from text nodes as part of the parsing operation.
+    const std::string xml("<Map>"
+                          "  <Layer>"
+                          "    <Datasource>"
+                          "      <Parameter name=\"empty\"><![CDATA[ ]]></Parameter>"
+                          "    </Datasource>"
+                          "  </Layer>"
+                          "</Map>");
+
+    mapnik::xml_tree tree("utf8");
+    tree.set_filename("xml_datasource_parameter_trim.cpp");
+    REQUIRE_NOTHROW(read_xml_string(xml, tree.root(), ""));
+
+    REQUIRE(tree.root().has_child("Map"));
+    mapnik::xml_node const &map = tree.root().get_child("Map");
+
+    REQUIRE(map.has_child("Layer"));
+    mapnik::xml_node const &layer = map.get_child("Layer");
+
+    REQUIRE(layer.has_child("Datasource"));
+    mapnik::xml_node const &datasource = layer.get_child("Datasource");
+
+    REQUIRE(datasource.has_child("Parameter"));
+    mapnik::xml_node const &parameter = datasource.get_child("Parameter");
+
+    // parser should call mapnik::util::trim on the text content and
+    // this should result in an empty text string in the parameter.
+    REQUIRE(parameter.get_text() == "");
+  }
+}
+