github/falco
1
0
Fork 0
mirror of https://github.com/falcosecurity/falco.git synced 2025-06-28 07:37:32 +00:00
Code Issues Projects Releases Wiki Activity

Fix(engine) Save parse positions when finding unresolved macros

Browse Source 
Now that ASTs contain parse positions, use them when reporting errors
about unknown macros.

When doing the first pass to find all macro references, save macros as
a map<macro name,parse position> instead of a set<macro name>. While
making that change, change the visitor struct to use references
instead of pointers.

In the second pass, when reporting any unresolved macro references,
also report the parse position.

The unit tests also check that the positions of macros are properly
returned in the resolved/unresolved maps.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
This commit is contained in:
Mark Stemm 2022-08-24 17:16:01 -07:00 committed by poiana
parent 83b12bab1d
commit 910b8ff858
3 changed files with 108 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
tests/engine/test_filter_macro_resolver.cpp
View File

@ -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<std::unique_ptr<expr>> 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<expr> filter = std::move(and_expr::create(filter_and));
std::vector<std::unique_ptr<expr>> 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<expr> macro = std::move(
unary_check_expr::create("test.field", "", "exists"));
std::shared_ptr<expr> filter = std::move(value_expr::create(macro_name));
std::shared_ptr<expr> 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<expr> a_macro = std::move(
unary_check_expr::create("one.field", "", "exists"));
std::shared_ptr<expr> b_macro = std::move(
unary_check_expr::create("another.field", "", "exists"));
std::vector<std::unique_ptr<expr>> 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<expr> filter = std::move(or_expr::create(filter_or));
std::vector<std::unique_ptr<expr>> 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<std::unique_ptr<expr>> 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<expr> a_macro = std::move(and_expr::create(a_macro_and));
std::shared_ptr<expr> b_macro = std::move(
unary_check_expr::create("another.field", "", "exists"));
std::shared_ptr<expr> filter = std::move(value_expr::create(a_macro_name));
std::shared_ptr<expr> filter = std::move(value_expr::create(a_macro_name, a_macro_pos));
std::vector<std::unique_ptr<expr>> 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<std::unique_ptr<expr>> 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<expr> 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<std::unique_ptr<expr>> 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<expr> a_macro = std::move(and_expr::create(a_macro_and));
std::shared_ptr<expr> filter = std::move(value_expr::create(a_macro_name));
std::shared_ptr<expr> 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<expr> macro = std::move(unary_check_expr::create("test.field", "", "exists"));
std::shared_ptr<expr> a_filter = std::move(value_expr::create(macro_name));
std::shared_ptr<expr> b_filter = std::move(value_expr::create(macro_name));
std::shared_ptr<expr> a_filter = std::move(value_expr::create(macro_name, macro_pos_1));
std::shared_ptr<expr> 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<unary_check_expr> macro = std::move(unary_check_expr::create("test.field", "", "exists"));
std::shared_ptr<expr> filter = std::move(value_expr::create(macro_name));
std::shared_ptr<expr> 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()));

24
userspace/engine/filter_macro_resolver.cpp
View File

@ -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<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)
@ -61,12 +57,12 @@ void filter_macro_resolver::set_macro(
m_macros[name] = macro;
}
const unordered_set<string>& 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<string>& 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();
}
}

27
userspace/engine/filter_macro_resolver.h
View File

@ -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<libsinsp::filter::ast::expr> macro);
/*!
\brief used in get_{resolved,unknown}_macros
*/
typedef std::unordered_map<std::string,libsinsp::filter::ast::pos_info> 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<std::string>& 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<std::string>& 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<libsinsp::filter::ast::expr> m_node_substitute;
std::unordered_set<std::string>* m_unknown_macros;
std::unordered_set<std::string>* 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<std::string> m_unknown_macros;
std::unordered_set<std::string> m_resolved_macros;
macro_info_map m_unknown_macros;
macro_info_map m_resolved_macros;
macro_defs m_macros;
};