From bb5e5fb4149df13488fa04623a50eca3ada13f7e Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Sat, 29 Jan 2022 12:27:41 +0000 Subject: [PATCH] [Rework] Allow to set a different behaviour for actions from settings Issue: #4025 --- src/libmime/message.c | 3 +- src/libmime/scan_result.c | 48 ++++++++++++++++++++++----- src/libmime/scan_result.h | 13 ++++++-- src/libserver/task.c | 4 +-- src/lua/lua_task.c | 70 ++++++++++++++++++++++----------------- 5 files changed, 92 insertions(+), 46 deletions(-) diff --git a/src/libmime/message.c b/src/libmime/message.c index 1676e4218..ec49b3b5e 100644 --- a/src/libmime/message.c +++ b/src/libmime/message.c @@ -862,7 +862,8 @@ rspamd_message_process_text_part_maybe (struct rspamd_task *task, rspamd_add_passthrough_result (task, action, RSPAMD_PASSTHROUGH_CRITICAL, - score, "Gtube pattern", "GTUBE", 0, NULL); + score, "Gtube pattern", + "GTUBE", 0, NULL); } rspamd_task_insert_result (task, GTUBE_SYMBOL, 0, NULL); diff --git a/src/libmime/scan_result.c b/src/libmime/scan_result.c index 16ec9b0c5..37244f60f 100644 --- a/src/libmime/scan_result.c +++ b/src/libmime/scan_result.c @@ -91,15 +91,15 @@ rspamd_create_metric_result (struct rspamd_task *task, if (task->cfg) { struct rspamd_action *act, *tmp; - metric_res->actions_limits = rspamd_mempool_alloc0 (task->task_pool, - sizeof (struct rspamd_action_result) * HASH_COUNT (task->cfg->actions)); + metric_res->actions_config = rspamd_mempool_alloc0 (task->task_pool, + sizeof (struct rspamd_action_config) * HASH_COUNT (task->cfg->actions)); i = 0; HASH_ITER (hh, task->cfg->actions, act, tmp) { if (!(act->flags & RSPAMD_ACTION_NO_THRESHOLD)) { - metric_res->actions_limits[i].cur_limit = act->threshold; + metric_res->actions_config[i].cur_limit = act->threshold; } - metric_res->actions_limits[i].action = act; + metric_res->actions_config[i].action = act; i ++; } @@ -122,9 +122,14 @@ rspamd_pr_sort (const struct rspamd_passthrough_result *pra, return prb->priority - pra->priority; } -void -rspamd_add_passthrough_result (struct rspamd_task *task, struct rspamd_action *action, guint priority, - double target_score, const gchar *message, const gchar *module, guint flags, +bool +rspamd_add_passthrough_result (struct rspamd_task *task, + struct rspamd_action *action, + guint priority, + double target_score, + const gchar *message, + const gchar *module, + uint flags, struct rspamd_scan_result *scan_result) { struct rspamd_passthrough_result *pr; @@ -133,6 +138,29 @@ rspamd_add_passthrough_result (struct rspamd_task *task, struct rspamd_action *a scan_result = task->result; } + /* Find the speicific action config */ + struct rspamd_action_config *action_config = NULL; + + for (unsigned int i = 0; i < HASH_COUNT (task->cfg->actions); i ++) { + struct rspamd_action_config *cur = &scan_result->actions_config[i]; + + /* We assume that all action pointers are static */ + if (cur->action == action) { + action_config = cur; + break; + } + } + + if (action_config && (action_config->flags & RSPAMD_ACTION_RESULT_DISABLED)) { + msg_info_task ("<%s>: NOT set pre-result to '%s' %s(%.2f): '%s' from %s(%d); action is disabled", + MESSAGE_FIELD_CHECK (task, message_id), action->name, + flags & RSPAMD_PASSTHROUGH_LEAST ? "*least " : "", + target_score, + message, module, priority); + + return false; + } + pr = rspamd_mempool_alloc (task->task_pool, sizeof (*pr)); pr->action = action; pr->priority = priority; @@ -160,6 +188,8 @@ rspamd_add_passthrough_result (struct rspamd_task *task, struct rspamd_action *a } scan_result->nresults ++; + + return true; } static inline gdouble @@ -777,7 +807,7 @@ rspamd_check_action_metric (struct rspamd_task *task, struct rspamd_passthrough_result **ppr, struct rspamd_scan_result *scan_result) { - struct rspamd_action_result *action_lim, + struct rspamd_action_config *action_lim, *noaction = NULL; struct rspamd_action *selected_action = NULL, *least_action = NULL; struct rspamd_passthrough_result *pr, *sel_pr = NULL; @@ -850,7 +880,7 @@ rspamd_check_action_metric (struct rspamd_task *task, * Select result by score */ for (i = scan_result->nactions - 1; i >= 0; i--) { - action_lim = &scan_result->actions_limits[i]; + action_lim = &scan_result->actions_config[i]; sc = action_lim->cur_limit; if (action_lim->action->action_type == METRIC_ACTION_NOACTION) { diff --git a/src/libmime/scan_result.h b/src/libmime/scan_result.h index c8bacf3e8..2f982fd1b 100644 --- a/src/libmime/scan_result.h +++ b/src/libmime/scan_result.h @@ -66,8 +66,15 @@ struct rspamd_passthrough_result { struct rspamd_passthrough_result *prev, *next; }; -struct rspamd_action_result { + +enum rspamd_action_config_flags { + RSPAMD_ACTION_RESULT_DEFAULT = 0, + RSPAMD_ACTION_RESULT_NO_THRESHOLD = (1u << 0u), + RSPAMD_ACTION_RESULT_DISABLED = (1u << 1u), +}; +struct rspamd_action_config { gdouble cur_limit; + int flags; struct rspamd_action *action; }; @@ -83,7 +90,7 @@ struct rspamd_scan_result { double negative_score; struct kh_rspamd_symbols_hash_s *symbols; /**< symbols of metric */ struct kh_rspamd_symbols_group_hash_s *sym_groups; /**< groups of symbols */ - struct rspamd_action_result *actions_limits; + struct rspamd_action_config *actions_config; const gchar *name; /**< for named results, NULL is the default result */ struct rspamd_task *task; /**< back reference */ gint symbol_cbref; /**< lua function that defines if a symbol can be inserted, -1 if unused */ @@ -121,7 +128,7 @@ struct rspamd_scan_result *rspamd_find_metric_result (struct rspamd_task *task, * @param message * @param module */ -void rspamd_add_passthrough_result (struct rspamd_task *task, +bool rspamd_add_passthrough_result (struct rspamd_task *task, struct rspamd_action *action, guint priority, double target_score, const gchar *message, const gchar *module, guint flags, diff --git a/src/libserver/task.c b/src/libserver/task.c index 244327b49..2b2dc727d 100644 --- a/src/libserver/task.c +++ b/src/libserver/task.c @@ -1673,12 +1673,12 @@ rspamd_task_get_required_score (struct rspamd_task *task, struct rspamd_scan_res } for (i = m->nactions - 1; i >= 0; i --) { - struct rspamd_action_result *action_lim = &m->actions_limits[i]; + struct rspamd_action_config *action_lim = &m->actions_config[i]; if (!isnan (action_lim->cur_limit) && !(action_lim->action->flags & (RSPAMD_ACTION_NO_THRESHOLD|RSPAMD_ACTION_HAM))) { - return m->actions_limits[i].cur_limit; + return m->actions_config[i].cur_limit; } } diff --git a/src/lua/lua_task.c b/src/lua/lua_task.c index b0ddc5e42..4a66ce865 100644 --- a/src/lua/lua_task.c +++ b/src/lua/lua_task.c @@ -5652,40 +5652,35 @@ lua_task_set_settings (lua_State *L) while ((cur = ucl_object_iterate (act, &it, true)) != NULL) { const gchar *act_name = ucl_object_key (cur); - double act_score = ucl_object_type (cur) == UCL_NULL ? - NAN : ucl_object_todouble (cur), old_score = NAN; + struct rspamd_action_config *action_config = NULL; + double act_score; int act_type; - gboolean found = FALSE; if (!rspamd_action_from_str (act_name, &act_type)) { act_type = -1; } for (i = 0; i < mres->nactions; i++) { - struct rspamd_action_result *act_res = &mres->actions_limits[i]; + struct rspamd_action_config *cur_act = &mres->actions_config[i]; - if (act_res->action->action_type == METRIC_ACTION_CUSTOM && + if (cur_act->action->action_type == METRIC_ACTION_CUSTOM && act_type == -1) { /* Compare by name */ - if (g_ascii_strcasecmp (act_name, act_res->action->name) == 0) { - old_score = act_res->cur_limit; - act_res->cur_limit = act_score; - found = TRUE; + if (g_ascii_strcasecmp (act_name, cur_act->action->name) == 0) { + action_config = cur_act; break; } } else { - if (act_res->action->action_type == act_type) { - old_score = act_res->cur_limit; - act_res->cur_limit = act_score; - found = TRUE; + if (cur_act->action->action_type == act_type) { + action_config = cur_act; break; } } } - if (!found) { - + if (!action_config) { + act_score = ucl_object_todouble(cur); if (!isnan (act_score)) { struct rspamd_action *new_act; @@ -5713,28 +5708,41 @@ lua_task_set_settings (lua_State *L) /* Insert it to the mres structure */ gsize new_actions_cnt = mres->nactions + 1; - struct rspamd_action_result *old_actions = mres->actions_limits; - - mres->actions_limits = rspamd_mempool_alloc (task->task_pool, - sizeof (struct rspamd_action_result) * new_actions_cnt); - memcpy (mres->actions_limits, old_actions, - sizeof (struct rspamd_action_result) * mres->nactions); - mres->actions_limits[mres->nactions].action = new_act; - mres->actions_limits[mres->nactions].cur_limit = act_score; + struct rspamd_action_config *old_actions = mres->actions_config; + + mres->actions_config = rspamd_mempool_alloc (task->task_pool, + sizeof (struct rspamd_action_config) * new_actions_cnt); + memcpy (mres->actions_config, old_actions, + sizeof (struct rspamd_action_config) * mres->nactions); + mres->actions_config[mres->nactions].action = new_act; + mres->actions_config[mres->nactions].cur_limit = act_score; mres->nactions ++; } /* Disabled/missing action is disabled one more time, not an error */ } else { - if (isnan (act_score)) { + /* Found the existing configured action */ + if (ucl_object_type (cur) == UCL_NULL) { + /* Disable action completely */ + action_config->flags |= RSPAMD_ACTION_RESULT_DISABLED; msg_info_task ("disabled action %s due to settings", - act_name); + action_config->action->name); } else { - msg_debug_task ("adjusted action %s: %.2f -> %.2f", - act_name, - old_score, - act_score); + act_score = ucl_object_todouble(cur); + if (isnan (act_score)) { + msg_info_task ("disabled action %s threshold (was %.2f) due to settings", + action_config->action->name, + action_config->cur_limit); + action_config->flags |= RSPAMD_ACTION_RESULT_NO_THRESHOLD; + } + else { + action_config->cur_limit = act_score; + msg_debug_task ("adjusted action %s: %.2f -> %.2f", + act_name, + action_config->cur_limit, + act_score); + } } } } @@ -6347,14 +6355,14 @@ lua_task_disable_action (lua_State *L) LUA_TRACE_POINT; struct rspamd_task *task = lua_check_task (L, 1); const gchar *action_name; - struct rspamd_action_result *action_res; + struct rspamd_action_config *action_res; action_name = luaL_checkstring (L, 2); if (task && action_name) { for (guint i = 0; i < task->result->nactions; i ++) { - action_res = &task->result->actions_limits[i]; + action_res = &task->result->actions_config[i]; if (strcmp (action_name, action_res->action->name) == 0) { if (isnan (action_res->cur_limit)) { -- 2.39.5