diff --git a/tests/engine/test_filter_macro_resolver.cpp b/tests/engine/test_filter_macro_resolver.cpp index d98fe7ce..b2b3ce06 100644 --- a/tests/engine/test_filter_macro_resolver.cpp +++ b/tests/engine/test_filter_macro_resolver.cpp @@ -20,9 +20,27 @@ limitations under the License. using namespace std; using namespace libsinsp::filter::ast; +static pos_info create_pos(uint32_t idx, uint32_t line, uint32_t col) +{ + pos_info ret; + ret.idx = idx; + ret.line = line; + ret.col = col; + + return ret; +} + +static bool operator==(const pos_info& p1, const pos_info& p2) +{ + return (p1.idx == p2.idx) && + (p1.line == p2.line) && + (p1.col == p2.col); +} + TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos = create_pos(12, 85, 27); SECTION("in the general case") { @@ -31,7 +49,7 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") std::vector> filter_and; filter_and.push_back(unary_check_expr::create("evt.name", "", "exists")); - filter_and.push_back(not_expr::create(value_expr::create(macro_name))); + filter_and.push_back(not_expr::create(value_expr::create(macro_name, macro_pos))); std::shared_ptr filter = std::move(and_expr::create(filter_and)); std::vector> expected_and; @@ -45,7 +63,8 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(expected.get())); @@ -61,7 +80,7 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") std::shared_ptr macro = std::move( unary_check_expr::create("test.field", "", "exists")); - std::shared_ptr filter = std::move(value_expr::create(macro_name)); + std::shared_ptr filter = std::move(value_expr::create(macro_name, macro_pos)); filter_macro_resolver resolver; resolver.set_macro(macro_name, macro); @@ -71,7 +90,8 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") REQUIRE(resolver.run(filter) == true); REQUIRE(filter.get() != old_filter_ptr); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(macro.get())); @@ -89,14 +109,17 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") string a_macro_name = macro_name + "_1"; string b_macro_name = macro_name + "_2"; + pos_info a_macro_pos = create_pos(11, 75, 43); + pos_info b_macro_pos = create_pos(91, 21, 9); + std::shared_ptr a_macro = std::move( unary_check_expr::create("one.field", "", "exists")); std::shared_ptr b_macro = std::move( unary_check_expr::create("another.field", "", "exists")); std::vector> filter_or; - filter_or.push_back(value_expr::create(a_macro_name)); - filter_or.push_back(value_expr::create(b_macro_name)); + filter_or.push_back(value_expr::create(a_macro_name, a_macro_pos)); + filter_or.push_back(value_expr::create(b_macro_name, b_macro_pos)); std::shared_ptr filter = std::move(or_expr::create(filter_or)); std::vector> expected_or; @@ -111,11 +134,16 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 2); - REQUIRE(resolver.get_resolved_macros().find(a_macro_name) - != resolver.get_resolved_macros().end()); - REQUIRE(resolver.get_resolved_macros().find(b_macro_name) - != resolver.get_resolved_macros().end()); + auto a_resolved_itr = resolver.get_resolved_macros().find(a_macro_name); + REQUIRE(a_resolved_itr != resolver.get_resolved_macros().end()); + REQUIRE(a_resolved_itr->first == a_macro_name); + REQUIRE(a_resolved_itr->second == a_macro_pos); + + auto b_resolved_itr = resolver.get_resolved_macros().find(b_macro_name); + REQUIRE(b_resolved_itr != resolver.get_resolved_macros().end()); REQUIRE(resolver.get_unknown_macros().empty()); + REQUIRE(b_resolved_itr->first == b_macro_name); + REQUIRE(b_resolved_itr->second == b_macro_pos); REQUIRE(filter->is_equal(expected_filter.get())); // second run @@ -130,15 +158,18 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") string a_macro_name = macro_name + "_1"; string b_macro_name = macro_name + "_2"; + pos_info a_macro_pos = create_pos(47, 1, 76); + pos_info b_macro_pos = create_pos(111, 65, 2); + std::vector> a_macro_and; a_macro_and.push_back(unary_check_expr::create("one.field", "", "exists")); - a_macro_and.push_back(value_expr::create(b_macro_name)); + a_macro_and.push_back(value_expr::create(b_macro_name, b_macro_pos)); std::shared_ptr a_macro = std::move(and_expr::create(a_macro_and)); std::shared_ptr b_macro = std::move( unary_check_expr::create("another.field", "", "exists")); - std::shared_ptr filter = std::move(value_expr::create(a_macro_name)); + std::shared_ptr filter = std::move(value_expr::create(a_macro_name, a_macro_pos)); std::vector> expected_and; expected_and.push_back(unary_check_expr::create("one.field", "", "exists")); @@ -152,10 +183,17 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 2); - REQUIRE(resolver.get_resolved_macros().find(a_macro_name) - != resolver.get_resolved_macros().end()); - REQUIRE(resolver.get_resolved_macros().find(b_macro_name) - != resolver.get_resolved_macros().end()); + auto a_resolved_itr = resolver.get_resolved_macros().find(a_macro_name); + REQUIRE(a_resolved_itr != resolver.get_resolved_macros().end()); + REQUIRE(a_resolved_itr->first == a_macro_name); + REQUIRE(a_resolved_itr->second == a_macro_pos); + + auto b_resolved_itr = resolver.get_resolved_macros().find(b_macro_name); + REQUIRE(b_resolved_itr != resolver.get_resolved_macros().end()); + REQUIRE(resolver.get_unknown_macros().empty()); + REQUIRE(b_resolved_itr->first == b_macro_name); + REQUIRE(b_resolved_itr->second == b_macro_pos); + REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(expected_filter.get())); @@ -170,18 +208,20 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") TEST_CASE("Should find unknown macros", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos = create_pos(9, 4, 2); SECTION("in the general case") { std::vector> filter_and; filter_and.push_back(unary_check_expr::create("evt.name", "", "exists")); - filter_and.push_back(not_expr::create(value_expr::create(macro_name))); + filter_and.push_back(not_expr::create(value_expr::create(macro_name, macro_pos))); std::shared_ptr filter = std::move(and_expr::create(filter_and)); filter_macro_resolver resolver; REQUIRE(resolver.run(filter) == false); REQUIRE(resolver.get_unknown_macros().size() == 1); - REQUIRE(*resolver.get_unknown_macros().begin() == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->first == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->second == macro_pos); REQUIRE(resolver.get_resolved_macros().empty()); } @@ -190,12 +230,15 @@ TEST_CASE("Should find unknown macros", "[rule_loader]") string a_macro_name = macro_name + "_1"; string b_macro_name = macro_name + "_2"; + pos_info a_macro_pos = create_pos(32, 84, 9); + pos_info b_macro_pos = create_pos(1, 0, 5); + std::vector> a_macro_and; a_macro_and.push_back(unary_check_expr::create("one.field", "", "exists")); - a_macro_and.push_back(value_expr::create(b_macro_name)); + a_macro_and.push_back(value_expr::create(b_macro_name, b_macro_pos)); std::shared_ptr a_macro = std::move(and_expr::create(a_macro_and)); - std::shared_ptr filter = std::move(value_expr::create(a_macro_name)); + std::shared_ptr filter = std::move(value_expr::create(a_macro_name, a_macro_pos)); auto expected_filter = clone(a_macro.get()); filter_macro_resolver resolver; @@ -204,9 +247,11 @@ TEST_CASE("Should find unknown macros", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == a_macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == a_macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == a_macro_pos); REQUIRE(resolver.get_unknown_macros().size() == 1); - REQUIRE(*resolver.get_unknown_macros().begin() == b_macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->first == b_macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->second == b_macro_pos); REQUIRE(filter->is_equal(expected_filter.get())); } } @@ -214,15 +259,19 @@ TEST_CASE("Should find unknown macros", "[rule_loader]") TEST_CASE("Should undefine macro", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos_1 = create_pos(12, 9, 3); + pos_info macro_pos_2 = create_pos(9, 6, 3); + std::shared_ptr macro = std::move(unary_check_expr::create("test.field", "", "exists")); - std::shared_ptr a_filter = std::move(value_expr::create(macro_name)); - std::shared_ptr b_filter = std::move(value_expr::create(macro_name)); + std::shared_ptr a_filter = std::move(value_expr::create(macro_name, macro_pos_1)); + std::shared_ptr b_filter = std::move(value_expr::create(macro_name, macro_pos_2)); filter_macro_resolver resolver; resolver.set_macro(macro_name, macro); REQUIRE(resolver.run(a_filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos_1); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(a_filter->is_equal(macro.get())); @@ -230,21 +279,24 @@ TEST_CASE("Should undefine macro", "[rule_loader]") REQUIRE(resolver.run(b_filter) == false); REQUIRE(resolver.get_resolved_macros().empty()); REQUIRE(resolver.get_unknown_macros().size() == 1); - REQUIRE(*resolver.get_unknown_macros().begin() == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->first == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->second == macro_pos_2); } // checks that the macro AST is cloned and not shared across resolved filters TEST_CASE("Should clone macro AST", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos = create_pos(5, 2, 8888); std::shared_ptr macro = std::move(unary_check_expr::create("test.field", "", "exists")); - std::shared_ptr filter = std::move(value_expr::create(macro_name)); + std::shared_ptr filter = std::move(value_expr::create(macro_name, macro_pos)); filter_macro_resolver resolver; - + resolver.set_macro(macro_name, macro); REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(macro.get())); diff --git a/userspace/engine/filter_macro_resolver.cpp b/userspace/engine/filter_macro_resolver.cpp index 0e86136f..81cf598d 100644 --- a/userspace/engine/filter_macro_resolver.cpp +++ b/userspace/engine/filter_macro_resolver.cpp @@ -21,12 +21,10 @@ using namespace libsinsp::filter; bool filter_macro_resolver::run(libsinsp::filter::ast::expr*& filter) { - visitor v; m_unknown_macros.clear(); m_resolved_macros.clear(); - v.m_unknown_macros = &m_unknown_macros; - v.m_resolved_macros = &m_resolved_macros; - v.m_macros = &m_macros; + + visitor v(m_unknown_macros, m_resolved_macros, m_macros); v.m_node_substitute = nullptr; filter->accept(&v); if (v.m_node_substitute) @@ -39,12 +37,10 @@ bool filter_macro_resolver::run(libsinsp::filter::ast::expr*& filter) bool filter_macro_resolver::run(std::shared_ptr& filter) { - visitor v; m_unknown_macros.clear(); m_resolved_macros.clear(); - v.m_unknown_macros = &m_unknown_macros; - v.m_resolved_macros = &m_resolved_macros; - v.m_macros = &m_macros; + + visitor v(m_unknown_macros, m_resolved_macros, m_macros); v.m_node_substitute = nullptr; filter->accept(&v); if (v.m_node_substitute) @@ -61,12 +57,12 @@ void filter_macro_resolver::set_macro( m_macros[name] = macro; } -const unordered_set& filter_macro_resolver::get_unknown_macros() const +const filter_macro_resolver::macro_info_map& filter_macro_resolver::get_unknown_macros() const { return m_unknown_macros; } -const unordered_set& filter_macro_resolver::get_resolved_macros() const +const filter_macro_resolver::macro_info_map& filter_macro_resolver::get_resolved_macros() const { return m_resolved_macros; } @@ -129,8 +125,8 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e) // we are supposed to get here only in case // of identier-only children from either a 'not', // an 'and' or an 'or'. - auto macro = m_macros->find(e->value); - if (macro != m_macros->end() && macro->second) // skip null-ptr macros + auto macro = m_macros.find(e->value); + if (macro != m_macros.end() && macro->second) // skip null-ptr macros { m_node_substitute = nullptr; auto new_node = ast::clone(macro->second.get()); @@ -141,11 +137,11 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e) { m_node_substitute = std::move(new_node); } - m_resolved_macros->insert(e->value); + m_resolved_macros[e->value] = e->get_pos(); } else { m_node_substitute = nullptr; - m_unknown_macros->insert(e->value); + m_unknown_macros[e->value] = e->get_pos(); } } diff --git a/userspace/engine/filter_macro_resolver.h b/userspace/engine/filter_macro_resolver.h index 8f71c43d..8c798e81 100644 --- a/userspace/engine/filter_macro_resolver.h +++ b/userspace/engine/filter_macro_resolver.h @@ -40,7 +40,7 @@ class filter_macro_resolver \return true if at least one of the defined macros is resolved */ bool run(libsinsp::filter::ast::expr*& filter); - + /*! \brief Version of run() that works with shared pointers */ @@ -58,12 +58,17 @@ class filter_macro_resolver std::string name, std::shared_ptr macro); + /*! + \brief used in get_{resolved,unknown}_macros + */ + typedef std::unordered_map macro_info_map; + /*! \brief Returns a set containing the names of all the macros substituted during the last invocation of run(). Should be non-empty if the last invocation of run() returned true. */ - const std::unordered_set& get_resolved_macros() const; + const macro_info_map& get_resolved_macros() const; /*! \brief Returns a set containing the names of all the macros @@ -71,8 +76,8 @@ class filter_macro_resolver A macro remains unresolved if it is found inside the processed filter but it was not defined with set_macro(); */ - const std::unordered_set& get_unknown_macros() const; - + const macro_info_map& get_unknown_macros() const; + private: typedef std::unordered_map< std::string, @@ -81,16 +86,18 @@ class filter_macro_resolver struct visitor : public libsinsp::filter::ast::expr_visitor { - visitor() = default; + visitor(macro_info_map& unknown_macros, macro_info_map& resolved_macros, macro_defs& macros) + : m_unknown_macros(unknown_macros), m_resolved_macros(resolved_macros), m_macros(macros) {} visitor(visitor&&) = default; visitor& operator = (visitor&&) = default; visitor(const visitor&) = delete; visitor& operator = (const visitor&) = delete; std::unique_ptr m_node_substitute; - std::unordered_set* m_unknown_macros; - std::unordered_set* m_resolved_macros; - macro_defs* m_macros; + macro_info_map& m_unknown_macros; + macro_info_map& m_resolved_macros; + + macro_defs& m_macros; void visit(libsinsp::filter::ast::and_expr* e) override; void visit(libsinsp::filter::ast::or_expr* e) override; @@ -101,7 +108,7 @@ class filter_macro_resolver void visit(libsinsp::filter::ast::binary_check_expr* e) override; }; - std::unordered_set m_unknown_macros; - std::unordered_set m_resolved_macros; + macro_info_map m_unknown_macros; + macro_info_map m_resolved_macros; macro_defs m_macros; };