[Fix] Core: Fix address rotation bug

Previously, upstream.get_addr function returned the new address of the
upstream. Unfortunately, it was used for printing addresses. It caused
the following situation: let's imagine we have A1 and A2 where A1 was
initially selected. So the connection was performed to A1:

                           Current addr   Selected addr

   Connect+---------+      A2+------>A1   A1
                    |
+-+Print failure<---+      A1+------>A2   A2
|                        +----+
+->Mark failure+-------->+ A2 |
                         +----+

But the failure OP as well as log message told about `A2` where the real
problem happened with `A1`.

This commit adds distinguishing between getting the next and the current
address of the upstream resolving this issue.

diff --git a/src/fuzzy_storage.c b/src/fuzzy_storage.c
index bbd6b36ca4c2a1e68fb219e9e47bf23b1cdca584..d0a4f3602fe507d257539f03393e59cd4a1fd03b 100644 (file)
--- a/src/fuzzy_storage.c
+++ b/src/fuzzy_storage.c
@@ -568,7 +568,7 @@ fuzzy_mirror_error_handler (struct rspamd_http_connection *conn, GError *err)
        msg_info ("abnormally closing connection from backend: %s:%s, "
                        "error: %e",
                        bk_conn->mirror->name,
-                       rspamd_inet_address_to_string (rspamd_upstream_addr (bk_conn->up)),
+                       rspamd_inet_address_to_string (rspamd_upstream_addr_cur (bk_conn->up)),
                        err);
 
        fuzzy_mirror_close_connection (bk_conn);
