Skip to content

Commit

Permalink
filterx-function: check arguments in constructor
Browse files Browse the repository at this point in the history 
This also fixes a race-condition related to the "retrieved" field of
FilterXFunctionArgs.

Signed-off-by: László Várady <[email protected]>
  • Loading branch information
@MrAnno
MrAnno committed Aug 23, 2024
1 parent fb649d7 commit eeefc6c
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 32 deletions.
70 changes: 45 additions & 25 deletions lib/filterx/expr-function.c
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/*
* Copyright (c) 2024 Axoflow
* Copyright (c) 2023 Balázs Scheidler <[email protected]>
* Copyright (c) 2023 shifter
*
Expand Down Expand Up @@ -43,7 +44,7 @@ filterx_function_error_quark(void)
typedef struct _FilterXSimpleFunction
{
FilterXFunction super;
FilterXFunctionArgs *args;
GPtrArray *args;
FilterXSimpleFunctionProto function_proto;
} FilterXSimpleFunction;

Expand Down Expand Up @@ -76,9 +77,9 @@ _args_get_expr(FilterXFunctionArgs *self, guint64 index)
}

static FilterXObject *
_args_get_object(FilterXFunctionArgs *self, guint64 index)
_get_arg_object(FilterXSimpleFunction *self, guint64 index)
{
FilterXExpr *expr = _args_get_expr(self, index);
FilterXExpr *expr = g_ptr_array_index(self->args, index);
if (!expr)
return NULL;

Expand All @@ -88,28 +89,17 @@ _args_get_object(FilterXFunctionArgs *self, guint64 index)
static GPtrArray *
_simple_function_eval_args(FilterXSimpleFunction *self)
{
guint64 len = filterx_function_args_len(self->args);

GPtrArray *res = g_ptr_array_new_full(len, (GDestroyNotify) filterx_object_unref);
GPtrArray *res = g_ptr_array_new_full(self->args->len, (GDestroyNotify) filterx_object_unref);

for (guint64 i = 0; i < len; i++)
for (guint64 i = 0; i < self->args->len; i++)
{
FilterXObject *obj = _args_get_object(self->args, i);
FilterXObject *obj = _get_arg_object(self, i);
if (obj == NULL)
goto error;

g_ptr_array_add(res, obj);
}

GError *error = NULL;
if (!filterx_function_args_check(self->args, &error))
{
filterx_simple_function_argument_error(&self->super.super, error->message, TRUE);
error->message = NULL;
g_clear_error(&error);
goto error;
}

return res;

error:
Expand All @@ -124,7 +114,7 @@ _simple_eval(FilterXExpr *s)

GPtrArray *args = NULL;

if (!filterx_function_args_empty(self->args))
if (self->args->len)
{
args = _simple_function_eval_args(self);
if (!args)
Expand All @@ -146,23 +136,53 @@ static void
_simple_free(FilterXExpr *s)
{
FilterXSimpleFunction *self = (FilterXSimpleFunction *) s;
filterx_function_args_free(self->args);
g_ptr_array_free(self->args, TRUE);
filterx_function_free_method(&self->super);
}

static inline gboolean
_simple_process_args(FilterXSimpleFunction *self, FilterXFunctionArgs *args, GError **error)
{
guint64 len = filterx_function_args_len(args);

for (guint64 i = 0; i < len; i++)
{
FilterXExpr *expr = filterx_function_args_get_expr(args, i);
if (!expr)
{
g_set_error(error, FILTERX_FUNCTION_ERROR, FILTERX_FUNCTION_ERROR_UNEXPECTED_ARGS, "can't get argument");
return FALSE;
}

g_ptr_array_add(self->args, expr);
}

return TRUE;
}

FilterXExpr *
filterx_simple_function_new(const gchar *function_name, FilterXFunctionArgs *args,
FilterXSimpleFunctionProto function_proto)
FilterXSimpleFunctionProto function_proto, GError **error)
{
FilterXSimpleFunction *self = g_new0(FilterXSimpleFunction, 1);

filterx_function_init_instance(&self->super, function_name);
self->super.super.eval = _simple_eval;
self->super.super.free_fn = _simple_free;
self->args = args;
self->function_proto = function_proto;

self->args = g_ptr_array_new_full(8, (GDestroyNotify) filterx_expr_unref);
if (!_simple_process_args(self, args, error) ||
!filterx_function_args_check(args, error))
goto error;

filterx_function_args_free(args);
return &self->super.super;

error:
filterx_function_args_free(args);
filterx_expr_unref(&self->super.super);
return NULL;
}

void
Expand Down Expand Up @@ -520,13 +540,13 @@ filterx_function_args_free(FilterXFunctionArgs *self)


static FilterXExpr *
_lookup_simple_function(GlobalConfig *cfg, const gchar *function_name, FilterXFunctionArgs *args)
_lookup_simple_function(GlobalConfig *cfg, const gchar *function_name, FilterXFunctionArgs *args, GError **error)
{
// Checking filterx builtin functions first
FilterXSimpleFunctionProto f = filterx_builtin_simple_function_lookup(function_name);
if (f != NULL)
{
return filterx_simple_function_new(function_name, args, f);
return filterx_simple_function_new(function_name, args, f, error);
}

// fallback to plugin lookup
Expand All @@ -539,7 +559,7 @@ _lookup_simple_function(GlobalConfig *cfg, const gchar *function_name, FilterXFu
if (f == NULL)
return NULL;

return filterx_simple_function_new(function_name, args, f);
return filterx_simple_function_new(function_name, args, f, error);
}

static FilterXExpr *
Expand Down Expand Up @@ -571,7 +591,7 @@ filterx_function_lookup(GlobalConfig *cfg, const gchar *function_name, GList *ar
if (!args)
return NULL;

FilterXExpr *expr = _lookup_simple_function(cfg, function_name, args);
FilterXExpr *expr = _lookup_simple_function(cfg, function_name, args, error);
if (expr)
return expr;

Expand Down
2 changes: 1 addition & 1 deletion lib/filterx/filterx-private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

// Builtin functions
FilterXExpr *filterx_simple_function_new(const gchar *function_name, FilterXFunctionArgs *args,
FilterXSimpleFunctionProto function_proto);
FilterXSimpleFunctionProto function_proto, GError **error);

gboolean filterx_builtin_simple_function_register_private(GHashTable *ht, const gchar *fn_name,
FilterXSimpleFunctionProto func);
Expand Down
4 changes: 3 additions & 1 deletion lib/filterx/tests/test_expr_condition.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,9 @@ _dummy_func(FilterXExpr *s, GPtrArray *args)

Test(expr_condition, test_condition_return_expr_result_on_missing_stmts)
{
FilterXExpr *func = filterx_simple_function_new("test_fn", filterx_function_args_new(NULL, NULL), _dummy_func);
GError *error = NULL;
FilterXExpr *func = filterx_simple_function_new("test_fn", filterx_function_args_new(NULL, NULL), _dummy_func, &error);
g_clear_error(&error);

FilterXExpr *cond = filterx_conditional_new(func);
FilterXObject *res = filterx_expr_eval(cond);
Expand Down
29 changes: 24 additions & 5 deletions lib/filterx/tests/test_expr_function.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@ FilterXObject *test_dummy_function(FilterXExpr *s, GPtrArray *args)

Test(expr_function, test_function_null_args)
{
GError *error = NULL;
FilterXExpr *func = filterx_simple_function_new("test_dummy", filterx_function_args_new(NULL, NULL),
test_dummy_function);
test_dummy_function, &error);
cr_assert_null(error);
g_clear_error(&error);

cr_assert_not_null(func);
filterx_expr_unref(func);
}
Expand All @@ -70,9 +74,15 @@ Test(expr_function, test_function_null_arg)
{
GList *args = NULL;
args = g_list_append(args, filterx_function_arg_new(NULL, NULL));

GError *error = NULL;
FilterXExpr *func = filterx_simple_function_new("test_dummy", filterx_function_args_new(args, NULL),
test_dummy_function);
cr_assert_not_null(func);
test_dummy_function, &error);

cr_assert_not_null(error);
g_clear_error(&error);

cr_assert_null(func);
filterx_expr_unref(func);
}

Expand All @@ -81,10 +91,14 @@ Test(expr_function, test_function_valid_arg)
GList *args = NULL;
args = g_list_append(args, filterx_function_arg_new(NULL, filterx_literal_new(filterx_string_new("bad format 1", -1))));

GError *error = NULL;
FilterXExpr *func = filterx_simple_function_new("test_dummy", filterx_function_args_new(args, NULL),
test_dummy_function);
test_dummy_function, &error);
cr_assert_not_null(func);

cr_assert_null(error);
g_clear_error(&error);

FilterXObject *res = filterx_expr_eval(func);
cr_assert_not_null(res);
cr_assert(filterx_object_is_type(res, &FILTERX_TYPE_NAME(string)));
Expand All @@ -100,8 +114,13 @@ Test(expr_function, test_function_multiple_args)
args = g_list_append(args, filterx_function_arg_new(NULL, filterx_literal_new(filterx_null_new())));
args = g_list_append(args, filterx_function_arg_new(NULL, filterx_literal_new(filterx_integer_new(443))));
args = g_list_append(args, filterx_function_arg_new(NULL, filterx_literal_new(filterx_string_new("foobar", -1))));

GError *error = NULL;
FilterXExpr *func = filterx_simple_function_new("test_dummy", filterx_function_args_new(args, NULL),
test_dummy_function);
test_dummy_function, &error);

cr_assert_null(error);
g_clear_error(&error);
cr_assert_not_null(func);
FilterXObject *res = filterx_expr_eval(func);
cr_assert_not_null(res);
Expand Down

0 comments on commit eeefc6c

Please sign in to comment.