commit bb5e5fb: [Rework] Allow to set a different behaviour for actions from settings

Vsevolod Stakhov vsevolod at highsecure.ru
Sat Jan 29 12:42:04 UTC 2022


Author: Vsevolod Stakhov
Date: 2022-01-29 12:27:41 +0000
URL: https://github.com/rspamd/rspamd/commit/bb5e5fb4149df13488fa04623a50eca3ada13f7e

[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)) {


More information about the Commits mailing list