From b8120ab14d695d5ae7392193a6135b5e7cef9c80 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Tue, 29 Jul 2014 09:21:47 +0200 Subject: Raise an exception when attachments are added after `mail` was called. Closes #16163 Adding attachments after a call to `mail` will result in invalid emails. This is related to the fact, that `mail` is making the required preparations before the email is ready to be sent. These change depending on your added attachments. --- actionmailer/CHANGELOG.md | 7 ++++++ actionmailer/lib/action_mailer/base.rb | 17 ++++++++++++++- actionmailer/test/base_test.rb | 39 ++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 4e3a9daf7d..bcfde6d96f 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,10 @@ +* Raise an exception when attachments are added after `mail` was called. + This is a safeguard to prevent invalid emails. + + Fixes #16163. + + *Yves Senn* + * Add `config.action_mailer.show_previews` configuration option. This config option can be used to enable the mail preview in environments diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 048f2ddc35..d9b88ec608 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -649,7 +649,22 @@ module ActionMailer # mail.attachments[0] # => Mail::Part (first attachment) # def attachments - @_message.attachments + if @_mail_was_called + LateAttachmentsProxy.new(@_message.attachments) + else + @_message.attachments + end + end + + class LateAttachmentsProxy < SimpleDelegator + def inline; _raise_error end + def []=(_name, _content); _raise_error end + + private + def _raise_error + raise RuntimeError, "Can't add attachments after `mail` was called.\n" \ + "Make sure to use `attachments[]=` before calling `mail`." + end end # The main method that creates the message and renders the email templates. There are diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 229ded8e04..6116d1e29f 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -232,6 +232,45 @@ class BaseTest < ActiveSupport::TestCase end end + test "adding attachments after mail was called raises exception" do + class LateAttachmentMailer < ActionMailer::Base + def welcome + mail body: "yay", from: "welcome@example.com", to: "to@example.com" + attachments['invoice.pdf'] = 'This is test File content' + end + end + + e = assert_raises(RuntimeError) { LateAttachmentMailer.welcome } + assert_match(/Can't add attachments after `mail` was called./, e.message) + end + + test "adding inline attachments after mail was called raises exception" do + class LateInlineAttachmentMailer < ActionMailer::Base + def welcome + mail body: "yay", from: "welcome@example.com", to: "to@example.com" + attachments.inline['invoice.pdf'] = 'This is test File content' + end + end + + e = assert_raises(RuntimeError) { LateInlineAttachmentMailer.welcome } + assert_match(/Can't add attachments after `mail` was called./, e.message) + end + + test "accessing attachments works after mail was called" do + class LateAttachmentAccessorMailer < ActionMailer::Base + def welcome + attachments['invoice.pdf'] = 'This is test File content' + mail body: "yay", from: "welcome@example.com", to: "to@example.com" + + unless attachments.map(&:filename) == ["invoice.pdf"] + raise Minitest::Assertion, "Should allow access to attachments" + end + end + end + + assert_nothing_raised { LateAttachmentAccessorMailer.welcome } + end + # Implicit multipart test "implicit multipart" do email = BaseMailer.implicit_multipart -- cgit v1.2.3