From 5aaf7e331675d9ef005785f07bd8202f1460f608 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 10 Oct 2019 08:50:48 +1100 Subject: [PATCH] FIX: during concurrent emails generation renderer should not be reused Our instance used for template rendering needs a lock to ensure there is no race condition where rendering happens on 2 threads at the same time. This can lead to local poisoning which can cause unexpected results in emails --- app/mailers/user_notifications.rb | 2 +- app/services/user_notification_renderer.rb | 14 ++++++++++---- lib/email/message_builder.rb | 2 +- lib/email/renderer.rb | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index de8ac561eb9..28e154f8de5 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -589,7 +589,7 @@ class UserNotifications < ActionMailer::Base end unless translation_override_exists - html = UserNotificationRenderer.instance.render( + html = UserNotificationRenderer.render( template: 'email/notification', format: :html, locals: { context_posts: context_posts, diff --git a/app/services/user_notification_renderer.rb b/app/services/user_notification_renderer.rb index edf11712838..64bb497dcfb 100644 --- a/app/services/user_notification_renderer.rb +++ b/app/services/user_notification_renderer.rb @@ -5,9 +5,15 @@ class UserNotificationRenderer < ActionView::Base include UserNotificationsHelper include EmailHelper - def self.instance - @instance ||= UserNotificationRenderer.with_view_paths( - Rails.configuration.paths["app/views"] - ) + LOCK = Mutex.new + + def self.render(*args) + LOCK.synchronize do + @instance ||= UserNotificationRenderer.with_view_paths( + Rails.configuration.paths["app/views"] + ) + @instance.render(*args) + end end + end diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 654c4ae3c67..df76f9170c6 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -94,7 +94,7 @@ module Email html_override.gsub!("%{respond_instructions}", "") end - html = UserNotificationRenderer.instance.render( + html = UserNotificationRenderer.render( template: 'layouts/email_template', format: :html, locals: { html_body: html_override.html_safe } diff --git a/lib/email/renderer.rb b/lib/email/renderer.rb index 2f9203bd6c3..feeb63c4112 100644 --- a/lib/email/renderer.rb +++ b/lib/email/renderer.rb @@ -18,7 +18,7 @@ module Email style = if @message.html_part Email::Styles.new(@message.html_part.body.to_s, @opts) else - unstyled = UserNotificationRenderer.instance.render( + unstyled = UserNotificationRenderer.render( template: 'layouts/email_template', format: :html, locals: { html_body: PrettyText.cook(text).html_safe }