Permalink
Browse files

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.
  • Loading branch information...
1 parent 6924f84 commit b8120ab14d695d5ae7392193a6135b5e7cef9c80 @senny senny committed Jul 29, 2014
Showing with 62 additions and 1 deletion.
  1. +7 −0 actionmailer/CHANGELOG.md
  2. +16 −1 actionmailer/lib/action_mailer/base.rb
  3. +39 −0 actionmailer/test/base_test.rb
@@ -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
@@ -649,7 +649,22 @@ def headers(args = nil)
# 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
@@ -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

0 comments on commit b8120ab

Please sign in to comment.