Maintenance: Improve template rendering.

This commit is contained in:
Martin Gruner 2021-09-23 12:04:18 +02:00 committed by Thorsten Eckel
parent 678161be2e
commit 95571b70c9
7 changed files with 66 additions and 15 deletions

View file

@ -324,7 +324,8 @@ returns
locale: data[:locale], locale: data[:locale],
timezone: data[:timezone], timezone: data[:timezone],
template: template[:subject], template: template[:subject],
escape: false escape: false,
trusted: true,
).render ).render
# strip off the extra newline at the end of the subject to avoid =0A suffixes (see #2726) # strip off the extra newline at the end of the subject to avoid =0A suffixes (see #2726)
@ -334,7 +335,8 @@ returns
objects: data[:objects], objects: data[:objects],
locale: data[:locale], locale: data[:locale],
timezone: data[:timezone], timezone: data[:timezone],
template: template[:body] template: template[:body],
trusted: true,
).render ).render
if !data[:raw] if !data[:raw]
@ -348,7 +350,8 @@ returns
objects: data[:objects], objects: data[:objects],
locale: data[:locale], locale: data[:locale],
timezone: data[:timezone], timezone: data[:timezone],
template: application_template template: application_template,
trusted: true,
).render ).render
end end
{ {

View file

@ -13,7 +13,8 @@ examples how to use
locale: 'de-de', locale: 'de-de',
timezone: 'America/Port-au-Prince', timezone: 'America/Port-au-Prince',
template: 'some template <b>#{ticket.title}</b> {config.fqdn}', template: 'some template <b>#{ticket.title}</b> {config.fqdn}',
escape: false escape: false,
trusted: false, # Allow ERB tags in the template?
).render ).render
message_body = NotificationFactory::Renderer.new( message_body = NotificationFactory::Renderer.new(
@ -27,11 +28,11 @@ examples how to use
=end =end
def initialize(objects:, template:, locale: nil, timezone: nil, escape: true) def initialize(objects:, template:, locale: nil, timezone: nil, escape: true, trusted: false) # rubocop:disable Metrics/ParameterLists
@objects = objects @objects = objects
@locale = locale || Locale.default @locale = locale || Locale.default
@timezone = timezone || Setting.get('timezone_default') @timezone = timezone || Setting.get('timezone_default')
@template = NotificationFactory::Template.new(template, escape) @template = NotificationFactory::Template.new(template, escape, trusted)
@escape = escape @escape = escape
end end

View file

@ -46,14 +46,16 @@ returns
locale: data[:locale], locale: data[:locale],
timezone: data[:timezone], timezone: data[:timezone],
template: template[:subject], template: template[:subject],
escape: false escape: false,
trusted: true
).render ).render
message_body = NotificationFactory::Renderer.new( message_body = NotificationFactory::Renderer.new(
objects: data[:objects], objects: data[:objects],
locale: data[:locale], locale: data[:locale],
timezone: data[:timezone], timezone: data[:timezone],
template: template[:body], template: template[:body],
escape: false escape: false,
trusted: true
).render ).render
if !data[:raw] if !data[:raw]
@ -68,7 +70,8 @@ returns
locale: data[:locale], locale: data[:locale],
timezone: data[:timezone], timezone: data[:timezone],
template: application_template, template: application_template,
escape: false escape: false,
trusted: true
).render ).render
end end
{ {

View file

@ -9,17 +9,21 @@ examples how to use
cleaned_template = NotificationFactory::Template.new( cleaned_template = NotificationFactory::Template.new(
'some template <b>#{ticket.title}</b> #{config.fqdn}', 'some template <b>#{ticket.title}</b> #{config.fqdn}',
true, true,
false, # Allow ERB tags in the template?
).to_s ).to_s
=end =end
def initialize(template, escape) def initialize(template, escape, trusted)
@template = template @template = template
@escape = escape @escape = escape
@trusted = trusted
end end
def to_s def to_s
@template.gsub(%r{\#{\s*(.*?)\s*}}m) do result = @template
result.gsub!(%r{<%(?!%)}, '<%%') if !@trusted
result.gsub(%r{\#{\s*(.*?)\s*}}m) do
# some browsers start adding HTML tags # some browsers start adding HTML tags
# fixes https://github.com/zammad/zammad/issues/385 # fixes https://github.com/zammad/zammad/issues/385
input_template = $1.gsub(%r{\A<.+?>\s*|\s*<.+?>\z}, '') input_template = $1.gsub(%r{\A<.+?>\s*|\s*<.+?>\z}, '')

View file

@ -6,7 +6,8 @@ FactoryBot.define do
locale { 'en-en' } locale { 'en-en' }
template { '' } template { '' }
escape { true } escape { true }
trusted { false }
initialize_with { new(objects: objects, locale: locale, template: template, escape: escape) } initialize_with { new(objects: objects, locale: locale, template: template, escape: escape, trusted: trusted) }
end end
end end

View file

@ -12,6 +12,21 @@ RSpec.describe NotificationFactory::Renderer do
expect(renderer.render).to eq '' expect(renderer.render).to eq ''
end end
context 'when rendering templates with ERB tags' do
let(:template) { '<%% <%= "<%" %> %%>' }
it 'ignores pre-existing ERB tags in an untrusted template' do
renderer = build :notification_factory_renderer, template: template
expect(renderer.render).to eq '<% <%= "<%" %> %%>'
end
it 'executes pre-existing ERB tags in a trusted template' do
renderer = build :notification_factory_renderer, template: template, trusted: true
expect(renderer.render).to eq '<% <% %%>'
end
end
it 'correctly renders chained object references' do it 'correctly renders chained object references' do
user = User.where(firstname: 'Nicole').first user = User.where(firstname: 'Nicole').first
ticket = create :ticket, customer: user ticket = create :ticket, customer: user

View file

@ -4,9 +4,11 @@ require 'rails_helper'
RSpec.describe NotificationFactory::Template do RSpec.describe NotificationFactory::Template do
subject(:template) do subject(:template) do
described_class.new(template_string, escape) described_class.new(template_string, escape, trusted)
end end
let(:trusted) { false }
describe '#to_s' do describe '#to_s' do
context 'for empty input template (incl. whitespace-only)' do context 'for empty input template (incl. whitespace-only)' do
let(:template_string) { "\#{ }" } let(:template_string) { "\#{ }" }
@ -28,6 +30,28 @@ RSpec.describe NotificationFactory::Template do
end end
end end
context 'for sanitizing the template string' do
let(:escape) { false }
context 'for strings containing ERB' do
let(:template_string) { '<%% <% "<%" %> <%# comment %> <%= "<%" %> <%- "" %> %%>' }
context 'for untrusted templates' do
it 'mutes all pre-existing ERB tags' do
expect(template.to_s).to eq('<%% <%% "<%%" %> <%%# comment %> <%%= "<%%" %> <%%- "" %> %%>')
end
end
context 'for trusted templates' do
let(:trusted) { true }
it 'keeps all pre-existing ERB tags' do
expect(template.to_s).to eq(template_string)
end
end
end
end
context 'for input template using #t helper' do context 'for input template using #t helper' do
let(:template_string) { "\#{t('some text')}" } let(:template_string) { "\#{t('some text')}" }
let(:escape) { false } let(:escape) { false }