Improve inline-image handling

* Add filename param to attach_inline_image

* Add attach_inline_image_file function
  (parallels EmailMessage.attach and attach_file)

* Use `Content-Disposition: inline` to decide
  whether an attachment should be handled inline
  (whether or not it's an image, and whether or not
  it has a Content-ID)

* Stop conflating filename and Content-ID, for
  ESPs that allow both. (Solves problem where
  Google Inbox was displaying inline images
  as attachments when sent through SendGrid.)

Author: medmunds

README.rst

@@ -107,7 +107,7 @@ or any other supported ESP where you see "mailgun":
    .. code-block:: python
 
         from django.core.mail import EmailMultiAlternatives
-        from anymail.message import attach_inline_image
+        from anymail.message import attach_inline_image_file
 
         msg = EmailMultiAlternatives(
             subject="Please activate your account",
@@ -117,7 +117,7 @@ or any other supported ESP where you see "mailgun":
             reply_to=["Helpdesk <[email protected]>"])
 
         # Include an inline image in the html:
-        logo_cid = attach_inline_image(msg, open("logo.jpg", "rb").read())
+        logo_cid = attach_inline_image_file(msg, "/path/to/logo.jpg")
         html = """<img alt="Logo" src="cid:{logo_cid}">
                   <p>Please <a href="http://example.com/activate">activate</a>
                   your account</p>""".format(logo_cid=logo_cid)

anymail/message.py

@@ -1,4 +1,6 @@
 from email.mime.image import MIMEImage
+from email.utils import unquote
+import os
 
 from django.core.mail import EmailMessage, EmailMultiAlternatives, make_msgid
 
@@ -28,23 +30,37 @@ def __init__(self, *args, **kwargs):
         # noinspection PyArgumentList
         super(AnymailMessageMixin, self).__init__(*args, **kwargs)
 
-    def attach_inline_image(self, content, subtype=None, idstring="img", domain=None):
+    def attach_inline_image_file(self, path, subtype=None, idstring="img", domain=None):
+        """Add inline image from file path to an EmailMessage, and return its content id"""
+        assert isinstance(self, EmailMessage)
+        return attach_inline_image_file(self, path, subtype, idstring, domain)
+
+    def attach_inline_image(self, content, filename=None, subtype=None, idstring="img", domain=None):
         """Add inline image and return its content id"""
         assert isinstance(self, EmailMessage)
-        return attach_inline_image(self, content, subtype, idstring, domain)
+        return attach_inline_image(self, content, filename, subtype, idstring, domain)
 
 
 class AnymailMessage(AnymailMessageMixin, EmailMultiAlternatives):
     pass
 
 
-def attach_inline_image(message, content, subtype=None, idstring="img", domain=None):
+def attach_inline_image_file(message, path, subtype=None, idstring="img", domain=None):
+    """Add inline image from file path to an EmailMessage, and return its content id"""
+    filename = os.path.basename(path)
+    with open(path, 'rb') as f:
+        content = f.read()
+    return attach_inline_image(message, content, filename, subtype, idstring, domain)
+
+
+def attach_inline_image(message, content, filename=None, subtype=None, idstring="img", domain=None):
     """Add inline image to an EmailMessage, and return its content id"""
-    cid = make_msgid(idstring, domain)  # Content ID per RFC 2045 section 7 (with <...>)
+    content_id = make_msgid(idstring, domain)  # Content ID per RFC 2045 section 7 (with <...>)
     image = MIMEImage(content, subtype)
-    image.add_header('Content-ID', cid)
+    image.add_header('Content-Disposition', 'inline', filename=filename)
+    image.add_header('Content-ID', content_id)
     message.attach(image)
-    return cid[1:-1]  # Without <...>, for use as the <img> tag src
+    return unquote(content_id)  # Without <...>, for use as the <img> tag src
 
 
 ANYMAIL_STATUSES = [

anymail/tests/test_mailgun_backend.py

@@ -14,7 +14,7 @@
 from django.utils.timezone import get_fixed_timezone, override as override_current_timezone
 
 from anymail.exceptions import AnymailAPIError, AnymailSerializationError, AnymailUnsupportedFeature
-from anymail.message import attach_inline_image
+from anymail.message import attach_inline_image_file
 
 from .mock_requests_backend import RequestsBackendMockAPITestCase
 from .utils import sample_image_content, sample_image_path, SAMPLE_IMAGE_FILENAME, AnymailTestMixin
@@ -161,8 +161,11 @@ def test_unicode_attachment_correctly_decoded(self):
         self.assertEqual(len(attachments), 1)
 
     def test_embedded_images(self):
-        image_data = sample_image_content()  # Read from a png file
-        cid = attach_inline_image(self.message, image_data)
+        image_filename = SAMPLE_IMAGE_FILENAME
+        image_path = sample_image_path(image_filename)
+        image_data = sample_image_content(image_filename)
+
+        cid = attach_inline_image_file(self.message, image_path)
         html_content = '<p>This has an <img src="cid:%s" alt="inline" /> image.</p>' % cid
         self.message.attach_alternative(html_content, "text/html")
 

anymail/tests/test_mailgun_integration.py

@@ -12,8 +12,7 @@
 from anymail.exceptions import AnymailAPIError
 from anymail.message import AnymailMessage
 
-from .utils import sample_image_content, AnymailTestMixin
-
+from .utils import AnymailTestMixin, sample_image_path
 
 MAILGUN_TEST_API_KEY = os.getenv('MAILGUN_TEST_API_KEY')
 MAILGUN_TEST_DOMAIN = os.getenv('MAILGUN_TEST_DOMAIN')
@@ -105,7 +104,7 @@ def test_all_options(self):
         )
         message.attach("attachment1.txt", "Here is some\ntext for you", "text/plain")
         message.attach("attachment2.csv", "ID,Name\n1,3", "text/csv")
