From a043f0433371eb20b4cb98fd244ef942375a8a80 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 23:24:11 +0100 Subject: [PATCH 1/7] filterx/filterx-scope: add filterx_variable_handle_is_message_tied() in addition to is_floating Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 2 +- lib/filterx/filterx-scope.c | 4 ++-- lib/filterx/filterx-variable.h | 12 ++++++++++++ lib/filterx/func-vars.c | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 4c962722d..d4bdc2321 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -82,7 +82,7 @@ _eval(FilterXExpr *s) return value; } - if (!filterx_variable_handle_is_floating(self->handle)) + if (filterx_variable_handle_is_message_tied(self->handle)) { FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msgs[0]); if(!msg_ref) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index ae673e762..51223caa6 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -158,7 +158,7 @@ filterx_scope_register_variable(FilterXScope *self, /* the scope needs to be synced with the message if it holds a * message-tied variable (e.g. $MSG) */ - if (!filterx_variable_handle_is_floating(handle)) + if (filterx_variable_handle_is_message_tied(handle)) self->syncable = TRUE; return v; } @@ -297,7 +297,7 @@ filterx_scope_clone(FilterXScope *other) { FilterXVariable *v = &g_array_index(other->variables, FilterXVariable, src_index); - if (filterx_variable_is_declared(v) || !filterx_variable_is_floating(v)) + if (filterx_variable_is_declared(v) || filterx_variable_is_message_tied(v)) { g_array_append_val(self->variables, *v); FilterXVariable *v_clone = &g_array_index(self->variables, FilterXVariable, dst_index); diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 7b2074423..9913c8152 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -68,6 +68,12 @@ filterx_variable_handle_is_floating(FilterXVariableHandle handle) return !!(handle & FILTERX_HANDLE_FLOATING_BIT); } +static inline gboolean +filterx_variable_handle_is_message_tied(FilterXVariableHandle handle) +{ + return !filterx_variable_handle_is_floating(handle); +} + static inline NVHandle filterx_variable_handle_to_nv_handle(FilterXVariableHandle handle) { @@ -80,6 +86,12 @@ filterx_variable_is_floating(FilterXVariable *v) return filterx_variable_handle_is_floating(v->handle); } +static inline gboolean +filterx_variable_is_message_tied(FilterXVariable *v) +{ + return filterx_variable_handle_is_message_tied(v->handle); +} + static inline NVHandle filterx_variable_get_nv_handle(FilterXVariable *v) { diff --git a/lib/filterx/func-vars.c b/lib/filterx/func-vars.c index f78286882..f760e31b2 100644 --- a/lib/filterx/func-vars.c +++ b/lib/filterx/func-vars.c @@ -42,7 +42,7 @@ _add_to_dict(FilterXVariable *variable, gpointer user_data) gssize name_len; const gchar *name_str = filterx_variable_get_name(variable, &name_len); - if (!filterx_variable_is_floating(variable)) + if (filterx_variable_is_message_tied(variable)) { g_string_assign(name_buf, "$"); g_string_append_len(name_buf, name_str, name_len); From 14c61b245e157d6284b3be4a711a5df52d67fc7a Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 16:09:44 +0100 Subject: [PATCH 2/7] filterx/filterx-eval: remove partial support for multiple LogMessage instances We do not really support message contexts (as does the original filter language and templates), so remove that support. This makes it simpler to initialize FilterXEvalContext as well as we do not have to manage the LogMessage array separately. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-template.c | 2 +- lib/filterx/expr-variable.c | 6 +++--- lib/filterx/filterx-eval.c | 7 +++---- lib/filterx/filterx-eval.h | 7 +++---- lib/filterx/filterx-pipe.c | 4 ++-- lib/filterx/func-sdata.c | 6 +++--- libtest/filterx-lib.c | 4 +--- 7 files changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/filterx/expr-template.c b/lib/filterx/expr-template.c index f94716ad3..27bce3001 100644 --- a/lib/filterx/expr-template.c +++ b/lib/filterx/expr-template.c @@ -46,7 +46,7 @@ _eval(FilterXExpr *s) /* FIXME: we could go directly to filterx_string_new() here to avoid a round trip in FilterXMessageValue */ /* FIXME/2: let's make this handle literal and trivial templates */ - log_template_format_value_and_type_with_context(self->template, context->msgs, context->num_msg, + log_template_format_value_and_type_with_context(self->template, &context->msg, 1, &context->template_eval_options, value, &t); /* NOTE: we borrow value->str here which is stored in a scratch buffer diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index d4bdc2321..670776d67 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -84,7 +84,7 @@ _eval(FilterXExpr *s) if (filterx_variable_handle_is_message_tied(self->handle)) { - FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msgs[0]); + FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msg); if(!msg_ref) return NULL; filterx_scope_register_variable(context->scope, self->handle, msg_ref); @@ -142,7 +142,7 @@ _isset(FilterXExpr *s) return filterx_variable_is_set(variable); FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; return log_msg_is_value_set(msg, self->handle); } @@ -159,7 +159,7 @@ _unset(FilterXExpr *s) return TRUE; } - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; if (log_msg_is_value_set(msg, self->handle)) _whiteout_variable(self, context); diff --git a/lib/filterx/filterx-eval.c b/lib/filterx/filterx-eval.c index 9d510a993..eb815b4c9 100644 --- a/lib/filterx/filterx-eval.c +++ b/lib/filterx/filterx-eval.c @@ -119,10 +119,8 @@ filterx_format_last_error_location(void) } FilterXEvalResult -filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr, LogMessage *msg) +filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr) { - context->msgs = &msg; - context->num_msg = 1; FilterXEvalResult result = FXE_FAILURE; FilterXObject *res = filterx_expr_eval(expr); @@ -147,7 +145,7 @@ filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr, LogMessage *ms } void -filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context) +filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, LogMessage *msg) { FilterXScope *scope; @@ -158,6 +156,7 @@ filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previ filterx_scope_make_writable(&scope); memset(context, 0, sizeof(*context)); + context->msg = msg; context->template_eval_options = DEFAULT_TEMPLATE_EVAL_OPTIONS; context->scope = scope; diff --git a/lib/filterx/filterx-eval.h b/lib/filterx/filterx-eval.h index 8c995cbf0..34d3a3f8a 100644 --- a/lib/filterx/filterx-eval.h +++ b/lib/filterx/filterx-eval.h @@ -47,8 +47,7 @@ typedef enum _FilterXEvalControl typedef struct _FilterXEvalContext FilterXEvalContext; struct _FilterXEvalContext { - LogMessage **msgs; - gint num_msg; + LogMessage *msg; FilterXScope *scope; FilterXError error; LogTemplateEvalOptions template_eval_options; @@ -62,14 +61,14 @@ FilterXScope *filterx_eval_get_scope(void); void filterx_eval_push_error(const gchar *message, FilterXExpr *expr, FilterXObject *object); void filterx_eval_push_error_info(const gchar *message, FilterXExpr *expr, gchar *info, gboolean free_info); void filterx_eval_set_context(FilterXEvalContext *context); -FilterXEvalResult filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr, LogMessage *msg); +FilterXEvalResult filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr); const gchar *filterx_eval_get_last_error(void); EVTTAG *filterx_format_last_error(void); EVTTAG *filterx_format_last_error_location(void); void filterx_eval_clear_errors(void); EVTTAG *filterx_format_eval_result(FilterXEvalResult result); -void filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context); +void filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, LogMessage *msg); void filterx_eval_deinit_context(FilterXEvalContext *context); static inline void diff --git a/lib/filterx/filterx-pipe.c b/lib/filterx/filterx-pipe.c index 20c7dad90..72c3fb4f5 100644 --- a/lib/filterx/filterx-pipe.c +++ b/lib/filterx/filterx-pipe.c @@ -68,7 +68,7 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o FilterXEvalResult eval_res; path_options = log_path_options_chain(&local_path_options, path_options); - filterx_eval_init_context(&eval_context, path_options->filterx_context); + filterx_eval_init_context(&eval_context, path_options->filterx_context, msg); if (filterx_scope_has_log_msg_changes(eval_context.scope)) filterx_scope_invalidate_log_msg_cache(eval_context.scope); @@ -79,7 +79,7 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o evt_tag_msg_reference(msg)); NVTable *payload = nv_table_ref(msg->payload); - eval_res = filterx_eval_exec(&eval_context, self->block, msg); + eval_res = filterx_eval_exec(&eval_context, self->block); msg_trace("<<<<<< filterx rule evaluation result", filterx_format_eval_result(eval_res), diff --git a/lib/filterx/func-sdata.c b/lib/filterx/func-sdata.c index 5505545bb..60f5b4abf 100644 --- a/lib/filterx/func-sdata.c +++ b/lib/filterx/func-sdata.c @@ -73,7 +73,7 @@ _eval(FilterXExpr *s) gboolean contains = FALSE; FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; for (guint8 i = 0; i < msg->num_sdata && !contains; i++) { @@ -127,7 +127,7 @@ filterx_simple_function_has_sdata(FilterXExpr *s, FilterXObject *args[], gsize a } FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; return filterx_boolean_new(msg->num_sdata != 0); } @@ -224,7 +224,7 @@ static gboolean _generate(FilterXExprGenerator *s, FilterXObject *fillable) { FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; const gchar *current_sd_id_start; gsize current_sd_id_len; diff --git a/libtest/filterx-lib.c b/libtest/filterx-lib.c index 2b46c74e9..d821e4843 100644 --- a/libtest/filterx-lib.c +++ b/libtest/filterx-lib.c @@ -193,9 +193,7 @@ init_libtest_filterx(void) FILTERX_TYPE_NAME(test_list) = FILTERX_TYPE_NAME(json_array); filterx_env.msg = create_sample_message(); - filterx_eval_init_context(&filterx_env.context, NULL); - filterx_env.context.msgs = &filterx_env.msg; - filterx_env.context.num_msg = 1; + filterx_eval_init_context(&filterx_env.context, NULL, filterx_env.msg); } From 32cd7af86e0892579d454d10cf7815a1fe19665f Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 09:35:10 +0100 Subject: [PATCH 3/7] filterx/expr-variable: make template macro based variables read only Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 670776d67..ee76cfaea 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -113,6 +113,12 @@ _assign(FilterXExpr *s, FilterXObject *new_value) FilterXScope *scope = filterx_eval_get_scope(); FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); + if (self->handle_is_macro) + { + filterx_eval_push_error("Macro based variable cannot be changed", &self->super, self->variable_name); + return FALSE; + } + if (!variable) { /* NOTE: we pass NULL as initial_value to make sure the new variable @@ -150,6 +156,13 @@ static gboolean _unset(FilterXExpr *s) { FilterXVariableExpr *self = (FilterXVariableExpr *) s; + + if (self->handle_is_macro) + { + filterx_eval_push_error("Macro based variable cannot be changed", &self->super, self->variable_name); + return FALSE; + } + FilterXEvalContext *context = filterx_eval_get_context(); FilterXVariable *variable = filterx_scope_lookup_variable(context->scope, self->handle); From 01720425f872d8875d7c79add565d1c551f9ae82 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 08:15:13 +0100 Subject: [PATCH 4/7] filterx/filterx-variable: move FilterXVariableHandle related functions to the front Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-variable.h | 47 +++++++++++++++++----------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 9913c8152..65346770b 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -29,25 +29,6 @@ #include "syslog-ng.h" #include "filterx-object.h" -#define FILTERX_SCOPE_MAX_GENERATION ((1UL << 20) - 1) - -typedef guint32 FilterXVariableHandle; - -typedef struct _FilterXVariable -{ - /* the MSB indicates that the variable is a floating one */ - FilterXVariableHandle handle; - /* - * assigned -- Indicates that the variable was assigned to a new value - * - * declared -- this variable is declared (e.g. retained for the entire input pipeline) - */ - guint32 assigned:1, - declared:1, - generation:20; - FilterXObject *value; -} FilterXVariable; - typedef enum { FX_VAR_MESSAGE, @@ -55,10 +36,9 @@ typedef enum FX_VAR_DECLARED, } FilterXVariableType; +#define FILTERX_SCOPE_MAX_GENERATION ((1UL << 20) - 1) -void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, - FilterXObject *initial_value, guint32 generation); -void filterx_variable_free(FilterXVariable *v); +typedef guint32 FilterXVariableHandle; #define FILTERX_HANDLE_FLOATING_BIT (1UL << 31) @@ -80,6 +60,27 @@ filterx_variable_handle_to_nv_handle(FilterXVariableHandle handle) return handle & ~FILTERX_HANDLE_FLOATING_BIT; } +FilterXVariableHandle filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type); + +typedef struct _FilterXVariable +{ + /* the MSB indicates that the variable is a floating one */ + FilterXVariableHandle handle; + /* + * assigned -- Indicates that the variable was assigned to a new value + * + * declared -- this variable is declared (e.g. retained for the entire input pipeline) + */ + guint32 assigned:1, + declared:1, + generation:20; + FilterXObject *value; +} FilterXVariable; + +void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, + FilterXObject *initial_value, guint32 generation); +void filterx_variable_free(FilterXVariable *v); + static inline gboolean filterx_variable_is_floating(FilterXVariable *v) { @@ -166,6 +167,4 @@ filterx_variable_set_declared(FilterXVariable *v, gboolean declared) v->declared = declared; } -FilterXVariableHandle filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type); - #endif From 24aed204631c87a067f1edb04b128051d87ad84b Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 08:36:54 +0100 Subject: [PATCH 5/7] filterx/filterx-variable: rename filterx_variable_free to _clear() It does not really free @self, so it only clears up allocations within an existing instance, these are usually called *_clear() functions. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 4 ++-- lib/filterx/filterx-variable.c | 2 +- lib/filterx/filterx-variable.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 51223caa6..f85d839b7 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -284,7 +284,7 @@ filterx_scope_new(void) g_atomic_counter_set(&self->ref_cnt, 1); self->variables = g_array_sized_new(FALSE, TRUE, sizeof(FilterXVariable), 16); - g_array_set_clear_func(self->variables, (GDestroyNotify) filterx_variable_free); + g_array_set_clear_func(self->variables, (GDestroyNotify) filterx_variable_clear); return self; } @@ -391,7 +391,7 @@ filterx_scope_invalidate_log_msg_cache(FilterXScope *self) if (!filterx_variable_is_floating(v) && self->syncable) { /* skip this variable */ - filterx_variable_free(v); + filterx_variable_clear(v); } else { diff --git a/lib/filterx/filterx-variable.c b/lib/filterx/filterx-variable.c index cd14418d6..95a10e08d 100644 --- a/lib/filterx/filterx-variable.c +++ b/lib/filterx/filterx-variable.c @@ -39,7 +39,7 @@ filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type) } void -filterx_variable_free(FilterXVariable *v) +filterx_variable_clear(FilterXVariable *v) { filterx_object_unref(v->value); } diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 65346770b..2cd498b52 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -79,7 +79,7 @@ typedef struct _FilterXVariable void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, FilterXObject *initial_value, guint32 generation); -void filterx_variable_free(FilterXVariable *v); +void filterx_variable_clear(FilterXVariable *v); static inline gboolean filterx_variable_is_floating(FilterXVariable *v) From ae4172537b2f6c8811bfd78a9efd493239147e4c Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 08:27:09 +0100 Subject: [PATCH 6/7] filterx/filterx-variable: introduce "variable_type" at the variable layer Previously the type enum was only used during the initialization of FilterXExprVariable which was then translated to a set of booleans at the variable layer. Also clean up naming a bit. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 27 ++++++++++++++------------- lib/filterx/filterx-scope.c | 21 ++++----------------- lib/filterx/filterx-scope.h | 4 +--- lib/filterx/filterx-variable.c | 13 ++++++++----- lib/filterx/filterx-variable.h | 21 +++++++++------------ lib/filterx/func-vars.c | 11 ++++------- 6 files changed, 40 insertions(+), 57 deletions(-) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index ee76cfaea..2eede2f4e 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -34,9 +34,10 @@ typedef struct _FilterXVariableExpr { FilterXExpr super; - FilterXObject *variable_name; + FilterXVariableType variable_type; NVHandle handle; - guint32 declared:1, handle_is_macro:1; + FilterXObject *variable_name; + guint32 handle_is_macro:1; } FilterXVariableExpr; static FilterXObject * @@ -61,7 +62,7 @@ _pull_variable_from_message(FilterXVariableExpr *self, FilterXEvalContext *conte static void _whiteout_variable(FilterXVariableExpr *self, FilterXEvalContext *context) { - filterx_scope_register_variable(context->scope, self->handle, NULL); + filterx_scope_register_variable(context->scope, FX_VAR_MESSAGE_TIED, self->handle, NULL); } static FilterXObject * @@ -87,7 +88,7 @@ _eval(FilterXExpr *s) FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msg); if(!msg_ref) return NULL; - filterx_scope_register_variable(context->scope, self->handle, msg_ref); + filterx_scope_register_variable(context->scope, FX_VAR_MESSAGE_TIED, self->handle, msg_ref); return msg_ref; } @@ -124,10 +125,7 @@ _assign(FilterXExpr *s, FilterXObject *new_value) /* NOTE: we pass NULL as initial_value to make sure the new variable * is considered changed due to the assignment */ - if (self->declared) - variable = filterx_scope_register_declared_variable(scope, self->handle, NULL); - else - variable = filterx_scope_register_variable(scope, self->handle, NULL); + variable = filterx_scope_register_variable(scope, self->variable_type, self->handle, NULL); } /* this only clones mutable objects */ @@ -217,7 +215,7 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) } static FilterXExpr * -filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) +filterx_variable_expr_new(FilterXString *name, FilterXVariableType variable_type) { FilterXVariableExpr *self = g_new0(FilterXVariableExpr, 1); @@ -231,9 +229,10 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) self->super.is_set = _isset; self->super.unset = _unset; + self->variable_type = variable_type; self->variable_name = (FilterXObject *) name; - self->handle = filterx_map_varname_to_handle(filterx_string_get_value_ref(self->variable_name, NULL), type); - if (type == FX_VAR_MESSAGE) + self->handle = filterx_map_varname_to_handle(filterx_string_get_value_ref(self->variable_name, NULL), variable_type); + if (variable_type == FX_VAR_MESSAGE_TIED) self->handle_is_macro = log_msg_is_handle_macro(filterx_variable_handle_to_nv_handle(self->handle)); return &self->super; @@ -242,7 +241,7 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) FilterXExpr * filterx_msg_variable_expr_new(FilterXString *name) { - return filterx_variable_expr_new(name, FX_VAR_MESSAGE); + return filterx_variable_expr_new(name, FX_VAR_MESSAGE_TIED); } FilterXExpr * @@ -257,5 +256,7 @@ filterx_variable_expr_declare(FilterXExpr *s) FilterXVariableExpr *self = (FilterXVariableExpr *) s; g_assert(s->eval == _eval); - self->declared = TRUE; + /* we can only declare a floating variable */ + g_assert(self->variable_type == FX_VAR_FLOATING); + self->variable_type = FX_VAR_DECLARED_FLOATING; } diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index f85d839b7..d742ae199 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -115,6 +115,7 @@ filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle) static FilterXVariable * _register_variable(FilterXScope *self, + FilterXVariableType variable_type, FilterXVariableHandle handle, FilterXObject *initial_value) { @@ -142,7 +143,7 @@ _register_variable(FilterXScope *self, FilterXVariable v; - filterx_variable_init_instance(&v, handle, initial_value, self->generation); + filterx_variable_init_instance(&v, variable_type, handle, initial_value, self->generation); g_array_insert_val(self->variables, v_index, v); return &g_array_index(self->variables, FilterXVariable, v_index); @@ -150,11 +151,11 @@ _register_variable(FilterXScope *self, FilterXVariable * filterx_scope_register_variable(FilterXScope *self, + FilterXVariableType variable_type, FilterXVariableHandle handle, FilterXObject *initial_value) { - FilterXVariable *v = _register_variable(self, handle, initial_value); - filterx_variable_set_declared(v, FALSE); + FilterXVariable *v = _register_variable(self, variable_type, handle, initial_value); /* the scope needs to be synced with the message if it holds a * message-tied variable (e.g. $MSG) */ @@ -163,20 +164,6 @@ filterx_scope_register_variable(FilterXScope *self, return v; } -FilterXVariable * -filterx_scope_register_declared_variable(FilterXScope *self, - FilterXVariableHandle handle, - FilterXObject *initial_value) - -{ - g_assert(filterx_variable_handle_is_floating(handle)); - - FilterXVariable *v = _register_variable(self, handle, initial_value); - filterx_variable_set_declared(v, TRUE); - - return v; -} - gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data) { diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 89f12c366..2e996d202 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -52,11 +52,9 @@ void filterx_scope_sync(FilterXScope *self, LogMessage *msg); FilterXVariable *filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle); FilterXVariable *filterx_scope_register_variable(FilterXScope *self, + FilterXVariableType variable_type, FilterXVariableHandle handle, FilterXObject *initial_value); -FilterXVariable *filterx_scope_register_declared_variable(FilterXScope *self, - FilterXVariableHandle handle, - FilterXObject *initial_value); gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data); void filterx_scope_invalidate_log_msg_cache(FilterXScope *self); diff --git a/lib/filterx/filterx-variable.c b/lib/filterx/filterx-variable.c index 95a10e08d..3b5b726e2 100644 --- a/lib/filterx/filterx-variable.c +++ b/lib/filterx/filterx-variable.c @@ -28,12 +28,12 @@ FilterXVariableHandle filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type) { - if (type == FX_VAR_MESSAGE) + if (type == FX_VAR_MESSAGE_TIED) name++; NVHandle nv_handle = log_msg_get_value_handle(name); - if (type == FX_VAR_MESSAGE) + if (type == FX_VAR_MESSAGE_TIED) return (FilterXVariableHandle) nv_handle; return (FilterXVariableHandle) nv_handle | FILTERX_HANDLE_FLOATING_BIT; } @@ -45,12 +45,15 @@ filterx_variable_clear(FilterXVariable *v) } void -filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, - FilterXObject *initial_value, guint32 generation) +filterx_variable_init_instance(FilterXVariable *v, + FilterXVariableType variable_type, + FilterXVariableHandle handle, + FilterXObject *initial_value, + guint32 generation) { v->handle = handle; + v->variable_type = variable_type; v->assigned = FALSE; - v->declared = FALSE; v->generation = generation; v->value = filterx_object_ref(initial_value); } diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 2cd498b52..447774c1c 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -31,9 +31,9 @@ typedef enum { - FX_VAR_MESSAGE, + FX_VAR_MESSAGE_TIED, FX_VAR_FLOATING, - FX_VAR_DECLARED, + FX_VAR_DECLARED_FLOATING, } FilterXVariableType; #define FILTERX_SCOPE_MAX_GENERATION ((1UL << 20) - 1) @@ -72,13 +72,16 @@ typedef struct _FilterXVariable * declared -- this variable is declared (e.g. retained for the entire input pipeline) */ guint32 assigned:1, - declared:1, + variable_type:2, generation:20; FilterXObject *value; } FilterXVariable; -void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, - FilterXObject *initial_value, guint32 generation); +void filterx_variable_init_instance(FilterXVariable *v, + FilterXVariableType variable_type, + FilterXVariableHandle handle, + FilterXObject *initial_value, + guint32 generation); void filterx_variable_clear(FilterXVariable *v); static inline gboolean @@ -134,7 +137,7 @@ filterx_variable_is_set(FilterXVariable *v) static inline gboolean filterx_variable_is_declared(FilterXVariable *v) { - return v->declared; + return v->variable_type == FX_VAR_DECLARED_FLOATING; } static inline gboolean @@ -161,10 +164,4 @@ filterx_variable_unassign(FilterXVariable *v) v->assigned = FALSE; } -static inline void -filterx_variable_set_declared(FilterXVariable *v, gboolean declared) -{ - v->declared = declared; -} - #endif diff --git a/lib/filterx/func-vars.c b/lib/filterx/func-vars.c index f760e31b2..6c24ceed5 100644 --- a/lib/filterx/func-vars.c +++ b/lib/filterx/func-vars.c @@ -111,14 +111,11 @@ _load_from_dict(FilterXObject *key, FilterXObject *value, gpointer user_data) return FALSE; } - gboolean is_floating = key_str[0] != '$'; - FilterXVariableHandle handle = filterx_map_varname_to_handle(key_str, is_floating ? FX_VAR_FLOATING : FX_VAR_MESSAGE); + FilterXVariableType variable_type = (key_str[0] == '$') ? FX_VAR_MESSAGE_TIED : FX_VAR_DECLARED_FLOATING; + FilterXVariableHandle handle = filterx_map_varname_to_handle(key_str, variable_type); FilterXVariable *variable = NULL; - if (is_floating) - variable = filterx_scope_register_declared_variable(scope, handle, NULL); - else - variable = filterx_scope_register_variable(scope, handle, NULL); + variable = filterx_scope_register_variable(scope, variable_type, handle, NULL); FilterXObject *cloned_value = filterx_object_clone(value); filterx_variable_set_value(variable, cloned_value); @@ -143,7 +140,7 @@ _load_from_dict(FilterXObject *key, FilterXObject *value, gpointer user_data) evt_tag_str("key", key_str), evt_tag_str("value", repr->str), evt_tag_str("type", value->type->name), - evt_tag_str("variable_type", is_floating ? "declared" : "message")); + evt_tag_int("variable_type", variable_type)); } return TRUE; From fd3ecd306ea3b2f54d943e3269beb28aa355e781 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 16:10:01 +0100 Subject: [PATCH 7/7] filterx: do not consider unmarshaling a change in value If we pull in a variable from the message and we need to unmarshal it (e.g. turn FilterXMessageValue to a more specific type like FilterXString or FilterXJSON), do not consider that a change of that variable. Changing it in-place, or assignment of a new value should be remain to be a change. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 4 +- lib/filterx/filterx-scope.c | 4 +- lib/filterx/filterx-variable.h | 6 +-- lib/filterx/func-vars.c | 2 +- .../filterx/test_filterx_scope.py | 48 +++++++++++++++++-- 5 files changed, 51 insertions(+), 13 deletions(-) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 2eede2f4e..c38539c19 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -104,7 +104,7 @@ _update_repr(FilterXExpr *s, FilterXObject *new_repr) FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); g_assert(variable != NULL); - filterx_variable_set_value(variable, new_repr); + filterx_variable_set_value(variable, new_repr, FALSE); } static gboolean @@ -130,7 +130,7 @@ _assign(FilterXExpr *s, FilterXObject *new_value) /* this only clones mutable objects */ new_value = filterx_object_clone(new_value); - filterx_variable_set_value(variable, new_value); + filterx_variable_set_value(variable, new_value, TRUE); filterx_object_unref(new_value); return TRUE; } diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index d742ae199..95c3658c1 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -130,9 +130,7 @@ _register_variable(FilterXScope *self, * it was a new value */ filterx_variable_set_generation(v_slot, self->generation); - filterx_variable_set_value(v_slot, initial_value); - /* consider this to be unset just as an initial registration is */ - filterx_variable_unassign(v_slot); + filterx_variable_set_value(v_slot, initial_value, FALSE); } return v_slot; } diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 447774c1c..b31afe52c 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -115,17 +115,17 @@ filterx_variable_get_value(FilterXVariable *v) } static inline void -filterx_variable_set_value(FilterXVariable *v, FilterXObject *new_value) +filterx_variable_set_value(FilterXVariable *v, FilterXObject *new_value, gboolean assignment) { filterx_object_unref(v->value); v->value = filterx_object_ref(new_value); - v->assigned = TRUE; + v->assigned = assignment; } static inline void filterx_variable_unset_value(FilterXVariable *v) { - filterx_variable_set_value(v, NULL); + filterx_variable_set_value(v, NULL, TRUE); } static inline gboolean diff --git a/lib/filterx/func-vars.c b/lib/filterx/func-vars.c index 6c24ceed5..d412998db 100644 --- a/lib/filterx/func-vars.c +++ b/lib/filterx/func-vars.c @@ -118,7 +118,7 @@ _load_from_dict(FilterXObject *key, FilterXObject *value, gpointer user_data) variable = filterx_scope_register_variable(scope, variable_type, handle, NULL); FilterXObject *cloned_value = filterx_object_clone(value); - filterx_variable_set_value(variable, cloned_value); + filterx_variable_set_value(variable, cloned_value, TRUE); filterx_object_unref(cloned_value); if (!variable) diff --git a/tests/light/functional_tests/filterx/test_filterx_scope.py b/tests/light/functional_tests/filterx/test_filterx_scope.py index d62b97207..78f8d4c6e 100644 --- a/tests/light/functional_tests/filterx/test_filterx_scope.py +++ b/tests/light/functional_tests/filterx/test_filterx_scope.py @@ -30,10 +30,14 @@ def render_filterx_exprs(expressions): return '\n'.join(f"filterx {{ {expr} }};" for expr in expressions) -def create_config(config, init_exprs, true_exprs=(), false_exprs=(), final_exprs=(), msg="foobar"): - file_true = config.create_file_destination(file_name="dest-true.log", template="'$MSG\n'") - file_false = config.create_file_destination(file_name="dest-false.log", template="'$MSG\n'") - file_final = config.create_file_destination(file_name="dest-final.log", template="'$MSG\n'") +def render_log_exprs(expressions): + return '\n'.join(expressions) + + +def create_config(config, init_exprs, init_log_exprs=(), true_exprs=(), false_exprs=(), final_log_exprs=(), final_exprs=(), msg="foobar", template="'$MSG\n'"): + file_true = config.create_file_destination(file_name="dest-true.log", template=template) + file_false = config.create_file_destination(file_name="dest-false.log", template=template) + file_final = config.create_file_destination(file_name="dest-final.log", template=template) preamble = f""" @version: {config.get_version()} @@ -76,6 +80,7 @@ def create_config(config, init_exprs, true_exprs=(), false_exprs=(), final_exprs log {{ source(genmsg); {render_filterx_exprs(init_exprs)}; + {render_log_exprs(init_log_exprs)} if {{ {render_filterx_exprs(true_exprs)} destination(dest_true); @@ -83,6 +88,7 @@ def create_config(config, init_exprs, true_exprs=(), false_exprs=(), final_exprs {render_filterx_exprs(false_exprs)} destination(dest_false); }}; + {render_log_exprs(final_log_exprs)} {render_filterx_exprs(final_exprs)} destination(dest_final); }}; @@ -182,6 +188,40 @@ def test_message_tied_variables_do_not_propagate_to_parallel_branches(config, sy assert file_false.read_log() == "kecske\n" +def test_message_tied_variables_are_not_considered_changed_just_by_unmarshaling(config, syslog_ng): + (file_true, file_false, file_final) = create_config( + config, init_exprs=[ + """ + # pull up the value from the message into a filterx variable + ${values.json}; + # cause an unmarshal into JSON + ${values.json}.emb_key1; + # $foo is set to the unmarshalled version of ${values.json} + $foo = ${values.json}; + """, + ], init_log_exprs=[ + # trigger a sync + """ + rewrite { + }; + """, + ], + # ${values.json} should retain to have spaces in them, since it was not actually changed, just unmarshalled + # ${foo} is reformatted from the unmarshalled value + # + # NOTE the extra spaces in the assertion below on the $foo part + template="'${values.json} -- $foo\n'", + ) + + syslog_ng.start(config) + + assert file_true.get_stats()["processed"] == 1 + assert "processed" not in file_false.get_stats() + (values_json, foo) = file_true.read_log().strip().split(' -- ') + assert values_json == """{"emb_key1": "emb_key1 value", "emb_key2": "emb_key2 value"}""" + assert foo == """{"emb_key1":"emb_key1 value","emb_key2":"emb_key2 value"}""" + + def test_floating_variables_are_dropped_at_the_end_of_the_scope(config, syslog_ng): (file_true, file_false, _) = create_config( config, [