From 5616d406b4725f2230abb5f455d17fcb7e3160d6 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sun, 31 May 2015 01:42:02 +0100 Subject: [PATCH 1/4] Trim whitespace in rapidxml parser text nodes. The libxml parser already trims text nodes, which leads to a difference in behaviour when reading map style XML. This patch normalises the behaviour between the two. --- src/rapidxml_loader.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rapidxml_loader.cpp b/src/rapidxml_loader.cpp index d380f0e88..97bcc503b 100644 --- a/src/rapidxml_loader.cpp +++ b/src/rapidxml_loader.cpp @@ -138,7 +138,11 @@ private: { 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; From b21ed59190064808902601694d7ee3abc7a4e973 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Sun, 31 May 2015 01:44:43 +0100 Subject: [PATCH 2/4] Add test for XML parser whitespace trimming behaviour. This tests the at the XML parser trims whitespace from XML text nodes. This was already the behaviour of the libxml2 parser, but not the rapidxml parser. Note that this makes #2878 pass for the rapidxml parser as well as the libxml2 parser. It seems that Travis uses the rapidxml parser only. --- test/unit/serialization/xml_parser_trim.cpp | 43 +++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 test/unit/serialization/xml_parser_trim.cpp diff --git a/test/unit/serialization/xml_parser_trim.cpp b/test/unit/serialization/xml_parser_trim.cpp new file mode 100644 index 000000000..cc95a20ad --- /dev/null +++ b/test/unit/serialization/xml_parser_trim.cpp @@ -0,0 +1,43 @@ +#include "catch.hpp" + +#include +#include +#include + +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("" + " " + " " + " " + " " + " " + ""); + + 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 ¶meter = 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() == ""); + } +} + From dd112b33e2222dc68ac72c1dc6aa6f44354bad4f Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Tue, 2 Jun 2015 18:10:59 +0100 Subject: [PATCH 3/4] Added visibility attribute to XML symbols. Thanks to @flippmoke, symbol visibility is exactly the cause of the previous compilation error. Until it's decided whether or not to turn this off (at least for testing & coverage builds?), I've added the necessary annotations to allow it to compile. --- include/mapnik/xml_loader.hpp | 6 +++--- include/mapnik/xml_node.hpp | 12 ++++++------ include/mapnik/xml_tree.hpp | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/mapnik/xml_loader.hpp b/include/mapnik/xml_loader.hpp index 2bc8f4af3..4126a3acd 100644 --- a/include/mapnik/xml_loader.hpp +++ b/include/mapnik/xml_loader.hpp @@ -28,9 +28,9 @@ 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 diff --git a/include/mapnik/xml_node.hpp b/include/mapnik/xml_node.hpp index 28be9ff0c..8020e511e 100644 --- a/include/mapnik/xml_node.hpp +++ b/include/mapnik/xml_node.hpp @@ -34,9 +34,9 @@ namespace mapnik { -class xml_tree; +class MAPNIK_DECL xml_tree; -class xml_attribute +class MAPNIK_DECL xml_attribute { public: xml_attribute(const char * value_); @@ -44,7 +44,7 @@ public: 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 +56,7 @@ protected: 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 +69,7 @@ protected: 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 +81,7 @@ protected: mutable std::string msg_; }; -class xml_node +class MAPNIK_DECL xml_node { public: using const_iterator = std::list::const_iterator; diff --git a/include/mapnik/xml_tree.hpp b/include/mapnik/xml_tree.hpp index 9cb29a8cc..f13cd4170 100644 --- a/include/mapnik/xml_tree.hpp +++ b/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"); From 1778d19b2283a1b08e40e5eadf5fa792d170567d Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Wed, 3 Jun 2015 15:56:37 +0100 Subject: [PATCH 4/4] Added headers required on Mac OS - not sure why not on Linux. --- include/mapnik/xml_loader.hpp | 3 +++ include/mapnik/xml_node.hpp | 3 +++ test/unit/serialization/xml_parser_trim.cpp | 1 + 3 files changed, 7 insertions(+) diff --git a/include/mapnik/xml_loader.hpp b/include/mapnik/xml_loader.hpp index 4126a3acd..1991fa685 100644 --- a/include/mapnik/xml_loader.hpp +++ b/include/mapnik/xml_loader.hpp @@ -23,6 +23,9 @@ #ifndef MAPNIK_LIBXML2_LOADER_HPP #define MAPNIK_LIBXML2_LOADER_HPP +// mapnik +#include // for MAPNIK_DECL + // stl #include diff --git a/include/mapnik/xml_node.hpp b/include/mapnik/xml_node.hpp index 8020e511e..106bbc673 100644 --- a/include/mapnik/xml_node.hpp +++ b/include/mapnik/xml_node.hpp @@ -23,6 +23,9 @@ #ifndef MAPNIK_XML_NODE_H #define MAPNIK_XML_NODE_H +//mapnik +#include // for MAPNIK_DECL + //boost #include diff --git a/test/unit/serialization/xml_parser_trim.cpp b/test/unit/serialization/xml_parser_trim.cpp index cc95a20ad..250a23757 100644 --- a/test/unit/serialization/xml_parser_trim.cpp +++ b/test/unit/serialization/xml_parser_trim.cpp @@ -1,6 +1,7 @@ #include "catch.hpp" #include +#include #include #include