Prevent stack overflow in ActionText PlaintextConversion

In our app, we encountered edge cases where the ActionText plaintext conversion caused stack overflows.
To address this, traverse a copy of the tree iteratively bottom-up, converting each node's content
to plaintext using the already-converted children content. Finally, extract the result from the root node.
This approach avoids recursion preventing stack overflows.

Author: intrip

actiontext/lib/action_text/plain_text_conversion.rb

@@ -7,69 +7,71 @@ module PlainTextConversion
     extend self
 
     def node_to_plain_text(node)
-      remove_trailing_newlines(plain_text_for_node(node))
+      remove_trailing_newlines(node_to_plain_text_content_tree(node).content)
     end
 
     private
-      def plain_text_for_node(node, index = 0)
+      def node_to_plain_text_content_tree(node)
+        BottomUpReplacer.replace_content(node.dup) { |n| plain_text_for_node(n) }
+      end
+
+      def plain_text_for_node(node)
         if respond_to?(plain_text_method_for_node(node), true)
-          send(plain_text_method_for_node(node), node, index)
+          send(plain_text_method_for_node(node), node)
         else
           plain_text_for_node_children(node)
         end
       end
 
       def plain_text_for_node_children(node)
-        texts = []
-        node.children.each_with_index do |child, index|
-          next if skippable?(child)
+        node.children.map(&:content).join
+      end
 
-          texts << plain_text_for_node(child, index)
-        end
-        texts.join
+      def plain_text_method_for_node(node)
+        :"plain_text_for_#{node.name}_node"
       end
 
-      def skippable?(node)
-        node.name == "script" || node.name == "style"
+      def plain_text_for_unsupported_node(node)
+        ""
       end
 
-      def plain_text_method_for_node(node)
-        :"plain_text_for_#{node.name}_node"
+      %i[ script style].each do |element|
+        alias_method :"plain_text_for_#{element}_node", :plain_text_for_unsupported_node
       end
 
-      def plain_text_for_block(node, index = 0)
+      def plain_text_for_block(node)
         "#{remove_trailing_newlines(plain_text_for_node_children(node))}\n\n"
       end
 
       %i[ h1 p ].each do |element|
         alias_method :"plain_text_for_#{element}_node", :plain_text_for_block
       end
 
-      def plain_text_for_list(node, index)
+      def plain_text_for_list(node)
         "#{break_if_nested_list(node, plain_text_for_block(node))}"
       end
 
       %i[ ul ol ].each do |element|
         alias_method :"plain_text_for_#{element}_node", :plain_text_for_list
       end
 
-      def plain_text_for_br_node(node, index)
+      def plain_text_for_br_node(node)
         "\n"
       end
 
-      def plain_text_for_text_node(node, index)
+      def plain_text_for_text_node(node)
         remove_trailing_newlines(node.text)
       end
 
-      def plain_text_for_div_node(node, index)
+      def plain_text_for_div_node(node)
         "#{remove_trailing_newlines(plain_text_for_node_children(node))}\n"
       end
 
-      def plain_text_for_figcaption_node(node, index)
+      def plain_text_for_figcaption_node(node)
         "[#{remove_trailing_newlines(plain_text_for_node_children(node))}]"
       end
 
-      def plain_text_for_blockquote_node(node, index)
+      def plain_text_for_blockquote_node(node)
         text = plain_text_for_block(node)
         return "“”" if text.blank?
 
@@ -79,8 +81,8 @@ def plain_text_for_blockquote_node(node, index)
         text
       end
 
-      def plain_text_for_li_node(node, index)
-        bullet = bullet_for_li_node(node, index)
+      def plain_text_for_li_node(node)
+        bullet = bullet_for_li_node(node)
         text = remove_trailing_newlines(plain_text_for_node_children(node))
         indentation = indentation_for_li_node(node)
 
@@ -91,8 +93,9 @@ def remove_trailing_newlines(text)
         text.chomp("")
       end
 
-      def bullet_for_li_node(node, index)
+      def bullet_for_li_node(node)
         if list_node_name_for_li_node(node) == "ol"
+          index = node.parent.children.index(node)
           "#{index + 1}."
         else
           "•"
@@ -121,5 +124,38 @@ def break_if_nested_list(node, text)
           text
         end
       end
+
+      class BottomUpReplacer
+        def self.replace_content(node, &block)
+          new(node).replace_content(&block)
+        end
+
+        def initialize(node)
+          @node = node
+        end
+
+        def replace_content(&block)
+          @node.tap do |node|
+            traverse_bottom_up(node) do |n|
+              n.content = block.call(n)
+            end
+          end
+        end
+
+        private
+          def traverse_bottom_up(node)
+            call_stack, processing_stack = [ node ], []
+
+            until call_stack.empty?
+              node = call_stack.pop
+              processing_stack.push(node)
+              call_stack.concat node.children
+            end
+
+            processing_stack.reverse_each do |node|
+              yield node
+            end
+          end
+      end
   end
 end

actiontext/test/unit/plain_text_conversion_test.rb

@@ -120,7 +120,7 @@ class ActionText::PlainTextConversionTest < ActiveSupport::TestCase
       "Hello world!\nHow are you?",
       ActionText::Fragment.wrap("<div>Hello world!</div><div></div>").tap do |fragment|
         node = fragment.source.children.last
-        1_000.times do
+        10_000.times do
           child = node.clone
           child.parent = node
           node = child
@@ -130,6 +130,14 @@ class ActionText::PlainTextConversionTest < ActiveSupport::TestCase
     )
   end
 
+  test "preserves the original tree" do
+    html = "<div>Hello <span>world!</span></div>"
+    fragment = ActionText::Fragment.wrap(html)
+    assert_no_changes -> { fragment.source.to_html } do
+      assert_equal "Hello world!", fragment.to_plain_text
+    end
+  end
+
   test "preserves non-linebreak whitespace after text" do
     assert_converted_to(
       "Hello world!",