Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new(libsinsp): add len() filter transformer #2131

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions userspace/libsinsp/filter/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,11 @@ static const std::vector<std::string> s_binary_list_ops = {

static constexpr const char* s_field_transformer_val = "val(";

static const std::vector<std::string> s_field_transformers = {
"tolower(",
"toupper(",
"b64(",
"basename(",
};
static const std::vector<std::string> s_field_transformers = {"tolower(",
"toupper(",
"b64(",
"basename(",
"len("};

static inline void update_pos(const char c, ast::pos_info& pos) {
pos.col++;
Expand Down
2 changes: 1 addition & 1 deletion userspace/libsinsp/filter/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class RE2;
// | 'startswith ' | 'bstartswith ' | 'endswith '
// ListOperator ::= 'intersects' | 'in' | 'pmatch'
// FieldTransformerVal ::= 'val('
// FieldTransformerType ::= 'tolower(' | 'toupper(' | 'b64(' | 'basename('
// FieldTransformerType ::= 'tolower(' | 'toupper(' | 'b64(' | 'basename(' | 'len('
//
// Tokens (Regular Expressions):
// Identifier ::= [a-zA-Z]+[a-zA-Z0-9_]*
Expand Down
77 changes: 71 additions & 6 deletions userspace/libsinsp/sinsp_filter_transformer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,16 @@
return true;
}

bool sinsp_filter_transformer::transform_type(ppm_param_type& t) const {
bool sinsp_filter_transformer::transform_type(ppm_param_type& t, uint32_t& flags) const {
bool is_list = flags & EPF_IS_LIST;
switch(m_type) {
case FTR_TOUPPER: {
switch(t) {
case PT_CHARBUF:
case PT_FSPATH:
case PT_FSRELPATH:
// for TOUPPER, the transformed type is the same as the input type
return true;
return !is_list;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this potentially a breaking change? It might be that we supported tolower (and similars) with list-type fields without noticing. Maybe I'm wrong, just checking in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, although I never tried it. I would suggest to test and document this behavior if we want it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have reverted this change and I'll add a couple of tests.

default:
return false;
}
Expand All @@ -82,7 +83,7 @@
case PT_FSPATH:
case PT_FSRELPATH:
// for TOLOWER, the transformed type is the same as the input type
return true;
return !is_list;
default:
return false;
}
Expand All @@ -92,7 +93,7 @@
case PT_CHARBUF:
case PT_BYTEBUF:
// for BASE64, the transformed type is the same as the input type
return true;
return !is_list;
default:
return false;
}
Expand All @@ -107,6 +108,23 @@
case PT_FSPATH:
case PT_FSRELPATH:
// for BASENAME, the transformed type is the same as the input type
return !is_list;
default:
return false;
}
}
case FTR_LEN: {
if(is_list) {
t = PT_UINT64;
flags = flags & ~EPF_IS_LIST;
return true;
}
switch(t) {
case PT_CHARBUF:
case PT_BYTEBUF:
case PT_FSPATH:
case PT_FSRELPATH:
t = PT_UINT64;
return true;
default:
return false;
Expand All @@ -119,8 +137,11 @@
}

bool sinsp_filter_transformer::transform_values(std::vector<extract_value_t>& vec,
ppm_param_type& t) {
if(!transform_type(t)) {
ppm_param_type& t,
uint32_t& flags) {
bool is_list = flags & EPF_IS_LIST;
ppm_param_type original_type = t;
if(!transform_type(t, flags)) {
throw_type_incompatibility_err(t, filter_transformer_type_str(m_type));
}

Expand Down Expand Up @@ -180,6 +201,50 @@
return true;
});
}
case FTR_LEN: {
assert((void("len() type must be PT_UINT64"), t == PT_UINT64));
if(is_list) {
uint64_t len = static_cast<uint64_t>(vec.size());
auto stored_val = store_scalar(len);
vec.clear();
vec.push_back(stored_val);
return true;
}

// not a list: could be string or buffer
bool is_string = false;
switch(original_type) {
case PT_CHARBUF:
case PT_FSPATH:
case PT_FSRELPATH:
is_string = true;
break;
case PT_BYTEBUF:
is_string = false;
break;

Check warning on line 224 in userspace/libsinsp/sinsp_filter_transformer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/sinsp_filter_transformer.cpp#L222-L224

Added lines #L222 - L224 were not covered by tests
default:
return false;
}

for(std::size_t i = 0; i < vec.size(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question -- is there really a scenario where the field is not a list-type and vec.size() > 1? Should we put an assert?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly do not know, but tests were using it like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can't think of a corner case (such as plugins) I'll remove the loop as it simplifies things.

uint64_t len;
if(vec[i].ptr == nullptr) {
vec[i] = store_scalar(0);
continue;

Check warning on line 233 in userspace/libsinsp/sinsp_filter_transformer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/sinsp_filter_transformer.cpp#L232-L233

Added lines #L232 - L233 were not covered by tests
}

if(is_string) {
len = static_cast<uint64_t>(
strnlen(reinterpret_cast<const char*>(vec[i].ptr), vec[i].len));
vec[i] = store_scalar(len);
continue;
}

len = static_cast<uint64_t>(vec[i].len);
vec[i] = store_scalar(len);

Check warning on line 244 in userspace/libsinsp/sinsp_filter_transformer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/sinsp_filter_transformer.cpp#L243-L244

Added lines #L243 - L244 were not covered by tests
}
return true;
}
default:
throw_unsupported_err(m_type);
return false;
Expand Down
17 changes: 15 additions & 2 deletions userspace/libsinsp/sinsp_filter_transformer.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
FTR_BASE64 = 2,
FTR_STORAGE = 3, // This transformer is only used internally
FTR_BASENAME = 4,
FTR_LEN = 5
};

static inline std::string filter_transformer_type_str(filter_transformer_type m) {
Expand All @@ -42,6 +43,8 @@
return "storage";
case FTR_BASENAME:
return "basename";
case FTR_LEN:
return "len";

Check warning on line 47 in userspace/libsinsp/sinsp_filter_transformer.h

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/sinsp_filter_transformer.h#L46-L47

Added lines #L46 - L47 were not covered by tests
default:
throw sinsp_exception("unknown field transfomer id " + std::to_string(m));
}
Expand All @@ -63,6 +66,9 @@
if(str == "basename") {
return filter_transformer_type::FTR_BASENAME;
}
if(str == "len") {
return filter_transformer_type::FTR_LEN;
}
throw sinsp_exception("unknown field transfomer '" + str + "'");
}