@@ -604,7 +604,7 @@ rspamd_fuzzy_send_update_mirror (struct rspamd_fuzzy_storage_ctx *ctx,
        }
 
        conn->sock = rspamd_inet_address_connect (
-                       rspamd_upstream_addr (conn->up),
+                       rspamd_upstream_addr_next (conn->up),
                        SOCK_STREAM, TRUE);
 
        if (conn->sock == -1) {

diff --git a/src/libserver/dns.c b/src/libserver/dns.c
index 7ad266c2f76e7133dfe565aa84457ce86489f4ba..4fbf40728ecb0016e0ce31772fdb7bb2f36906cc 100644 (file)
--- a/src/libserver/dns.c
+++ b/src/libserver/dns.c
@@ -256,7 +256,7 @@ rspamd_dns_server_init (struct upstream *up, guint idx, gpointer ud)
        void *serv;
        struct rdns_upstream_elt *elt;
 
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
 
        if (r->cfg) {
                serv = rdns_resolver_add_server (r->r, rspamd_inet_address_to_string (addr),

diff --git a/src/libserver/fuzzy_backend_redis.c b/src/libserver/fuzzy_backend_redis.c
index fbccac5ab7a3052964cd49cc256797fd2bc110a5..956979d427feba741e3f6aa8dba25c1d4b638d66 100644 (file)
--- a/src/libserver/fuzzy_backend_redis.c
+++ b/src/libserver/fuzzy_backend_redis.c
@@ -648,7 +648,7 @@ rspamd_fuzzy_backend_check_redis (struct rspamd_fuzzy_backend *bk,
                        0);
 
        session->up = up;
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        g_assert (addr != NULL);
        session->ctx = rspamd_redis_pool_connect (backend->pool,
                        backend->dbname, backend->password,
@@ -774,7 +774,7 @@ rspamd_fuzzy_backend_count_redis (struct rspamd_fuzzy_backend *bk,
                        0);
 
        session->up = up;
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        g_assert (addr != NULL);
        session->ctx = rspamd_redis_pool_connect (backend->pool,
                        backend->dbname, backend->password,
@@ -899,7 +899,7 @@ rspamd_fuzzy_backend_version_redis (struct rspamd_fuzzy_backend *bk,
                        0);
 
        session->up = up;
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        g_assert (addr != NULL);
        session->ctx = rspamd_redis_pool_connect (backend->pool,
                        backend->dbname, backend->password,
@@ -1459,7 +1459,7 @@ rspamd_fuzzy_backend_update_redis (struct rspamd_fuzzy_backend *bk,
                        0);
 
        session->up = up;
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        g_assert (addr != NULL);
        session->ctx = rspamd_redis_pool_connect (backend->pool,
                        backend->dbname, backend->password,

diff --git a/src/libstat/backends/redis_backend.c b/src/libstat/backends/redis_backend.c
index d7402db983296676f0ef4d2a9811427f22d19430..d54767c129f4a166bb4191148384a42e39e9c1c1 100644 (file)
--- a/src/libstat/backends/redis_backend.c
+++ b/src/libstat/backends/redis_backend.c
@@ -975,7 +975,7 @@ rspamd_redis_async_stat_cb (struct rspamd_stat_async_elt *elt, gpointer d)
                                        0);
 
        g_assert (cbdata->selected != NULL);
-       addr = rspamd_upstream_addr (cbdata->selected);
+       addr = rspamd_upstream_addr_next (cbdata->selected);
        g_assert (addr != NULL);
 
        if (rspamd_inet_address_get_af (addr) == AF_UNIX) {
@@ -1522,7 +1522,7 @@ rspamd_redis_runtime (struct rspamd_task *task,
        rt->stcf = stcf;
        rt->redis_object_expanded = object_expanded;
 
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        g_assert (addr != NULL);
 
        if (rspamd_inet_address_get_af (addr) == AF_UNIX) {
@@ -1693,7 +1693,7 @@ rspamd_redis_learn_tokens (struct rspamd_task *task, GPtrArray *tokens,
                }
        }
 
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        g_assert (addr != NULL);
 
        if (rspamd_inet_address_get_af (addr) == AF_UNIX) {

diff --git a/src/libstat/learn_cache/redis_cache.c b/src/libstat/learn_cache/redis_cache.c
index 6a0aa1da77acdd40f1c862dbf6dab7b4902662ef..aea024e06d6cddcb52109493a73295ccf8630ff3 100644 (file)
--- a/src/libstat/learn_cache/redis_cache.c
+++ b/src/libstat/learn_cache/redis_cache.c
@@ -385,7 +385,7 @@ rspamd_stat_cache_redis_runtime (struct rspamd_task *task,
        rt->task = task;
        rt->ctx = ctx;
 
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        g_assert (addr != NULL);
 
        if (rspamd_inet_address_get_af (addr) == AF_UNIX) {

diff --git a/src/libutil/upstream.c b/src/libutil/upstream.c
index eb88e501a1ca1deb15fea66db138c3936a340d59..938c49011d46f95092d101917350b434ab2a9d78 100644 (file)
--- a/src/libutil/upstream.c
+++ b/src/libutil/upstream.c
@@ -630,7 +630,7 @@ rspamd_upstream_dtor (struct upstream *up)
 }
 
 rspamd_inet_addr_t*
-rspamd_upstream_addr (struct upstream *up)
+rspamd_upstream_addr_next (struct upstream *up)
 {
        guint idx, next_idx;
        struct upstream_addr_elt *e1, *e2;
@@ -646,6 +646,12 @@ rspamd_upstream_addr (struct upstream *up)
        return e2->addr;
 }
 
+rspamd_inet_addr_t*
+rspamd_upstream_addr_cur (const struct upstream *up)
+{
+       return g_ptr_array_index (up->addrs.addr, up->addrs.cur);
+}
+
 const gchar*
 rspamd_upstream_name (struct upstream *up)
 {

diff --git a/src/libutil/upstream.h b/src/libutil/upstream.h
index 5c0c92afc1dc6cfe1e8c5fa3b6b42426c4ac928d..4db9627656d72c5257be1ded77bb895d1c01bd51 100644 (file)
--- a/src/libutil/upstream.h
+++ b/src/libutil/upstream.h
@@ -207,12 +207,19 @@ void rspamd_upstreams_add_watch_callback (struct upstream_list *ups,
                                                                                  GFreeFunc free_func,
                                                                                  gpointer ud);
 
+/**
+ * Returns the next IP address of the upstream (internal rotation)
+ * @param up
+ * @return
+ */
+rspamd_inet_addr_t* rspamd_upstream_addr_next (struct upstream *up);
+
 /**
  * Returns the current IP address of the upstream
  * @param up
  * @return
  */
-rspamd_inet_addr_t* rspamd_upstream_addr (struct upstream *up);
+rspamd_inet_addr_t* rspamd_upstream_addr_cur (const struct upstream *up);
 
 /**
  * Add custom address for an upstream (ownership of addr is transferred to upstream)

diff --git a/src/lua/lua_upstream.c b/src/lua/lua_upstream.c
index 1a4d6b12899b1650abb43803da7aeb54d5d1284d..37c541d9dcb673c899ae620c267d53f9af3fb1b2 100644 (file)
--- a/src/lua/lua_upstream.c
+++ b/src/lua/lua_upstream.c
@@ -110,7 +110,7 @@ lua_upstream_get_addr (lua_State *L)
        struct upstream *up = lua_check_upstream (L);
 
        if (up) {
-               rspamd_lua_ip_push (L, rspamd_upstream_addr (up));
+               rspamd_lua_ip_push (L, rspamd_upstream_addr_next (up));
        }
        else {
                lua_pushnil (L);

diff --git a/src/plugins/fuzzy_check.c b/src/plugins/fuzzy_check.c
index 760429ba21a812f953aefef1abf3184d35a33336..58cdd3376ee377da8a656fe72b9c5016db220107 100644 (file)
--- a/src/plugins/fuzzy_check.c
+++ b/src/plugins/fuzzy_check.c
@@ -2209,7 +2209,7 @@ fuzzy_check_io_callback (gint fd, short what, void *arg)
                msg_err_task ("got error on IO with server %s(%s), on %s, %d, %s",
                        rspamd_upstream_name (session->server),
                                rspamd_inet_address_to_string_pretty (
-                                               rspamd_upstream_addr (session->server)),
+                                               rspamd_upstream_addr_cur (session->server)),
                        session->state == 1 ? "read" : "write",
                        errno,
                        strerror (errno));
@@ -2255,7 +2255,7 @@ fuzzy_check_timer_callback (gint fd, short what, void *arg)
                msg_err_task ("got IO timeout with server %s(%s), after %d retransmits",
                                rspamd_upstream_name (session->server),
                                rspamd_inet_address_to_string_pretty (
-                                               rspamd_upstream_addr (session->server)),
+                                               rspamd_upstream_addr_cur (session->server)),
                                session->retransmits);
                rspamd_upstream_fail (session->server, FALSE);
                if (session->item) {
@@ -2464,7 +2464,7 @@ fuzzy_controller_io_callback (gint fd, short what, void *arg)
                msg_err_task ("got error in IO with server %s(%s), %d, %s",
                                rspamd_upstream_name (session->server),
                                rspamd_inet_address_to_string_pretty (
-                                               rspamd_upstream_addr (session->server)),
+                                               rspamd_upstream_addr_cur (session->server)),
                                errno, strerror (errno));
                rspamd_upstream_fail (session->server, FALSE);
        }
@@ -2568,7 +2568,7 @@ fuzzy_controller_timer_callback (gint fd, short what, void *arg)
                                "after %d retransmits",
                                rspamd_upstream_name (session->server),
                                rspamd_inet_address_to_string_pretty (
-                                               rspamd_upstream_addr (session->server)),
+                                               rspamd_upstream_addr_cur (session->server)),
                                session->retransmits);
 
                if (session->session) {
@@ -2725,7 +2725,7 @@ register_fuzzy_client_call (struct rspamd_task *task,
                selected = rspamd_upstream_get (rule->servers, RSPAMD_UPSTREAM_ROUND_ROBIN,
                                NULL, 0);
                if (selected) {
-                       addr = rspamd_upstream_addr (selected);
+                       addr = rspamd_upstream_addr_next (selected);
                        if ((sock = rspamd_inet_address_connect (addr, SOCK_DGRAM, TRUE)) == -1) {
                                msg_warn_task ("cannot connect to %s(%s), %d, %s",
                                                rspamd_upstream_name (selected),
@@ -2853,7 +2853,7 @@ register_fuzzy_controller_call (struct rspamd_http_connection_entry *entry,
        while ((selected = rspamd_upstream_get (rule->servers,
                        RSPAMD_UPSTREAM_SEQUENTIAL, NULL, 0))) {
                /* Create UDP socket */
-               addr = rspamd_upstream_addr (selected);
+               addr = rspamd_upstream_addr_next (selected);
 
                if ((sock = rspamd_inet_address_connect (addr,
                                SOCK_DGRAM, TRUE)) == -1) {
@@ -3216,7 +3216,7 @@ fuzzy_check_send_lua_learn (struct fuzzy_rule *rule,
                while ((selected = rspamd_upstream_get (rule->servers,
                                RSPAMD_UPSTREAM_SEQUENTIAL, NULL, 0))) {
                        /* Create UDP socket */
-                       addr = rspamd_upstream_addr (selected);
+                       addr = rspamd_upstream_addr_next (selected);
 
                        if ((sock = rspamd_inet_address_connect (addr,
                                        SOCK_DGRAM, TRUE)) == -1) {

diff --git a/src/plugins/surbl.c b/src/plugins/surbl.c
index 94d88334efcf1fde19454d64639e5a2ef54ae72b..5949f5bb6edf4ef550a646e321a3724fd05e8b39 100644 (file)
--- a/src/plugins/surbl.c
+++ b/src/plugins/surbl.c
@@ -1624,7 +1624,8 @@ surbl_redirector_error (struct rspamd_http_connection *conn,
 
        task = param->task;
        msg_err_surbl ("connection with http server %s terminated incorrectly: %e",
-               rspamd_inet_address_to_string (rspamd_upstream_addr (param->redirector)),
+               rspamd_inet_address_to_string (
+                               rspamd_upstream_addr_cur (param->redirector)),
                err);
        rspamd_upstream_fail (param->redirector, FALSE);
        rspamd_session_remove_event (param->task->s, free_redirector_session,
@@ -1715,7 +1716,7 @@ register_redirector_call (struct rspamd_url *url, struct rspamd_task *task,
                                RSPAMD_UPSTREAM_ROUND_ROBIN, url->host, url->hostlen);
 
                if (selected) {
-                       s = rspamd_inet_address_connect (rspamd_upstream_addr (selected),
+                       s = rspamd_inet_address_connect (rspamd_upstream_addr_next (selected),
                                        SOCK_STREAM, TRUE);
                }
 

diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c
index bb0028f0bfc1861f47403c03ba603f36993b23e3..31aeceb1229923a6c9eb660cb3c959d9ad259bf2 100644 (file)
--- a/src/rspamd_proxy.c
+++ b/src/rspamd_proxy.c
@@ -1309,7 +1309,8 @@ proxy_backend_mirror_error_handler (struct rspamd_http_connection *conn, GError
        msg_info_session ("abnormally closing connection from backend: %s:%s, "
                        "error: %e",
                        bk_conn->name,
-                       rspamd_inet_address_to_string (rspamd_upstream_addr (bk_conn->up)),
+                       rspamd_inet_address_to_string (
+                                       rspamd_upstream_addr_cur (bk_conn->up)),
                        err);
 
        if (err) {
@@ -1337,7 +1338,8 @@ proxy_backend_mirror_finish_handler (struct rspamd_http_connection *conn,
                        bk_conn->parser_from_ref, msg->body_buf.begin, msg->body_buf.len)) {
                msg_warn_session ("cannot parse results from the mirror backend %s:%s",
                                bk_conn->name,
-                               rspamd_inet_address_to_string (rspamd_upstream_addr (bk_conn->up)));
+                               rspamd_inet_address_to_string (
+                                               rspamd_upstream_addr_cur (bk_conn->up)));
                bk_conn->err = "cannot parse ucl";
        }
 
@@ -1387,7 +1389,7 @@ proxy_open_mirror_connections (struct rspamd_proxy_session *session)
                }
 
                bk_conn->backend_sock = rspamd_inet_address_connect (
-                               rspamd_upstream_addr (bk_conn->up),
+                               rspamd_upstream_addr_next (bk_conn->up),
                                SOCK_STREAM, TRUE);
 
                if (bk_conn->backend_sock == -1) {
@@ -1432,7 +1434,7 @@ proxy_open_mirror_connections (struct rspamd_proxy_session *session)
 
                if (m->local ||
                                rspamd_inet_address_is_local (
-                                               rspamd_upstream_addr (bk_conn->up), FALSE)) {
+                                               rspamd_upstream_addr_cur (bk_conn->up), FALSE)) {
 
                        if (session->fname) {
                                rspamd_http_message_add_header (msg, "File", session->fname);
@@ -1509,7 +1511,8 @@ proxy_backend_master_error_handler (struct rspamd_http_connection *conn, GError
        session = bk_conn->s;
        msg_info_session ("abnormally closing connection from backend: %s, error: %e,"
                        " retries left: %d",
-               rspamd_inet_address_to_string (rspamd_upstream_addr (session->master_conn->up)),
+               rspamd_inet_address_to_string (
+                               rspamd_upstream_addr_cur (session->master_conn->up)),
                err,
                session->ctx->max_retries - session->retries);
        session->retries ++;
@@ -1531,7 +1534,7 @@ proxy_backend_master_error_handler (struct rspamd_http_connection *conn, GError
                        msg_info_session ("retry connection to: %s"
                                        " retries left: %d",
                                        rspamd_inet_address_to_string (
-                                                       rspamd_upstream_addr (session->master_conn->up)),
+                                                       rspamd_upstream_addr_cur (session->master_conn->up)),
                                        session->ctx->max_retries - session->retries);
                }
        }
@@ -1821,14 +1824,15 @@ retry:
                }
 
                session->master_conn->backend_sock = rspamd_inet_address_connect (
-                               rspamd_upstream_addr (session->master_conn->up),
+                               rspamd_upstream_addr_next (session->master_conn->up),
                                SOCK_STREAM, TRUE);
 
                if (session->master_conn->backend_sock == -1) {
                        msg_err_session ("cannot connect upstream: %s(%s)",
                                        host ? hostbuf : "default",
-                                                       rspamd_inet_address_to_string (rspamd_upstream_addr (
-                                                                       session->master_conn->up)));
+                                                       rspamd_inet_address_to_string (
+                                                                       rspamd_upstream_addr_cur (
+                                                                                       session->master_conn->up)));
                        rspamd_upstream_fail (session->master_conn->up, TRUE);
                        session->retries ++;
                        goto retry;
@@ -1872,7 +1876,8 @@ retry:
 
                if (backend->local ||
                                rspamd_inet_address_is_local (
-                                               rspamd_upstream_addr (session->master_conn->up), FALSE)) {
+                                               rspamd_upstream_addr_cur (
+                                                               session->master_conn->up), FALSE)) {
 
                        if (session->fname) {
                                rspamd_http_message_add_header (msg, "File", session->fname);

diff --git a/test/rspamd_upstream_test.c b/test/rspamd_upstream_test.c
index 4e4f1ae87533155d859011506429f5ea2c61fefd..668651c8f6cfd8f080e508af1013fc6a4d11198e 100644 (file)
--- a/test/rspamd_upstream_test.c
+++ b/test/rspamd_upstream_test.c
@@ -87,27 +87,27 @@ rspamd_upstream_test_func (void)
        rspamd_parse_inet_address (&paddr, "::1", 0);
        g_assert (rspamd_upstream_add_addr (up, paddr));
        /* Rewind to start */
-       addr = rspamd_upstream_addr (up);
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
+       addr = rspamd_upstream_addr_next (up);
        /* cur should be zero here */
-       addr = rspamd_upstream_addr (up);
-       next_addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
+       next_addr = rspamd_upstream_addr_next (up);
        g_assert (rspamd_inet_address_get_af (addr) == AF_INET);
        g_assert (rspamd_inet_address_get_af (next_addr) == AF_INET);
-       next_addr = rspamd_upstream_addr (up);
+       next_addr = rspamd_upstream_addr_next (up);
        g_assert (rspamd_inet_address_get_af (next_addr) == AF_INET6);
-       next_addr = rspamd_upstream_addr (up);
+       next_addr = rspamd_upstream_addr_next (up);
        g_assert (rspamd_inet_address_get_af (next_addr) == AF_INET);
-       next_addr = rspamd_upstream_addr (up);
+       next_addr = rspamd_upstream_addr_next (up);
        g_assert (rspamd_inet_address_get_af (next_addr) == AF_INET);
-       next_addr = rspamd_upstream_addr (up);
+       next_addr = rspamd_upstream_addr_next (up);
        g_assert (rspamd_inet_address_get_af (next_addr) == AF_INET6);
        /* Test errors with IPv6 */
        rspamd_upstream_fail (up, TRUE);
        /* Now we should have merely IPv4 addresses in rotation */
-       addr = rspamd_upstream_addr (up);
+       addr = rspamd_upstream_addr_next (up);
        for (i = 0; i < 256; i++) {
-               next_addr = rspamd_upstream_addr (up);
+               next_addr = rspamd_upstream_addr_next (up);
                g_assert (rspamd_inet_address_get_af (addr) == AF_INET);
                g_assert (rspamd_inet_address_get_af (next_addr) == AF_INET);
                g_assert (rspamd_inet_address_compare (addr, next_addr) != 0);