Skip to content

Commit

Permalink
Support condition parse errors in rule loading results
Browse files Browse the repository at this point in the history 
In #2098 and #2158, we reworked how rules loading errors/warnings were
returned to provide a richer set of information, including
locations/context for the errors/warnings.

That did *not* include locations within condition expressions,
though. When parsing a condition expression resulted in a
warning/error, the location simply pointed to the condition property
of the rule.

This commit improves this to handle parse errors:

- When libsinsp::filter::parser::parse() throws an exception, use
  get_pos() to get the position within the condition string.
- Add a new context() constructor that takes a filter pos_info instead
  of a YAML::Mark.

Now that positions aren't always related to the location of yaml
nodes, Make up a generic "position" struct for locations and convert
YAML::Mark and parser positions to a position struct.

Also allow a context to contain an alternate content string which is
used to build the snippet. For contexts related to condition strings,
the content is the condition.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
  • Loading branch information
@mstemm @poiana
mstemm authored and poiana committed Sep 7, 2022
1 parent af95455 commit 7a5a4c3
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 55 deletions.
152 changes: 99 additions & 53 deletions userspace/engine/rule_loader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,34 +36,82 @@ using namespace std;
using namespace libsinsp::filter;

rule_loader::context::context(const std::string& name)
: name(name)
{
// This ensures that every context has one location, even if
// that location is effectively the whole document.
location loc = {name, position(), "rules content", ""};
m_locs.push_back(loc);
}

rule_loader::context::context(const YAML::Node &item,
const std::string item_type,
const std::string item_name,
const context& parent)
: name(parent.name)
{
init(parent.name(), position(item.Mark()), item_type, item_name, parent);
}

rule_loader::context::context(const libsinsp::filter::parser::pos_info& pos,
const std::string& condition,
const context& parent)
: alt_content(condition)
{

// Contexts based on conditions don't use the
// filename. Instead the "name" is just the condition, and
// uses a short prefix of the condition.
std::string name = "\"" + condition.substr(0, 20) + "...\"";
std::replace(name.begin(), name.end(), '\n', ' ');
std::replace(name.begin(), name.end(), '\r', ' ');

std::string item_type = "condition expression";
std::string item_name = "";

// Convert the parser position to a context location. Both
// they have the same basic info (position, line, column).
// parser line/columns are 1-indexed while yaml marks are
// 0-indexed, though.
position condpos;
condpos.pos = pos.idx;
condpos.line = pos.line-1;
condpos.column = pos.col-1;

init(name, condpos, item_type, item_name, parent);
}

const std::string& rule_loader::context::name() const
{
// All valid contexts should have at least one location.
if(m_locs.empty())
{
throw falco_exception("rule_loader::context without location?");
}

return m_locs.front().name;
}

void rule_loader::context::init(const std::string& name,
const position& pos,
const std::string item_type,
const std::string item_name,
const context& parent)
{
// Copy parent locations
m_locs = parent.m_locs;

// Add current item to back
location loc = {item.Mark(), item_type, item_name};
location loc = {name, pos, item_type, item_name};
m_locs.push_back(loc);
}

std::string rule_loader::context::as_string()
{
std::ostringstream os;

// If no locations (can happen for initial file-level
// context), just note it was somewhere in the file
// All valid contexts should have at least one location.
if(m_locs.empty())
{
os << "In " << name << ":" << std::endl;
return os.str();
throw falco_exception("rule_loader::context without location?");
}

bool first = true;
Expand All @@ -81,9 +129,9 @@ std::string rule_loader::context::as_string()
os << ": ";

os << "("
<< name << ":"
<< loc.mark.line << ":"
<< loc.mark.column
<< loc.name << ":"
<< loc.pos.line << ":"
<< loc.pos.column
<< ")" << std::endl;
}

Expand All @@ -96,67 +144,58 @@ nlohmann::json rule_loader::context::as_json()

ret["locations"] = nlohmann::json::array();

// All valid contexts should have at least one location.
if(m_locs.empty())
{
throw falco_exception("rule_loader::context without location?");
}

for(auto& loc : m_locs)
{
nlohmann::json jloc, jpos;

jloc["item_type"] = "file";
jloc["item_name"] = "";
jloc["item_type"] = loc.item_type;
jloc["item_name"] = loc.item_name;

jpos["filename"] = name;
jpos["line"] = 0;
jpos["column"] = 0;
jpos["offset"] = 0;
jpos["name"] = loc.name;
jpos["line"] = loc.pos.line;
jpos["column"] = loc.pos.column;
jpos["offset"] = loc.pos.pos;

jloc["position"] = jpos;

ret["locations"].push_back(jloc);
}
else
{
for(auto& loc : m_locs)
{
nlohmann::json jloc, jpos;

jloc["item_type"] = loc.item_type;
jloc["item_name"] = loc.item_name;

jpos["filename"] = name;
jpos["line"] = loc.mark.line;
jpos["column"] = loc.mark.column;
jpos["offset"] = loc.mark.pos;

jloc["position"] = jpos;

ret["locations"].push_back(jloc);
}
}

return ret;
}

