]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] html: eliminate DOM recursion to survive deeply nested messages
authorVsevolod Stakhov <vsevolod@rspamd.com>
Wed, 22 Jul 2026 12:44:09 +0000 (13:44 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Wed, 22 Jul 2026 12:44:09 +0000 (13:44 +0100)
Convert html_append_tag_content, traverse_block_tags and
html_debug_structure from native-stack recursion to explicit heap
stacks, and maintain html_tag::depth incrementally instead of walking
the parent chain per tag. With max_tags = 8192 a message can legally
nest ~8190 elements, which overflowed 512KB worker-thread stacks.

Add a regression test parsing 8190 nested divs.

src/libserver/html/html.cxx
src/libserver/html/html.hxx
src/libserver/html/html_tag.hxx
src/libserver/html/html_tests.cxx

index 7a7116050a0ebc81d76122c05b3f18128fcf490a..c926465f7620da8852aaf4f048d17ba5ffa40dcf 100644 (file)
@@ -1870,222 +1870,227 @@ html_append_tag_content(rspamd_mempool_t *pool,
                                                khash_t(rspamd_url_hash) * url_set,
                                                lua_State *L) -> goffset
 {
-       auto is_visible = true, is_block = false, is_spaces = false, is_transparent = false;
-       goffset next_tag_offset = tag->closing.end,
-                       initial_parsed_offset = hc->parsed.size(),
-                       initial_invisible_offset = hc->invisible.size();
-
-       auto calculate_final_tag_offsets = [&]() -> void {
-               if (is_visible) {
-                       tag->content_offset = initial_parsed_offset;
-                       tag->closing.start = hc->parsed.size();
-               }
-               else {
-                       tag->content_offset = initial_invisible_offset;
-                       tag->closing.start = hc->invisible.size();
-               }
+       struct append_frame {
+               html_tag *tag;
+               std::size_t next_child = 0;
+               goffset next_tag_offset = 0;
+               goffset initial_parsed_offset = 0;
+               goffset initial_invisible_offset = 0;
+               goffset cur_offset = 0;
+               bool entered = false;
+               bool finished = false;
+               bool is_visible = true;
+               bool is_block = false;
+               bool is_spaces = false;
+               bool is_transparent = false;
        };
 
-       if (tag->closing.end == -1) {
-               if (tag->closing.start != -1) {
-                       next_tag_offset = tag->closing.start;
-                       tag->closing.end = tag->closing.start;
+       auto calculate_final_tag_offsets = [hc](append_frame &frame) -> void {
+               if (frame.is_visible) {
+                       frame.tag->content_offset = frame.initial_parsed_offset;
+                       frame.tag->closing.start = hc->parsed.size();
                }
                else {
-                       next_tag_offset = tag->content_offset;
-                       tag->closing.end = tag->content_offset;
+                       frame.tag->content_offset = frame.initial_invisible_offset;
+                       frame.tag->closing.start = hc->invisible.size();
                }
-       }
-       if (tag->closing.start == -1) {
-               tag->closing.start = tag->closing.end;
-       }
+       };
 
-       auto append_margin = [&](char c) -> void {
-               /* We do care about visible margins only */
-               if (is_visible) {
-                       if (!hc->parsed.empty() && hc->parsed.back() != c && hc->parsed.back() != '\n') {
-                               if (hc->parsed.back() == ' ') {
-                                       /* We also strip extra spaces at the end, but limiting the start */
-                                       auto last = std::make_reverse_iterator(hc->parsed.begin() + initial_parsed_offset);
-                                       auto first = std::find_if(hc->parsed.rbegin(), last,
-                                                                                         [](auto ch) -> auto {
-                                                                                                 return ch != ' ';
-                                                                                         });
-                                       hc->parsed.erase(first.base(), hc->parsed.end());
-                                       g_assert(hc->parsed.size() >= initial_parsed_offset);
-                               }
-                               hc->parsed.push_back(c);
+       auto append_margin = [hc](append_frame &frame, char margin) -> void {
+               if (frame.is_visible && !hc->parsed.empty() &&
+                       hc->parsed.back() != margin && hc->parsed.back() != '\n') {
+                       if (hc->parsed.back() == ' ') {
+                               auto last = std::make_reverse_iterator(
+                                       hc->parsed.begin() + frame.initial_parsed_offset);
+                               auto first = std::find_if(hc->parsed.rbegin(), last,
+                                                                                 [](auto ch) -> auto {
+                                                                                         return ch != ' ';
+                                                                                 });
+                               hc->parsed.erase(first.base(), hc->parsed.end());
+                               g_assert(hc->parsed.size() >= frame.initial_parsed_offset);
                        }
+                       hc->parsed.push_back(margin);
                }
        };
 
-       if (tag->id == Tag_BR || tag->id == Tag_HR) {
+       auto enter_frame = [&](append_frame &frame) -> void {
+               auto *cur = frame.tag;
+               frame.next_tag_offset = cur->closing.end;
+               frame.initial_parsed_offset = hc->parsed.size();
+               frame.initial_invisible_offset = hc->invisible.size();
 
-               if (!(tag->flags & FL_IGNORE)) {
-                       hc->parsed.append("\n");
+               if (cur->closing.end == -1) {
+                       if (cur->closing.start != -1) {
+                               frame.next_tag_offset = cur->closing.start;
+                               cur->closing.end = cur->closing.start;
+                       }
+                       else {
+                               frame.next_tag_offset = cur->content_offset;
+                               cur->closing.end = cur->content_offset;
+                       }
+               }
+               if (cur->closing.start == -1) {
+                       cur->closing.start = cur->closing.end;
                }
 
-               auto ret = tag->content_offset;
-               calculate_final_tag_offsets();
-
-               return ret;
-       }
-       else if ((tag->id == Tag_HEAD && (tag->flags & FL_IGNORE)) || (tag->flags & CM_HEAD)) {
-               auto ret = tag->closing.end;
-               calculate_final_tag_offsets();
-
-               return ret;
-       }
+               if (cur->id == Tag_BR || cur->id == Tag_HR) {
+                       if (!(cur->flags & FL_IGNORE)) {
+                               hc->parsed.append("\n");
+                       }
+                       frame.next_tag_offset = cur->content_offset;
+                       calculate_final_tag_offsets(frame);
+                       frame.finished = true;
+                       frame.entered = true;
+                       return;
+               }
+               if ((cur->id == Tag_HEAD && (cur->flags & FL_IGNORE)) ||
+                       (cur->flags & CM_HEAD)) {
+                       frame.next_tag_offset = cur->closing.end;
+                       calculate_final_tag_offsets(frame);
+                       frame.finished = true;
+                       frame.entered = true;
+                       return;
+               }
 
-       if ((tag->flags & (FL_COMMENT | FL_XML | FL_IGNORE | CM_HEAD))) {
-               is_visible = false;
-       }
-       else {
-               if (!tag->block) {
-                       is_visible = true;
+               if ((cur->flags & (FL_COMMENT | FL_XML | FL_IGNORE | CM_HEAD))) {
+                       frame.is_visible = false;
                }
-               else if (!tag->block->is_visible()) {
-                       if (!tag->block->is_transparent()) {
-                               is_visible = false;
+               else if (cur->block && !cur->block->is_visible()) {
+                       if (!cur->block->is_transparent() ||
+                               (cur->block->has_display() &&
+                                cur->block->display == css::css_display_value::DISPLAY_HIDDEN)) {
+                               frame.is_visible = false;
                        }
                        else {
-                               if (tag->block->has_display() &&
-                                       tag->block->display == css::css_display_value::DISPLAY_HIDDEN) {
-                                       is_visible = false;
-                               }
-                               else {
-                                       is_transparent = true;
-                               }
+                               frame.is_transparent = true;
                        }
                }
-               else {
-                       if (tag->block->display == css::css_display_value::DISPLAY_BLOCK) {
-                               is_block = true;
+               else if (cur->block) {
+                       if (cur->block->display == css::css_display_value::DISPLAY_BLOCK) {
+                               frame.is_block = true;
                        }
-                       else if (tag->block->display == css::css_display_value::DISPLAY_TABLE_ROW) {
-                               is_spaces = true;
+                       else if (cur->block->display == css::css_display_value::DISPLAY_TABLE_ROW) {
+                               frame.is_spaces = true;
                        }
                }
-       }
 
-       if (is_block) {
-               append_margin('\n');
-       }
-       else if (is_spaces) {
-               append_margin(' ');
-       }
+               if (frame.is_block) {
+                       append_margin(frame, '\n');
+               }
+               else if (frame.is_spaces) {
+                       append_margin(frame, ' ');
+               }
 
-       goffset cur_offset = tag->content_offset;
+               frame.cur_offset = cur->content_offset;
+               frame.entered = true;
+       };
 
-       for (auto *cld: tag->children) {
-               auto enclosed_start = cld->tag_start;
-               goffset initial_part_len = enclosed_start - cur_offset;
+       auto finish_frame = [&](append_frame &frame) -> void {
+               auto *cur = frame.tag;
 
-               if (initial_part_len > 0) {
-                       if (is_visible) {
-                               html_append_parsed(hc,
-                                                                  {start + cur_offset, std::size_t(initial_part_len)},
-                                                                  is_transparent, len, hc->parsed);
-                       }
-                       else {
+               if (frame.cur_offset < cur->closing.start) {
+                       goffset final_part_len = cur->closing.start - frame.cur_offset;
+
+                       if (final_part_len > 0) {
+                               auto &dest = frame.is_visible ? hc->parsed : hc->invisible;
                                html_append_parsed(hc,
-                                                                  {start + cur_offset, std::size_t(initial_part_len)},
-                                                                  is_transparent, len, hc->invisible);
+                                                                  {start + frame.cur_offset, std::size_t(final_part_len)},
+                                                                  frame.is_transparent, len, dest);
                        }
                }
 
-               auto next_offset = html_append_tag_content(pool, start, len,
-                                                                                                  hc, cld, exceptions, url_set, L);
-
-               /* Do not allow shifting back */
-               if (next_offset > cur_offset) {
-                       cur_offset = next_offset;
+               if (frame.is_block) {
+                       append_margin(frame, '\n');
+               }
+               else if (frame.is_spaces) {
+                       append_margin(frame, ' ');
                }
-       }
 
-       if (cur_offset < tag->closing.start) {
-               goffset final_part_len = tag->closing.start - cur_offset;
+               if (frame.is_visible) {
+                       if (cur->id == Tag_A) {
+                               auto written_len = hc->parsed.size() - frame.initial_parsed_offset;
+                               html_process_displayed_href_tag(
+                                       pool, hc,
+                                       {hc->parsed.data() + frame.initial_parsed_offset,
+                                        std::size_t(written_len)},
+                                       cur, exceptions, url_set, frame.initial_parsed_offset, L);
 
-               if (final_part_len > 0) {
-                       if (is_visible) {
-                               html_append_parsed(hc,
-                                                                  {start + cur_offset, std::size_t(final_part_len)},
-                                                                  is_transparent,
-                                                                  len,
-                                                                  hc->parsed);
-                       }
-                       else {
-                               html_append_parsed(hc,
-                                                                  {start + cur_offset, std::size_t(final_part_len)},
-                                                                  is_transparent,
-                                                                  len,
-                                                                  hc->invisible);
-                       }
-               }
-       }
-       if (is_block) {
-               append_margin('\n');
-       }
-       else if (is_spaces) {
-               append_margin(' ');
-       }
-
-       if (is_visible) {
-               if (tag->id == Tag_A) {
-                       auto written_len = hc->parsed.size() - initial_parsed_offset;
-                       html_process_displayed_href_tag(pool, hc,
-                                                                                       {hc->parsed.data() + initial_parsed_offset, std::size_t(written_len)},
-                                                                                       tag, exceptions,
-                                                                                       url_set, initial_parsed_offset,
-                                                                                       L);
-                       /* Count display URL mismatches when URL is present */
-                       if (std::holds_alternative<rspamd_url *>(tag->extra)) {
-                               auto *u = std::get<rspamd_url *>(tag->extra);
-                               if (u && (u->flags & RSPAMD_URL_FLAG_DISPLAY_URL) && (u->flags & RSPAMD_URL_FLAG_HTML_DISPLAYED)) {
-                                       /* html_process_displayed_href_tag sets linked_url when display URL differs */
-                                       if (u->ext && u->ext->linked_url && u->ext->linked_url != u) {
+                               if (std::holds_alternative<rspamd_url *>(cur->extra)) {
+                                       auto *u = std::get<rspamd_url *>(cur->extra);
+                                       if (u && (u->flags & RSPAMD_URL_FLAG_DISPLAY_URL) &&
+                                               (u->flags & RSPAMD_URL_FLAG_HTML_DISPLAYED) &&
+                                               u->ext && u->ext->linked_url && u->ext->linked_url != u) {
                                                hc->features.links.display_mismatch_links++;
                                        }
                                }
                        }
-               }
-               else if (tag->id == Tag_IMG) {
-                       /* Process ALT if presented */
-                       auto maybe_alt = tag->find_alt();
+                       else if (cur->id == Tag_IMG) {
+                               auto maybe_alt = cur->find_alt();
 
-                       if (maybe_alt) {
-                               if (!hc->parsed.empty() && !g_ascii_isspace(hc->parsed.back())) {
-                                       /* Add a space */
-                                       hc->parsed += ' ';
-                               }
+                               if (maybe_alt) {
+                                       if (!hc->parsed.empty() && !g_ascii_isspace(hc->parsed.back())) {
+                                               hc->parsed += ' ';
+                                       }
 
-                               hc->parsed.append(maybe_alt.value());
+                                       hc->parsed.append(maybe_alt.value());
 
-                               if (!hc->parsed.empty() && !g_ascii_isspace(hc->parsed.back())) {
-                                       /* Add a space */
-                                       hc->parsed += ' ';
+                                       if (!hc->parsed.empty() && !g_ascii_isspace(hc->parsed.back())) {
+                                               hc->parsed += ' ';
+                                       }
                                }
                        }
                }
-       }
-       else {
-               /* Invisible stuff */
-               if (std::holds_alternative<rspamd_url *>(tag->extra)) {
-                       auto *url_enclosed = std::get<rspamd_url *>(tag->extra);
+               else if (std::holds_alternative<rspamd_url *>(cur->extra)) {
+                       auto *url_enclosed = std::get<rspamd_url *>(cur->extra);
 
-                       /*
-                        * TODO: when hash is fixed to include flags we need to remove and add
-                        * url to the hash set
-                        */
                        if (url_enclosed) {
                                url_enclosed->flags |= RSPAMD_URL_FLAG_INVISIBLE;
                        }
                }
-       }
 
-       calculate_final_tag_offsets();
+               calculate_final_tag_offsets(frame);
+       };
+
+       std::vector<append_frame> stack;
+       stack.reserve(128);
+       stack.push_back({tag});
+       goffset result = tag->closing.end;
+
+       while (!stack.empty()) {
+               auto &frame = stack.back();
+
+               if (!frame.entered) {
+                       enter_frame(frame);
+               }
+
+               if (!frame.finished && frame.next_child < frame.tag->children.size()) {
+                       auto *child = frame.tag->children[frame.next_child++];
+                       goffset initial_part_len = child->tag_start - frame.cur_offset;
 
-       return next_tag_offset;
+                       if (initial_part_len > 0) {
+                               auto &dest = frame.is_visible ? hc->parsed : hc->invisible;
+                               html_append_parsed(hc,
+                                                                  {start + frame.cur_offset, std::size_t(initial_part_len)},
+                                                                  frame.is_transparent, len, dest);
+                       }
+
+                       stack.push_back({child});
+                       continue;
+               }
+
+               if (!frame.finished) {
+                       finish_frame(frame);
+               }
+
+               result = frame.next_tag_offset;
+               stack.pop_back();
+
+               if (!stack.empty() && result > stack.back().cur_offset) {
+                       stack.back().cur_offset = result;
+               }
+       }
+
+       return result;
 }
 
 auto html_process_input(struct rspamd_task *task,
@@ -2470,14 +2475,9 @@ auto html_process_input(struct rspamd_task *task,
 
                /* Track DOM tag count and max depth */
                hc->features.tags_count++;
-               {
-                       unsigned int depth = 0;
-                       for (auto *pdepth = cur_tag->parent; pdepth != nullptr; pdepth = pdepth->parent) {
-                               depth++;
-                       }
-                       if (depth > hc->features.max_dom_depth) {
-                               hc->features.max_dom_depth = depth;
-                       }
+               cur_tag->depth = cur_tag->parent ? cur_tag->parent->depth + 1 : 0;
+               if (cur_tag->depth > hc->features.max_dom_depth) {
+                       hc->features.max_dom_depth = cur_tag->depth;
                }
 
                if (!(cur_tag->flags & CM_EMPTY)) {
@@ -3194,10 +3194,15 @@ auto html_debug_structure(const html_content &hc) -> std::string
        std::string output;
 
        if (hc.root_tag) {
-               auto rec_functor = [&](const html_tag *t, int level, auto rec_functor) -> void {
-                       std::string pluses(level, '+');
+               std::vector<std::pair<const html_tag *, int>> stack{{hc.root_tag, 1}};
+
+               while (!stack.empty()) {
+                       auto [t, level] = stack.back();
+                       stack.pop_back();
 
                        if (!(t->flags & (FL_VIRTUAL | FL_IGNORE))) {
+                               std::string pluses(level, '+');
+
                                if (t->flags & FL_XML) {
                                        output += fmt::format("{}xml;", pluses);
                                }
@@ -3207,12 +3212,11 @@ auto html_debug_structure(const html_content &hc) -> std::string
                                }
                                level++;
                        }
-                       for (const auto *cld: t->children) {
-                               rec_functor(cld, level, rec_functor);
-                       }
-               };
 
-               rec_functor(hc.root_tag, 1, rec_functor);
+                       for (auto it = t->children.rbegin(); it != t->children.rend(); ++it) {
+                               stack.emplace_back(*it, level);
+                       }
+               }
        }
 
        return output;
index 6f23f0fb953a4e1bb26b504f4b58503e8b11e4a7..09d55dd1f4da97dea91d367401f378dc8b03a8bd 100644 (file)
@@ -88,39 +88,52 @@ struct html_content {
        auto traverse_block_tags(fu2::function<bool(const html_tag *)> &&func,
                                                         traverse_type how = traverse_type::PRE_ORDER) const -> bool
        {
-
                if (root_tag == nullptr) {
                        return false;
                }
 
-               auto rec_functor_pre_order = [&](const html_tag *root, auto &&rec) -> bool {
-                       if (func(root)) {
+               switch (how) {
+               case traverse_type::PRE_ORDER: {
+                       std::vector<const html_tag *> stack{root_tag};
 
-                               for (const auto *c: root->children) {
-                                       if (!rec(c, rec)) {
-                                               return false;
-                                       }
-                               }
+                       while (!stack.empty()) {
+                               auto *tag = stack.back();
+                               stack.pop_back();
 
-                               return true;
-                       }
-                       return false;
-               };
-               auto rec_functor_post_order = [&](const html_tag *root, auto &&rec) -> bool {
-                       for (const auto *c: root->children) {
-                               if (!rec(c, rec)) {
+                               if (!func(tag)) {
                                        return false;
                                }
+
+                               for (auto it = tag->children.rbegin(); it != tag->children.rend(); ++it) {
+                                       stack.push_back(*it);
+                               }
                        }
 
-                       return func(root);
-               };
+                       return true;
+               }
+               case traverse_type::POST_ORDER: {
+                       using stack_entry = std::pair<const html_tag *, bool>;
+                       std::vector<stack_entry> stack{{root_tag, false}};
+
+                       while (!stack.empty()) {
+                               auto [tag, visited] = stack.back();
+                               stack.pop_back();
 
-               switch (how) {
-               case traverse_type::PRE_ORDER:
-                       return rec_functor_pre_order(root_tag, rec_functor_pre_order);
-               case traverse_type::POST_ORDER:
-                       return rec_functor_post_order(root_tag, rec_functor_post_order);
+                               if (visited) {
+                                       if (!func(tag)) {
+                                               return false;
+                                       }
+                               }
+                               else {
+                                       stack.emplace_back(tag, true);
+                                       for (auto it = tag->children.rbegin(); it != tag->children.rend(); ++it) {
+                                               stack.emplace_back(*it, false);
+                                       }
+                               }
+                       }
+
+                       return true;
+               }
                default:
                        RSPAMD_UNREACHABLE;
                }
index 6df1955d4aae225726614c54d86117ca3856d0e2..3c309ad053ff3447fa030342f73941e57bba5f50 100644 (file)
@@ -1312,7 +1312,8 @@ struct html_tag {
        html_tag_extra_t extra;
        mutable struct html_block *block = nullptr;
        std::vector<struct html_tag *> children;
-       struct html_tag *parent;
+       struct html_tag *parent = nullptr;
+       unsigned int depth = 0;
 
        // Template method to find component by type
        template<typename T>
@@ -1464,6 +1465,8 @@ struct html_tag {
                flags = 0;
                block = nullptr;
                children.clear();
+               parent = nullptr;
+               depth = 0;
                closing.clear();
        }
 
index 3be836a2d5c81505ef2fafa57dd4167ee69423bf..e3ab6e5693b0ac6986d59a4f0047cf342099529a 100644 (file)
@@ -299,6 +299,51 @@ TEST_SUITE("html")
 
                rspamd_mempool_delete(pool);
        }
+
+       TEST_CASE("html deep nesting")
+       {
+               /* Just below max_tags, so every div is parsed as a real DOM node */
+               constexpr unsigned int num_divs = 8190;
+
+               rspamd_url_init(NULL);
+               auto *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(),
+                                                                               "html", 0);
+               struct rspamd_task fake_task;
+               memset(&fake_task, 0, sizeof(fake_task));
+               fake_task.task_pool = pool;
+
+               std::string input;
+               input.reserve(num_divs * 11 + sizeof("deep"));
+               for (unsigned int i = 0; i < num_divs; i++) {
+                       input += "<div>";
+               }
+               input += "deep";
+               for (unsigned int i = 0; i < num_divs; i++) {
+                       input += "</div>";
+               }
+
+               GByteArray *tmp = g_byte_array_sized_new(input.size());
+               g_byte_array_append(tmp, (const uint8_t *) input.data(), input.size());
+               auto *hc = html_process_input(&fake_task, tmp, nullptr, nullptr, nullptr, true, nullptr);
+               CHECK(hc != nullptr);
+               /* A virtual root tag is inserted above the first div */
+               CHECK(hc->features.max_dom_depth == num_divs);
+               CHECK(hc->parsed.find("deep") != std::string::npos);
+
+               auto dump = html_debug_structure(*hc);
+               CHECK(!dump.empty());
+
+               auto traversed = 0U;
+               hc->traverse_block_tags([&](const html_tag *) -> bool {
+                       traversed++;
+                       return true;
+               },
+                                                               html_content::traverse_type::POST_ORDER);
+               CHECK(traversed == num_divs + 1);
+
+               g_byte_array_free(tmp, TRUE);
+               rspamd_mempool_delete(pool);
+       }
 }
 
 } /* namespace rspamd::html */