FIX: Notification emails with attachments are incorrectly structured

Two behaviors in the mail gem collide:

 1. Attachments are added as extra parts at the top level,
 2. When there are both text and html parts, the content type is set to
    'multipart/alternative'.

Since attachments aren't alternative renderings, for emails that contain
attachments and both html and text parts, some coercing is necessary.
This commit is contained in:
Daniel Waterworth 2020-03-12 10:31:16 +00:00
parent 1b8793e7a4
commit 59578dfc5b
2 changed files with 54 additions and 0 deletions

View File

@ -270,6 +270,50 @@ module Email
) )
end end
end end
fix_parts_after_attachments!
end
#
# Two behaviors in the mail gem collide:
#
# 1. Attachments are added as extra parts at the top level,
# 2. When there are both text and html parts, the content type is set
# to 'multipart/alternative'.
#
# Since attachments aren't alternative renderings, for emails that contain
# attachments and both html and text parts, some coercing is necessary.
#
# When there are alternative rendering and attachments, this method causes
# the top level to be 'multipart/mixed' and puts the html and text parts
# into a nested 'multipart/alternative' part.
#
# Due to mail gem magic, @message.text_part and @message.html_part still
# refer to the same objects.
#
def fix_parts_after_attachments!
has_attachments = @message.attachments.present?
has_alternative_renderings =
@message.html_part.present? && @message.text_part.present?
if has_attachments && has_alternative_renderings
@message.content_type = "multipart/mixed"
html_part = @message.html_part
@message.html_part = nil
text_part = @message.text_part
@message.text_part = nil
content = Mail::Part.new do
content_type "multipart/alternative"
part html_part
part text_part
end
@message.parts.unshift(content)
end
end end
def header_value(name) def header_value(name)

View File

@ -412,6 +412,16 @@ describe Email::Sender do
expect(message.attachments.map(&:filename)) expect(message.attachments.map(&:filename))
.to contain_exactly(*[small_pdf, csv_file].map(&:original_filename)) .to contain_exactly(*[small_pdf, csv_file].map(&:original_filename))
end end
it "structures the email as a multipart/mixed with a multipart/alternative first part" do
SiteSetting.email_total_attachment_size_limit_kb = 10_000
Email::Sender.new(message, :valid_type).send
expect(message.content_type).to start_with("multipart/mixed")
expect(message.parts.size).to eq(4)
expect(message.parts[0].content_type).to start_with("multipart/alternative")
expect(message.parts[0].parts.size).to eq(2)
end
end end
context 'with a deleted post' do context 'with a deleted post' do