From 80f7567fb425b59488893aa7a5cc59bf823d6670 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Fri, 6 Mar 2026 08:44:21 +0000 Subject: [PATCH] [Fix] Preserve duplicate URLs across MIME parts Do not suppress URLs from mime_part:get_urls() when the same URL was already seen in another MIME part. This restores per-part URL visibility for multipart/alternative messages and keeps text/plain URLs available even when text/html contains the same links. --- src/libserver/html/html.cxx | 25 +++++++++------- src/libserver/url.c | 21 +++++++++---- src/lua/lua_task.c | 12 ++++---- test/lua/unit/task.lua | 59 +++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 22 deletions(-) diff --git a/src/libserver/html/html.cxx b/src/libserver/html/html.cxx index a67d35bf72..62aeef3bb6 100644 --- a/src/libserver/html/html.cxx +++ b/src/libserver/html/html.cxx @@ -1400,6 +1400,14 @@ struct rspamd_html_url_query_cbd { uint32_t parent_flags; /* Flags from outer URL to propagate */ }; +static inline auto +html_part_add_url(GPtrArray *part_urls, struct rspamd_url *url) -> void +{ + if (part_urls) { + g_ptr_array_add(part_urls, url); + } +} + static gboolean html_url_query_callback(struct rspamd_url *url, gsize start_offset, gsize end_offset, gpointer ud) @@ -1426,8 +1434,8 @@ html_url_query_callback(struct rspamd_url *url, gsize start_offset, /* Propagate source/classification flags from the parent (outer) URL */ url->flags |= (cbd->parent_flags & RSPAMD_URL_FLAG_PROPAGATE_MASK); - if (rspamd_url_set_add_or_increase(cbd->url_set, url, false) && cbd->part_urls) { - g_ptr_array_add(cbd->part_urls, url); + if (rspamd_url_set_add_or_increase(cbd->url_set, url, false)) { + html_part_add_url(cbd->part_urls, url); } return TRUE; @@ -1454,9 +1462,7 @@ html_process_query_url(rspamd_mempool_t *pool, struct rspamd_url *url, html_url_query_callback, &qcbd, L); } - if (part_urls) { - g_ptr_array_add(part_urls, url); - } + html_part_add_url(part_urls, url); } static auto @@ -1576,9 +1582,8 @@ html_process_img_tag(rspamd_mempool_t *pool, existing->flags |= img->url->flags; existing->count++; } - else if (part_urls) { - /* New url */ - g_ptr_array_add(part_urls, img->url); + else { + html_part_add_url(part_urls, img->url); } } } @@ -2378,9 +2383,7 @@ auto html_process_input(struct rspamd_task *task, url->count++; } } - if (part_urls) { - g_ptr_array_add(part_urls, url); - } + html_part_add_url(part_urls, url); /* Minimal link features collection */ hc->features.links.total_links++; diff --git a/src/libserver/url.c b/src/libserver/url.c index 5034bd5e56..4c2871070b 100644 --- a/src/libserver/url.c +++ b/src/libserver/url.c @@ -3624,6 +3624,14 @@ struct rspamd_url_mimepart_cbdata { uint32_t parent_flags; /* Flags from outer URL to propagate to query URLs */ }; +static inline void +rspamd_mime_part_add_url(struct rspamd_mime_part *part, struct rspamd_url *url) +{ + if (part && part->urls) { + g_ptr_array_add(part->urls, url); + } +} + static gboolean rspamd_url_query_callback(struct rspamd_url *url, gsize start_offset, gsize end_offset, gpointer ud) @@ -3655,10 +3663,9 @@ rspamd_url_query_callback(struct rspamd_url *url, gsize start_offset, /* Propagate source/classification flags from the parent (outer) URL */ url->flags |= (cbd->parent_flags & RSPAMD_URL_FLAG_PROPAGATE_MASK); + rspamd_mime_part_add_url(cbd->part ? cbd->part->mime_part : NULL, url); + if (rspamd_url_set_add_or_increase(MESSAGE_FIELD(task, urls), url, false)) { - if (cbd->part && cbd->part->mime_part->urls) { - g_ptr_array_add(cbd->part->mime_part->urls, url); - } url->part_order = cbd->cur_part_order++; @@ -3727,14 +3734,16 @@ rspamd_url_text_part_callback(struct rspamd_url *url, gsize start_offset, url->flags |= RSPAMD_URL_FLAG_FROM_TEXT; } - if (rspamd_url_set_add_or_increase(MESSAGE_FIELD(task, urls), url, false) && - cbd->part->mime_part->urls) { + rspamd_mime_part_add_url(cbd->part ? cbd->part->mime_part : NULL, url); + + if (rspamd_url_set_add_or_increase(MESSAGE_FIELD(task, urls), url, false)) { + rspamd_mime_part_add_url(cbd->part ? cbd->part->mime_part : NULL, url); + url->part_order = cbd->cur_part_order++; if (cbd->cur_url_order) { url->order = (*cbd->cur_url_order)++; } - g_ptr_array_add(cbd->part->mime_part->urls, url); } cbd->part->exceptions = g_list_prepend( diff --git a/src/lua/lua_task.c b/src/lua/lua_task.c index 239bd896e7..441618b545 100644 --- a/src/lua/lua_task.c +++ b/src/lua/lua_task.c @@ -2953,10 +2953,12 @@ inject_url_query_callback(struct rspamd_url *url, gsize start_offset, url->flags |= RSPAMD_URL_FLAG_QUERY; - if (rspamd_url_set_add_or_increase(MESSAGE_FIELD(task, urls), url, false) && cbd->mpart_urls) { + if (cbd->mpart_urls) { g_ptr_array_add(cbd->mpart_urls, url); } + rspamd_url_set_add_or_increase(MESSAGE_FIELD(task, urls), url, false); + return TRUE; } @@ -2997,10 +2999,10 @@ lua_task_inject_url(lua_State *L) rspamd_lua_check_udata_maybe(L, 3, rspamd_mimepart_classname)); } if (task && task->message && url && url->url) { - if (rspamd_url_set_add_or_increase(MESSAGE_FIELD(task, urls), url->url, false)) { - if (mpart && mpart->urls) { - inject_url_query(task, url->url, mpart->urls); - } + rspamd_url_set_add_or_increase(MESSAGE_FIELD(task, urls), url->url, false); + + if (mpart && mpart->urls) { + inject_url_query(task, url->url, mpart->urls); } } else { diff --git a/test/lua/unit/task.lua b/test/lua/unit/task.lua index ec2a1cd4c2..9a0b98122b 100644 --- a/test/lua/unit/task.lua +++ b/test/lua/unit/task.lua @@ -167,4 +167,63 @@ Thank you, task:destroy() end) + + test("Part URLs are not deduplicated across MIME parts", function() + local msg = table.concat { + hdrs, + 'Content-Type: multipart/alternative; boundary=XXX\n', + '\n', + '--XXX\n', + 'Content-Type: text/plain\n', + '\n', + 'Visit and \n', + '\n', + '--XXX\n', + 'Content-Type: text/html\n', + '\n', + '' .. + 'A' .. + 'B' .. + '\n', + '\n', + '--XXX--\n', + } + local res, task = rspamd_task.load_from_string(msg, rspamd_config) + assert_true(res, "failed to load message") + task:process_message() + + local parts = task:get_parts() + assert_true(#parts >= 2, "should have at least two MIME parts") + + local function uniq_urls(part) + local seen = {} + + return fun.totable(fun.filter(function(v) + if seen[v] then + return false + end + + seen[v] = true + return true + end, fun.map(function(u) + return u:get_host() .. '/' .. u:get_path() + end, part:get_urls()))) + end + + assert_rspamd_table_eq_sorted({ + actual = uniq_urls(parts[#parts - 1]), + expect = { + 'example.com/a', 'example.com/b' + } + }) + + assert_rspamd_table_eq_sorted({ + actual = uniq_urls(parts[#parts]), + expect = { + 'example.com/a', 'example.com/b' + } + }) + + task:destroy() + end) end) -- 2.47.3