diff --git a/.travis.yml b/.travis.yml index 0dcfac089..4be27fe6f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ sudo: false git: depth: 10 - submodules: true + submodules: false env: global: @@ -43,6 +43,15 @@ before_install: - export MASON_PUBLISH=${MASON_PUBLISH:-false} - if [[ ${TRAVIS_BRANCH} != 'master' ]]; then export MASON_PUBLISH=false; fi - if [[ ${TRAVIS_PULL_REQUEST} != 'false' ]]; then export MASON_PUBLISH=false; fi + - git submodule update --init --depth=10 || + git submodule foreach 'test "$sha1" = "`git rev-parse HEAD`" || + git ls-remote origin "refs/pull/*/head" | + while read hash ref; do + if test "$hash" = "$sha1"; then + git config --add remote.origin.fetch "+$ref:$ref"; + fi + done' + - git submodule update --init --depth=10 install: - if [[ $(uname -s) == 'Linux' ]]; then diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp index e979e3f7b..6613d5cde 100644 --- a/src/svg/svg_parser.cpp +++ b/src/svg/svg_parser.cpp @@ -137,29 +137,36 @@ double parse_double(T & error_messages, const char* str) template double parse_svg_value(T & error_messages, const char* str, bool & percent) { - using namespace boost::spirit::qi; using skip_type = boost::spirit::ascii::space_type; using boost::phoenix::ref; - double_type double_; - char_type char_; - _1_type _1; + qi::double_type double_; + qi::lit_type lit; + qi::_1_type _1; double val = 0.0; - symbols units; + qi::symbols units; units.add + ("px", 1.0) ("pt", DPI/72.0) ("pc", DPI/6.0) ("mm", DPI/25.4) ("cm", DPI/2.54) ("in", (double)DPI) ; - if (!phrase_parse(str, str + std::strlen(str), - double_[ref(val) = _1, ref(percent) = false] + const char* cur = str; // phrase_parse modifies the first iterator + const char* end = str + std::strlen(str); + if (!qi::phrase_parse(cur, end, + double_[ref(val) = _1][ref(percent) = false] > - (units[ ref(val) *= _1] | - char_('%')[ref(val) *= 0.01, ref(percent) = true]), + lit('%')[ref(val) *= 0.01][ref(percent) = true]), skip_type())) { - error_messages.emplace_back("Failed to parse SVG value: \"" + std::string(str) + "\""); + error_messages.emplace_back("Failed to parse SVG value: '" + std::string(str) + "'"); + } + else if (cur != end) + { + error_messages.emplace_back("Failed to parse SVG value: '" + std::string(str) + + "', trailing garbage: '" + cur + "'"); } return val; } diff --git a/test/data b/test/data index 24844dead..e4178f3ce 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 24844dead6d23fd541aba147b69703a605fc7351 +Subproject commit e4178f3ce9798f93024a49d6cd0a87d98ec0f6b0 diff --git a/test/unit/svg/svg_parser_test.cpp b/test/unit/svg/svg_parser_test.cpp index e0bfd9ecc..3fee6a452 100644 --- a/test/unit/svg/svg_parser_test.cpp +++ b/test/unit/svg/svg_parser_test.cpp @@ -34,6 +34,42 @@ #include #include +namespace // internal +{ + struct test_parser + { + mapnik::svg_storage_type path; + mapnik::svg::vertex_stl_adapter stl_storage; + mapnik::svg::svg_path_adapter svg_path; + mapnik::svg::svg_converter_type svg; + mapnik::svg::svg_parser p; + + test_parser() + : stl_storage(path.source()) + , svg_path(stl_storage) + , svg(svg_path, path.attributes()) + , p(svg) + {} + + mapnik::svg::svg_parser* operator->() + { + return &p; + } + }; + + template + std::string join(C const& container) + { + std::string result; + for (auto const& str : container) + { + if (!result.empty()) result += "\n "; + result += str; + } + return result; + } +} + TEST_CASE("SVG parser") { SECTION("SVG i/o") @@ -49,142 +85,112 @@ TEST_CASE("SVG parser") { SECTION("SVG::parse i/o") { std::string svg_name("FAIL"); - - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse(svg_name)) + char const* expected_errors[] = { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "Unable to open 'FAIL'"); - } + "Unable to open 'FAIL'" + }; + + test_parser p; + REQUIRE(!p->parse(svg_name)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG::parse_from_string syntax error") { std::string svg_name("./test/data/svg/invalid.svg"); + char const* expected_errors[] = + { + "Unable to parse '\n\n'" + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "Unable to parse '\n\n'"); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG::parse_from_string syntax error") { std::string svg_name("./test/data/svg/invalid.svg"); - - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse(svg_name)) + char const* expected_errors[] = { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "svg_parser::parse - Unable to parse './test/data/svg/invalid.svg'"); - } + "svg_parser::parse - Unable to parse './test/data/svg/invalid.svg'" + }; + + test_parser p; + REQUIRE(!p->parse(svg_name)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG parser color ") { std::string svg_name("./test/data/svg/color_fail.svg"); + char const* expected_errors[] = + { + "Failed to parse color: \"fail\"", + "Failed to parse SVG value: 'fail'", + "Failed to parse color: \"fail\"", + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 3); - REQUIRE(errors[0] == "Failed to parse color: \"fail\""); - REQUIRE(errors[1] == "Failed to parse SVG value: \"fail\""); - REQUIRE(errors[2] == "Failed to parse color: \"fail\""); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG - cope with erroneous geometries") { std::string svg_name("./test/data/svg/errors.svg"); + char const* expected_errors[] = + { + "parse_rect: Invalid width", + "Failed to parse SVG value: 'FAIL'", + "parse_rect: Invalid height", + "parse_rect: Invalid rx", + "parse_rect: Invalid ry", + "Failed to parse SVG value: '100invalidunit', trailing garbage: 'validunit'", + "unable to parse invalid svg ", + "unable to parse invalid svg with id 'fail-path'", + "unable to parse invalid svg with id 'fail-path'", + "parse_circle: Invalid radius", + "Failed to parse 'points'", + "Failed to parse 'points'", + "parse_ellipse: Invalid rx", + "parse_ellipse: Invalid ry", + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 13); - REQUIRE(errors[0] == "parse_rect: Invalid width"); - REQUIRE(errors[1] == "Failed to parse SVG value: \"FAIL\""); - REQUIRE(errors[2] == "parse_rect: Invalid height"); - REQUIRE(errors[3] == "parse_rect: Invalid rx"); - REQUIRE(errors[4] == "parse_rect: Invalid ry"); - REQUIRE(errors[5] == "unable to parse invalid svg "); - REQUIRE(errors[6] == "unable to parse invalid svg with id 'fail-path'"); - REQUIRE(errors[7] == "unable to parse invalid svg with id 'fail-path'"); - REQUIRE(errors[8] == "parse_circle: Invalid radius"); - REQUIRE(errors[9] == "Failed to parse 'points'"); - REQUIRE(errors[10] == "Failed to parse 'points'"); - REQUIRE(errors[11] == "parse_ellipse: Invalid rx"); - REQUIRE(errors[12] == "parse_ellipse: Invalid ry"); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG parser double % ") { std::string svg_name("./test/data/svg/gradient-radial-error.svg"); + char const* expected_errors[] = + { + "Failed to parse SVG value: 'FAIL'" + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "Failed to parse SVG value: \"FAIL\""); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG parser display=none") @@ -320,16 +326,10 @@ TEST_CASE("SVG parser") { std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - p.parse_from_string(svg_str); - auto width = svg.width(); - auto height = svg.height(); + test_parser p; + REQUIRE(p->parse_from_string(svg_str)); + auto width = p.svg.width(); + auto height = p.svg.height(); REQUIRE(width == 100); REQUIRE(height == 100); } @@ -604,41 +604,33 @@ TEST_CASE("SVG parser") { SECTION("SVG missing def") { std::string svg_name("./test/data/svg/gradient-nodef.svg"); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - REQUIRE(!p.parse(svg_name)); - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 2); - REQUIRE(errors[0] == "Failed to find gradient fill: MyGradient"); - REQUIRE(errors[1] == "Failed to find gradient stroke: MyGradient"); + char const* expected_errors[] = + { + "Failed to find gradient fill: MyGradient", + "Failed to find gradient stroke: MyGradient", + }; + + test_parser p; + REQUIRE(!p->parse(svg_name)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG missing id") { - std::string svg_name("./test/data/svg/gradient-no-id.svg"); + char const* expected_errors[] = + { + "Failed to find gradient fill: MyGradient", + "Failed to find gradient stroke: MyGradient", + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 2); - REQUIRE(errors[0] == "Failed to find gradient fill: MyGradient"); - REQUIRE(errors[1] == "Failed to find gradient stroke: MyGradient"); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG missing inheritance")