]> git.ipfire.org Git - thirdparty/rspamd.git/commit
[Fix] url_redirector: cache write missing on splice, brittle mid-walk (#6017)
authorDmitriy Alekseev <1865999+dragoangel@users.noreply.github.com>
Fri, 8 May 2026 07:22:02 +0000 (09:22 +0200)
committerGitHub <noreply@github.com>
Fri, 8 May 2026 07:22:02 +0000 (08:22 +0100)
commitcb511ba2daa880d4c5c7c2ec7e4ea8d0173610b2
tree6c4b862d616f449627d3fb747546e0e2991e602f
parent31f324d13fc38edc938c6c2ab60ec13fec3eed2b
[Fix] url_redirector: cache write missing on splice, brittle mid-walk (#6017)

* [Fix] url_redirector: cache write missing on splice, brittle mid-walk handling, silent stale-lock drops

- Persist live-resolved chain when http_walk splices into step (terminal path now calls finalize_chain instead of apply_redirect_chain only). Previously the new link from orig_url to a cached redir_url was never written, leaving 'processing' marker at hash(orig_url) until 13s expiry
- Resume via http_walk on true cache miss mid-walk instead of giving up with a truncated chain (covers expired/evicted downstream links in cached chains)
- Differentiate cache miss from 'processing' lock mid-walk: lock means another worker is resolving (apply partial), miss means cache gone (extend via HTTP)
- Surface redis errors during chain walk via dedicated rspamd_logger.errx
- Add debug log when SET NX lock claim fails (held by another worker or stale processing marker after crash); previously it was a silent drop
- Add debug log 'no URLs matched redirector_hosts_map' at handler exit when message had URLs but selected=0, exposing cold-start window where redirector_hosts_map multimap has not finished loading

Signed-off-by: Dmitriy Alekseev <1865999+dragoangel@users.noreply.github.com>
* [Fix] url_redirector: address review feedback on PR #6017

- Hoist the per-call finish() closure out of step() into a free
  step_finish() helper -- no closure allocation per cache hop.
- Capture tostring(last) once as last_str to avoid re-running the URL
  __tostring slow path on each error/debug branch in step()'s redis cb.
- Drop redundant tostring(ndata) in redis_reserve_cb debug log
  (rspamd_logger %s handles tostring internally).
- Replace task:get_urls() or {} with task:has_urls() in the no-match
  debug branch -- returns (bool, count) without materialising the URL
  table, so production traffic doesn't pay for an allocation just to
  feed a debug-only log line.
- Move task:has_urls() to the top of url_redirector_handler -- when
  the message has no URLs at all, return early and skip the CTA scan
  and extract_specific_urls call entirely; reuse the same n_urls in
  the no-match debug branch.

Signed-off-by: Dmitriy Alekseev <1865999+dragoangel@users.noreply.github.com>
* [Fix] url_redirector: clear bridged URL from seen before handing off to http_walk

step() marks seen[val]=true after appending each cached hop. When the
cache-miss mid-walk branch then bridges to http_walk on the same hop,
http_walk re-marks via seen[tostring(url)] and -- since cache writer
stores tostring(url), making val and tostring(rspamd_url.create(val))
round-trip-stable -- collides with step's mark. The cycle guard
false-fires on the bridged URL, truncating the chain and skipping the
live extension. Clear seen[last_str] before the http_walk bridge so
its own marking is the first one for that URL.

* [Chore] url_redirector: remove unneeded guards

Signed-off-by: Dmitriy Alekseev <1865999+dragoangel@users.noreply.github.com>
* [Fix] url_redirector: finalize on http_walk cycle to release the processing lock

The cycle branch called apply_redirect_chain, which only updates the task
(set_redirected, inject_url, insert_result) and never touches Redis.

Signed-off-by: Dmitriy Alekseev <1865999+dragoangel@users.noreply.github.com>
---------

Signed-off-by: Dmitriy Alekseev <1865999+dragoangel@users.noreply.github.com>
src/plugins/lua/url_redirector.lua