Expand All @@ -72,9 +78,9 @@

sinsp_filter_transformer(filter_transformer_type t): m_type(t) {};

bool transform_type(ppm_param_type& t) const;
bool transform_type(ppm_param_type& t, uint32_t& flags) const;

bool transform_values(std::vector<extract_value_t>& vals, ppm_param_type& t);
bool transform_values(std::vector<extract_value_t>& vals, ppm_param_type& t, uint32_t& flags);

private:
using str_transformer_func_t = std::function<bool(std::string_view in, storage_t& out)>;
Expand All @@ -83,6 +89,13 @@
ppm_param_type t,
str_transformer_func_t mod);

template<class T>
extract_value_t store_scalar(T value) {
uint8_t* bytes = reinterpret_cast<uint8_t*>(&value);
storage_t& stored_val = m_storage_values.emplace_back(bytes, bytes + sizeof(T));
return {static_cast<uint8_t*>(stored_val.data()), static_cast<uint32_t>(stored_val.size())};
}

filter_transformer_type m_type;
std::vector<storage_t> m_storage_values;
};
15 changes: 9 additions & 6 deletions userspace/libsinsp/sinsp_filtercheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ char* sinsp_filter_check::tostring(sinsp_evt* evt) {
}

auto ftype = get_transformed_field_info()->m_type;
if(m_field->m_flags & EPF_IS_LIST) {
if(get_transformed_field_info()->m_flags & EPF_IS_LIST) {
std::string res = "(";
for(auto& val : m_extracted_values) {
if(res.size() > 1) {
Expand Down Expand Up @@ -478,7 +478,7 @@ Json::Value sinsp_filter_check::tojson(sinsp_evt* evt) {
}

auto ftype = get_transformed_field_info()->m_type;
if(m_field->m_flags & EPF_IS_LIST) {
if(get_transformed_field_info()->m_flags & EPF_IS_LIST) {
for(auto& val : m_extracted_values) {
jsonval.append(rawval_to_json(val.ptr, ftype, m_field->m_print_format, val.len));
}
Expand Down Expand Up @@ -1045,11 +1045,12 @@ void sinsp_filter_check::add_transformer(filter_transformer_type trtype) {
// apply type transformation, both as a feasibility check and
// as an information to be returned later on
sinsp_filter_transformer tr(trtype);
if(!tr.transform_type(m_transformed_field->m_type)) {
if(!tr.transform_type(m_transformed_field->m_type, m_transformed_field->m_flags)) {
throw sinsp_exception("can't add field transformer: type '" +
std::string(param_type_to_string(m_transformed_field->m_type)) +
"' is not supported by '" + filter_transformer_type_str(trtype) +
"' transformer applied on field '" +
"' transformer applied on " +
(m_transformed_field->is_list() ? "list " : "") + "field '" +
std::string(get_field_info()->m_name) + "'");
}

Expand All @@ -1062,9 +1063,11 @@ void sinsp_filter_check::add_transformer(filter_transformer_type trtype) {
}

bool sinsp_filter_check::apply_transformers(std::vector<extract_value_t>& values) {
auto type = get_field_info()->m_type;
const filtercheck_field_info* field_info = get_field_info();
auto field_type = field_info->m_type;
auto field_flags = field_info->m_flags;
for(auto& tr : m_transformers) {
if(!tr.transform_values(values, type)) {
if(!tr.transform_values(values, field_type, field_flags)) {
return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion userspace/libsinsp/sinsp_filtercheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class sinsp_filter_check {
//
// Extract the field from the event. If sanitize_strings is true, any
// string values are sanitized to remove nonprintable characters.
// By default, this fills the vector with only one value, retireved by calling the single-result
// By default, this fills the vector with only one value, retrieved by calling the single-result
// extract method.
// If a NULL value is returned by extract, the vector is emptied.
// Subclasses are meant to either override this, or the single-valued extract method.
Expand Down
2 changes: 1 addition & 1 deletion userspace/libsinsp/sinsp_filtercheck_fd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ static const filtercheck_field_info sinsp_filter_check_fd_fields[] = {
"FD full name raw. Just like fd.name, but only used if fd is a file path. File path is "
"kept raw with limited sanitization and without deriving the absolute path."},
{PT_CHARBUF,
EPF_IS_LIST | EPF_ARG_ALLOWED | EPF_NO_RHS | EPF_NO_TRANSFORMER,
EPF_IS_LIST | EPF_ARG_ALLOWED | EPF_NO_RHS,
PF_DEC,
"fd.types",
"FD Type",
Expand Down
2 changes: 1 addition & 1 deletion userspace/libsinsp/test/filter_parser.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ TEST(parser, supported_operators) {

TEST(parser, supported_field_transformers) {
std::string expected_val = "val";
std::vector<std::string> expected = {"tolower", "toupper", "b64", "basename"};
std::vector<std::string> expected = {"tolower", "toupper", "b64", "basename", "len"};

auto actual = parser::supported_field_transformers();
ASSERT_EQ(actual.size(), expected.size());
Expand Down
Loading
Loading