commit 092129b: [Project] Various fixes

Vsevolod Stakhov vsevolod at highsecure.ru
Fri Jun 14 19:00:23 UTC 2019


Author: Vsevolod Stakhov
Date: 2019-06-14 18:28:31 +0100
URL: https://github.com/rspamd/rspamd/commit/092129b5216cb55de385ee6138a29c537142596c

[Project] Various fixes

---
 lualib/lua_settings.lua         | 16 +++++++--
 src/libserver/cfg_utils.c       | 11 +++++--
 src/libserver/protocol.c        |  4 +++
 src/libserver/rspamd_symcache.c | 72 ++++++++++++++++++++++++-----------------
 src/lua/lua_config.c            |  6 ++--
 src/lua/lua_task.c              |  7 ++--
 src/plugins/lua/settings.lua    | 24 ++++++++++++--
 7 files changed, 96 insertions(+), 44 deletions(-)

diff --git a/lualib/lua_settings.lua b/lualib/lua_settings.lua
index 6723e4171..5cd2fb8fa 100644
--- a/lualib/lua_settings.lua
+++ b/lualib/lua_settings.lua
@@ -26,23 +26,27 @@ local on_load_added = false
 
 local function register_settings_cb()
   for _,set in pairs(known_ids) do
-    local s = set.settings
+    local s = set.settings.apply
     local enabled_symbols = {}
+    local seen_enabled = false
     local disabled_symbols = {}
+    local seen_disabled = false
 
     -- Enabled map
     if s.symbols_enabled then
       for _,sym in ipairs(s.symbols_enabled) do
         enabled_symbols[sym] = true
+        seen_enabled = true
       end
     end
     if s.groups_enabled then
       for _,gr in ipairs(s.groups_enabled) do
-        local syms = rspamd_config:get_group_symbols()
+        local syms = rspamd_config:get_group_symbols(gr)
 
         if syms then
           for _,sym in ipairs(syms) do
             enabled_symbols[sym] = true
+            seen_enabled = true
           end
         end
       end
@@ -52,20 +56,26 @@ local function register_settings_cb()
     if s.symbols_disabled then
       for _,sym in ipairs(s.symbols_disabled) do
         disabled_symbols[sym] = true
+        seen_disabled = true
       end
     end
     if s.groups_disabled then
       for _,gr in ipairs(s.groups_disabled) do
-        local syms = rspamd_config:get_group_symbols()
+        local syms = rspamd_config:get_group_symbols(gr)
 
         if syms then
           for _,sym in ipairs(syms) do
             disabled_symbols[sym] = true
+            seen_disabled = true
           end
         end
       end
     end
 
+    -- Deal with complexity to avoid mess in C
+    if not seen_enabled then enabled_symbols = nil end
+    if not seen_disabled then disabled_symbols = nil end
+
     rspamd_config:register_settings_id(set.name, enabled_symbols, disabled_symbols)
 
     -- Remove to avoid clash
diff --git a/src/libserver/cfg_utils.c b/src/libserver/cfg_utils.c
index 0c47ec22e..0f38cc389 100644
--- a/src/libserver/cfg_utils.c
+++ b/src/libserver/cfg_utils.c
@@ -246,6 +246,7 @@ void
 rspamd_config_free (struct rspamd_config *cfg)
 {
 	struct rspamd_config_post_load_script *sc, *sctmp;
+	struct rspamd_config_settings_elt *set, *stmp;
 	struct rspamd_worker_log_pipe *lp, *ltmp;
 
 	DL_FOREACH_SAFE (cfg->finish_callbacks, sc, sctmp) {
@@ -258,6 +259,10 @@ rspamd_config_free (struct rspamd_config *cfg)
 		g_free (sc);
 	}
 
+	DL_FOREACH_SAFE (cfg->setting_ids, set, stmp) {
+		REF_RELEASE (set);
+	}
+
 	rspamd_map_remove_all (cfg);
 	rspamd_mempool_destructors_enforce (cfg->cfg_pool);
 
@@ -2352,7 +2357,7 @@ rspamd_config_name_to_id (const gchar *name, gsize namelen)
 	guint64 h;
 
 	h = rspamd_cryptobox_fast_hash_specific (RSPAMD_CRYPTOBOX_XXHASH64,
-			name, strlen (name), 0x0);
+			name, namelen, 0x0);
 	/* Take the lower part of hash as LE number */
 	return ((guint32)GUINT64_TO_LE (h));
 }
@@ -2416,7 +2421,7 @@ rspamd_config_register_settings_id (struct rspamd_config *cfg,
 		}
 
 		REF_INIT_RETAIN (nelt, rspamd_config_settings_elt_dtor);
-		msg_warn_config ("replace settings id %d (%s)", id, name);
+		msg_warn_config ("replace settings id %ud (%s)", id, name);
 		rspamd_symcache_process_settings_elt (cfg->cache, elt);
 		DL_APPEND (cfg->setting_ids, nelt);
 
