Refactoring: Migrate notification_factory_template_test.rb to RSpec

lib/notification_factory/template.rb

@@ -17,35 +17,31 @@ examples how to use
   end
 
   def to_s
-    strip_html
+    @template.gsub(/\#{\s*(.*?)\s*}/m) do
-  end
-
-  def strip_html
       # some browsers start adding HTML tags
       # fixes https://github.com/zammad/zammad/issues/385
-    @template.gsub(/\#\{\s*t\((.+?)\)\s*\}/m) do
+      input_template = $1.gsub(/\A<.+?>\s*|\s*<.+?>\z/, '')
-      content = $1
+
-      if content =~ /^'(.+?)'$/
+      case input_template
-        %(<%= t "#{strip_content($1)}", #{@escape} %>)
+      when /\At\('(.+?)'\)\z/m
+        %(<%= t "#{sanitize_text($1)}", #{@escape} %>)
+      when /\At\((.+?)\)\z/m
+        %(<%= t d"#{sanitize_object_name($1)}", #{@escape} %>)
+      when /\Aconfig\.(.+?)\z/m
+        %(<%= c "#{sanitize_object_name($1)}", #{@escape} %>)
       else
-        %(<%= t d"#{strip_variable(content)}", #{@escape} %>)
+        %(<%= d "#{sanitize_object_name(input_template)}", #{@escape} %>)
       end
-    end.gsub(/\#\{\s*config\.(.+?)\s*\}/m) do
-      %(<%= c "#{strip_variable($1)}", #{@escape} %>)
-    end.gsub(/\#\{(.*?)\}/m) do
-      %(<%= d "#{strip_variable($1)}", #{@escape} %>)
     end
   end
 
-  def strip_content(string)
+  def sanitize_text(string)
-    string&.gsub(/\t|\r|\n/, '')
+    string&.tr("\t\r\n", '')
-          &.gsub(/"/, '\"')
+          &.gsub(/(?<!\\)(?=")/, '\\')
   end
 
-  def strip_variable(string)
+  def sanitize_object_name(string)
-    string&.gsub(/\t|\r|\n|"|'|§|;/, '')
+    string&.tr("\t\r\n\f \"'§;", '')
-          &.gsub(/\s*/, '')
-          &.gsub(/<.+?>/, '')
   end
 
 end

spec/lib/notification_factory/template_spec.rb

@@ -0,0 +1,91 @@
+require 'rails_helper'
+
+RSpec.describe NotificationFactory::Template do
+  subject(:template) do
+    NotificationFactory::Template.new(template_string, escape)
+  end
+
+  describe '#to_s' do
+    context 'for empty input template (incl. whitespace-only)' do
+      let(:template_string) { "\#{ }" }
+
+      context 'with escape = true' do
+        let(:escape) { true }
+
+        it 'returns an ERB template with the #d helper, and passes escape arg as string' do
+          expect(template.to_s).to eq('<%= d "", true %>')
+        end
+      end
+
+      context 'with escape = false' do
+        let(:escape) { false }
+
+        it 'returns an ERB template with the #d helper, and passes escape arg as string' do
+          expect(template.to_s).to eq('<%= d "", false %>')
+        end
+      end
+    end
+
+    context 'for input template using #t helper' do
+      let(:template_string) { "\#{t('some text')}" }
+      let(:escape) { false }
+
+      it 'returns an ERB template with the #t helper, and passes escape arg as string' do
+        expect(template.to_s).to eq('<%= t "some text", false %>')
+      end
+
+      context 'with double-quotes in argument' do
+        let(:template_string) { "\#{t('some \"text\"')}" }
+
+        it 'adds backslash-escaping' do
+          expect(template.to_s).to eq('<%= t "some \"text\"", false %>')
+        end
+      end
+    end
+
+    # Regression test for https://github.com/zammad/zammad/issues/385
+    context 'with HTML auto-injected by browser' do
+      let(:escape) { true }
+
+      context 'for <a> tags wrapped around "ticket.id"' do
+        let(:template_string) { <<~'TEMPLATE'.chomp }
+          #{<a href="http://ticket.id" title="http://ticket.id" target="_blank">ticket.id</a>}
+        TEMPLATE
+
+        it 'strips tag from resulting ERB template' do
+          expect(template.to_s).to eq('<%= d "ticket.id", true %>')
+        end
+      end
+
+      context 'for <a> tags wrapped around "config.fqdn"' do
+        let(:template_string) { <<~'TEMPLATE'.chomp }
+          #{<a href="http://config.fqdn" title="http://config.fqdn" target="_blank">config.fqdn</a>}
+        TEMPLATE
+
+        it 'strips tag from resulting ERB template' do
+          expect(template.to_s).to eq('<%= c "fqdn", true %>')
+        end
+      end
+
+      context 'for <a> tags surrounded by whitespace' do
+        let(:template_string) { <<~'TEMPLATE'.chomp }
+          #{   <a href="http://ticket.id" title="http://ticket.id" target="_blank">ticket.id  </a>  }
+        TEMPLATE
+
+        it 'strips tag and spaces from template' do
+          expect(template.to_s).to eq('<%= d "ticket.id", true %>')
+        end
+      end
+
+      context 'for unpaired <a> tag and trailing whitespace' do
+        let(:template_string) { <<~'TEMPLATE'.chomp }
+          #{<a href="http://ticket.id" title="http://ticket.id" target="_blank">ticket.id  }
+        TEMPLATE
+
+        it 'strips tag and spaces from template' do
+          expect(template.to_s).to eq('<%= d "ticket.id", true %>')
+        end
+      end
+    end
+  end
+end

test/unit/notification_factory_template_test.rb

@@ -1,79 +0,0 @@
-require 'test_helper'
-
-class NotificationFactoryTemplateTest < ActiveSupport::TestCase
-
-  # RSpec incoming!
-  def described_class
-    NotificationFactory::Template
-  end
-
-  test 'regular browser html' do
-
-    # ensures https://github.com/zammad/zammad/issues/385
-    template_before = '#{<a href="http://ticket.id" title="http://ticket.id" target="_blank">ticket.id</a>}'
-    template_after  = '<%= d "ticket.id", true %>'
-
-    result = described_class.new(template_before, true).to_s
-    assert_equal(template_after, result)
-
-    template_before = '#{<a href="http://ticket.id" title="http://ticket.id" target="_blank">config.fqdn</a>}'
-    template_after  = '<%= d "config.fqdn", true %>'
-
-    result = described_class.new(template_before, true).to_s
-    assert_equal(template_after, result)
-  end
-
-  test 'spaced browser html' do
-
-    # ensures https://github.com/zammad/zammad/issues/385
-    template_before = '#{   <a href="http://ticket.id" title="http://ticket.id" target="_blank">ticket.id  </a>  }'
-    template_after  = '<%= d "ticket.id", true %>'
-
-    result = described_class.new(template_before, true).to_s
-    assert_equal(template_after, result)
-  end
-
-  test 'broken browser html' do
-
-    # ensures https://github.com/zammad/zammad/issues/385
-    template_before = '#{<a href="http://ticket.id" title="http://ticket.id" target="_blank">ticket.id  }'
-    template_after  = '<%= d "ticket.id", true %>'
-
-    result = described_class.new(template_before, true).to_s
-    assert_equal(template_after, result)
-  end
-
-  test 'empty tag' do
-
-    template_before = '#{}'
-    template_after  = '<%= d "", true %>'
-
-    result = described_class.new(template_before, true).to_s
-    assert_equal(template_after, result)
-  end
-
-  test 'empty tag with space' do
-
-    template_before = '#{ }'
-    template_after  = '<%= d "", false %>'
-
-    result = described_class.new(template_before, false).to_s
-    assert_equal(template_after, result)
-  end
-
-  test 'translation' do
-
-    template_before = "\#{t('some text')}"
-    template_after  = '<%= t "some text", false %>'
-
-    result = described_class.new(template_before, false).to_s
-    assert_equal(template_after, result)
-
-    template_before = "\#{t('some \"text\"')}"
-    template_after  = '<%= t "some \"text\"", false %>'
-
-    result = described_class.new(template_before, false).to_s
-    assert_equal(template_after, result)
-  end
-
-end