Merge pull request #76 from gjtorikian/stacking-text-chunk-changes

Support stacking text chunk changes

ext/selma/src/html/text_chunk.rs

@@ -6,6 +6,26 @@ use magnus::{exception, method, Error, Module, RClass, Symbol, Value};
 
 struct HTMLTextChunk {
     text_chunk: NativeRefWrap<TextChunk<'static>>,
+    buffer: String,
+}
+
+macro_rules! clone_buffer_if_not_empty {
+    ($binding:expr, $buffer:expr) => {
+        if !$binding.buffer.is_empty() {
+            $buffer.clone_from(&$binding.buffer);
+        }
+    };
+}
+
+// if this is the first time we're processing this text chunk (buffer is empty),
+// we carry on. Otherwise, we need to use the buffer text, not the text chunk,
+// because lol-html is not designed in such a way to keep track of text chunks.
+macro_rules! set_text_chunk_to_buffer {
+    ($text_chunk:expr, $buffer:expr) => {
+        if !$buffer.is_empty() {
+            $text_chunk.set_str($buffer);
+        }
+    };
 }
 
 #[magnus::wrap(class = "Selma::HTML::TextChunk")]
@@ -18,6 +38,7 @@ impl SelmaHTMLTextChunk {
     pub fn new(ref_wrap: NativeRefWrap<TextChunk<'static>>) -> Self {
         Self(RefCell::new(HTMLTextChunk {
             text_chunk: ref_wrap,
+            buffer: String::new(),
         }))
     }
 
@@ -96,16 +117,25 @@ impl SelmaHTMLTextChunk {
 
     fn replace(&self, args: &[Value]) -> Result<String, Error> {
         let mut binding = self.0.borrow_mut();
+        let mut buffer = String::new();
+
+        clone_buffer_if_not_empty!(binding, buffer);
+
         let text_chunk = binding.text_chunk.get_mut().unwrap();
 
+        set_text_chunk_to_buffer!(text_chunk, buffer);
+
         let (text_str, content_type) = match crate::scan_text_args(args) {
             Ok((text_str, content_type)) => (text_str, content_type),
             Err(err) => return Err(err),
         };
-
         text_chunk.replace(&text_str, content_type);
 
-        Ok(text_chunk.as_str().to_string())
+        text_chunk.set_str(text_str.clone());
+
+        binding.buffer = text_chunk.as_str().to_string();
+
+        Ok(text_str)
     }
 }
 

ext/selma/src/rewriter.rs

@@ -540,7 +540,8 @@ impl SelmaRewriter {
         // prevents missing `handle_text_chunk` function
         let content = text_chunk.as_str();
 
-        // seems that sometimes lol-html returns blank text / EOLs?
+        // lol-html sometimes returns blank text if
+        // last_in_text_node() is true
         if content.is_empty() {
             return Ok(());
         }

lib/selma/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Selma
-  VERSION = "0.4.6.1"
+  VERSION = "0.4.7"
 end

test/selma_maliciousness_test.rb

@@ -276,5 +276,5 @@ def test_deleted_content_does_not_segfault
     1000.times do
       selma.rewrite(html)
     end
-  end
+  end if ci?
 end

test/selma_rewriter_text_test.rb

@@ -144,6 +144,68 @@ def test_that_it_works_for_text_reject
     assert_equal("<div><p>Hello @gjtorikian: <code>@gjtorik</code></p><br/> <pre>@gjtorik</pre></div>", modified_doc)
   end
 
+  class TextRewriteOne
+    SELECTOR = Selma::Selector.new(match_text_within: "*")
+
+    def selector
+      SELECTOR
+    end
+
+    def handle_text_chunk(text)
+      text.replace(text.to_s.tr("1", "2"), as: :text)
+    end
+  end
+
+  class TextRewriteTwo
+    SELECTOR = Selma::Selector.new(match_text_within: "*")
+
+    def selector
+      SELECTOR
+    end
+
+    def handle_text_chunk(text)
+      text.replace(text.to_s.tr("2", "3"), as: :text)
+    end
+  end
+
+  def test_that_it_stacks_two_text_changes
+    frag = "<div>1 + 2 = 6</div>"
+    modified_doc = Selma::Rewriter.new(sanitizer: nil, handlers: [TextRewriteOne.new, TextRewriteTwo.new]).rewrite(frag)
+
+    assert_equal("<div>3 + 3 = 6</div>", modified_doc)
+  end
+
+  class HTMLRewriteOne
+    SELECTOR = Selma::Selector.new(match_text_within: "*")
+
+    def selector
+      SELECTOR
+    end
+
+    def handle_text_chunk(text)
+      text.replace(text.to_s.sub("1", "<strong>1</strong>"), as: :html)
+    end
+  end
+
+  class HTMLRewriteTwo
+    SELECTOR = Selma::Selector.new(match_text_within: "*")
+
+    def selector
+      SELECTOR
+    end
+
+    def handle_text_chunk(text)
+      text.replace(text.to_s.sub("2", "<em>2</em>"), as: :html)
+    end
+  end
+
+  def test_that_it_stacks_two_html_changes
+    frag = "<div>1 + 2 = 3</div>"
+    modified_doc = Selma::Rewriter.new(sanitizer: nil, handlers: [HTMLRewriteOne.new, HTMLRewriteTwo.new]).rewrite(frag)
+
+    assert_equal("<div><strong>1</strong> + <em>2</em> = 3</div>", modified_doc)
+  end
+
   class TextStringResizeHandler
     DEFAULT_IGNORED_ANCESTOR_TAGS = ["pre", "code", "tt"].freeze
 

test/test_helper.rb

@@ -25,6 +25,10 @@ def nest_html_content(html_content, depth)
   "#{"<span>" * depth}#{html_content}#{"</span>" * depth}"
 end
 
+def ci?
+  ENV["CI"] == "true"
+end
+
 FIXTURES_DIR = "test/fixtures"
 
 def load_fixture(file)