-        cid = message.attach_inline_image(sample_image_content(), domain=MAILGUN_TEST_DOMAIN)
+        cid = message.attach_inline_image_file(sample_image_path(), domain=MAILGUN_TEST_DOMAIN)
         message.attach_alternative(
             "<div>This is the <i>html</i> body <img src='cid:%s'></div>" % cid,
             "text/html")

anymail/tests/test_sendgrid_backend.py

@@ -16,7 +16,7 @@
 from django.utils.timezone import get_fixed_timezone, override as override_current_timezone
 
 from anymail.exceptions import AnymailAPIError, AnymailSerializationError, AnymailUnsupportedFeature
-from anymail.message import attach_inline_image
+from anymail.message import attach_inline_image_file
 
 from .mock_requests_backend import RequestsBackendMockAPITestCase
 from .utils import sample_image_content, sample_image_path, SAMPLE_IMAGE_FILENAME, AnymailTestMixin
@@ -209,8 +209,11 @@ def test_unicode_attachment_correctly_decoded(self):
                          ('Une pièce jointe.html', '<p>\u2019</p>', 'text/html'))
 
     def test_embedded_images(self):
-        image_data = sample_image_content()  # Read from a png file
-        cid = attach_inline_image(self.message, image_data)
+        image_filename = SAMPLE_IMAGE_FILENAME
+        image_path = sample_image_path(image_filename)
+        image_data = sample_image_content(image_filename)
+
+        cid = attach_inline_image_file(self.message, image_path)  # Read from a png file
         html_content = '<p>This has an <img src="cid:%s" alt="inline" /> image.</p>' % cid
         self.message.attach_alternative(html_content, "text/html")
 
@@ -219,11 +222,10 @@ def test_embedded_images(self):
         self.assertEqual(data['html'], html_content)
 
         files = self.get_api_call_files()
-        filename = cid  # (for now)
         self.assertEqual(files, {
-            'files[%s]' % filename: (filename, image_data, "image/png"),
+            'files[%s]' % image_filename: (image_filename, image_data, "image/png"),
         })
-        self.assertEqual(data['content[%s]' % filename], cid)
+        self.assertEqual(data['content[%s]' % image_filename], cid)
 
     def test_attached_images(self):
         image_filename = SAMPLE_IMAGE_FILENAME

anymail/tests/test_sendgrid_integration.py

@@ -10,8 +10,7 @@
 from anymail.exceptions import AnymailAPIError
 from anymail.message import AnymailMessage
 
-from .utils import sample_image_content, AnymailTestMixin
-
+from .utils import AnymailTestMixin, sample_image_path
 
 SENDGRID_TEST_API_KEY = os.getenv('SENDGRID_TEST_API_KEY')
 
@@ -75,16 +74,14 @@ def test_all_options(self):
         )
         message.attach("attachment1.txt", "Here is some\ntext for you", "text/plain")
         message.attach("attachment2.csv", "ID,Name\n1,Amy Lina", "text/csv")
-        cid = message.attach_inline_image(sample_image_content())
+        cid = message.attach_inline_image_file(sample_image_path())
         message.attach_alternative(
             "<p><b>HTML:</b> with <a href='http://example.com'>link</a>"
             "and image: <img src='cid:%s'></div>" % cid,
             "text/html")
 
         message.send()
         self.assertEqual(message.anymail_status.status, {'queued'})  # SendGrid always queues
-        message_id = message.anymail_status.message_id
-        print(message_id)
 
     @override_settings(ANYMAIL_SENDGRID_API_KEY="Hey, that's not an API key!")
     def test_invalid_api_key(self):

anymail/utils.py

@@ -2,7 +2,7 @@
 from base64 import b64encode
 from datetime import datetime
 from email.mime.base import MIMEBase
