]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] upstream: make addr accessors and all_upstreams pending-safe
authorVsevolod Stakhov <vsevolod@rspamd.com>
Sat, 25 Apr 2026 19:10:30 +0000 (20:10 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Sat, 25 Apr 2026 19:10:30 +0000 (20:10 +0100)
The PENDING_RESOLVE upstream state introduced earlier kept pending
entries out of the alive list, but `:all_upstreams()` walks the full
`ups` array and exposes them to Lua callers - which then crashed in
`s:get_addr()` because `rspamd_upstream_addr_next/cur/port` indexed
a NULL `addrs.addr`.

Defensive fix at the C accessor layer:

* rspamd_upstream_addr_next / _cur now return NULL when the upstream
  has no addresses (NULL or empty array). This is the safe layer that
  every other consumer eventually goes through.
* rspamd_upstream_port returns the parsed `deferred_port` for pending
  upstreams (so callers that just want a port get a sensible answer)
  and -1 if even that is unknown.
* lua_upstream:get_addr() pushes nil when the C side has no address.

Audit of `:all_upstreams()` callers, all updated to skip pending:

* lua_redis prepare_redis_call (SCRIPT LOAD broadcast): if
  `s:get_addr()` is nil, mark the slot as "tempfail" so the next
  retry will pick it up once DNS comes back, log, and skip it.
* rspamadm statistics_dump connect_to_upstream: log and return early
  before opening a redis connection with a nil host.
* clickhouse plugin check_clickhouse_upstream: skip with an info log
  so the periodic check tries again next tick.

The DKIM Vault helper already passes `upstream = ... or nil` to
http.request and lets the HTTP layer fall back to URL-based connect,
which remains the right behaviour.

lualib/lua_redis.lua
lualib/rspamadm/statistics_dump.lua
src/libutil/upstream.c
src/lua/lua_upstream.c
src/plugins/lua/clickhouse.lua

index 4e4d192bc417bd974b5e5c8ff6f1294d0f8e49bd..e6465fa17d852acbcc48912042d6d54d1ec53380 100644 (file)
@@ -1421,8 +1421,16 @@ local function prepare_redis_call(script)
   end
 
   for idx, s in ipairs(servers) do
+    local server_addr = s:get_addr()
+    if not server_addr then
+      -- Pending DNS resolution; mark as tempfail so the next attempt retries
+      script.servers_ready[idx] = "tempfail"
+      logger.infox(rspamd_config,
+          'skipping SCRIPT LOAD for upstream %s: address not resolved yet',
+          s:get_name())
+    else
     local cur_opts = {
-      host = s:get_addr(),
+      host = server_addr,
       timeout = script.redis_params['timeout'],
       cmd = 'SCRIPT',
       args = { 'LOAD', script.script },
@@ -1447,6 +1455,7 @@ local function prepare_redis_call(script)
     end
 
     table.insert(options, cur_opts)
+    end
   end
 
   return options
index 2d5123c5c76ebb17fc418b6052e25830738ba5d1..d790086e4d2278230aafd6ae752c8445390911f4 100644 (file)
@@ -260,8 +260,14 @@ end
 
 local function connect_to_upstream(up, redis_params)
   local rspamd_redis = require "rspamd_redis"
+  local up_addr = up:get_addr()
+  if not up_addr then
+    rspamd_logger.errx("cannot connect to redis %s: address not resolved yet",
+        up:get_name())
+    return false, nil
+  end
   local ret, conn = rspamd_redis.connect_sync({
-    host = up:get_addr(),
+    host = up_addr,
     timeout = redis_params.timeout,
     config = rspamd_config,
     ev_base = rspamadm_ev_base,
index 3b2a62f0798d83fae6597f80dc70f75477462af3..a341407c29140af13daf328f7d9dd81a4aa0be1d 100644 (file)
@@ -1406,8 +1406,8 @@ rspamd_upstream_dtor(struct upstream *up)
 rspamd_inet_addr_t *
 rspamd_upstream_addr_next(struct upstream *up)
 {
-       unsigned int idx = up->addrs.cur, next_idx = up->addrs.cur, cur_af,
-                                min_errors, min_errors_idx;
+       unsigned int idx, next_idx, cur_af,
+               min_errors, min_errors_idx;
        struct upstream_addr_elt *e1, *e2;
 
        /*
@@ -1418,6 +1418,14 @@ rspamd_upstream_addr_next(struct upstream *up)
         * 4) If we cannot find such element, then we return the next element (switching AF)
         */
 
+       if (up == NULL || up->addrs.addr == NULL ||
+               up->addrs.addr->len == 0) {
+               /* Pending DNS resolution or never had any addresses */
+               return NULL;
+       }
+
+       idx = up->addrs.cur;
+       next_idx = up->addrs.cur;
        e1 = g_ptr_array_index(up->addrs.addr, up->addrs.cur);
        cur_af = rspamd_inet_address_get_af(e1->addr);
        min_errors = e1->errors;
@@ -1467,6 +1475,11 @@ rspamd_upstream_addr_cur(const struct upstream *up)
 {
        struct upstream_addr_elt *elt;
 
+       if (up == NULL || up->addrs.addr == NULL ||
+               up->addrs.addr->len == 0) {
+               return NULL;
+       }
+
        elt = g_ptr_array_index(up->addrs.addr, up->addrs.cur);
 
        return elt->addr;
@@ -1482,6 +1495,12 @@ int rspamd_upstream_port(struct upstream *up)
 {
        struct upstream_addr_elt *elt;
 
+       if (up == NULL || up->addrs.addr == NULL ||
+               up->addrs.addr->len == 0) {
+               /* Pending or never resolved: fall back to the parsed port if known */
+               return up != NULL ? (int) up->deferred_port : -1;
+       }
+
        elt = g_ptr_array_index(up->addrs.addr, up->addrs.cur);
        return rspamd_inet_address_get_port(elt->addr);
 }
index 181ef93988592463859325015f08189b595f1035..6cab4e7e43e975d12d41a1c3112c662eba7b6303 100644 (file)
@@ -113,7 +113,14 @@ lua_upstream_get_addr(lua_State *L)
        struct rspamd_lua_upstream *up = lua_check_upstream(L, 1);
 
        if (up) {
-               rspamd_lua_ip_push(L, rspamd_upstream_addr_next(up->up));
+               rspamd_inet_addr_t *addr = rspamd_upstream_addr_next(up->up);
+               if (addr) {
+                       rspamd_lua_ip_push(L, addr);
+               }
+               else {
+                       /* Upstream has no addresses yet (pending DNS resolution) */
+                       lua_pushnil(L);
+               }
        }
        else {
                lua_pushnil(L);
index c5fac01519e1436642dd0fded79f3a55c933ec24..a0075b76191eb1e574d156f8e21fb6a0d6d5c4b0 100644 (file)
@@ -1594,6 +1594,12 @@ local function check_rspamd_table(upstream, ev_base, cfg)
 end
 
 local function check_clickhouse_upstream(upstream, ev_base, cfg)
+  if not upstream:get_addr() then
+    rspamd_logger.infox(rspamd_config,
+        'skipping clickhouse upstream %s: address not resolved yet',
+        upstream:get_name())
+    return
+  end
   local ch_params = {
     ev_base = ev_base,
     config = cfg,