Django

Code

Ticket #9367 (closed: fixed)

Opened 1 year ago

Last modified 9 months ago

EmailMultiAlternatives does not properly handle attachments

Reported by: loekje Assigned to: lukeplant
Milestone: Component: django.core.mail
Version: 1.0 Keywords:
Cc: patrys@pld-linux.org, walter+django@wjd.nu Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

EmailMultiAlternatives? does not properly attach (PDF) files. The generated email only contains a multipart/alternative part, whereas in order to additionally support attachments the multipart/alternative part should be wrapped in a multipart/mixed part. This allows for the inclusion of attachments in addition to different alternative views.

I would like to suggest to add this functionality to either the EmailMessage? class or to update the EmailMultiAlternatives? class to have proper support of both alternative views AND attachments. I have included an example on how to support this behavior. It has been tested to work with Outlook and Thunderbird mail clients.

Attachments

mail.py (2.8 kB) - added by loekje on 10/14/08 15:35:02.
Example of EmailAlternativesMessage? with support of including, next to alternative views, attachments
mail.py.patch (5.7 kB) - added by loekje on 10/24/08 14:55:12.

Change History

10/14/08 15:35:02 changed by loekje

  • attachment mail.py added.

Example of EmailAlternativesMessage? with support of including, next to alternative views, attachments

10/14/08 15:39:51 changed by loekje

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

Just to be clear on what the result is of the following code for the EmailMultiAlternatives? implementation in Django 1.0 when invoking the following code:

subject, from_email, to = 'hello', 'from@example.com', 'to@example.com'
text_content = 'This is an important message.'
html_content = '<p>This is an <strong>important</strong> message.</p>'
msg = EmailMultiAlternatives(subject, text_content, from_email, [to])
msg.attach_alternative(html_content, "text/html")
msg.attach_file('/tmp/file.pdf')
msg.send()

A message will be sent, with the two views (typically the HTML view will show up in the email client) and the file.pdf attachment is included in the email. However, it cannot be opened in email client because it does not show any attachments (and why should the email client, it is supposed to be an alternative view...)

(follow-up: ↓ 3 ) 10/14/08 21:53:41 changed by mtredinnick

  • summary changed from EmailMultiAlternatives does not properly attach (PDF) files. to EmailMultiAlternatives does not properly handle attachments.
  • stage changed from Unreviewed to Accepted.

We should make sure that attach_file() works correctly and actually attaches the file even in the subclass. That should be all that's needed here.

I can't work out what your attachment here is trying to do, since it's a whole file. Can you attach a proper patch if you're trying to fix attach_file()?

(in reply to: ↑ 2 ) 10/16/08 16:45:35 changed by anonymous

Replying to mtredinnick:

We should make sure that attach_file() works correctly and actually attaches the file even in the subclass. That should be all that's needed here. I can't work out what your attachment here is trying to do, since it's a whole file. Can you attach a proper patch if you're trying to fix attach_file()?

Ok. I will submit a patch in the coming days.

10/24/08 14:55:12 changed by loekje

  • attachment mail.py.patch added.

10/24/08 14:55:35 changed by loekje

  • has_patch set to 1.

Patch has been attached. In the patch the EmailMultiAlternatives? wraps the multipart/alternative views in a multipart/mixed message that also includes the attachments. The changes made to EmailMessage? are merely for reusing code in the derived EmailMutliAlternatives? class. In the original EmailMultiAlternatives? all attachments (including the alternative views) would be attached as multipart/alternative. As a result of this the alternative views are correctly presented in the browser, however the attachments are also considered to be an alternative view (instead of the real attachment they are).

11/10/08 12:48:02 changed by loekje

  • owner changed from nobody to mtredinnick.

11/26/08 04:01:23 changed by patrys

  • cc set to patrys@pld-linux.org.

12/01/08 22:24:20 changed by mtredinnick

  • owner deleted.

Not sure why this is assigned to me. I'm not going to have time to look at it just yet.

05/06/09 22:46:27 changed by thejaswi_puthraya

  • component changed from Uncategorized to django.core.mail.

06/12/09 03:05:40 changed by wdoekes

  • cc changed from patrys@pld-linux.org to patrys@pld-linux.org, walter+django@wjd.nu.

06/12/09 07:40:40 changed by lukeplant

  • owner set to lukeplant.

06/12/09 08:56:22 changed by lukeplant

It's still quite difficult to see what the patch does, due to the way diff interprets things, but I've had a proper look. attach_file() doesn't work correctly in the subclass because the subclass works by overriding the value of multipart_subtype, which is used by the base class to build the message. So the fundamental way EmailMultiAlternatives works is broken for normal attachments, hence the need for quite a big patch. But it looks like a good patch.

I've added a test, I'll commit shortly. I'm taking it on trust that this is necessary to fix behaviour in Outlook (the previous format of email worked OK in my mail client, but Outlook's behaviour seems sensible as loekje argues).

06/12/09 08:56:40 changed by lukeplant

  • status changed from new to closed.
  • resolution set to fixed.

(In [10983]) Fixed #9367 - EmailMultiAlternatives? does not properly handle attachments.

Thanks to Loek Engels for the bulk of the patch.

06/12/09 09:09:36 changed by lukeplant

(In [10984]) [1.0.X] Fixed #9367 - EmailMultiAlternatives? does not properly handle attachments.

Thanks to Loek Engels for the bulk of the patch.

Backport of r10983 from trunk


Add/Change #9367 (EmailMultiAlternatives does not properly handle attachments)




Change Properties
Action