-from email.utils import parseaddr, formatdate
+from email.utils import formatdate, parseaddr, unquote
 from time import mktime
 
 import six
@@ -100,12 +100,12 @@ class Attachment(object):
     """A normalized EmailMessage.attachments item with additional functionality
 
     Normalized to have these properties:
-    name: attachment filename; may be empty string
+    name: attachment filename; may be None
     content: bytestream
     mimetype: the content type; guessed if not explicit
     inline: bool, True if attachment has a Content-ID header
-    content_id: for inline, the Content-ID (*with* <>)
-    cid: for inline, the Content-ID *without* <>
+    content_id: for inline, the Content-ID (*with* <>); may be None
+    cid: for inline, the Content-ID *without* <>; may be empty string
     """
 
     def __init__(self, attachment, encoding):
@@ -121,11 +121,12 @@ def __init__(self, attachment, encoding):
             self.name = attachment.get_filename()
             self.content = attachment.get_payload(decode=True)
             self.mimetype = attachment.get_content_type()
-            # Treat image attachments that have content ids as inline:
-            if attachment.get_content_maintype() == "image" and attachment["Content-ID"] is not None:
+
+            if get_content_disposition(attachment) == 'inline':
                 self.inline = True
-                self.content_id = attachment["Content-ID"]  # including the <...>
-                self.cid = self.content_id[1:-1]  # without the <, >
+                self.content_id = attachment["Content-ID"]  # probably including the <...>
+                if self.content_id is not None:
+                    self.cid = unquote(self.content_id)  # without the <, >
         else:
             (self.name, self.content, self.mimetype) = attachment
 
@@ -145,6 +146,18 @@ def b64content(self):
         return b64encode(content).decode("ascii")
 
 