@@ -2442,7 +2447,7 @@ rspamd_config_register_settings_id (struct rspamd_config *cfg,
 			elt->symbols_disabled = ucl_object_ref (symbols_disabled);
 		}
 
-		msg_info_config ("register new settings id %d (%s)", id, name);
+		msg_info_config ("register new settings id %ud (%s)", id, name);
 		REF_INIT_RETAIN (elt, rspamd_config_settings_elt_dtor);
 		rspamd_symcache_process_settings_elt (cfg->cache, elt);
 		DL_APPEND (cfg->setting_ids, elt);
diff --git a/src/libserver/protocol.c b/src/libserver/protocol.c
index 1ee5a2992..7df5b27c5 100644
--- a/src/libserver/protocol.c
+++ b/src/libserver/protocol.c
@@ -491,6 +491,10 @@ rspamd_protocol_handle_headers (struct rspamd_task *task,
 						msg_warn_protocol ("unknown settings id: %V",
 								hv);
 					}
+					else {
+						msg_debug_protocol ("applied settings id %V -> %ud", hv,
+								task->settings_elt->id);
+					}
 				}
 				break;
 			case 'u':
diff --git a/src/libserver/rspamd_symcache.c b/src/libserver/rspamd_symcache.c
index 1ae279e73..25707b447 100644
--- a/src/libserver/rspamd_symcache.c
+++ b/src/libserver/rspamd_symcache.c
@@ -1442,7 +1442,7 @@ rspamd_symcache_is_item_allowed (struct rspamd_task *task,
 			rspamd_symcache_check_id_list (&item->forbidden_ids,
 					id)) {
 			msg_debug_cache_task ("deny execution of %s as it is forbidden for "
-						 "settings id %d",
+						 "settings id %ud",
 						 item->symbol,
 						 id);
 			return FALSE;
@@ -1453,7 +1453,7 @@ rspamd_symcache_is_item_allowed (struct rspamd_task *task,
 				!rspamd_symcache_check_id_list (&item->allowed_ids,
 						id)) {
 				msg_debug_cache_task ("deny execution of %s as it is not listed "
-									  "as allowed for settings id %d",
+									  "as allowed for settings id %ud",
 						item->symbol,
 						id);
 				return FALSE;
@@ -1461,7 +1461,7 @@ rspamd_symcache_is_item_allowed (struct rspamd_task *task,
 		}
 		else {
 			msg_debug_cache_task ("allow execution of %s for "
-								  "settings id %d as it can be only disabled explicitly",
+								  "settings id %ud as it can be only disabled explicitly",
 					item->symbol,
 					id);
 		}
@@ -3217,24 +3217,31 @@ rspamd_symcache_process_settings_elt (struct rspamd_symcache *cache,
 		iter = NULL;
 
 		while ((cur = ucl_object_iterate (elt->symbols_disabled, &iter, true)) != NULL) {
-			item = rspamd_symcache_find_filter (cache,
-					ucl_object_tostring (cur), false);
-
-			if (item->is_virtual) {
-				/*
-				 * Virtual symbols are special:
-				 * we ignore them in symcache but prevent them from being
-				 * inserted.
-				 */
-				msg_debug_cache ("skip virtual symbol %s for settings id %d (%s)",
-						ucl_object_tostring (cur), id, elt->name);
+			const gchar *sym = ucl_object_key (cur);
+			item = rspamd_symcache_find_filter (cache, sym, false);
+
+			if (item) {
+				if (item->is_virtual) {
+					/*
+					 * Virtual symbols are special:
+					 * we ignore them in symcache but prevent them from being
+					 * inserted.
+					 */
+					msg_debug_cache ("skip virtual symbol %s for settings id %ud (%s)",
+							sym, id, elt->name);
+				}
+				else {
+					/* Normal symbol, disable it */
+					rspamd_symcache_add_id_to_list (cache->static_pool,
+							&item->forbidden_ids, id);
+					msg_debug_cache ("deny symbol %s for settings %ud (%s)",
+							sym, id, elt->name);
+				}
 			}
 			else {
-				/* Normal symbol, disable it */
-				rspamd_symcache_add_id_to_list (cache->static_pool,
-						&item->forbidden_ids, id);
-				msg_debug_cache ("deny symbol %s for settings %d (%s)",
-						ucl_object_tostring (cur), id, elt->name);
+				msg_warn_cache ("cannot find a symbol to disable %s "
+					"when processing settings %ud (%s)",
+					sym, id, elt->name);
 			}
 		}
 	}
@@ -3244,20 +3251,27 @@ rspamd_symcache_process_settings_elt (struct rspamd_symcache *cache,
 
 		while ((cur = ucl_object_iterate (elt->symbols_enabled, &iter, true)) != NULL) {
 			/* Here, we resolve parent and explicitly allow it */
-			item = rspamd_symcache_find_filter (cache,
-					ucl_object_tostring (cur), true);
+			const gchar *sym = ucl_object_key (cur);
+			item = rspamd_symcache_find_filter (cache, sym, true);
 
-			if (elt->symbols_disabled &&
+			if (item) {
+				if (elt->symbols_disabled &&
 					ucl_object_lookup (elt->symbols_disabled, item->symbol)) {
-				msg_err_cache ("conflict in %s: cannot enable disabled symbol %s, "
-				   "wanted to enable symbol %s",
-						elt->name, item->symbol, ucl_object_tostring (cur));
+					msg_err_cache ("conflict in %s: cannot enable disabled symbol %s, "
+								   "wanted to enable symbol %s",
+							elt->name, item->symbol, sym);
+				}
+				else {
+					rspamd_symcache_add_id_to_list (cache->static_pool,
+							&item->allowed_ids, id);
+					msg_debug_cache ("allow execution of symbol %s for settings %ud (%s)",
+							sym, id, elt->name);
+				}
 			}
 			else {
-				rspamd_symcache_add_id_to_list (cache->static_pool,
-						&item->allowed_ids, id);
-				msg_debug_cache ("allow execution of symbol %s for settings %d (%s)",
-						ucl_object_tostring (cur), id, elt->name);
+				msg_warn_cache ("cannot find a symbol to enable %s "
+								"when processing settings %ud (%s)",
+						sym, id, elt->name);
 			}
 		}
 	}
diff --git a/src/lua/lua_config.c b/src/lua/lua_config.c
index 4eac4a0e4..cff27ca1e 100644
--- a/src/lua/lua_config.c
+++ b/src/lua/lua_config.c
@@ -3342,15 +3342,15 @@ lua_config_register_settings_id (lua_State *L)
 
 		sym_enabled = ucl_object_lua_import (L, 3);
 
-		if (sym_enabled == NULL || ucl_object_type (sym_enabled) != UCL_ARRAY) {
+		if (sym_enabled != NULL && ucl_object_type (sym_enabled) != UCL_OBJECT) {
 			ucl_object_unref (sym_enabled);
 
 			return luaL_error (L, "invalid symbols enabled");
 		}
 
-		sym_disabled = ucl_object_lua_import (L, 3);
+		sym_disabled = ucl_object_lua_import (L, 4);
 
-		if (sym_disabled == NULL || ucl_object_type (sym_disabled) != UCL_ARRAY) {
+		if (sym_disabled != NULL && ucl_object_type (sym_disabled) != UCL_OBJECT) {
 			ucl_object_unref (sym_enabled);
 			ucl_object_unref (sym_disabled);
 
diff --git a/src/lua/lua_task.c b/src/lua/lua_task.c
index 850eabf2e..8359ccd9c 100644
--- a/src/lua/lua_task.c
+++ b/src/lua/lua_task.c
@@ -4906,9 +4906,10 @@ lua_task_set_settings_id (lua_State *L)
 	if (task != NULL && id != 0) {
 
 		if (task->settings_elt) {
-			return luaL_error (L, "settings id has been already set to %d (%s)",
-					task->settings_elt->id, task->settings_elt->name);
-
+			if (task->settings_elt->id != id) {
+				return luaL_error (L, "settings id has been already set to %d (%s)",
+						task->settings_elt->id, task->settings_elt->name);
+			}
 		}
 		else {
 			task->settings_elt = rspamd_config_find_settings_id_ref (task->cfg,
diff --git a/src/plugins/lua/settings.lua b/src/plugins/lua/settings.lua
index 630095006..990fa47c3 100644
--- a/src/plugins/lua/settings.lua
+++ b/src/plugins/lua/settings.lua
@@ -449,10 +449,28 @@ local function check_settings(task)
 
         -- Can use xor here but more complicated for reading
         if (result and not s.rule.inverse) or (not result and s.rule.inverse) then
-          rspamd_logger.infox(task, "<%s> apply settings according to rule %s (%s matched)",
-            task:get_message_id(), s.name, table.concat(matched, ','))
           if s.rule['apply'] then
-            apply_settings(task, s.rule.apply, s.rule.id)
+            if s.rule.id then
+              -- Extract static settings
+              local cached = lua_settings.settings_by_id(s.rule.id)
+
+              if not cached then
+                rspamd_logger.errx(task, 'unregistered settings id found: %s!', s.rule.id)
+              else
+                rspamd_logger.infox(task, "<%s> apply static settings %s (id = %s); %s matched",
+                    task:get_message_id(),
+                    cached.name, s.rule.id,
+                    table.concat(matched, ','))
+                apply_settings(task, cached.settings, s.rule.id)
+              end
+
+            else
+              -- Dynamic settings
+              rspamd_logger.infox(task, "<%s> apply settings according to rule %s (%s matched)",
+                  task:get_message_id(), s.name, table.concat(matched, ','))
+              apply_settings(task, s.rule.apply, nil)
+            end
+
             applied = true
           end
           if s.rule['symbols'] then


More information about the Commits mailing list