sij/mapnik
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Enforce consistent error handling policy - always throw on fatal errors (both strict and non-strict). In strict mode throw on first parsing error. Remove return values from parse,parse_from_string and traverse_tree methods. Update unit tests.

Browse source
This commit is contained in:
artemp 2017-07-31 09:18:12 +01:00
parent 7f4adc2d73
commit a6230559f1
4 changed files with 122 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
include/mapnik/svg/svg_parser.hpp
View file

@ -69,8 +69,8 @@ public:
explicit svg_parser(svg_converter_type & path, bool strict = false); explicit svg_parser(svg_converter_type & path, bool strict = false);
~svg_parser(); ~svg_parser();
error_handler & err_handler(); error_handler & err_handler();
bool parse(std::string const& filename); void parse(std::string const& filename);
bool parse_from_string(std::string const& svg); void parse_from_string(std::string const& svg);
svg_converter_type & path_; svg_converter_type & path_;
bool is_defs_; bool is_defs_;
bool strict_; bool strict_;

9
src/marker_cache.cpp
View file

@ -175,14 +175,14 @@ std::shared_ptr<mapnik::marker const> marker_cache::find(std::string const& uri,
svg_path_adapter svg_path(stl_storage); svg_path_adapter svg_path(stl_storage);
svg_converter_type svg(svg_path, marker_path->attributes()); svg_converter_type svg(svg_path, marker_path->attributes());
svg_parser p(svg, strict); svg_parser p(svg, strict);
p.parse_from_string(known_svg_string);
if (!p.parse_from_string(known_svg_string) && !strict) if (!strict)
{ {
for (auto const& msg : p.err_handler().error_messages()) for (auto const& msg : p.err_handler().error_messages())
{ {
MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\""; MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\"";
} }
//return std::make_shared<mapnik::marker const>(mapnik::marker_null());
} }
//svg.arrange_orientations(); //svg.arrange_orientations();
double lox,loy,hix,hiy; double lox,loy,hix,hiy;
@ -215,15 +215,14 @@ std::shared_ptr<mapnik::marker const> marker_cache::find(std::string const& uri,
svg_path_adapter svg_path(stl_storage); svg_path_adapter svg_path(stl_storage);
svg_converter_type svg(svg_path, marker_path->attributes()); svg_converter_type svg(svg_path, marker_path->attributes());
svg_parser p(svg, strict); svg_parser p(svg, strict);
p.parse(uri);
if (!strict)
if (!p.parse(uri) && !strict)
{ {
for (auto const& msg : p.err_handler().error_messages()) for (auto const& msg : p.err_handler().error_messages())
{ {
MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\""; MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\"";
} }
//return std::make_shared<mapnik::marker const>(mapnik::marker_null());
} }
//svg.arrange_orientations(); //svg.arrange_orientations();
double lox,loy,hix,hiy; double lox,loy,hix,hiy;

20
src/svg/svg_parser.cpp
View file

@ -81,7 +81,7 @@ namespace mapnik { namespace svg {
namespace rapidxml = boost::property_tree::detail::rapidxml; namespace rapidxml = boost::property_tree::detail::rapidxml;
bool traverse_tree(svg_parser& parser, rapidxml::xml_node<char> const* node); void traverse_tree(svg_parser& parser, rapidxml::xml_node<char> const* node);
void end_element(svg_parser& parser, rapidxml::xml_node<char> const* node); void end_element(svg_parser& parser, rapidxml::xml_node<char> const* node);
void parse_path(svg_parser& parser, rapidxml::xml_node<char> const* node); void parse_path(svg_parser& parser, rapidxml::xml_node<char> const* node);
void parse_element(svg_parser& parser, char const* name, rapidxml::xml_node<char> const* node); void parse_element(svg_parser& parser, char const* name, rapidxml::xml_node<char> const* node);
@ -386,7 +386,7 @@ std::pair<unsigned,bool> parse_preserve_aspect_ratio(T & err_handler, char const
} }
bool traverse_tree(svg_parser & parser, rapidxml::xml_node<char> const* node) void traverse_tree(svg_parser & parser, rapidxml::xml_node<char> const* node)
{ {
auto const* name = node->name(); auto const* name = node->name();
switch (node->type()) switch (node->type())
@ -482,7 +482,6 @@ bool traverse_tree(svg_parser & parser, rapidxml::xml_node<char> const* node)
default: default:
break; break;
} }
return true;
} }
@ -1422,7 +1421,7 @@ svg_parser::svg_parser(svg_converter<svg_path_adapter,
svg_parser::~svg_parser() {} svg_parser::~svg_parser() {}
bool svg_parser::parse(std::string const& filename) void svg_parser::parse(std::string const& filename)
{ {
#ifdef _WINDOWS #ifdef _WINDOWS
std::basic_ifstream<char> stream(mapnik::utf8_to_utf16(filename)); std::basic_ifstream<char> stream(mapnik::utf8_to_utf16(filename));
@ -1433,8 +1432,7 @@ bool svg_parser::parse(std::string const& filename)
{ {
std::stringstream ss; std::stringstream ss;
ss << "Unable to open '" << filename << "'"; ss << "Unable to open '" << filename << "'";
err_handler_.on_error(ss.str()); throw std::runtime_error(ss.str());
return false;
} }
stream.unsetf(std::ios::skipws); stream.unsetf(std::ios::skipws);
@ -1452,8 +1450,7 @@ bool svg_parser::parse(std::string const& filename)
{ {
std::stringstream ss; std::stringstream ss;
ss << "svg_parser::parse - Unable to parse '" << filename << "'"; ss << "svg_parser::parse - Unable to parse '" << filename << "'";
err_handler_.on_error(ss.str()); throw std::runtime_error(ss.str());
return false;
} }
for (rapidxml::xml_node<char> const* child = doc.first_node(); for (rapidxml::xml_node<char> const* child = doc.first_node();
@ -1461,10 +1458,9 @@ bool svg_parser::parse(std::string const& filename)
{ {
traverse_tree(*this, child); traverse_tree(*this, child);
} }
return err_handler_.error_messages().empty() ? true : false;
} }
bool svg_parser::parse_from_string(std::string const& svg) void svg_parser::parse_from_string(std::string const& svg)
{ {
const int flags = rapidxml::parse_trim_whitespace | rapidxml::parse_validate_closing_tags; const int flags = rapidxml::parse_trim_whitespace | rapidxml::parse_validate_closing_tags;
rapidxml::xml_document<> doc; rapidxml::xml_document<> doc;
@ -1478,15 +1474,13 @@ bool svg_parser::parse_from_string(std::string const& svg)
{ {
std::stringstream ss; std::stringstream ss;
ss << "Unable to parse '" << svg << "'"; ss << "Unable to parse '" << svg << "'";
err_handler_.on_error(ss.str()); throw std::runtime_error(ss.str());
return false;
} }
for (rapidxml::xml_node<char> const* child = doc.first_node(); for (rapidxml::xml_node<char> const* child = doc.first_node();
child; child = child->next_sibling()) child; child = child->next_sibling())
{ {
traverse_tree(*this, child); traverse_tree(*this, child);
} }
return err_handler_.error_messages().empty() ? true : false;
} }
svg_parser::error_handler & svg_parser::err_handler() svg_parser::error_handler & svg_parser::err_handler()

134
test/unit/svg/svg_parser_test.cpp
View file

@ -44,11 +44,11 @@ namespace // internal
mapnik::svg::svg_converter_type svg; mapnik::svg::svg_converter_type svg;
mapnik::svg::svg_parser p; mapnik::svg::svg_parser p;
test_parser() explicit test_parser(bool strict = false)
: stl_storage(path.source()) : stl_storage(path.source())
, svg_path(stl_storage) , svg_path(stl_storage)
, svg(svg_path, path.attributes()) , svg(svg_path, path.attributes())
, p(svg) , p(svg, strict)
{} {}
mapnik::svg::svg_parser* operator->() mapnik::svg::svg_parser* operator->()
@ -91,8 +91,14 @@ TEST_CASE("SVG parser") {
}; };
test_parser p; test_parser p;
REQUIRE(!p->parse(svg_name)); try
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); {
p->parse(svg_name);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == join(expected_errors));
}
} }
SECTION("SVG::parse_from_string syntax error") SECTION("SVG::parse_from_string syntax error")
@ -108,8 +114,14 @@ TEST_CASE("SVG parser") {
std::istreambuf_iterator<char>()); std::istreambuf_iterator<char>());
test_parser p; test_parser p;
REQUIRE(!p->parse_from_string(svg_str)); try
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); {
p->parse_from_string(svg_str);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == join(expected_errors));
}
} }
SECTION("SVG::parse_from_string syntax error") SECTION("SVG::parse_from_string syntax error")
@ -121,8 +133,14 @@ TEST_CASE("SVG parser") {
}; };
test_parser p; test_parser p;
REQUIRE(!p->parse(svg_name)); try
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); {
p->parse(svg_name);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == join(expected_errors));
}
} }
SECTION("SVG parser color <fail>") SECTION("SVG parser color <fail>")
@ -140,9 +158,22 @@ TEST_CASE("SVG parser") {
std::string svg_str((std::istreambuf_iterator<char>(in)), std::string svg_str((std::istreambuf_iterator<char>(in)),
std::istreambuf_iterator<char>()); std::istreambuf_iterator<char>());
test_parser p; {
REQUIRE(!p->parse_from_string(svg_str)); test_parser p;
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); p->parse_from_string(svg_str);
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors));
}
{
test_parser p(true);
try
{
p->parse_from_string(svg_str);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == std::string(expected_errors[0]));
}
}
} }
SECTION("SVG - cope with erroneous geometries") SECTION("SVG - cope with erroneous geometries")
@ -170,9 +201,24 @@ TEST_CASE("SVG parser") {
std::string svg_str((std::istreambuf_iterator<char>(in)), std::string svg_str((std::istreambuf_iterator<char>(in)),
std::istreambuf_iterator<char>()); std::istreambuf_iterator<char>());
test_parser p; {
REQUIRE(!p->parse_from_string(svg_str)); test_parser p;
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); p->parse_from_string(svg_str);
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors));
}
{
// strict
test_parser p(true);
try
{
p->parse_from_string(svg_str);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == std::string(expected_errors[0]));
}
}
} }
SECTION("SVG parser double % <fail>") SECTION("SVG parser double % <fail>")
@ -188,9 +234,22 @@ TEST_CASE("SVG parser") {
std::string svg_str((std::istreambuf_iterator<char>(in)), std::string svg_str((std::istreambuf_iterator<char>(in)),
std::istreambuf_iterator<char>()); std::istreambuf_iterator<char>());
test_parser p; {
REQUIRE(!p->parse_from_string(svg_str)); test_parser p;
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); p->parse_from_string(svg_str);
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors));
}
{
test_parser p(true);
try
{
p->parse_from_string(svg_str);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == std::string(expected_errors[0]));
}
}
} }
SECTION("SVG parser display=none") SECTION("SVG parser display=none")
@ -327,7 +386,7 @@ TEST_CASE("SVG parser") {
std::string svg_str((std::istreambuf_iterator<char>(in)), std::string svg_str((std::istreambuf_iterator<char>(in)),
std::istreambuf_iterator<char>()); std::istreambuf_iterator<char>());
test_parser p; test_parser p;
REQUIRE(p->parse_from_string(svg_str)); p->parse_from_string(svg_str);
auto width = p.svg.width(); auto width = p.svg.width();
auto height = p.svg.height(); auto height = p.svg.height();
REQUIRE(width == 100); REQUIRE(width == 100);
@ -610,10 +669,22 @@ TEST_CASE("SVG parser") {
"Failed to find gradient fill: MyGradient", "Failed to find gradient fill: MyGradient",
"Failed to find gradient stroke: MyGradient", "Failed to find gradient stroke: MyGradient",
}; };
{
test_parser p; test_parser p;
REQUIRE(!p->parse(svg_name)); p->parse(svg_name);
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors));
}
{
test_parser p(true);
try
{
p->parse(svg_name);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == std::string(expected_errors[0]));
}
}
} }
SECTION("SVG missing <gradient> id") SECTION("SVG missing <gradient> id")
@ -629,9 +700,22 @@ TEST_CASE("SVG parser") {
std::string svg_str((std::istreambuf_iterator<char>(in)), std::string svg_str((std::istreambuf_iterator<char>(in)),
std::istreambuf_iterator<char>()); std::istreambuf_iterator<char>());
test_parser p; {
REQUIRE(!p->parse_from_string(svg_str)); test_parser p;
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); p->parse_from_string(svg_str);
REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors));
}
{
test_parser p(true);
try
{
p->parse_from_string(svg_str);
}
catch (std::exception const& ex)
{
REQUIRE(ex.what() == std::string(expected_errors[0]));
}
}
} }
SECTION("SVG missing <gradient> inheritance") SECTION("SVG missing <gradient> inheritance")