std::string rule_loader::context::snippet(const falco::load_result::rules_contents_t& rules_contents,
size_t snippet_width) const
{
std::string ret;

// All valid contexts should have at least one location.
if(m_locs.empty())
{
return "<No context available>\n";
throw falco_exception("rule_loader::context without location?");
}

rule_loader::context::location loc = m_locs.back();
auto it = rules_contents.find(loc.name);

auto it = rules_contents.find(name);
if(alt_content.empty() && it == rules_contents.end())
{
return "<No context for file + " + loc.name + ">\n";
}

if(it == rules_contents.end())
// If not using alt content, the last location's name must be found in rules_contents
const std::string& snip_content = (!alt_content.empty() ? alt_content : it->second.get());

if(snip_content.empty())
{
return "<No context available>\n";
}

const std::string& snip_content = it->second;

size_t from = loc.mark.pos;
size_t from = loc.pos.pos;

// In some cases like this, where the content ends with a
// dangling property value:
Expand All @@ -172,10 +211,10 @@ std::string rule_loader::context::snippet(const falco::load_result::rules_conten
// However, some lines can be very very long, so the walk
// forwards/walk backwards is capped at a maximum of
// snippet_width/2 characters in either direction.
for(; from > 0 && snip_content.at(from) != '\n' && (loc.mark.pos - from) < (snippet_width/2); from--);
for(; from > 0 && snip_content.at(from) != '\n' && (loc.pos.pos - from) < (snippet_width/2); from--);

size_t to = loc.mark.pos;
for(; to < snip_content.size()-1 && snip_content.at(to) != '\n' && (to - loc.mark.pos) < (snippet_width/2); to++);
size_t to = loc.pos.pos;
for(; to < snip_content.size()-1 && snip_content.at(to) != '\n' && (to - loc.pos.pos) < (snippet_width/2); to++);

// Don't include the newlines
if(snip_content.at(from) == '\n')
Expand All @@ -187,24 +226,29 @@ std::string rule_loader::context::snippet(const falco::load_result::rules_conten
to--;
}

ret = snip_content.substr(from, to-from+1);
std::string ret = snip_content.substr(from, to-from+1);

if(snip_content.empty())
{
return "<No context available>\n";
}

// Replace the initial/end characters with '...' if the walk
// forwards/backwards was incomplete
if(loc.mark.pos - from >= (snippet_width/2))
if(loc.pos.pos - from >= (snippet_width/2))
{
ret.replace(0, 2, "...");
ret.replace(0, 3, "...");
}

if(to - loc.mark.pos >= (snippet_width/2))
if(to - loc.pos.pos >= (snippet_width/2))
{
ret.replace(ret.size()-2, ret.size(), "...");
ret.replace(ret.size()-3, 3, "...");
}

ret += "\n";

// Add a blank line with a marker at the position within the snippet
ret += std::string(loc.mark.pos-from, ' ') + '^' + "\n";
ret += std::string(loc.pos.pos-from, ' ') + '^' + "\n";

return ret;
}
Expand Down Expand Up @@ -804,10 +848,12 @@ static shared_ptr<ast::expr> parse_condition(
}
catch (const sinsp_exception& e)
{
rule_loader::context parsectx(p.get_pos(), condition, ctx);

throw rule_loader::rule_load_exception(
load_result::LOAD_ERR_COMPILE_CONDITION,
e.what(),
ctx);
parsectx);
}
}

Expand Down
48 changes: 46 additions & 2 deletions userspace/engine/rule_loader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,26 @@ class rule_loader
public:
static const size_t default_snippet_width = 160;

struct position
{
position() : pos(0), line(0), column(0) {};
position(const YAML::Mark& mark) : pos(mark.pos), line(mark.line), column(mark.column) {};
~position() = default;
int pos;
int line;
int column;
};

struct location
{
// A name for the content this location refers
// to. Will generally be a filename, can also
// refer to a rule/macro condition when the
// location points into a condition string.
std::string name;

// The original location in the document
YAML::Mark mark;
position pos;

// The kind of item at this location
// (e.g. "list", "macro", "rule", "exception", etc)
Expand All @@ -57,23 +73,51 @@ class rule_loader
const std::string item_type,
const std::string item_name,
const context& parent);

// Build a context from a condition expression +
// parser position. This does not use the original
// yaml content because:
// - YAML block indicators will remove whitespace/newlines/wrapping
// from the YAML node containing the condition expression.
// - When compiling, the condition expression has expanded
// macro and list references with their values.
context(const libsinsp::filter::parser::pos_info& pos,
const std::string& condition,
const context& parent);

virtual ~context() = default;

// Return the content name (generally filename) for
// this context
const std::string& name() const;

// Return a snippet of the provided rules content
// corresponding to this context.
// Uses the provided rules_contents to look up the original
// rules content for a given location name.
// (If this context has a non-empty alt_content, it
// will be used to create the snippet, ignoring the
// provided rules_contents).
std::string snippet(const falco::load_result::rules_contents_t& rules_contents, size_t snippet_width = default_snippet_width) const;

std::string as_string();
nlohmann::json as_json();

private:
std::string name;
void init(const std::string& name,
const position& pos,
const std::string item_type,
const std::string item_name,
const context& parent);

// A chain of locations from the current item, its
// parent, possibly older ancestors.
std::vector<location> m_locs;

// If non-empty, this content will be used when
// creating snippets. Used for contexts involving
// condition expressions.
std::string alt_content;
};

struct warning
Expand Down

0 comments on commit 7a5a4c3

Please sign in to comment.