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

filterx: various small fixes #97

Merged
merged 11 commits into from
May 22, 2024
13 changes: 7 additions & 6 deletions lib/filterx/expr-comparison.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,13 @@ _evaluate_as_num(FilterXObject *lhs, FilterXObject *rhs, gint operator)
static gboolean
_evaluate_type_aware(FilterXObject *lhs, FilterXObject *rhs, gint operator)
{
if (filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(string)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(bytes)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(protobuf)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(message_value)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(json_object)) || // TODO: we should have generic map and array cmp
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(json_array)))
if (lhs->type == rhs->type &&
(filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(string)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(bytes)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(protobuf)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(message_value)) ||
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(json_object)) || // TODO: we should have generic map and array cmp
filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(json_array))))
return _evaluate_as_string(lhs, rhs, operator);

if (filterx_object_is_type(lhs, &FILTERX_TYPE_NAME(null)) ||
Expand Down
35 changes: 35 additions & 0 deletions lib/filterx/expr-function.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "filterx/filterx-eval.h"
#include "filterx/expr-literal.h"
#include "filterx/object-string.h"
#include "filterx/object-primitive.h"
#include "filterx/object-null.h"
#include "plugin.h"
#include "cfg.h"
Expand Down Expand Up @@ -320,6 +321,40 @@ filterx_function_args_get_named_literal_string(FilterXFunctionArgs *self, const
return str;
}

gboolean
filterx_function_args_get_named_literal_boolean(FilterXFunctionArgs *self, const gchar *name, gboolean *exists,
gboolean *error)
{
*error = FALSE;

FilterXExpr *expr = filterx_function_args_get_named_expr(self, name);
*exists = !!expr;
if (!expr)
return FALSE;

FilterXObject *obj = NULL;
gboolean value = FALSE;

if (!filterx_expr_is_literal(expr))
goto error;

obj = filterx_expr_eval(expr);
if (!obj)
goto error;

if (!filterx_boolean_unwrap(obj, &value))
goto error;

goto success;

error:
*error = TRUE;
success:
filterx_object_unref(obj);
filterx_expr_unref(expr);
return value;
}

void
filterx_function_args_free(FilterXFunctionArgs *self)
{
Expand Down
2 changes: 2 additions & 0 deletions lib/filterx/expr-function.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ FilterXExpr *filterx_function_args_get_named_expr(FilterXFunctionArgs *self, con
FilterXObject *filterx_function_args_get_named_object(FilterXFunctionArgs *self, const gchar *name, gboolean *exists);
const gchar *filterx_function_args_get_named_literal_string(FilterXFunctionArgs *self, const gchar *name,
gsize *len, gboolean *exists);
gboolean filterx_function_args_get_named_literal_boolean(FilterXFunctionArgs *self, const gchar *name,
gboolean *exists, gboolean *error);
void filterx_function_args_free(FilterXFunctionArgs *self);

FilterXExpr *filterx_function_lookup(GlobalConfig *cfg, const gchar *function_name, GList *args, GError **error);
Expand Down
18 changes: 17 additions & 1 deletion lib/filterx/object-json-array.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ _marshal(FilterXObject *s, GString *repr, LogMessageValueType *t)
return TRUE;
}

static gboolean
_repr(FilterXObject *s, GString *repr)
{
FilterXJsonArray *self = (FilterXJsonArray *) s;

const gchar *json_repr = json_object_to_json_string_ext(self->jso, JSON_C_TO_STRING_PLAIN);
g_string_append(repr, json_repr);

return TRUE;
}

static gboolean
_map_to_json(FilterXObject *s, struct json_object **jso, FilterXObject **assoc_object)
{
Expand Down Expand Up @@ -106,7 +117,11 @@ _get_subscript(FilterXList *s, guint64 index)

struct json_object *jso = json_object_array_get_idx(self->jso, index);
if (!jso)
return NULL;
{
/* NULL is returned if the stored value is null. */
if (json_object_array_length(self->jso) > index + 1)
return NULL;
}

return filterx_json_convert_json_to_object_cached(&s->super, &self->root_container, jso);
}
Expand Down Expand Up @@ -351,6 +366,7 @@ FILTERX_DEFINE_TYPE(json_array, FILTERX_TYPE_NAME(list),
.truthy = _truthy,
.free_fn = _free,
.marshal = _marshal,
.repr = _repr,
.map_to_json = _map_to_json,
.clone = _clone,
.list_factory = filterx_json_array_new_empty,
Expand Down
12 changes: 12 additions & 0 deletions lib/filterx/object-json-object.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ _marshal(FilterXObject *s, GString *repr, LogMessageValueType *t)
return TRUE;
}

static gboolean
_repr(FilterXObject *s, GString *repr)
{
FilterXJsonObject *self = (FilterXJsonObject *) s;

const gchar *json_repr = json_object_to_json_string_ext(self->jso, JSON_C_TO_STRING_PLAIN);
g_string_append(repr, json_repr);

return TRUE;
}

static gboolean
_map_to_json(FilterXObject *s, struct json_object **jso, FilterXObject **assoc_object)
{
Expand Down Expand Up @@ -253,6 +264,7 @@ FILTERX_DEFINE_TYPE(json_object, FILTERX_TYPE_NAME(dict),
.truthy = _truthy,
.free_fn = _free,
.marshal = _marshal,
.repr = _repr,
.map_to_json = _map_to_json,
.clone = _clone,
.list_factory = filterx_json_array_new_empty,
Expand Down
13 changes: 13 additions & 0 deletions lib/filterx/object-json.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ _convert_json_to_object(FilterXObject *self, FilterXWeakRef *root_container, str
{
switch (json_object_get_type(jso))
{
case json_type_null:
return filterx_null_new();
case json_type_double:
return filterx_double_new(json_object_get_double(jso));
case json_type_boolean:
Expand Down Expand Up @@ -113,8 +115,15 @@ filterx_json_convert_json_to_object_cached(FilterXObject *self, FilterXWeakRef *
*
* Changing the value of the double to the same value, ditches this,
* but only if necessary.
*
* This only works starting with 0.14, before that json_object_set_double()
* does not drop the string user data, so we cannot check if it is our
* filterx object or the string, which means we cannot cache doubles.
*/

if (JSON_C_MAJOR_VERSION == 0 && JSON_C_MINOR_VERSION < 14)
return _convert_json_to_object(self, root_container, jso);

json_object_set_double(jso, json_object_get_double(jso));
}

Expand All @@ -132,6 +141,10 @@ filterx_json_convert_json_to_object_cached(FilterXObject *self, FilterXWeakRef *
void
filterx_json_associate_cached_object(struct json_object *jso, FilterXObject *filterx_obj)
{
/* null JSON value turns into NULL pointer. */
if (!jso)
return;

filterx_eval_store_weak_ref(filterx_obj);

/* we are not storing a reference in userdata to avoid circular
Expand Down
9 changes: 5 additions & 4 deletions lib/filterx/object-primitive.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,10 @@ _double_map_to_json(FilterXObject *s, struct json_object **object, FilterXObject
gboolean
double_repr(double val, GString *repr)
{
gchar buf[G_ASCII_DTOSTR_BUF_SIZE];
g_ascii_dtostr(buf, sizeof(buf), val);
g_string_append(repr, buf);
gsize init_len = repr->len;
g_string_set_size(repr, init_len + G_ASCII_DTOSTR_BUF_SIZE);
g_ascii_dtostr(repr->str + init_len, G_ASCII_DTOSTR_BUF_SIZE, val);
g_string_set_size(repr, init_len + strlen(repr->str + init_len));
return TRUE;
}

Expand Down Expand Up @@ -317,7 +318,7 @@ static gboolean
_repr(FilterXObject *s, GString *repr)
{
LogMessageValueType t;
return filterx_object_marshal(s, repr, &t);
return filterx_object_marshal_append(s, repr, &t);
}

FILTERX_DEFINE_TYPE(integer, FILTERX_TYPE_NAME(object),
Expand Down
4 changes: 3 additions & 1 deletion lib/filterx/object-string.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,14 @@ _bytes_repr(FilterXObject *s, GString *repr)
gsize target_len = self->str_len * 2;
gsize repr_len = repr->len;
g_string_set_size(repr, target_len + repr_len);
format_hex_string_with_delimiter(self->str, self->str_len, repr->str + repr_len, target_len, 0);
format_hex_string_with_delimiter(self->str, self->str_len, repr->str + repr_len, target_len + 1, 0);
return TRUE;
}

FilterXObject *
filterx_bytes_new(const gchar *mem, gssize mem_len)
{
g_assert(mem_len != -1);
FilterXString *self = (FilterXString *) filterx_string_new(mem, mem_len);
self->super.type = &FILTERX_TYPE_NAME(bytes);
return &self->super;
Expand All @@ -201,6 +202,7 @@ filterx_bytes_new(const gchar *mem, gssize mem_len)
FilterXObject *
filterx_protobuf_new(const gchar *mem, gssize mem_len)
{
g_assert(mem_len != -1);
FilterXString *self = (FilterXString *) filterx_bytes_new(mem, mem_len);
self->super.type = &FILTERX_TYPE_NAME(protobuf);
return &self->super;
Expand Down
34 changes: 20 additions & 14 deletions lib/filterx/tests/test_expr_comparison.c
Original file line number Diff line number Diff line change
Expand Up @@ -533,10 +533,17 @@ Test(expr_comparison, test_string_to_datetime_string_based_comparison)
FCMPX_NE | FCMPX_STRING_BASED, TRUE);
}

// FCMPX_TYPE_AWARE tests
// in case of LHS type is one of: [string, bytes, json, protobuf, message_value], uses FCMPX_STRING_BASED comparsion
// in case of any of LHS or RHS is filterx null, compares filterx types (null less than any)
// uses FCMPX_NUM_BASED in any other cases
/*
* Type aware tests.
*
* In case of both side's types are the same and they are one of: [string, bytes, json, protobuf, message_value],
* we do a string based comparison.
*
* If the one of the sides is null, and we check for eq or ne, they are equal if the other is null,
* and not equal if the other is not null.
*
* In any other scenario we do a number based comparison, which includes NaN logic.
*/

Test(expr_comparison, test_null_cases_type_aware_comparison)
{
Expand All @@ -552,14 +559,13 @@ Test(expr_comparison, test_null_cases_type_aware_comparison)

_assert_comparison(filterx_string_new("foobar", 6), filterx_null_new(), FCMPX_EQ | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_string_new("foobar", 6), filterx_null_new(), FCMPX_LT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_string_new("foobar", 6), filterx_null_new(), FCMPX_GT | FCMPX_TYPE_AWARE, TRUE);
_assert_comparison(filterx_string_new("foobar", 6), filterx_null_new(), FCMPX_GT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_string_new("foobar", 6), filterx_null_new(), FCMPX_NE | FCMPX_TYPE_AWARE, TRUE);
}

Test(expr_comparison, test_string_cases_type_aware_comparison)
{
// TODO: fix float precision error in g_ascii_dtostr 3.14 -> "3.1400000000000000000000001"
// _assert_comparison(filterx_string_new("3.14", 4), filterx_double_new(3.14), FCMPX_EQ | FCMPX_TYPE_AWARE, TRUE);
_assert_comparison(filterx_string_new("3.14", 4), filterx_double_new(3.14), FCMPX_EQ | FCMPX_TYPE_AWARE, TRUE);
MrAnno marked this conversation as resolved.
Show resolved Hide resolved

// string - integer
_assert_comparison(filterx_string_new("443", 3), filterx_integer_new(443), FCMPX_EQ | FCMPX_TYPE_AWARE, TRUE);
Expand All @@ -570,29 +576,29 @@ Test(expr_comparison, test_string_cases_type_aware_comparison)
// check if compared as string
_assert_comparison(filterx_string_new("a443", 4), filterx_integer_new(443), FCMPX_EQ | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_string_new("a443", 4), filterx_integer_new(443), FCMPX_LT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_string_new("a443", 4), filterx_integer_new(443), FCMPX_GT | FCMPX_TYPE_AWARE, TRUE);
_assert_comparison(filterx_string_new("a443", 4), filterx_integer_new(443), FCMPX_GT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_string_new("a443", 4), filterx_integer_new(443), FCMPX_NE | FCMPX_TYPE_AWARE, TRUE);

// bytes - boolean
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(true), FCMPX_EQ | FCMPX_TYPE_AWARE, TRUE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(true), FCMPX_EQ | FCMPX_TYPE_AWARE, FALSE);
Copy link
Member

Choose a reason for hiding this comment

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

This would have been great, but I guess it's not a big deal as it works with strings.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can write custom parsing logic for these, and convert them to 0 or 1 numbers then follow through with the number comparison. It might make sense.

_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(true), FCMPX_LT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(true), FCMPX_GT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(true), FCMPX_NE | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(true), FCMPX_NE | FCMPX_TYPE_AWARE, TRUE);

_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(false), FCMPX_EQ | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(false), FCMPX_LT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(false), FCMPX_GT | FCMPX_TYPE_AWARE, TRUE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(false), FCMPX_GT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_bytes_new("true", 4), filterx_boolean_new(false), FCMPX_NE | FCMPX_TYPE_AWARE, TRUE);

// protobuf - double
_assert_comparison(filterx_protobuf_new("472", 3), filterx_double_new(472.0), FCMPX_EQ | FCMPX_TYPE_AWARE, TRUE);
_assert_comparison(filterx_protobuf_new("472", 3), filterx_double_new(472.0), FCMPX_EQ | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_protobuf_new("472", 3), filterx_double_new(472.0), FCMPX_LT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_protobuf_new("472", 3), filterx_double_new(472.0), FCMPX_GT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_protobuf_new("472", 3), filterx_double_new(472.0), FCMPX_NE | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_protobuf_new("472", 3), filterx_double_new(472.0), FCMPX_NE | FCMPX_TYPE_AWARE, TRUE);

_assert_comparison(filterx_protobuf_new("a472", 4), filterx_double_new(472.0), FCMPX_EQ | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_protobuf_new("a472", 4), filterx_double_new(472.0), FCMPX_LT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_protobuf_new("a472", 4), filterx_double_new(472.0), FCMPX_GT | FCMPX_TYPE_AWARE, TRUE);
_assert_comparison(filterx_protobuf_new("a472", 4), filterx_double_new(472.0), FCMPX_GT | FCMPX_TYPE_AWARE, FALSE);
_assert_comparison(filterx_protobuf_new("a472", 4), filterx_double_new(472.0), FCMPX_NE | FCMPX_TYPE_AWARE, TRUE);
}

Expand Down
3 changes: 3 additions & 0 deletions lib/filterx/tests/test_object_boolean.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,11 @@ Test(filterx_boolean, test_filterx_boolean_repr)
{
FilterXObject *obj = filterx_boolean_new(TRUE);
GString *repr = scratch_buffers_alloc();
g_string_assign(repr, "foo");
cr_assert(filterx_object_repr(obj, repr));
cr_assert_str_eq("true", repr->str);
cr_assert(filterx_object_repr_append(obj, repr));
cr_assert_str_eq("truetrue", repr->str);
filterx_object_unref(obj);
}

Expand Down
18 changes: 15 additions & 3 deletions lib/filterx/tests/test_object_bytes.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ Test(filterx_bytes, test_filterx_bytes_typecast_null_object_arg)
Test(filterx_bytes, test_filterx_bytes_typecast_from_bytes)
{
GPtrArray *args = g_ptr_array_new_with_free_func((GDestroyNotify) filterx_object_unref);
FilterXObject *in = filterx_bytes_new("byte sequence", -1);
FilterXObject *in = filterx_bytes_new("byte \0sequence", 14);
g_ptr_array_add(args, in);

FilterXObject *obj = filterx_typecast_bytes(args);
Expand Down Expand Up @@ -106,7 +106,7 @@ Test(filterx_bytes, test_filterx_bytes_typecast_from_string)
Test(filterx_bytes, test_filterx_bytes_typecast_from_protobuf)
{
GPtrArray *args = g_ptr_array_new_with_free_func((GDestroyNotify) filterx_object_unref);
FilterXObject *in = filterx_protobuf_new("not a valid protobuf!", -1);
FilterXObject *in = filterx_protobuf_new("not a valid \0protobuf!", 22);
g_ptr_array_add(args, in);

FilterXObject *obj = filterx_typecast_bytes(args);
Expand All @@ -116,12 +116,24 @@ Test(filterx_bytes, test_filterx_bytes_typecast_from_protobuf)
gsize size;
const gchar *bytes = filterx_bytes_get_value(obj, &size);

cr_assert(memcmp("not a valid protobuf!", bytes, size) == 0);
cr_assert(memcmp("not a valid \0protobuf!", bytes, size) == 0);

g_ptr_array_free(args, TRUE);
filterx_object_unref(obj);
}

Test(filterx_bytes, filterx_bytes_repr)
{
FilterXObject *obj = filterx_bytes_new("\0\1\2\3", 4);
GString *repr = scratch_buffers_alloc();
g_string_assign(repr, "foo");
cr_assert(filterx_object_repr(obj, repr));
cr_assert_str_eq("00010203", repr->str);
cr_assert(filterx_object_repr_append(obj, repr));
cr_assert_str_eq("0001020300010203", repr->str);
filterx_object_unref(obj);
}

static void
setup(void)
{
Expand Down
11 changes: 6 additions & 5 deletions lib/filterx/tests/test_object_datetime.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,19 +172,20 @@ Test(filterx_datetime, test_filterx_datetime_typecast_from_datetime)

Test(filterx_datetime, test_filterx_datetime_repr)
{
const gchar *test_time_str = "2024-03-18T12:34:13+234";
GPtrArray *args = g_ptr_array_new_with_free_func((GDestroyNotify) filterx_object_unref);
FilterXObject *in = filterx_string_new(test_time_str, -1);
FilterXObject *in = filterx_string_new("2024-03-18T12:34:13+0900", -1);
g_ptr_array_add(args, in);

FilterXObject *obj = filterx_typecast_datetime(args);
cr_assert_not_null(obj);
cr_assert(filterx_object_is_type(obj, &FILTERX_TYPE_NAME(datetime)));

GString *repr = scratch_buffers_alloc();

cr_assert(filterx_object_repr(in, repr));
cr_assert_str_eq(test_time_str, repr->str);
g_string_assign(repr, "foo");
cr_assert(filterx_object_repr(obj, repr));
cr_assert_str_eq("2024-03-18T12:34:13.000+09:00", repr->str);
cr_assert(filterx_object_repr_append(obj, repr));
cr_assert_str_eq("2024-03-18T12:34:13.000+09:002024-03-18T12:34:13.000+09:00", repr->str);

g_ptr_array_free(args, TRUE);
filterx_object_unref(obj);
Expand Down
3 changes: 3 additions & 0 deletions lib/filterx/tests/test_object_double.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,11 @@ Test(filterx_double, test_filterx_double_repr)
{
FilterXObject *obj = filterx_double_new(123.456);
GString *repr = scratch_buffers_alloc();
g_string_assign(repr, "foo");
cr_assert(filterx_object_repr(obj, repr));
cr_assert_str_eq("123.456", repr->str);
cr_assert(filterx_object_repr_append(obj, repr));
cr_assert_str_eq("123.456123.456", repr->str);
filterx_object_unref(obj);
}

Expand Down
Loading
Loading