From 60fbe1e896eb5e20452e1157eca5fd3259bee48e Mon Sep 17 00:00:00 2001 From: medmunds Date: Mon, 10 Jan 2022 15:27:12 -0800 Subject: [PATCH] Fix: treat first text/plain alternative as plaintext body Improve handling of alternative parts and `content_subtype` to match how Django's SMTP backend handles some unusual cases. Change Test backend to support (and record) text/* alternative parts. (But still reject other types of alternatives.) Fixes #252 --- CHANGELOG.rst | 15 +++++++++ anymail/backends/base.py | 37 ++++++++++++++++----- anymail/backends/test.py | 7 +++- tests/test_general_backend.py | 61 +++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b20f3d6..16b9967 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -25,6 +25,20 @@ Release history ^^^^^^^^^^^^^^^ .. This extra heading level keeps the ToC from becoming unmanageably long +vNext +----- + +*Unreleased changes on main branch* + +Fixes +~~~~~ + +* Allow `attach_alternative("content", "text/plain")` in place of setting + an EmailMessage's `body`, and generally improve alternative part + handling for consistency with Django's SMTP EmailBackend. + (Thanks to `@cjsoftuk`_ for reporting the issue.) + + v8.4 ---- @@ -1241,6 +1255,7 @@ Features .. _@anstosa: https://github.com/anstosa .. _@calvin: https://github.com/calvin .. _@chrisgrande: https://github.com/chrisgrande +.. _@cjsoftuk: https://github.com/cjsoftuk .. _@costela: https://github.com/costela .. _@decibyte: https://github.com/decibyte .. _@dominik-lekse: https://github.com/dominik-lekse diff --git a/anymail/backends/base.py b/anymail/backends/base.py index fead8fa..d8c639b 100644 --- a/anymail/backends/base.py +++ b/anymail/backends/base.py @@ -281,9 +281,7 @@ class BasePayload: else: value = converter(value) if value is not UNSET: - if attr == 'body': - setter = self.set_html_body if message.content_subtype == 'html' else self.set_text_body - elif attr == 'from_email': + if attr == 'from_email': setter = self.set_from_email_list elif attr == 'extra_headers': setter = self.process_extra_headers @@ -452,6 +450,18 @@ class BasePayload: # headers is a CaseInsensitiveDict, and is a copy (so is safe to modify) self.unsupported_feature('extra_headers') + def set_body(self, body): + # Interpret message.body depending on message.content_subtype. + # (Subclasses should generally implement set_text_body and set_html_body + # rather than overriding this.) + content_subtype = self.message.content_subtype + if content_subtype == "plain": + self.set_text_body(body) + elif content_subtype == "html": + self.set_html_body(body) + else: + self.add_alternative(body, "text/%s" % content_subtype) + def set_text_body(self, body): raise NotImplementedError("%s.%s must implement set_text_body" % (self.__class__.__module__, self.__class__.__name__)) @@ -461,17 +471,28 @@ class BasePayload: (self.__class__.__module__, self.__class__.__name__)) def set_alternatives(self, alternatives): + # Handle treating first text/{plain,html} alternatives as bodies. + # (Subclasses should generally implement add_alternative + # rather than overriding this.) + has_plain_body = self.message.content_subtype == "plain" and self.message.body + has_html_body = self.message.content_subtype == "html" and self.message.body for content, mimetype in alternatives: - if mimetype == "text/html": - # This assumes that there's at most one html alternative, - # and so it should be the html body. (Most ESPs don't - # support multiple html alternative parts anyway.) + if mimetype == "text/plain" and not has_plain_body: + self.set_text_body(content) + has_plain_body = True + elif mimetype == "text/html" and not has_html_body: self.set_html_body(content) + has_html_body = True else: self.add_alternative(content, mimetype) def add_alternative(self, content, mimetype): - self.unsupported_feature("alternative part with type '%s'" % mimetype) + if mimetype == "text/plain": + self.unsupported_feature("multiple plaintext parts") + elif mimetype == "text/html": + self.unsupported_feature("multiple html parts") + else: + self.unsupported_feature("alternative part with type '%s'" % mimetype) def set_attachments(self, attachments): for attachment in attachments: diff --git a/anymail/backends/test.py b/anymail/backends/test.py index 8df150f..b3e0920 100644 --- a/anymail/backends/test.py +++ b/anymail/backends/test.py @@ -107,7 +107,12 @@ class TestPayload(BasePayload): self.params['html_body'] = body def add_alternative(self, content, mimetype): - self.unsupported_feature("alternative part with type '%s'" % mimetype) + # For testing purposes, we allow all "text/*" alternatives, + # but not any other mimetypes. + if mimetype.startswith('text'): + self.params.setdefault('alternatives', []).append((content, mimetype)) + else: + self.unsupported_feature("alternative part with type '%s'" % mimetype) def add_attachment(self, attachment): self.params.setdefault('attachments', []).append(attachment) diff --git a/tests/test_general_backend.py b/tests/test_general_backend.py index b778845..ebe73bf 100644 --- a/tests/test_general_backend.py +++ b/tests/test_general_backend.py @@ -426,6 +426,67 @@ class SpecialHeaderTests(TestBackendTestCase): self.message.send() +class AlternativePartsTests(TestBackendTestCase): + """Anymail should handle alternative parts consistently with Django's SMTP backend""" + + def test_default_usage(self): + """Body defaults to text/plain, use alternative for html""" + self.message.body = "plain body" + self.message.attach_alternative("html body", "text/html") + self.message.send() + params = self.get_send_params() + self.assertEqual(params['text_body'], "plain body") + self.assertEqual(params['html_body'], "html body") + self.assertNotIn('alternatives', params) + + def test_content_subtype_html(self): + """Change body to text/html, use alternative for plain""" + self.message.content_subtype = "html" + self.message.body = "html body" + self.message.attach_alternative("plain body", "text/plain") + self.message.send() + params = self.get_send_params() + self.assertEqual(params['text_body'], "plain body") + self.assertEqual(params['html_body'], "html body") + self.assertNotIn('alternatives', params) + + def test_attach_plain_and_html(self): + """Use alternatives for both bodies""" + message = AnymailMessage(subject="Subject", from_email="from@example.com", to=["to@example.com"]) + message.attach_alternative("plain body", "text/plain") + message.attach_alternative("html body", "text/html") + message.send() + params = self.get_send_params() + self.assertEqual(params['text_body'], "plain body") + self.assertEqual(params['html_body'], "html body") + self.assertNotIn('alternatives', params) + + def test_additional_plain_part(self): + """Two plaintext bodies""" + # In theory this is supported (e.g., for different languages or charsets), + # though MUAs are unlikely to display anything after the first. + self.message.body = "plain body" + self.message.attach_alternative("second plain body", "text/plain") + self.message.send() + params = self.get_send_params() + self.assertEqual(params['text_body'], "plain body") + self.assertEqual(params['alternatives'], [("second plain body", "text/plain")]) + + def test_exotic_content_subtype(self): + """Change body to text/calendar, use alternatives for plain and html""" + # This is unlikely to work with most ESPs, but we can try to communicate the intent... + # (You probably want an attachment rather than an alternative part.) + self.message.content_subtype = "calendar" + self.message.body = "BEGIN:VCALENDAR..." + self.message.attach_alternative("plain body", "text/plain") + self.message.attach_alternative("html body", "text/html") + self.message.send() + params = self.get_send_params() + self.assertEqual(params['text_body'], "plain body") + self.assertEqual(params['html_body'], "html body") + self.assertEqual(params['alternatives'], [("BEGIN:VCALENDAR...", "text/calendar")]) + + class BatchSendDetectionTestCase(TestBackendTestCase): """Tests shared code to consistently determine whether to use batch send"""