diff --git a/unit_tests/engine/test_filter_macro_resolver.cpp b/unit_tests/engine/test_filter_macro_resolver.cpp index d7f92f3c..3e2a1880 100644 --- a/unit_tests/engine/test_filter_macro_resolver.cpp +++ b/unit_tests/engine/test_filter_macro_resolver.cpp @@ -18,6 +18,8 @@ limitations under the License. #include #include +namespace filter_ast = libsinsp::filter::ast; + static std::vector::const_iterator find_value( const std::vector& values, const std::string& ref) @@ -35,19 +37,19 @@ static std::vector::const_iterator find_value TEST(MacroResolver, should_resolve_macros_on_a_filter_AST) { - libsinsp::filter::ast::pos_info macro_pos(12, 85, 27); + filter_ast::pos_info macro_pos(12, 85, 27); - std::shared_ptr macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"); + std::shared_ptr macro = filter_ast::unary_check_expr::create(filter_ast::field_expr::create("test.field", ""), "exists"); - std::vector> filter_and; - filter_and.push_back(libsinsp::filter::ast::unary_check_expr::create("evt.name", "", "exists")); - filter_and.push_back(libsinsp::filter::ast::not_expr::create(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos))); - std::shared_ptr filter = libsinsp::filter::ast::and_expr::create(filter_and); + std::vector> filter_and; + filter_and.push_back(filter_ast::unary_check_expr::create(filter_ast::field_expr::create("evt.name", ""), "exists")); + filter_and.push_back(filter_ast::not_expr::create(filter_ast::identifier_expr::create(MACRO_NAME, macro_pos))); + std::shared_ptr filter = filter_ast::and_expr::create(filter_and); - std::vector> expected_and; - expected_and.push_back(libsinsp::filter::ast::unary_check_expr::create("evt.name", "", "exists")); - expected_and.push_back(libsinsp::filter::ast::not_expr::create(clone(macro.get()))); - std::shared_ptr expected = libsinsp::filter::ast::and_expr::create(expected_and); + std::vector> expected_and; + expected_and.push_back(filter_ast::unary_check_expr::create(filter_ast::field_expr::create("evt.name", ""), "exists")); + expected_and.push_back(filter_ast::not_expr::create(clone(macro.get()))); + std::shared_ptr expected = filter_ast::and_expr::create(expected_and); filter_macro_resolver resolver; resolver.set_macro(MACRO_NAME, macro); @@ -69,17 +71,17 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST) TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_single_node) { - libsinsp::filter::ast::pos_info macro_pos(12, 85, 27); + filter_ast::pos_info macro_pos(12, 85, 27); - std::shared_ptr macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"); + std::shared_ptr macro = filter_ast::unary_check_expr::create(filter_ast::field_expr::create("test.field", ""), "exists"); - std::shared_ptr filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos); + std::shared_ptr filter = filter_ast::identifier_expr::create(MACRO_NAME, macro_pos); filter_macro_resolver resolver; resolver.set_macro(MACRO_NAME, macro); // first run - libsinsp::filter::ast::expr* old_filter_ptr = filter.get(); + filter_ast::expr* old_filter_ptr = filter.get(); ASSERT_TRUE(resolver.run(filter)); ASSERT_NE(filter.get(), old_filter_ptr); ASSERT_EQ(resolver.get_resolved_macros().size(), 1); @@ -99,21 +101,21 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_single_node) TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_multiple_macros) { - libsinsp::filter::ast::pos_info a_macro_pos(11, 75, 43); - libsinsp::filter::ast::pos_info b_macro_pos(91, 21, 9); + filter_ast::pos_info a_macro_pos(11, 75, 43); + filter_ast::pos_info b_macro_pos(91, 21, 9); - std::shared_ptr a_macro = libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists"); - std::shared_ptr b_macro = libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists"); + std::shared_ptr a_macro = filter_ast::unary_check_expr::create(filter_ast::field_expr::create("one.field", ""), "exists"); + std::shared_ptr b_macro = filter_ast::unary_check_expr::create(filter_ast::field_expr::create("another.field", ""), "exists"); - std::vector> filter_or; - filter_or.push_back(libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos)); - filter_or.push_back(libsinsp::filter::ast::value_expr::create(MACRO_B_NAME, b_macro_pos)); - std::shared_ptr filter = libsinsp::filter::ast::or_expr::create(filter_or); + std::vector> filter_or; + filter_or.push_back(filter_ast::identifier_expr::create(MACRO_A_NAME, a_macro_pos)); + filter_or.push_back(filter_ast::identifier_expr::create(MACRO_B_NAME, b_macro_pos)); + std::shared_ptr filter = filter_ast::or_expr::create(filter_or); - std::vector> expected_or; + std::vector> expected_or; expected_or.push_back(clone(a_macro.get())); expected_or.push_back(clone(b_macro.get())); - std::shared_ptr expected_filter = libsinsp::filter::ast::or_expr::create(expected_or); + std::shared_ptr expected_filter = filter_ast::or_expr::create(expected_or); filter_macro_resolver resolver; resolver.set_macro(MACRO_A_NAME, a_macro); @@ -143,23 +145,23 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_multiple_macros) TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_nested_macros) { - libsinsp::filter::ast::pos_info a_macro_pos(47, 1, 76); - libsinsp::filter::ast::pos_info b_macro_pos(111, 65, 2); + filter_ast::pos_info a_macro_pos(47, 1, 76); + filter_ast::pos_info b_macro_pos(111, 65, 2); - std::vector> a_macro_and; - a_macro_and.push_back(libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists")); - a_macro_and.push_back(libsinsp::filter::ast::value_expr::create(MACRO_B_NAME, b_macro_pos)); - std::shared_ptr a_macro = libsinsp::filter::ast::and_expr::create(a_macro_and); + std::vector> a_macro_and; + a_macro_and.push_back(filter_ast::unary_check_expr::create(filter_ast::field_expr::create("one.field", ""), "exists")); + a_macro_and.push_back(filter_ast::identifier_expr::create(MACRO_B_NAME, b_macro_pos)); + std::shared_ptr a_macro = filter_ast::and_expr::create(a_macro_and); - std::shared_ptr b_macro = - libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists"); + std::shared_ptr b_macro = + filter_ast::unary_check_expr::create(filter_ast::field_expr::create("another.field", ""), "exists"); - std::shared_ptr filter = libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos); + std::shared_ptr filter = filter_ast::identifier_expr::create(MACRO_A_NAME, a_macro_pos); - std::vector> expected_and; - expected_and.push_back(libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists")); - expected_and.push_back(libsinsp::filter::ast::unary_check_expr::create("another.field", "", "exists")); - std::shared_ptr expected_filter = libsinsp::filter::ast::and_expr::create(expected_and); + std::vector> expected_and; + expected_and.push_back(filter_ast::unary_check_expr::create(filter_ast::field_expr::create("one.field", ""), "exists")); + expected_and.push_back(filter_ast::unary_check_expr::create(filter_ast::field_expr::create("another.field", ""), "exists")); + std::shared_ptr expected_filter = filter_ast::and_expr::create(expected_and); filter_macro_resolver resolver; resolver.set_macro(MACRO_A_NAME, a_macro); @@ -191,12 +193,12 @@ TEST(MacroResolver, should_resolve_macros_on_a_filter_AST_nested_macros) TEST(MacroResolver, should_find_unknown_macros) { - libsinsp::filter::ast::pos_info macro_pos(9, 4, 2); + filter_ast::pos_info macro_pos(9, 4, 2); - std::vector> filter_and; - filter_and.push_back(libsinsp::filter::ast::unary_check_expr::create("evt.name", "", "exists")); - filter_and.push_back(libsinsp::filter::ast::not_expr::create(libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos))); - std::shared_ptr filter = libsinsp::filter::ast::and_expr::create(filter_and); + std::vector> filter_and; + filter_and.push_back(filter_ast::unary_check_expr::create(filter_ast::field_expr::create("evt.name", ""), "exists")); + filter_and.push_back(filter_ast::not_expr::create(filter_ast::identifier_expr::create(MACRO_NAME, macro_pos))); + std::shared_ptr filter = filter_ast::and_expr::create(filter_and); filter_macro_resolver resolver; ASSERT_FALSE(resolver.run(filter)); @@ -208,15 +210,15 @@ TEST(MacroResolver, should_find_unknown_macros) TEST(MacroResolver, should_find_unknown_nested_macros) { - libsinsp::filter::ast::pos_info a_macro_pos(32, 84, 9); - libsinsp::filter::ast::pos_info b_macro_pos(1, 0, 5); + filter_ast::pos_info a_macro_pos(32, 84, 9); + filter_ast::pos_info b_macro_pos(1, 0, 5); - std::vector> a_macro_and; - a_macro_and.push_back(libsinsp::filter::ast::unary_check_expr::create("one.field", "", "exists")); - a_macro_and.push_back(libsinsp::filter::ast::value_expr::create(MACRO_B_NAME, b_macro_pos)); - std::shared_ptr a_macro = libsinsp::filter::ast::and_expr::create(a_macro_and); + std::vector> a_macro_and; + a_macro_and.push_back(filter_ast::unary_check_expr::create(filter_ast::field_expr::create("one.field", ""), "exists")); + a_macro_and.push_back(filter_ast::identifier_expr::create(MACRO_B_NAME, b_macro_pos)); + std::shared_ptr a_macro = filter_ast::and_expr::create(a_macro_and); - std::shared_ptr filter = libsinsp::filter::ast::value_expr::create(MACRO_A_NAME, a_macro_pos); + std::shared_ptr filter = filter_ast::identifier_expr::create(MACRO_A_NAME, a_macro_pos); auto expected_filter = clone(a_macro.get()); filter_macro_resolver resolver; @@ -234,12 +236,12 @@ TEST(MacroResolver, should_find_unknown_nested_macros) TEST(MacroResolver, should_undefine_macro) { - libsinsp::filter::ast::pos_info macro_pos_1(12, 9, 3); - libsinsp::filter::ast::pos_info macro_pos_2(9, 6, 3); + filter_ast::pos_info macro_pos_1(12, 9, 3); + filter_ast::pos_info macro_pos_2(9, 6, 3); - std::shared_ptr macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"); - std::shared_ptr a_filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos_1); - std::shared_ptr b_filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos_2); + std::shared_ptr macro = filter_ast::unary_check_expr::create(filter_ast::field_expr::create("test.field", ""), "exists"); + std::shared_ptr a_filter = filter_ast::identifier_expr::create(MACRO_NAME, macro_pos_1); + std::shared_ptr b_filter = filter_ast::identifier_expr::create(MACRO_NAME, macro_pos_2); filter_macro_resolver resolver; resolver.set_macro(MACRO_NAME, macro); @@ -261,9 +263,9 @@ TEST(MacroResolver, should_undefine_macro) /* checks that the macro AST is cloned and not shared across resolved filters */ TEST(MacroResolver, should_clone_macro_AST) { - libsinsp::filter::ast::pos_info macro_pos(5, 2, 8888); - std::shared_ptr macro = libsinsp::filter::ast::unary_check_expr::create("test.field", "", "exists"); - std::shared_ptr filter = libsinsp::filter::ast::value_expr::create(MACRO_NAME, macro_pos); + filter_ast::pos_info macro_pos(5, 2, 8888); + std::shared_ptr macro = filter_ast::unary_check_expr::create(filter_ast::field_expr::create("test.field", ""), "exists"); + std::shared_ptr filter = filter_ast::identifier_expr::create(MACRO_NAME, macro_pos); filter_macro_resolver resolver; resolver.set_macro(MACRO_NAME, macro); @@ -274,6 +276,6 @@ TEST(MacroResolver, should_clone_macro_AST) ASSERT_TRUE(resolver.get_unknown_macros().empty()); ASSERT_TRUE(filter->is_equal(macro.get())); - macro->field = "another.field"; + macro->left = filter_ast::field_expr::create("another.field", ""); ASSERT_FALSE(filter->is_equal(macro.get())); } diff --git a/userspace/engine/filter_details_resolver.cpp b/userspace/engine/filter_details_resolver.cpp index 5939f1f6..4d7cad4b 100644 --- a/userspace/engine/filter_details_resolver.cpp +++ b/userspace/engine/filter_details_resolver.cpp @@ -17,9 +17,11 @@ limitations under the License. #include "filter_details_resolver.h" +#include + using namespace libsinsp::filter; -std::string get_field_name(const std::string& name, const std::string& arg) +static inline std::string get_field_name(const std::string& name, const std::string& arg) { std::string fld = name; if (!arg.empty()) @@ -41,30 +43,22 @@ void filter_details::reset() void filter_details_resolver::run(ast::expr* filter, filter_details& details) { visitor v(details); - // note: we may have ASTs composed on only one macro ref - v.m_expect_macro = true; filter->accept(&v); } void filter_details_resolver::visitor::visit(ast::and_expr* e) { - m_expect_macro = false; for(size_t i = 0; i < e->children.size(); i++) { - m_expect_macro = true; e->children[i]->accept(this); - m_expect_macro = false; } } void filter_details_resolver::visitor::visit(ast::or_expr* e) { - m_expect_macro = false; for(size_t i = 0; i < e->children.size(); i++) { - m_expect_macro = true; e->children[i]->accept(this); - m_expect_macro = false; } } @@ -99,36 +93,56 @@ void filter_details_resolver::visitor::visit(ast::list_expr* e) void filter_details_resolver::visitor::visit(ast::binary_check_expr* e) { - m_expect_macro = false; - m_details.fields.insert(get_field_name(e->field, e->arg)); + m_last_node_field_name.clear(); + e->left->accept(this); + if (m_last_node_field_name.empty()) + { + throw std::runtime_error("can't find field info in binary check expression"); + } + m_details.fields.insert(m_last_node_field_name); m_details.operators.insert(e->op); m_expect_list = true; - m_expect_evtname = e->field == "evt.type" || e->field == "evt.asynctype"; - e->value->accept(this); + m_expect_evtname = m_last_node_field_name == "evt.type" || m_last_node_field_name == "evt.asynctype"; + e->right->accept(this); m_expect_evtname = false; m_expect_list = false; } void filter_details_resolver::visitor::visit(ast::unary_check_expr* e) { - m_expect_macro = false; - m_details.fields.insert(get_field_name(e->field, e->arg)); + m_last_node_field_name.clear(); + e->left->accept(this); + if (m_last_node_field_name.empty()) + { + throw std::runtime_error("can't find field info in unary check expression"); + } + m_details.fields.insert(m_last_node_field_name); m_details.operators.insert(e->op); } +void filter_details_resolver::visitor::visit(ast::identifier_expr* e) +{ + // todo(jasondellaluce): maybe throw an error if we encounter an unknown macro? + if(m_details.known_macros.find(e->identifier) != m_details.known_macros.end()) + { + m_details.macros.insert(e->identifier); + } +} + void filter_details_resolver::visitor::visit(ast::value_expr* e) { - if (m_expect_macro) - { - if(m_details.known_macros.find(e->value) != m_details.known_macros.end()) - { - m_details.macros.insert(e->value); - } - // todo(jasondellaluce): should we throw an error if we - // encounter an unknown macro? - } - else if (m_expect_evtname) + if (m_expect_evtname) { m_details.evtnames.insert(e->value); } } + +void filter_details_resolver::visitor::visit(ast::field_expr* e) +{ + m_last_node_field_name = get_field_name(e->field, e->arg); +} + +void filter_details_resolver::visitor::visit(ast::field_transformer_expr* e) +{ + e->value->accept(this); +} diff --git a/userspace/engine/filter_details_resolver.h b/userspace/engine/filter_details_resolver.h index 798285bf..8387da89 100644 --- a/userspace/engine/filter_details_resolver.h +++ b/userspace/engine/filter_details_resolver.h @@ -60,22 +60,25 @@ private: explicit visitor(filter_details& details) : m_details(details), m_expect_list(false), - m_expect_macro(false), - m_expect_evtname(false) {} + m_expect_evtname(false), + m_last_node_field_name() {} visitor(visitor&&) = default; visitor(const visitor&) = delete; void visit(libsinsp::filter::ast::and_expr* e) override; void visit(libsinsp::filter::ast::or_expr* e) override; void visit(libsinsp::filter::ast::not_expr* e) override; + void visit(libsinsp::filter::ast::identifier_expr* e) override; void visit(libsinsp::filter::ast::value_expr* e) override; void visit(libsinsp::filter::ast::list_expr* e) override; void visit(libsinsp::filter::ast::unary_check_expr* e) override; void visit(libsinsp::filter::ast::binary_check_expr* e) override; + void visit(libsinsp::filter::ast::field_expr* e) override; + void visit(libsinsp::filter::ast::field_transformer_expr* e) override; filter_details& m_details; bool m_expect_list; - bool m_expect_macro; bool m_expect_evtname; + std::string m_last_node_field_name; }; }; diff --git a/userspace/engine/filter_macro_resolver.cpp b/userspace/engine/filter_macro_resolver.cpp index f872ac96..3c22775c 100644 --- a/userspace/engine/filter_macro_resolver.cpp +++ b/userspace/engine/filter_macro_resolver.cpp @@ -101,8 +101,6 @@ void filter_macro_resolver::visitor::visit(ast::list_expr* e) void filter_macro_resolver::visitor::visit(ast::binary_check_expr* e) { - // avoid exploring checks, so that we can be sure that each - // value_expr* node visited is a macro identifier m_node_substitute = nullptr; } @@ -113,10 +111,22 @@ void filter_macro_resolver::visitor::visit(ast::unary_check_expr* e) 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'. - const auto& macro = m_macros.find(e->value); + m_node_substitute = nullptr; +} + +void filter_macro_resolver::visitor::visit(ast::field_expr* e) +{ + m_node_substitute = nullptr; +} + +void filter_macro_resolver::visitor::visit(ast::field_transformer_expr* e) +{ + m_node_substitute = nullptr; +} + +void filter_macro_resolver::visitor::visit(ast::identifier_expr* e) +{ + const auto& macro = m_macros.find(e->identifier); if (macro != m_macros.end() && macro->second) // skip null-ptr macros { // note: checks for loop detection @@ -126,7 +136,7 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e) auto msg = "reference loop in macro '" + macro->first + "'"; m_errors.push_back({msg, e->get_pos()}); m_node_substitute = nullptr; - m_unknown_macros.push_back({e->value, e->get_pos()}); + m_unknown_macros.push_back({e->identifier, e->get_pos()}); return; } @@ -140,12 +150,12 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e) { m_node_substitute = std::move(new_node); } - m_resolved_macros.push_back({e->value, e->get_pos()}); + m_resolved_macros.push_back({e->identifier, e->get_pos()}); m_macros_path.pop_back(); } else { m_node_substitute = nullptr; - m_unknown_macros.push_back({e->value, e->get_pos()}); + m_unknown_macros.push_back({e->identifier, e->get_pos()}); } } diff --git a/userspace/engine/filter_macro_resolver.h b/userspace/engine/filter_macro_resolver.h index 605f6d01..7e1edff7 100644 --- a/userspace/engine/filter_macro_resolver.h +++ b/userspace/engine/filter_macro_resolver.h @@ -35,7 +35,7 @@ class filter_macro_resolver according with all the definitions added through set_macro(), by replacing the reference with a clone of the macro AST. \param filter The filter AST to be processed. Note that the pointer - is passed by reference and be modified in order to apply + is passed by reference and be transformed in order to apply the substutions. In that case, the old pointer is owned by this class and is deleted automatically. \return true if at least one of the defined macros is resolved @@ -121,10 +121,13 @@ class filter_macro_resolver void visit(libsinsp::filter::ast::and_expr* e) override; void visit(libsinsp::filter::ast::or_expr* e) override; void visit(libsinsp::filter::ast::not_expr* e) override; + void visit(libsinsp::filter::ast::identifier_expr* e) override; void visit(libsinsp::filter::ast::value_expr* e) override; void visit(libsinsp::filter::ast::list_expr* e) override; void visit(libsinsp::filter::ast::unary_check_expr* e) override; void visit(libsinsp::filter::ast::binary_check_expr* e) override; + void visit(libsinsp::filter::ast::field_expr* e) override; + void visit(libsinsp::filter::ast::field_transformer_expr* e) override; }; std::vector m_errors; diff --git a/userspace/engine/filter_warning_resolver.cpp b/userspace/engine/filter_warning_resolver.cpp index 730a5d97..e497b5c0 100644 --- a/userspace/engine/filter_warning_resolver.cpp +++ b/userspace/engine/filter_warning_resolver.cpp @@ -49,14 +49,22 @@ bool filter_warning_resolver::run( void filter_warning_resolver::visitor::visit( libsinsp::filter::ast::binary_check_expr* e) { - if (is_unsafe_field(e->field) && is_equality_operator(e->op)) + m_last_node_is_unsafe_field = false; + e->left->accept(this); + if (m_last_node_is_unsafe_field && is_equality_operator(e->op)) { m_is_equality_check = true; - e->value->accept(this); + e->right->accept(this); m_is_equality_check = false; } } +void filter_warning_resolver::visitor::visit( + libsinsp::filter::ast::field_expr* e) +{ + m_last_node_is_unsafe_field = is_unsafe_field(e->field); +} + void filter_warning_resolver::visitor::visit( libsinsp::filter::ast::value_expr* e) { diff --git a/userspace/engine/filter_warning_resolver.h b/userspace/engine/filter_warning_resolver.h index 1dc0d645..8de59082 100644 --- a/userspace/engine/filter_warning_resolver.h +++ b/userspace/engine/filter_warning_resolver.h @@ -49,17 +49,22 @@ public: private: struct visitor : public libsinsp::filter::ast::base_expr_visitor { - visitor(): m_is_equality_check(false), m_warnings(nullptr) {} + visitor(): + m_is_equality_check(false), + m_last_node_is_unsafe_field(false), + m_warnings(nullptr) {} visitor(visitor&&) = default; visitor& operator = (visitor&&) = default; visitor(const visitor&) = delete; visitor& operator = (const visitor&) = delete; bool m_is_equality_check; + bool m_last_node_is_unsafe_field; std::set* m_warnings; void visit(libsinsp::filter::ast::value_expr* e) override; void visit(libsinsp::filter::ast::list_expr* e) override; void visit(libsinsp::filter::ast::binary_check_expr* e) override; + void visit(libsinsp::filter::ast::field_expr* e) override; }; };