diff --git a/include/mapnik/svg/svg_parser.hpp b/include/mapnik/svg/svg_parser.hpp index 7a8e88f91..fab0b8e19 100644 --- a/include/mapnik/svg/svg_parser.hpp +++ b/include/mapnik/svg/svg_parser.hpp @@ -69,8 +69,8 @@ public: explicit svg_parser(svg_converter_type & path, bool strict = false); ~svg_parser(); error_handler & err_handler(); - bool parse(std::string const& filename); - bool parse_from_string(std::string const& svg); + void parse(std::string const& filename); + void parse_from_string(std::string const& svg); svg_converter_type & path_; bool is_defs_; bool strict_; diff --git a/src/marker_cache.cpp b/src/marker_cache.cpp index 2e223ec90..d7ea7facb 100644 --- a/src/marker_cache.cpp +++ b/src/marker_cache.cpp @@ -175,14 +175,14 @@ std::shared_ptr marker_cache::find(std::string const& uri, svg_path_adapter svg_path(stl_storage); svg_converter_type svg(svg_path, marker_path->attributes()); 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()) { MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\""; } - //return std::make_shared(mapnik::marker_null()); } //svg.arrange_orientations(); double lox,loy,hix,hiy; @@ -215,15 +215,14 @@ std::shared_ptr marker_cache::find(std::string const& uri, svg_path_adapter svg_path(stl_storage); svg_converter_type svg(svg_path, marker_path->attributes()); svg_parser p(svg, strict); + p.parse(uri); - - if (!p.parse(uri) && !strict) + if (!strict) { for (auto const& msg : p.err_handler().error_messages()) { MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\""; } - //return std::make_shared(mapnik::marker_null()); } //svg.arrange_orientations(); double lox,loy,hix,hiy; diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp index d6082d58c..5ecab6740 100644 --- a/src/svg/svg_parser.cpp +++ b/src/svg/svg_parser.cpp @@ -81,7 +81,7 @@ namespace mapnik { namespace svg { namespace rapidxml = boost::property_tree::detail::rapidxml; -bool traverse_tree(svg_parser& parser, rapidxml::xml_node const* node); +void traverse_tree(svg_parser& parser, rapidxml::xml_node const* node); void end_element(svg_parser& parser, rapidxml::xml_node const* node); void parse_path(svg_parser& parser, rapidxml::xml_node const* node); void parse_element(svg_parser& parser, char const* name, rapidxml::xml_node const* node); @@ -386,7 +386,7 @@ std::pair parse_preserve_aspect_ratio(T & err_handler, char const } -bool traverse_tree(svg_parser & parser, rapidxml::xml_node const* node) +void traverse_tree(svg_parser & parser, rapidxml::xml_node const* node) { auto const* name = node->name(); switch (node->type()) @@ -482,7 +482,6 @@ bool traverse_tree(svg_parser & parser, rapidxml::xml_node const* node) default: break; } - return true; } @@ -1422,7 +1421,7 @@ svg_parser::svg_parser(svg_converter stream(mapnik::utf8_to_utf16(filename)); @@ -1433,8 +1432,7 @@ bool svg_parser::parse(std::string const& filename) { std::stringstream ss; ss << "Unable to open '" << filename << "'"; - err_handler_.on_error(ss.str()); - return false; + throw std::runtime_error(ss.str()); } stream.unsetf(std::ios::skipws); @@ -1452,8 +1450,7 @@ bool svg_parser::parse(std::string const& filename) { std::stringstream ss; ss << "svg_parser::parse - Unable to parse '" << filename << "'"; - err_handler_.on_error(ss.str()); - return false; + throw std::runtime_error(ss.str()); } for (rapidxml::xml_node const* child = doc.first_node(); @@ -1461,10 +1458,9 @@ bool svg_parser::parse(std::string const& filename) { 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; rapidxml::xml_document<> doc; @@ -1478,15 +1474,13 @@ bool svg_parser::parse_from_string(std::string const& svg) { std::stringstream ss; ss << "Unable to parse '" << svg << "'"; - err_handler_.on_error(ss.str()); - return false; + throw std::runtime_error(ss.str()); } for (rapidxml::xml_node const* child = doc.first_node(); child; child = child->next_sibling()) { traverse_tree(*this, child); } - return err_handler_.error_messages().empty() ? true : false; } svg_parser::error_handler & svg_parser::err_handler() diff --git a/test/unit/svg/svg_parser_test.cpp b/test/unit/svg/svg_parser_test.cpp index d076909bb..b49e3d7d4 100644 --- a/test/unit/svg/svg_parser_test.cpp +++ b/test/unit/svg/svg_parser_test.cpp @@ -44,11 +44,11 @@ namespace // internal mapnik::svg::svg_converter_type svg; mapnik::svg::svg_parser p; - test_parser() + explicit test_parser(bool strict = false) : stl_storage(path.source()) , svg_path(stl_storage) , svg(svg_path, path.attributes()) - , p(svg) + , p(svg, strict) {} mapnik::svg::svg_parser* operator->() @@ -91,8 +91,14 @@ TEST_CASE("SVG parser") { }; test_parser p; - REQUIRE(!p->parse(svg_name)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + try + { + p->parse(svg_name); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == join(expected_errors)); + } } SECTION("SVG::parse_from_string syntax error") @@ -108,8 +114,14 @@ TEST_CASE("SVG parser") { std::istreambuf_iterator()); test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + try + { + p->parse_from_string(svg_str); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == join(expected_errors)); + } } SECTION("SVG::parse_from_string syntax error") @@ -121,8 +133,14 @@ TEST_CASE("SVG parser") { }; test_parser p; - REQUIRE(!p->parse(svg_name)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + try + { + p->parse(svg_name); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == join(expected_errors)); + } } SECTION("SVG parser color ") @@ -140,9 +158,22 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + 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") @@ -170,9 +201,24 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + 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 % ") @@ -188,9 +234,22 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + 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") @@ -327,7 +386,7 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); test_parser p; - REQUIRE(p->parse_from_string(svg_str)); + p->parse_from_string(svg_str); auto width = p.svg.width(); auto height = p.svg.height(); REQUIRE(width == 100); @@ -610,10 +669,22 @@ TEST_CASE("SVG parser") { "Failed to find gradient fill: MyGradient", "Failed to find gradient stroke: MyGradient", }; - - test_parser p; - REQUIRE(!p->parse(svg_name)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + p->parse(svg_name); + 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 id") @@ -629,9 +700,22 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + 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 inheritance")