diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3fd83e1..673cd04 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -45,10 +45,14 @@ Features Fixes ~~~~~ +* **Mailgun:** Raise `AnymailUnsupportedFeature` error when attempting to send an + attachment without a filename (or inline attachment without a *Content-ID*), because + Mailgun silently drops these attachments from the sent message. (See + `docs `__. + Thanks `@costela`_ for identifying this undocumented Mailgun API limitation.) * **Mailgun:** Fix problem where attachments with non-ASCII filenames would be lost. (Works around Requests/urllib3 issue encoding multipart/form-data filenames in a way - that isn't RFC 7578 compliant. Thanks to `@decibyte`_ for catching the problem and - `@costela`_ for identifying the related Mailgun API oddity.) + that isn't RFC 7578 compliant. Thanks to `@decibyte`_ for catching the problem.) v4.2 diff --git a/anymail/backends/mailgun.py b/anymail/backends/mailgun.py index 7bb2c39..9696b56 100644 --- a/anymail/backends/mailgun.py +++ b/anymail/backends/mailgun.py @@ -188,9 +188,13 @@ class MailgunPayload(RequestsPayload): if attachment.inline: field = "inline" name = attachment.cid + if not name: + self.unsupported_feature("inline attachments without Content-ID") else: field = "attachment" name = attachment.name + if not name: + self.unsupported_feature("attachments without filenames") self.files.append( (field, (name, attachment.content, attachment.mimetype)) ) diff --git a/docs/esps/mailgun.rst b/docs/esps/mailgun.rst index b3fb727..0122fa2 100644 --- a/docs/esps/mailgun.rst +++ b/docs/esps/mailgun.rst @@ -147,12 +147,41 @@ values directly to Mailgun. You can use any of the (non-file) parameters listed Limitations and quirks ---------------------- +**Attachments require filenames** + Mailgun has an `undocumented API requirement`_ that every attachment must have a + filename. Attachments with missing filenames are silently dropped from the sent + message. Similarly, every inline attachment must have a :mailheader:`Content-ID`. + + To avoid unexpected behavior, Anymail will raise an + :exc:`~anymail.exceptions.AnymailUnsupportedFeature` error if you attempt to send + a message through Mailgun with any attachments that don't have filenames (or inline + attachments that don't have :mailheader:`Content-ID`\s). + + Ensure your attachments have filenames by using + :class:`message.attach_file(filename) `, + :class:`message.attach(content, filename="...") `, + or if you are constructing your own MIME objects to attach, + :meth:`mimeobj.add_header("Content-Disposition", "attachment", filename="...") `. + + Ensure your inline attachments have Content-IDs by using Anymail's + :ref:`inline image helpers `, or if you are constructing your own MIME objects, + :meth:`mimeobj.add_header("Content-ID", "...") ` and + :meth:`mimeobj.add_header("Content-Disposition", "inline") `. + + .. versionchanged:: 4.3 + + Earlier Anymail releases did not check for these cases, and attachments + without filenames/Content-IDs would be ignored by Mailgun without notice. + **Envelope sender uses only domain** Anymail's :attr:`~anymail.message.AnymailMessage.envelope_sender` is used to select your Mailgun :ref:`sender domain `. For obvious reasons, only the domain portion applies. You can use anything before the @, and it will be ignored. +.. _undocumented API requirement: + https://mailgun.uservoice.com/forums/156243-feature-requests/suggestions/35668606 + .. _mailgun-templates: diff --git a/tests/test_mailgun_backend.py b/tests/test_mailgun_backend.py index 93da086..c303c3f 100644 --- a/tests/test_mailgun_backend.py +++ b/tests/test_mailgun_backend.py @@ -138,16 +138,19 @@ class MailgunBackendStandardEmailTests(MailgunBackendMockAPITestCase): png_content = b"PNG\xb4 pretend this is the contents of a png file" self.message.attach(filename="test.png", content=png_content) - # Should work with a MIMEBase object (also tests no filename)... + # Should work with a MIMEBase object... pdf_content = b"PDF\xb4 pretend this is valid pdf data" mimeattachment = MIMEBase('application', 'pdf') mimeattachment.set_payload(pdf_content) + mimeattachment["Content-Disposition"] = 'attachment; filename="custom filename"' # Mailgun requires filename self.message.attach(mimeattachment) # And also with an message/rfc822 attachment forwarded_email_content = sample_email_content() forwarded_email = message_from_bytes(forwarded_email_content) rfcmessage = MIMEBase("message", "rfc822") + rfcmessage.add_header("Content-Disposition", "attachment", + filename="forwarded message") # Mailgun requires filename rfcmessage.attach(forwarded_email) self.message.attach(rfcmessage) @@ -157,13 +160,15 @@ class MailgunBackendStandardEmailTests(MailgunBackendMockAPITestCase): self.assertEqual(len(attachments), 4) self.assertEqual(attachments[0], ('test.txt', text_content, 'text/plain')) self.assertEqual(attachments[1], ('test.png', png_content, 'image/png')) # type inferred from filename - self.assertEqual(attachments[2], (None, pdf_content, 'application/pdf')) # no filename + self.assertEqual(attachments[2], ("custom filename", pdf_content, 'application/pdf')) # Email messages can get a bit changed with respect to whitespace characters # in headers, without breaking the message, so we tolerate that: - self.assertEqual(attachments[3][0], None) + self.assertEqual(attachments[3][0], "forwarded message") self.assertEqualIgnoringHeaderFolding( attachments[3][1], - b'Content-Type: message/rfc822\nMIME-Version: 1.0\n\n' + forwarded_email_content) + b'Content-Type: message/rfc822\nMIME-Version: 1.0\n' + + b'Content-Disposition: attachment; filename="forwarded message"\n' + + b'\n' + forwarded_email_content) self.assertEqual(attachments[3][2], 'message/rfc822') # Make sure the image attachment is not treated as embedded: @@ -218,6 +223,23 @@ class MailgunBackendStandardEmailTests(MailgunBackendMockAPITestCase): self.assertIn("Testing for filename*=utf-8''problems", data) self.assertIn(u"The attached message should have an attachment named 'vedhæftet fil.txt'", data) + def test_attachment_missing_filename(self): + """Mailgun silently drops attachments without filenames, so warn the caller""" + mimeattachment = MIMEBase('application', 'pdf') + mimeattachment.set_payload(b"PDF\xb4 pretend this is valid pdf data") + mimeattachment["Content-Disposition"] = 'attachment' + self.message.attach(mimeattachment) + + with self.assertRaisesMessage(AnymailUnsupportedFeature, "attachments without filenames"): + self.message.send() + + def test_inline_missing_contnet_id(self): + mimeattachment = MIMEImage(b"imagedata", "x-fakeimage") + mimeattachment["Content-Disposition"] = 'inline; filename="fakeimage.txt"' + self.message.attach(mimeattachment) + with self.assertRaisesMessage(AnymailUnsupportedFeature, "inline attachments without Content-ID"): + self.message.send() + def test_embedded_images(self): image_filename = SAMPLE_IMAGE_FILENAME image_path = sample_image_path(image_filename) @@ -247,6 +269,7 @@ class MailgunBackendStandardEmailTests(MailgunBackendMockAPITestCase): self.message.attach_file(image_path) # option 1: attach as a file image = MIMEImage(image_data) # option 2: construct the MIMEImage and attach it directly + image.set_param("filename", "custom-filename", "Content-Disposition") # Mailgun requires filenames self.message.attach(image) self.message.send() @@ -254,7 +277,7 @@ class MailgunBackendStandardEmailTests(MailgunBackendMockAPITestCase): attachments = [value for (field, value) in files if field == 'attachment'] self.assertEqual(len(attachments), 2) self.assertEqual(attachments[0], (image_filename, image_data, 'image/png')) - self.assertEqual(attachments[1], (None, image_data, 'image/png')) # name unknown -- not attached as file + self.assertEqual(attachments[1], ("custom-filename", image_data, 'image/png')) # Make sure the image attachments are not treated as inline: inlines = [value for (field, value) in files if field == 'inline'] self.assertEqual(len(inlines), 0)