]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] Enable deps in pre/postfilters and fix settings flow
authorVsevolod Stakhov <vsevolod@rspamd.com>
Tue, 31 Mar 2026 08:55:34 +0000 (09:55 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Tue, 31 Mar 2026 08:55:34 +0000 (09:55 +0100)
Add dependency checking to process_pre_postfilters, allowing
prefilters, postfilters and idempotent filters to declare deps
on symbols in the same stage. Previously deps only worked for
the FILTER stage.

Use this to make SETTINGS_APPLY depend on SETTINGS_CHECK (and
REDIS_SETTINGS* when configured), ensuring proper ordering via
the dep system instead of fragile priority hacks.

Restore immediate settings application in collectors for backward
compatibility - SETTINGS_APPLY now only triggers when multiple
layers need merging.

Also fix missing #include <algorithm> for std::sort on GCC.

src/libserver/settings_merge.cxx
src/libserver/symcache/symcache_runtime.cxx
src/plugins/lua/settings.lua

index 8808c306fff5201b95aa60a79db9c4884cb26d34..051f804b1fb83a0e7a7127835232a607c45f60e2 100644 (file)
@@ -19,6 +19,7 @@
 #include "cfg_file.h"
 #include "contrib/ankerl/unordered_dense.h"
 
+#include <algorithm>
 #include <string>
 #include <vector>
 
index e4d8e2d0e1103b0ad823d60bce380c8ae027faff..bc1ffc01c2d1a9e30179e89ff8c5f84e84d51c9f 100644 (file)
@@ -386,6 +386,16 @@ auto symcache_runtime::process_pre_postfilters(struct rspamd_task *task,
                                }
                        }
 
+                       /* Check dependencies for pre/postfilters */
+                       if (!item->deps.empty()) {
+                               if (!check_item_deps(task, cache, item, dyn_item, false)) {
+                                       msg_debug_cache_task_lambda("blocked execution of %d(%s) in "
+                                                                                               "pre/postfilter stage unless deps are resolved",
+                                                                                               item->id, item->symbol.c_str());
+                                       return false;
+                               }
+                       }
+
                        return process_symbol(task, cache, item, dyn_item);
                }
 
index 90625a85010cf2806afb3362a7f2b437a5f4ee1e..379289c1976d42295c87c7e93c3e936bd75b8851 100644 (file)
@@ -34,6 +34,7 @@ local fun = require "fun"
 local rspamd_mempool = require "rspamd_mempool"
 
 local redis_params
+local redis_sym_names -- populated if redis settings are configured
 
 local settings = {}
 local N = "settings"
@@ -117,14 +118,23 @@ local function apply_settings_side_effects(task, to_apply)
   end
 end
 
--- Legacy apply for single-layer case (backward compat)
+-- Apply settings immediately AND collect for potential merge
 local function apply_settings(task, to_apply, id, name)
+  -- Collect for potential multi-layer merge
   collect_settings_layer(task, SETTINGS_LAYER.RULE, name, id, to_apply)
 
+  -- Also apply immediately for backward compatibility
+  -- (SETTINGS_APPLY will re-apply merged result if multiple layers exist)
+  task:set_settings(to_apply)
+  task:cache_set('settings', to_apply)
+  task:cache_set('settings_name', name or 'unknown')
+
   if id then
     task:set_settings_id(id)
   end
 
+  apply_settings_side_effects(task, to_apply)
+
   return true
 end
 
@@ -341,12 +351,17 @@ local function check_settings(task)
   local function maybe_apply_query_settings()
     if query_apply then
       if id_elt then
