]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Minor] Address review comments
authorVsevolod Stakhov <vsevolod@rspamd.com>
Tue, 7 Oct 2025 20:24:16 +0000 (21:24 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Tue, 7 Oct 2025 20:24:16 +0000 (21:24 +0100)
src/fuzzy_storage.c
src/plugins/fuzzy_check.c

index 6077593d0d39541edf48a68082dfb187551d55e8..da2d920911cdb8120c5998f6a900b3f53360c9cd 100644 (file)
@@ -2322,7 +2322,11 @@ fuzzy_session_destroy(gpointer d)
 
        rspamd_inet_address_free(session->addr);
        rspamd_explicit_memzero(session->nm, sizeof(session->nm));
-       session->worker->nconns--;
+
+       /* Only decrement nconns if this is not a TCP command session */
+       if (session->tcp_session == NULL) {
+               session->worker->nconns--;
+       }
 
        if (session->ip_stat) {
                REF_RELEASE(session->ip_stat);
@@ -2702,7 +2706,7 @@ rspamd_fuzzy_tcp_io(EV_P_ ev_io *w, int revents)
                                if (cmd_session->ip_stat) {
                                        REF_RETAIN(cmd_session->ip_stat);
                                }
-                               session->common.worker->nconns++;
+                               /* Don't increment nconns here - TCP session already counted the connection */
 
                                /* Set TCP session pointer so replies go to TCP queue */
                                cmd_session->tcp_session = session;
@@ -2715,9 +2719,12 @@ rspamd_fuzzy_tcp_io(EV_P_ ev_io *w, int revents)
                                        session->common.cmd_type = cmd_session->cmd_type;
                                        memcpy(&session->common.cmd, &cmd_session->cmd, sizeof(session->common.cmd));
 
-                                       /* Note: key and extensions ownership transferred to cmd_session */
+                                       /* Transfer ownership of key and extensions to TCP session */
+                                       /* Clear cmd_session pointers to avoid double-free */
                                        session->common.key = cmd_session->key;
                                        session->common.extensions = cmd_session->extensions;
+                                       cmd_session->key = NULL;
+                                       cmd_session->extensions = NULL;
                                        memcpy(session->common.nm, cmd_session->nm, sizeof(session->common.nm));
 
                                        /* Process command - replies will go to TCP queue via tcp_session pointer */
index c3d7defe0bdddb8425075e688239e14e895c5d72..ff9fc6dfbe12ba3c0eba3c3eef605478244ec815 100644 (file)
@@ -602,8 +602,6 @@ fuzzy_tcp_connect_async(struct fuzzy_rule *rule,
                return NULL;
        }
 
-       rspamd_inet_address_set_port(addr, rspamd_upstream_port(upstream));
-
        /* Create non-blocking TCP socket */
        fd = rspamd_inet_address_connect(addr, SOCK_STREAM, TRUE);
        if (fd == -1) {
@@ -1080,8 +1078,8 @@ fuzzy_tcp_send_command(struct fuzzy_tcp_connection *conn,
                buf->data = g_malloc(io->io.iov_len);
                memcpy(buf->data, io->io.iov_base, io->io.iov_len);
 
-               /* Set frame size in network byte order (big endian) */
-               buf->size_hdr = GUINT16_TO_BE((uint16_t) io->io.iov_len);
+               /* Set frame size in little endian byte order */
+               buf->size_hdr = GUINT16_TO_LE((uint16_t) io->io.iov_len);
                buf->total_len = sizeof(buf->size_hdr) + io->io.iov_len;
                buf->bytes_written = 0;
 
@@ -1351,7 +1349,8 @@ fuzzy_tcp_read_handler(struct fuzzy_tcp_connection *conn)
                        if (processed_offset < conn->bytes_unprocessed) {
                                uint16_t first_byte = conn->cur_frame_state & 0xFF;
                                uint16_t second_byte = conn->read_buf[processed_offset];
-                               conn->cur_frame_state = 0xC000 | ((first_byte << 8) | second_byte);
+                               /* Reconstruct in little-endian byte order */
+                               conn->cur_frame_state = 0xC000 | (first_byte | (second_byte << 8));
                                processed_offset++;
                        }
                        else {
@@ -1543,71 +1542,58 @@ fuzzy_parse_rule(struct rspamd_config *cfg, const ucl_object_t *obj,
        if ((value = ucl_object_lookup(obj, "no_subject")) != NULL) {
                rule->no_subject = ucl_obj_toboolean(value);
        }
-
        /* TCP configuration */
        if ((value = ucl_object_lookup(obj, "tcp")) != NULL) {
-               const char *tcp_mode = NULL;
-
-               msg_debug_config("rule %s: found 'tcp' option, type=%d",
-                                                name ? name : "default", ucl_object_type(value));
+               const ucl_object_t *tcp_obj;
 
                if (ucl_object_type(value) == UCL_BOOLEAN) {
-                       if (ucl_object_toboolean(value)) {
-                               rule->tcp_enabled = TRUE;
-                               rule->tcp_auto = FALSE;
-                       }
+                       /* Simple boolean: enable/disable TCP */
+                       rule->tcp_enabled = ucl_object_toboolean(value);
+                       rule->tcp_auto = FALSE;
                }
-               else if (ucl_object_type(value) == UCL_STRING) {
-                       tcp_mode = ucl_object_tostring(value);
+               else if (ucl_object_type(value) == UCL_OBJECT) {
+                       /* Object with detailed TCP configuration */
 
-                       if (g_ascii_strcasecmp(tcp_mode, "yes") == 0 ||
-                               g_ascii_strcasecmp(tcp_mode, "true") == 0) {
-                               rule->tcp_enabled = TRUE;
-                               rule->tcp_auto = FALSE;
+                       if ((tcp_obj = ucl_object_lookup(value, "enabled")) != NULL) {
+                               rule->tcp_enabled = ucl_object_toboolean(tcp_obj);
                        }
-                       else if (g_ascii_strcasecmp(tcp_mode, "auto") == 0) {
-                               rule->tcp_auto = TRUE;
-                               rule->tcp_enabled = FALSE;
+
+                       if ((tcp_obj = ucl_object_lookup(value, "auto")) != NULL) {
+                               rule->tcp_auto = ucl_object_toboolean(tcp_obj);
                        }
-                       else if (g_ascii_strcasecmp(tcp_mode, "no") == 0 ||
-                                        g_ascii_strcasecmp(tcp_mode, "false") == 0) {
-                               rule->tcp_enabled = FALSE;
-                               rule->tcp_auto = FALSE;
+
+                       if ((tcp_obj = ucl_object_lookup(value, "threshold")) != NULL) {
+                               rule->tcp_threshold = ucl_object_todouble(tcp_obj);
                        }
                        else {
-                               msg_warn_config("unknown tcp mode: %s, TCP disabled", tcp_mode);
+                               rule->tcp_threshold = 1.0; /* Default: >1 req/sec */
                        }
-               }
-       }
-       else {
-               msg_debug_config("rule %s: 'tcp' option not found in configuration",
-                                                name ? name : "default");
-       }
 
-       if ((value = ucl_object_lookup(obj, "tcp_threshold")) != NULL) {
-               msg_debug_config("rule %s: found tcp_threshold=%.2f",
-                                                name ? name : "default", ucl_obj_todouble(value));
-               rule->tcp_threshold = ucl_obj_todouble(value);
-       }
-       else {
-               rule->tcp_threshold = 1.0; /* Default: 1 req/sec */
-       }
+                       if ((tcp_obj = ucl_object_lookup(value, "window")) != NULL) {
+                               rule->tcp_window = ucl_object_todouble(tcp_obj);
+                       }
+                       else {
+                               rule->tcp_window = 1.0; /* Default: 1 second window */
+                       }
 
-       if ((value = ucl_object_lookup(obj, "tcp_window")) != NULL) {
-               rule->tcp_window = ucl_obj_todouble(value);
+                       if ((tcp_obj = ucl_object_lookup(value, "timeout")) != NULL) {
+                               rule->tcp_timeout = ucl_object_todouble(tcp_obj);
+                       }
+                       else {
+                               rule->tcp_timeout = 5.0; /* Default: 5 seconds */
+                       }
+               }
        }
        else {
-               rule->tcp_window = 1.0; /* Default: 1 second window */
+               /* Default values if no TCP configuration */
+               rule->tcp_enabled = FALSE;
+               rule->tcp_auto = FALSE;
+               rule->tcp_threshold = 1.0;
+               rule->tcp_window = 1.0;
+               rule->tcp_timeout = 5.0;
        }
 
-       if ((value = ucl_object_lookup(obj, "tcp_timeout")) != NULL) {
-               rule->tcp_timeout = ucl_obj_todouble(value);
-       }
-       else {
-               rule->tcp_timeout = 5.0; /* Default: 5 seconds */
-       }
 
-       /* Log TCP configuration */
        if (rule->tcp_enabled) {
                msg_info_config("rule %s: TCP explicitly enabled (timeout=%.1fs)",
                                                rule->name ? rule->name : "default", rule->tcp_timeout);
@@ -1893,57 +1879,6 @@ fuzzy_parse_rule(struct rspamd_config *cfg, const ucl_object_t *obj,
                rule->weight_threshold = ucl_object_todouble(value);
        }
 
-       /* TCP configuration */
-       if ((value = ucl_object_lookup(obj, "tcp")) != NULL) {
-               const ucl_object_t *tcp_obj;
-
-               if (ucl_object_type(value) == UCL_BOOLEAN) {
-                       /* Simple boolean: enable/disable TCP */
-                       rule->tcp_enabled = ucl_object_toboolean(value);
-                       rule->tcp_auto = FALSE;
-               }
-               else if (ucl_object_type(value) == UCL_OBJECT) {
-                       /* Object with detailed TCP configuration */
-
-                       if ((tcp_obj = ucl_object_lookup(value, "enabled")) != NULL) {
-                               rule->tcp_enabled = ucl_object_toboolean(tcp_obj);
-                       }
-
-                       if ((tcp_obj = ucl_object_lookup(value, "auto")) != NULL) {
-                               rule->tcp_auto = ucl_object_toboolean(tcp_obj);
-                       }
-
-                       if ((tcp_obj = ucl_object_lookup(value, "threshold")) != NULL) {
-                               rule->tcp_threshold = ucl_object_todouble(tcp_obj);
-                       }
-                       else {
-                               rule->tcp_threshold = 1.0; /* Default: >1 req/sec */
-                       }
-
-                       if ((tcp_obj = ucl_object_lookup(value, "window")) != NULL) {
-                               rule->tcp_window = ucl_object_todouble(tcp_obj);
-                       }
-                       else {
-                               rule->tcp_window = 1.0; /* Default: 1 second window */
-                       }
-
-                       if ((tcp_obj = ucl_object_lookup(value, "timeout")) != NULL) {
-                               rule->tcp_timeout = ucl_object_todouble(tcp_obj);
-                       }
-                       else {
-                               rule->tcp_timeout = 5.0; /* Default: 5 seconds */
-                       }
-               }
-       }
-       else {
-               /* Default values if no TCP configuration */
-               rule->tcp_enabled = FALSE;
-               rule->tcp_auto = FALSE;
-               rule->tcp_threshold = 1.0;
-               rule->tcp_window = 1.0;
-               rule->tcp_timeout = 5.0;
-       }
-
        /* Initialize rate tracker */
        rule->rate_tracker.requests_count = 0;
        rule->rate_tracker.window_start = 0;
@@ -5057,7 +4992,6 @@ register_fuzzy_client_call(struct rspamd_task *task,
                        msg_info_task("fuzzy_check: sending %d commands via UDP to %s",
                                                  (int) commands->len, rspamd_upstream_name(selected));
                        addr = rspamd_upstream_addr_cur(selected);
-                       rspamd_inet_address_set_port(addr, rspamd_upstream_port(selected));
 
                        if ((sock = rspamd_inet_address_connect(addr, SOCK_DGRAM, TRUE)) == -1) {
                                msg_warn_task("cannot connect to %s(%s), %d, %s",