+def get_content_disposition(mimeobj):
+    """Return the message's content-disposition if it exists, or None.
+
+    Backport of py3.5 :func:`~email.message.Message.get_content_disposition`
+    """
+    value = mimeobj.get('content-disposition')
+    if value is None:
+        return None
+    # _splitparam(value)[0].lower() :
+    return str(value).partition(';')[0].strip().lower()
+
+
 def get_anymail_setting(setting, default=UNSET, allow_bare=False):
     """Returns a Django Anymail setting.
 

docs/esps/mandrill.rst

@@ -88,14 +88,14 @@ Changes to settings
   the values from :setting:`ANYMAIL_SEND_DEFAULTS`.
 
 ``MANDRILL_SUBACCOUNT``
-  Use :attr:`esp_extra` in :setting:`ANYMAIL_MANDRILL_SEND_DEFAULTS`:
+  Use :setting:`ANYMAIL_MANDRILL_SEND_DEFAULTS`:
 
     .. code-block:: python
 
         ANYMAIL = {
             ...
             "MANDRILL_SEND_DEFAULTS": {
-                "esp_extra": {"subaccount": "<your subaccount>"}
+                "subaccount": "<your subaccount>"
             }
         }
 
@@ -149,3 +149,17 @@ Changes to EmailMessage attributes
   to your code. In the future, the Mandrill-only attributes
   will be moved into the
   :attr:`~anymail.message.AnymailMessage.esp_extra` dict.
+
+**Inline images**
+  Djrill (incorrectly) used the presence of a :mailheader:`Content-ID`
+  header to decide whether to treat an image as inline. Anymail
+  looks for :mailheader:`Content-Disposition: inline`.
+
+  If you were constructing MIMEImage inline image attachments
+  for your Djrill messages, in addition to setting the Content-ID,
+  you should also add::
+
+      image.add_header('Content-Disposition', 'inline')
+
+  Or better yet, use Anymail's new :ref:`inline-images`
+  helper functions to attach your inline images.

docs/sending/anymail_additions.rst

@@ -182,7 +182,15 @@ ESP send options (AnymailMessage)
     :class:`AnymailMessageMixin` objects. Unlike the attributes above,
     they can't be used on an arbitrary :class:`~django.core.mail.EmailMessage`.)
 
-    .. method:: attach_inline_image(content, subtype=None, idstring="img", domain=None)
+    .. method:: attach_inline_image_file(path, subtype=None, idstring="img", domain=None)
+
+        Attach an inline (embedded) image to the message and return its :mailheader:`Content-ID`.
+
+        This calls :func:`attach_inline_image_file` on the message. See :ref:`inline-images`
+        for details and an example.
+
+
+    .. method:: attach_inline_image(content, filename=None, subtype=None, idstring="img", domain=None)
 
         Attach an inline (embedded) image to the message and return its :mailheader:`Content-ID`.
 
@@ -303,9 +311,17 @@ ESP send status
 Inline images
 -------------
 
-Anymail includes a convenience function to simplify attaching inline images to email.
+Anymail includes convenience functions to simplify attaching inline images to email.
+
+These functions work with *any* Django :class:`~django.core.mail.EmailMessage` --
+they're not specific to Anymail email backends. You can use them with messages sent
+through Django's SMTP backend or any other that properly supports MIME attachments.
+
+(Both functions are also available as convenience methods on Anymail's
+:class:`~anymail.message.AnymailMessage` and :class:`~anymail.message.AnymailMessageMixin`
+classes.)
 
-.. function:: attach_inline_image(message, content, subtype=None, idstring="img", domain=None)
+.. function:: attach_inline_image_file(message, path, subtype=None, idstring="img", domain=None)
 
     Attach an inline (embedded) image to the message and return its :mailheader:`Content-ID`.
 
@@ -315,23 +331,20 @@ Anymail includes a convenience function to simplify attaching inline images to e
     .. code-block:: python
 
         from django.core.mail import EmailMultiAlternatives
-        from anymail.message import attach_inline_image
-
-        # read image content -- be sure to open the file in binary mode:
-        with f = open("path/to/picture.jpg", "rb"):
-            raw_image_data = f.read()
+        from anymail.message import attach_inline_image_file
 
         message = EmailMultiAlternatives( ... )
-        cid = attach_inline_image(message, raw_image_data)
+        cid = attach_inline_image_file(message, 'path/to/picture.jpg')
         html = '... <img alt="Picture" src="cid:%s"> ...' % cid
-        message.attach_alternative(html, "text/html")
+        message.attach_alternative(html, 'text/html')
 
         message.send()
 
 
     `message` must be an :class:`~django.core.mail.EmailMessage` (or subclass) object.
 
-    `content` must be the binary image data (e.g., read from a file).
+    `path` must be the pathname to an image file. (Its basename will also be used as the
+    attachment's filename, which may be visible in some email clients.)
 
     `subtype` is an optional MIME :mimetype:`image` subtype, e.g., `"png"` or `"jpg"`.
     By default, this is determined automatically from the content.
@@ -342,14 +355,23 @@ Anymail includes a convenience function to simplify attaching inline images to e
     (But be aware the default `domain` can leak your server's local hostname
     in the resulting email.)
 
-    This function works with *any* Django :class:`~django.core.mail.EmailMessage` --
-    it's not specific to Anymail email backends. You can use it with messages sent
-    through Django's SMTP backend or any other that properly supports MIME attachments.
 
-    (This function is also available as the
-    :meth:`~anymail.message.AnymailMessage.attach_inline_image` method
-    on Anymail's :class:`~anymail.message.AnymailMessage` and
-    :class:`~anymail.message.AnymailMessageMixin` classes.)
+.. function:: attach_inline_image(message, content, filename=None, subtype=None, idstring="img", domain=None)
+
+    This is a version of :func:`attach_inline_image_file` that accepts raw
+    image data, rather than reading it from a file.
+
+    `message` must be an :class:`~django.core.mail.EmailMessage` (or subclass) object.
+
+    `content` must be the binary image data
+
+    `filename` is an optional `str` that will be used as as the attachment's
+    filename -- e.g., `"picture.jpg"`. This may be visible in email clients that
+    choose to display the image as an attachment as well as making it available
+    for inline use (this is up to the email client). It should be a base filename,
+    without any path info.
+
+    `subtype`, `idstring` and `domain` are as described in :func:`attach_inline_image_file`
 
 
 .. _send-defaults:

docs/sending/django_email.rst

@@ -73,14 +73,20 @@ will send.
 
 .. rubric:: Inline images
 
-If your message has any image attachments with :mailheader:`Content-ID` headers,
-Anymail will tell your ESP to treat them as inline images rather than ordinary
-attached files.
-
-You can construct an inline image attachment yourself with Python's
-:class:`python:email.mime.image.MIMEImage`, or you can use the convenience
-function :func:`~message.attach_inline_image` included with
-Anymail. See :ref:`inline-images` in the "Anymail additions" section.
+If your message has any attachments with :mailheader:`Content-Disposition: inline`
+headers, Anymail will tell your ESP to treat them as inline rather than ordinary
+attached files. If you want to reference an attachment from an `<img>` in your
+HTML source, the attachment also needs a :mailheader:`Content-ID` header.
+
+Anymail's comes with :func:`~message.attach_inline_image` and
+:func:`~message.attach_inline_image_file` convenience functions that
+do the right thing. See :ref:`inline-images` in the "Anymail additions" section.
+
+(If you prefer to do the work yourself, Python's :class:`~email.mime.image.MIMEImage`
+and :meth:`~email.message.Message.add_header` should be helpful.)
+
+Even if you mark an attachment as inline, some email clients may decide to also
+display it as an attachment. This is largely outside your control.
 
 
 .. _message-headers: