diff --git a/lib/notification_factory/template.rb b/lib/notification_factory/template.rb index 98ce92347..945eba14d 100644 --- a/lib/notification_factory/template.rb +++ b/lib/notification_factory/template.rb @@ -17,35 +17,31 @@ examples how to use end def to_s - strip_html - end + @template.gsub(/\#{\s*(.*?)\s*}/m) do + # some browsers start adding HTML tags + # fixes https://github.com/zammad/zammad/issues/385 + input_template = $1.gsub(/\A<.+?>\s*|\s*<.+?>\z/, '') - def strip_html - # some browsers start adding HTML tags - # fixes https://github.com/zammad/zammad/issues/385 - @template.gsub(/\#\{\s*t\((.+?)\)\s*\}/m) do - content = $1 - if content =~ /^'(.+?)'$/ - %(<%= t "#{strip_content($1)}", #{@escape} %>) + case input_template + 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) - string&.gsub(/\t|\r|\n/, '') - &.gsub(/"/, '\"') + def sanitize_text(string) + string&.tr("\t\r\n", '') + &.gsub(/(?/, '') + def sanitize_object_name(string) + string&.tr("\t\r\n\f \"'§;", '') end end diff --git a/spec/lib/notification_factory/template_spec.rb b/spec/lib/notification_factory/template_spec.rb new file mode 100644 index 000000000..094f5ca46 --- /dev/null +++ b/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 tags wrapped around "ticket.id"' do + let(:template_string) { <<~'TEMPLATE'.chomp } + #{ticket.id} + TEMPLATE + + it 'strips tag from resulting ERB template' do + expect(template.to_s).to eq('<%= d "ticket.id", true %>') + end + end + + context 'for tags wrapped around "config.fqdn"' do + let(:template_string) { <<~'TEMPLATE'.chomp } + #{config.fqdn} + TEMPLATE + + it 'strips tag from resulting ERB template' do + expect(template.to_s).to eq('<%= c "fqdn", true %>') + end + end + + context 'for tags surrounded by whitespace' do + let(:template_string) { <<~'TEMPLATE'.chomp } + #{ ticket.id } + TEMPLATE + + it 'strips tag and spaces from template' do + expect(template.to_s).to eq('<%= d "ticket.id", true %>') + end + end + + context 'for unpaired tag and trailing whitespace' do + let(:template_string) { <<~'TEMPLATE'.chomp } + #{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 diff --git a/test/unit/notification_factory_template_test.rb b/test/unit/notification_factory_template_test.rb deleted file mode 100644 index 38ca7367c..000000000 --- a/test/unit/notification_factory_template_test.rb +++ /dev/null @@ -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 = '#{ticket.id}' - template_after = '<%= d "ticket.id", true %>' - - result = described_class.new(template_before, true).to_s - assert_equal(template_after, result) - - template_before = '#{config.fqdn}' - 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 = '#{ ticket.id }' - 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 = '#{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