-        collect_settings_layer(task, SETTINGS_LAYER.PROFILE, id_elt.name, id_elt.id, query_apply)
-        rspamd_logger.infox(task, "collected settings id %s(%s); priority %s",
+        apply_settings(task, query_apply, id_elt.id, id_elt.name)
+        rspamd_logger.infox(task, "applied settings id %s(%s); priority %s",
             id_elt.name, id_elt.id, priority_to_string(priority))
       else
+        -- Collect at HTTP layer + apply immediately
         collect_settings_layer(task, SETTINGS_LAYER.HTTP, 'HTTP query', 0, query_apply)
-        rspamd_logger.infox(task, "collected settings from query; priority %s",
+        task:set_settings(query_apply)
+        task:cache_set('settings', query_apply)
+        task:cache_set('settings_name', 'HTTP query')
+        apply_settings_side_effects(task, query_apply)
+        rspamd_logger.infox(task, "applied settings from query; priority %s",
             priority_to_string(priority))
       end
     end
@@ -474,9 +489,14 @@ local function gen_settings_external_cb(name)
             name, ucl_err)
       else
         local obj = parser:get_object()
-        rspamd_logger.infox(task, "<%s> collect settings from external map %s",
+        rspamd_logger.infox(task, "<%s> apply settings from external map %s",
             name, task:get_message_id())
+        -- Collect for merge AND apply immediately
         collect_settings_layer(task, SETTINGS_LAYER.RULE, 'external_map:' .. name, 0, obj)
+        task:set_settings(obj)
+        task:cache_set('settings', obj)
+        task:cache_set('settings_name', 'external_map:' .. name)
+        apply_settings_side_effects(task, obj)
       end
     else
       rspamd_logger.infox(task, "<%s> no settings returned from the external map %s: %s (code = %s)",
@@ -1301,9 +1321,14 @@ local function gen_redis_callback(handler, id)
                   ucl_err)
             else
               local obj = parser:get_object()
-              rspamd_logger.infox(task, "<%s> collect settings from redis rule %s",
+              rspamd_logger.infox(task, "<%s> apply settings from redis rule %s",
                   task:get_message_id(), id)
+              -- Collect for merge AND apply immediately
               collect_settings_layer(task, SETTINGS_LAYER.PER_USER, 'redis:' .. tostring(id), 0, obj)
+              task:set_settings(obj)
+              task:cache_set('settings', obj)
+              task:cache_set('settings_name', 'redis:' .. tostring(id))
+              apply_settings_side_effects(task, obj)
               break
             end
           end
@@ -1366,15 +1391,18 @@ if redis_section then
     end
   end
 
+  redis_sym_names = {}
   fun.each(function(id, h)
+    local redis_sym_name = 'REDIS_SETTINGS' .. tostring(id)
     rspamd_config:register_symbol({
-      name = 'REDIS_SETTINGS' .. tostring(id),
+      name = redis_sym_name,
       type = 'prefilter',
       callback = gen_redis_callback(h, id),
       priority = lua_util.symbols_priorities.top,
       flags = 'empty,nostat',
       augmentations = { string.format("timeout=%f", redis_params.timeout or 0.0) },
     })
+    redis_sym_names[#redis_sym_names + 1] = redis_sym_name
   end, redis_key_handlers)
 end
 
@@ -1387,60 +1415,58 @@ module_sym_id = rspamd_config:register_symbol({
 })
 
 -- SETTINGS_APPLY runs after all settings collectors (SETTINGS_CHECK, REDIS_SETTINGS*)
--- have finished, merges collected layers, and applies the result.
--- Priority high (9) < top (10) ensures proper ordering via prefilter priority mechanism.
+-- via explicit dependencies. It merges multiple layers when present.
+-- Each collector also applies immediately for backward compat (single-layer case).
+-- SETTINGS_APPLY only re-applies when multiple layers need merging.
 rspamd_config:register_symbol({
   name = 'SETTINGS_APPLY',
   type = 'prefilter',
   callback = function(task)
     local layers = task:cache_get('settings_layers')
-    if not layers or #layers == 0 then
-      lua_util.debugm(N, task, "no settings layers collected, nothing to apply")
+    if not layers or #layers <= 1 then
+      -- Single layer (or none): already applied immediately by the collector
       return
     end
 
-    if #layers == 1 then
-      -- Single layer: apply directly without merge overhead
-      local layer = layers[1]
-      lua_util.debugm(N, task, "single settings layer %s, applying directly", layer.name)
-      task:set_settings(layer.apply)
+    -- Multiple layers: merge via C infrastructure and re-apply
+    rspamd_logger.infox(task, "merging %s settings layers", #layers)
+
+    -- Collect settings_id from the highest-priority layer that has one
+    local best_settings_id = nil
+    for _, layer in ipairs(layers) do
       if layer.settings_id and layer.settings_id ~= 0 then
-        task:set_settings_id(layer.settings_id)
-      end
-      apply_settings_side_effects(task, layer.apply)
-    else
-      -- Multiple layers: merge via C infrastructure
-      rspamd_logger.infox(task, "merging %s settings layers", #layers)
-
-      -- Also collect settings_id from the highest-priority layer that has one
-      local best_settings_id = nil
-      for _, layer in ipairs(layers) do
-        if layer.settings_id and layer.settings_id ~= 0 then
-          best_settings_id = layer.settings_id
-        end
+        best_settings_id = layer.settings_id
       end
+    end
 
-      local merged = task:merge_and_apply_settings(layers)
-      if merged then
-        if best_settings_id then
-          task:set_settings_id(best_settings_id)
-        end
-        -- Apply Lua-side effects from the merged result
-        local merged_settings = task:get_settings()
-        if merged_settings then
-          apply_settings_side_effects(task, merged_settings)
-        end
-      else
-        rspamd_logger.warnx(task, "settings merge returned no result")
+    local merged = task:merge_and_apply_settings(layers)
+    if merged then
+      if best_settings_id then
+        task:set_settings_id(best_settings_id)
+      end
+      -- Apply Lua-side effects from the merged result
+      local merged_settings = task:get_settings()
+      if merged_settings then
+        apply_settings_side_effects(task, merged_settings)
       end
+    else
+      rspamd_logger.warnx(task, "settings merge returned no result")
     end
-
-    task:cache_set('settings_applied', true)
   end,
-  priority = lua_util.symbols_priorities.high,
+  priority = lua_util.symbols_priorities.top,
   flags = 'empty,nostat,explicit_disable,ignore_passthrough',
 })
 
+-- SETTINGS_APPLY depends on SETTINGS_CHECK (waits for it to finish collecting)
+rspamd_config:register_dependency('SETTINGS_APPLY', 'SETTINGS_CHECK')
+
+-- Also depend on REDIS_SETTINGS symbols if redis is configured
+if redis_sym_names then
+  for _, sym_name in ipairs(redis_sym_names) do
+    rspamd_config:register_dependency('SETTINGS_APPLY', sym_name)
+  end
+end
+
 local set_section = rspamd_config:get_all_opt("settings")
 
 if set_section and set_section[1] and type(set_section[1]) == "string" then