From: Vsevolod Stakhov <vsevolod@rspamd.com>
Date: Wed, 3 Jun 2026 09:40:48 +0000 (+0100)
Subject: [Fix] ratelimit: Track all buckets in selector rules
X-Git-Tag: 4.1.0~8^2
X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=refs%2Fpull%2F6076%2Fhead;p=thirdparty%2Frspamd.git

[Fix] ratelimit: Track all buckets in selector rules

When a rule defines several buckets (e.g. "200 / 1h" plus "30 / 1m")
and uses a selector, limit_to_prefixes keyed the prefixes table by the
selector value alone. Every bucket therefore mapped to the same Redis
key and only the last bucket in the array was ever tracked, so the
other limits were silently ignored.

Give each bucket a distinct key by prefixing the selector value with a
per-bucket id (burst + rate), mirroring the burst component that
gen_rate_key already prepends for the non-selector path. The
non-selector path is unchanged, so its existing Redis keys are kept.

Adds a functional regression test (two buckets, burst 2 + burst 20)
that fails before the fix because the restrictive bucket is ignored.

Closes #6059
---

diff --git a/src/plugins/lua/ratelimit.lua b/src/plugins/lua/ratelimit.lua
index 695ef3ddb0..f0a6c836ca 100644
--- a/src/plugins/lua/ratelimit.lua
+++ b/src/plugins/lua/ratelimit.lua
@@ -280,19 +280,31 @@ local function make_prefix(redis_key, name, bucket)
   }
 end
 
+-- Distinguishes buckets within a single rule so that, e.g. "200 / 1h" and
+-- "30 / 1m" map to separate Redis keys instead of collapsing onto the same
+-- selector value (see issue #6059). Uses burst + rate, mirroring the burst
+-- component that gen_rate_key already prepends for the non-selector path.
+local function bucket_id(bucket)
+  return string.format('%s:%s',
+    lua_util.round(100000.0 / bucket.burst), bucket.rate)
+end
+
 local function limit_to_prefixes(task, k, v, prefixes)
   local n = 0
   for _, bucket in ipairs(v.buckets) do
     if v.selector then
       local selectors = lua_selectors.process_selectors(task, v.selector)
       if selectors then
+        local bid = bucket_id(bucket)
         local combined = lua_selectors.combine_selectors(task, selectors, ':')
         if type(combined) == 'string' then
-          prefixes[combined] = make_prefix(combined, k, bucket)
+          local rkey = bid .. ':' .. combined
+          prefixes[rkey] = make_prefix(rkey, k, bucket)
           n = n + 1
         else
           fun.each(function(p)
-            prefixes[p] = make_prefix(p, k, bucket)
+            local rkey = bid .. ':' .. p
+            prefixes[rkey] = make_prefix(rkey, k, bucket)
             n = n + 1
           end, combined)
         end
diff --git a/test/functional/cases/500_ratelimit.robot b/test/functional/cases/500_ratelimit.robot
index 85e7b353cd..0490306d11 100644
--- a/test/functional/cases/500_ratelimit.robot
+++ b/test/functional/cases/500_ratelimit.robot
@@ -46,3 +46,9 @@ RATELIMIT CHECK BUILTIN
 
 RATELIMIT CHECK SELECTOR
   Basic Test  foo@example.net  special@example.net  2
+
+RATELIMIT CHECK MULTIPLE BUCKETS
+  # Regression for #6059: with two buckets (burst 2 and burst 20) the most
+  # restrictive one must fire. Before the fix only the last bucket (burst 20)
+  # was tracked, so a soft reject would never happen at the third message.
+  Basic Test  bar@example.net  multi@example.net  2
diff --git a/test/functional/configs/ratelimit.conf b/test/functional/configs/ratelimit.conf
index 36bae88fee..9e6034b77e 100644
--- a/test/functional/configs/ratelimit.conf
+++ b/test/functional/configs/ratelimit.conf
@@ -19,5 +19,20 @@ ratelimit {
         rate = "1 / 1s";
       }
     }
+    to_multi_buckets {
+      # Two buckets in one rule: the restrictive one (burst = 2) must apply,
+      # not just the last one in the array (burst = 20). Regression for #6059.
+      selector = "id('multi');to.in('multi@example.net')";
+      bucket = [
+        {
+          burst = 2;
+          rate = "1 / 1s";
+        },
+        {
+          burst = 20;
+          rate = "1 / 1s";
+        }
+      ];
+    }
   }
 }