From 401dfacfc5e87670a4391022805b999b52d19031 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Fri, 5 Feb 2016 00:39:35 +0100 Subject: [PATCH 1/4] test/image_filter: prevent SIGSEGV when parsing fails --- test/unit/imaging/image_filter.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/unit/imaging/image_filter.cpp b/test/unit/imaging/image_filter.cpp index 4ee5b1a72..2b43c4d2d 100644 --- a/test/unit/imaging/image_filter.cpp +++ b/test/unit/imaging/image_filter.cpp @@ -434,8 +434,11 @@ SECTION("test colorize-alpha - parsing correct input") { std::string s("colorize-alpha(#0000ff 0%, #00ff00 100%)"); std::vector f; - CHECK(parse_image_filters(s, f)); + REQUIRE(parse_image_filters(s, f)); mapnik::filter::colorize_alpha const & ca = mapnik::util::get(f.front()); + CHECK(ca.size() == 2); + + CHECKED_IF(ca.size() > 0) { mapnik::filter::color_stop const & s2 = ca[0]; CHECK( s2.color.alpha() == 0xff ); @@ -445,6 +448,7 @@ SECTION("test colorize-alpha - parsing correct input") { CHECK( s2.offset == 0.0 ); } + CHECKED_IF(ca.size() > 1) { mapnik::filter::color_stop const & s2 = ca[1]; CHECK( s2.color.alpha() == 0xff ); From 959d4ded23bca4942f8b1453359eb4fa68091a27 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sun, 31 Jan 2016 02:40:25 +0100 Subject: [PATCH 2/4] image_filter_grammar: rewrite using Nabialek trick --- include/mapnik/image_filter_grammar.hpp | 30 ++-- include/mapnik/image_filter_grammar_impl.hpp | 151 +++++++++---------- 2 files changed, 93 insertions(+), 88 deletions(-) diff --git a/include/mapnik/image_filter_grammar.hpp b/include/mapnik/image_filter_grammar.hpp index 176c84e02..7ab2d121d 100644 --- a/include/mapnik/image_filter_grammar.hpp +++ b/include/mapnik/image_filter_grammar.hpp @@ -20,8 +20,8 @@ * *****************************************************************************/ -#ifndef MAPNIK_IMAGE_FILITER_GRAMMAR_HPP -#define MAPNIK_IMAGE_FILITER_GRAMMAR_HPP +#ifndef MAPNIK_IMAGE_FILTER_GRAMMAR_HPP +#define MAPNIK_IMAGE_FILTER_GRAMMAR_HPP // mapnik #include @@ -66,21 +66,29 @@ template struct image_filter_grammar : qi::grammar { + using alternative_type = qi::rule; + image_filter_grammar(); + qi::rule start; - qi::rule filter; - qi::rule, void(ContType&), qi::ascii::space_type> agg_blur_filter; - qi::rule, - void(ContType&), qi::ascii::space_type> scale_hsla_filter; - qi::rule, void(ContType&), qi::ascii::space_type> colorize_alpha_filter; + qi::rule> filter; qi::rule no_args; + qi::symbols alternatives; qi::uint_parser< unsigned, 10, 1, 3 > radius_; css_color_grammar css_color_; - qi::rule color_stop_offset; + qi::rule color_stop_; + qi::rule color_stop_offset; phoenix::function percent_offset; - qi::rule, void(ContType&), qi::ascii::space_type> color_to_alpha_filter; + +private: + alternative_type & add(std::string const& symbol); + + static constexpr unsigned max_alternatives = 16; + unsigned num_alternatives = 0; + alternative_type alternative_storage[max_alternatives]; }; -} +} // namespace mapnik -#endif // MAPNIK_IMAGE_FILITER_PARSER_HPP +#endif // MAPNIK_IMAGE_FILTER_GRAMMAR_HPP diff --git a/include/mapnik/image_filter_grammar_impl.hpp b/include/mapnik/image_filter_grammar_impl.hpp index 168d9a112..98de7119f 100644 --- a/include/mapnik/image_filter_grammar_impl.hpp +++ b/include/mapnik/image_filter_grammar_impl.hpp @@ -26,15 +26,16 @@ #pragma GCC diagnostic push #include -#include #include #pragma GCC diagnostic pop -BOOST_FUSION_ADAPT_STRUCT( - mapnik::filter::color_stop, - (mapnik::color, color ) - (double, offset) -) +namespace { // internal + + BOOST_PHOENIX_ADAPT_FUNCTION( + typename std::remove_reference::type, ovo, // = optional_value_or + boost::get_optional_value_or, 2) + +} // namespace internal namespace mapnik { @@ -48,94 +49,90 @@ image_filter_grammar::image_filter_grammar() qi::lit_type lit; qi::_val_type _val; qi::_1_type _1; + qi::_2_type _2; + qi::_3_type _3; + qi::_4_type _4; + qi::_5_type _5; + qi::_6_type _6; + qi::_7_type _7; + qi::_8_type _8; qi::_a_type _a; - qi::_b_type _b; - qi::_c_type _c; - qi::_d_type _d; - qi::_e_type _e; - qi::_f_type _f; - qi::_g_type _g; - qi::_h_type _h; - qi::_r1_type _r1; + qi::attr_type attr; qi::double_type double_; + qi::hold_type hold; + qi::omit_type omit; using phoenix::push_back; using phoenix::construct; - using phoenix::at_c; start = -(filter % *lit(',')) ; - filter = - lit("emboss") >> no_args [push_back(_val,construct())] - | - lit("blur") >> no_args [push_back(_val,construct())] - | - lit("gray") >> no_args [push_back(_val,construct())] - | - lit("edge-detect") >> no_args [push_back(_val,construct())] - | - lit("sobel") >> no_args [push_back(_val,construct())] - | - lit("sharpen") >> no_args [push_back(_val,construct())] - | - lit("x-gradient") >> no_args [push_back(_val,construct())] - | - lit("y-gradient") >> no_args [push_back(_val,construct())] - | - lit("invert") >> no_args [push_back(_val,construct())] - | - lit("color-blind-protanope") >> no_args [push_back(_val,construct())] - | - lit("color-blind-deuteranope") >> no_args [push_back(_val,construct())] - | - lit("color-blind-tritanope") >> no_args [push_back(_val,construct())] - | - agg_blur_filter(_val) - | - scale_hsla_filter(_val) - | - colorize_alpha_filter(_val) - | - color_to_alpha_filter(_val) + filter = omit[alternatives[_a = _1]] >> qi::lazy(*_a) ; - agg_blur_filter = (lit("agg-stack-blur")[_a = 1, _b = 1] - >> -( lit('(') >> -( radius_[_a = _1][_b = _1] - >> -(lit(',') >> radius_[_b = _1])) >> lit(')'))) - [push_back(_r1, construct(_a,_b))] - ; + add("emboss") = no_args >> attr(construct()); + add("blur") = no_args >> attr(construct()); + add("gray") = no_args >> attr(construct()); + add("edge-detect") = no_args >> attr(construct()); + add("sobel") = no_args >> attr(construct()); + add("sharpen") = no_args >> attr(construct()); + add("x-gradient") = no_args >> attr(construct()); + add("y-gradient") = no_args >> attr(construct()); + add("invert") = no_args >> attr(construct()); + add("color-blind-protanope") = no_args >> attr(construct()); + add("color-blind-deuteranope") = no_args >> attr(construct()); + add("color-blind-tritanope") = no_args >> attr(construct()); - scale_hsla_filter = lit("scale-hsla") - >> lit('(') - >> double_[_a = _1] >> lit(',') >> double_[_b = _1] >> lit(',') - >> double_[_c = _1] >> lit(',') >> double_[_d = _1] >> lit(',') - >> double_[_e = _1] >> lit(',') >> double_[_f = _1] >> lit(',') - >> double_[_g = _1] >> lit(',') >> double_[_h = _1] >> lit(')') - [push_back(_r1, construct(_a,_b,_c,_d,_e,_f,_g,_h))] - ; - - colorize_alpha_filter = lit("colorize-alpha")[_a = construct()] - >> lit('(') - >> (css_color_[at_c<0>(_b) = _1, at_c<1>(_b) = 0] - >> -color_stop_offset(_b)) [push_back(_a,_b)] - >> -(+(lit(',') >> css_color_[at_c<0>(_b) =_1,at_c<1>(_b) = 0] - >> -color_stop_offset(_b))[push_back(_a,_b)]) - >> lit(')') [push_back(_r1,_a)] - ; - - color_stop_offset = (double_ >> lit('%'))[at_c<1>(_r1) = percent_offset(_1)] + add("agg-stack-blur") = + (lit('(') >> radius_ >> -( lit(',') >> radius_ ) >> lit(')')) + [push_back(_val, construct(_1, ovo(_2, _1)))] | - double_[at_c<1>(_r1) = _1] + no_args + [push_back(_val, construct(1, 1))] ; - color_to_alpha_filter = lit("color-to-alpha") - >> lit('(') - >> css_color_[_a = _1] - >> lit(')') - [push_back(_r1,construct(_a))] + add("scale-hsla") = + (lit('(') + >> double_ >> lit(',') >> double_ >> lit(',') + >> double_ >> lit(',') >> double_ >> lit(',') + >> double_ >> lit(',') >> double_ >> lit(',') + >> double_ >> lit(',') >> double_ >> lit(')')) + [push_back(_val, construct(_1,_2,_3,_4,_5,_6,_7,_8))] + ; + + add("colorize-alpha") = qi::as() + [lit('(') >> color_stop_ % lit(',') >> lit(')')] + [push_back(_val, _1)] + ; + + color_stop_ = (css_color_ >> -color_stop_offset) + [_val = construct(_1, ovo(_2, 0.0))] + ; + + color_stop_offset = double_[_val = _1] + >> -lit('%')[_val = percent_offset(_val)] + ; + + add("color-to-alpha") = + hold[lit('(') >> css_color_ >> lit(')')] + [push_back(_val, construct(_1))] ; no_args = -(lit('(') >> lit(')')); } +template +auto image_filter_grammar::add(std::string const& symbol) + -> alternative_type & +{ + if (num_alternatives >= max_alternatives) + { + throw std::length_error("too many alternatives in image_filter_grammar"); + } + + alternative_storage[num_alternatives].name(symbol); + alternatives.add(symbol, &alternative_storage[num_alternatives]); + return alternative_storage[num_alternatives++]; } + +} // namespace mapnik From f19b8e8eff8411a5c5e9f40b73c868e5ff201d4d Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Mon, 15 Feb 2016 02:23:19 +0100 Subject: [PATCH 3/4] test/expressions: add checks for backslash-escapes in strings --- test/unit/core/expressions_test.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/unit/core/expressions_test.cpp b/test/unit/core/expressions_test.cpp index 933117a6b..946b4f155 100644 --- a/test/unit/core/expressions_test.cpp +++ b/test/unit/core/expressions_test.cpp @@ -89,6 +89,8 @@ TEST_CASE("expressions") // unicode TRY_CHECK(parse_and_dump("'single-quoted string'") == "'single-quoted string'"); TRY_CHECK(parse_and_dump("\"double-quoted string\"") == "'double-quoted string'"); + TRY_CHECK(parse_and_dump("'escaped \\' apostrophe'") == "'escaped \\' apostrophe'"); + TRY_CHECK(parse_and_dump("'escaped \\\\ backslash'") == "'escaped \\\\ backslash'"); // floating point constants TRY_CHECK(parse_and_dump("pi") == "3.14159"); @@ -159,8 +161,13 @@ TEST_CASE("expressions") // regex // replace TRY_CHECK(eval(" [foo].replace('(\\B)|( )','$1 ') ") == tr.transcode("b a r")); + // 'foo' =~ s:(\w)\1:$1x:r + TRY_CHECK(eval(" 'foo'.replace('(\\w)\\1', '$1x') ") == tr.transcode("fox")); + TRY_CHECK(parse_and_dump(" 'foo'.replace('(\\w)\\1', '$1x') ") == "'foo'.replace('(\\w)\\1','$1x')"); // match TRY_CHECK(eval(" [name].match('Québec') ") == true); - + // 'Québec' =~ m:^Q\S*$: + TRY_CHECK(eval(" [name].match('^Q\\S*$') ") == true); + TRY_CHECK(parse_and_dump(" [name].match('^Q\\S*$') ") == "[name].match('^Q\\S*$')"); } From 2d88fec4583439b51c178dd162e1da323579e730 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Mon, 15 Feb 2016 02:45:29 +0100 Subject: [PATCH 4/4] to_expression_string: fix backslash-escapes in strings --- include/mapnik/value.hpp | 47 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/include/mapnik/value.hpp b/include/mapnik/value.hpp index 289759839..1192442b7 100644 --- a/include/mapnik/value.hpp +++ b/include/mapnik/value.hpp @@ -710,14 +710,55 @@ struct to_unicode_impl struct to_expression_string_impl { + struct EscapingByteSink : U_NAMESPACE_QUALIFIER ByteSink + { + std::string dest_; + char quote_; + + explicit EscapingByteSink(char quote) + : quote_(quote) + {} + + virtual void Append(const char* data, int32_t n) + { + // reserve enough room to hold the appended chunk and quotes; + // if another chunk follows, or any character needs escaping, + // the string will grow naturally + if (dest_.empty()) + { + dest_.reserve(2 + static_cast(n)); + dest_.append(1, quote_); + } + else + { + dest_.reserve(dest_.size() + n + 1); + } + + for (auto end = data + n; data < end; ++data) + { + if (*data == '\\' || *data == quote_) + dest_.append(1, '\\'); + dest_.append(1, *data); + } + } + + virtual void Flush() + { + if (dest_.empty()) + dest_.append(2, quote_); + else + dest_.append(1, quote_); + } + }; + explicit to_expression_string_impl(char quote = '\'') : quote_(quote) {} std::string operator() (value_unicode_string const& val) const { - std::string utf8; - to_utf8(val,utf8); - return quote_ + utf8 + quote_; + EscapingByteSink sink(quote_); + val.toUTF8(sink); + return sink.dest_; } std::string operator() (value_integer val) const