From: Vsevolod Stakhov Date: Sat, 25 Apr 2026 19:10:30 +0000 (+0100) Subject: [Fix] upstream: make addr accessors and all_upstreams pending-safe X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a7e47e607943f366cd1be714e678522f3400a95;p=thirdparty%2Frspamd.git [Fix] upstream: make addr accessors and all_upstreams pending-safe 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. --- diff --git a/lualib/lua_redis.lua b/lualib/lua_redis.lua index 4e4d192bc4..e6465fa17d 100644 --- a/lualib/lua_redis.lua +++ b/lualib/lua_redis.lua @@ -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 diff --git a/lualib/rspamadm/statistics_dump.lua b/lualib/rspamadm/statistics_dump.lua index 2d5123c5c7..d790086e4d 100644 --- a/lualib/rspamadm/statistics_dump.lua +++ b/lualib/rspamadm/statistics_dump.lua @@ -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, diff --git a/src/libutil/upstream.c b/src/libutil/upstream.c index 3b2a62f079..a341407c29 100644 --- a/src/libutil/upstream.c +++ b/src/libutil/upstream.c @@ -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); } diff --git a/src/lua/lua_upstream.c b/src/lua/lua_upstream.c index 181ef93988..6cab4e7e43 100644 --- a/src/lua/lua_upstream.c +++ b/src/lua/lua_upstream.c @@ -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); diff --git a/src/plugins/lua/clickhouse.lua b/src/plugins/lua/clickhouse.lua index c5fac01519..a0075b7619 100644 --- a/src/plugins/lua/clickhouse.lua +++ b/src/plugins/lua/clickhouse.lua @@ -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,