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.

3 files changed, 62 insertions, 1 deletions

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