Cleanup: centralize Reply-To header handling; case-insensitive headers

Django allows setting the reply address with either message.reply_to
or message.extra_headers["Reply-To"]. If both are supplied, the extra
headers version takes precedence. (See EmailMessage.message().)

Several Anymail backends had duplicate logic to handle conflicting
properties. Move that logic into the base Payload.

(Also prepares for common handling of extra_headers['From'], later.)

Related changes:

* Use CaseInsensitiveDict for processing extra_headers.
  This is potentially a breaking change, but any code that was trying
  to send multiple headers differing only in case was likely already
  broken. (Email header field names are case-insensitive, per RFC-822.)

* Handle CaseInsensitiveDict in RequestsPayload.serialize_json().
  (Several backends had duplicate code for handling this, too.)

* Fixes SparkPost backend, which had been incorrectly treating
  message.reply_to and message.extra_headers['Reply-To'] differently.
This commit is contained in:
medmunds
2018-02-26 12:02:48 -08:00
parent ec0ee336a2
commit bd9d92f5a0
9 changed files with 75 additions and 33 deletions

View File

@@ -4,6 +4,7 @@ import six
from django.conf import settings from django.conf import settings
from django.core.mail.backends.base import BaseEmailBackend from django.core.mail.backends.base import BaseEmailBackend
from django.utils.timezone import is_naive, get_current_timezone, make_aware, utc from django.utils.timezone import is_naive, get_current_timezone, make_aware, utc
from requests.structures import CaseInsensitiveDict
from ..exceptions import AnymailCancelSend, AnymailError, AnymailUnsupportedFeature, AnymailRecipientsRefused from ..exceptions import AnymailCancelSend, AnymailError, AnymailUnsupportedFeature, AnymailRecipientsRefused
from ..message import AnymailStatus from ..message import AnymailStatus
@@ -268,6 +269,8 @@ class BasePayload(object):
setter = self.set_html_body if message.content_subtype == 'html' else self.set_text_body setter = self.set_html_body if message.content_subtype == 'html' else self.set_text_body
elif attr == 'from_email': elif attr == 'from_email':
setter = self.set_from_email_list setter = self.set_from_email_list
elif attr == 'extra_headers':
setter = self.process_extra_headers
else: else:
# AttributeError here? Your Payload subclass is missing a set_<attr> implementation # AttributeError here? Your Payload subclass is missing a set_<attr> implementation
setter = getattr(self, 'set_%s' % attr) setter = getattr(self, 'set_%s' % attr)
@@ -278,6 +281,21 @@ class BasePayload(object):
raise AnymailUnsupportedFeature("%s does not support %s" % (self.esp_name, feature), raise AnymailUnsupportedFeature("%s does not support %s" % (self.esp_name, feature),
email_message=self.message, payload=self, backend=self.backend) email_message=self.message, payload=self, backend=self.backend)
def process_extra_headers(self, headers):
# Handle some special-case headers, and pass the remainder to set_extra_headers.
# (Subclasses shouldn't need to override this.)
headers = CaseInsensitiveDict(headers) # email headers are case-insensitive per RFC-822 et seq
reply_to = headers.pop('Reply-To', None)
if reply_to:
# message.extra_headers['Reply-To'] will override message.reply_to
# (because the extra_headers attr is processed after reply_to).
# This matches the behavior of Django's EmailMessage.message().
self.set_reply_to(parse_address_list([reply_to]))
if headers:
self.set_extra_headers(headers)
# #
# Attribute validators # Attribute validators
# #
@@ -380,6 +398,7 @@ class BasePayload(object):
self.unsupported_feature('reply_to') self.unsupported_feature('reply_to')
def set_extra_headers(self, headers): def set_extra_headers(self, headers):
# headers is a CaseInsensitiveDict, and is a copy (so is safe to modify)
self.unsupported_feature('extra_headers') self.unsupported_feature('extra_headers')
def set_text_body(self, body): def set_text_body(self, body):

View File

@@ -1,7 +1,7 @@
import json import json
import requests import requests
# noinspection PyUnresolvedReferences from requests.structures import CaseInsensitiveDict
from six.moves.urllib.parse import urljoin from six.moves.urllib.parse import urljoin
from anymail.utils import get_anymail_setting from anymail.utils import get_anymail_setting
@@ -156,8 +156,16 @@ class RequestsPayload(BasePayload):
Useful for implementing serialize_data in a subclass, Useful for implementing serialize_data in a subclass,
""" """
try: try:
return json.dumps(data) return json.dumps(data, default=self._json_default)
except TypeError as err: except TypeError as err:
# Add some context to the "not JSON serializable" message # Add some context to the "not JSON serializable" message
raise AnymailSerializationError(orig_err=err, email_message=self.message, raise AnymailSerializationError(orig_err=err, email_message=self.message,
backend=self.backend, payload=self) backend=self.backend, payload=self)
@staticmethod
def _json_default(o):
"""json.dump default function that handles some common Payload data types"""
if isinstance(o, CaseInsensitiveDict): # used for headers
return dict(o)
raise TypeError("Object of type '%s' is not JSON serializable" %
o.__class__.__name__)

View File

@@ -1,7 +1,5 @@
import re import re
from requests.structures import CaseInsensitiveDict
from ..exceptions import AnymailRequestsAPIError from ..exceptions import AnymailRequestsAPIError
from ..message import AnymailRecipientStatus from ..message import AnymailRecipientStatus
from ..utils import get_anymail_setting from ..utils import get_anymail_setting
@@ -149,12 +147,9 @@ class PostmarkPayload(RequestsPayload):
self.data["ReplyTo"] = reply_to self.data["ReplyTo"] = reply_to
def set_extra_headers(self, headers): def set_extra_headers(self, headers):
header_dict = CaseInsensitiveDict(headers)
if 'Reply-To' in header_dict:
self.data["ReplyTo"] = header_dict.pop('Reply-To')
self.data["Headers"] = [ self.data["Headers"] = [
{"Name": key, "Value": value} {"Name": key, "Value": value}
for key, value in header_dict.items() for key, value in headers.items()
] ]
def set_text_body(self, body): def set_text_body(self, body):

View File

@@ -7,7 +7,7 @@ from requests.structures import CaseInsensitiveDict
from .base_requests import AnymailRequestsBackend, RequestsPayload from .base_requests import AnymailRequestsBackend, RequestsPayload
from ..exceptions import AnymailConfigurationError, AnymailRequestsAPIError, AnymailWarning from ..exceptions import AnymailConfigurationError, AnymailRequestsAPIError, AnymailWarning
from ..message import AnymailRecipientStatus from ..message import AnymailRecipientStatus
from ..utils import BASIC_NUMERIC_TYPES, get_anymail_setting, timestamp, update_deep, parse_address_list from ..utils import BASIC_NUMERIC_TYPES, get_anymail_setting, timestamp, update_deep
class EmailBackend(AnymailRequestsBackend): class EmailBackend(AnymailRequestsBackend):
@@ -102,14 +102,7 @@ class SendGridPayload(RequestsPayload):
self.ensure_message_id() self.ensure_message_id()
self.build_merge_data() self.build_merge_data()
headers = self.data["headers"] if not self.data["headers"]:
if "Reply-To" in headers:
# Reply-To must be in its own param
reply_to = headers.pop('Reply-To')
self.set_reply_to(parse_address_list([reply_to]))
if len(headers) > 0:
self.data["headers"] = dict(headers) # flatten to normal dict for json serialization
else:
del self.data["headers"] # don't send empty headers del self.data["headers"] # don't send empty headers
return self.serialize_json(self.data) return self.serialize_json(self.data)

View File

@@ -129,8 +129,10 @@ class SendGridPayload(RequestsPayload):
self.data["x-smtpapi"] = self.serialize_json(self.data["x-smtpapi"]) self.data["x-smtpapi"] = self.serialize_json(self.data["x-smtpapi"])
# Serialize extra headers to json: # Serialize extra headers to json:
headers = self.data["headers"] if self.data["headers"]:
self.data["headers"] = self.serialize_json(dict(headers.items())) self.data["headers"] = self.serialize_json(self.data["headers"])
else:
del self.data["headers"]
return self.data return self.data

View File

@@ -3,7 +3,7 @@ from requests.structures import CaseInsensitiveDict
from .base_requests import AnymailRequestsBackend, RequestsPayload from .base_requests import AnymailRequestsBackend, RequestsPayload
from ..exceptions import AnymailRequestsAPIError from ..exceptions import AnymailRequestsAPIError
from ..message import AnymailRecipientStatus from ..message import AnymailRecipientStatus
from ..utils import get_anymail_setting, parse_address_list from ..utils import get_anymail_setting
class EmailBackend(AnymailRequestsBackend): class EmailBackend(AnymailRequestsBackend):
@@ -88,15 +88,8 @@ class SendinBluePayload(RequestsPayload):
def serialize_data(self): def serialize_data(self):
"""Performs any necessary serialization on self.data, and returns the result.""" """Performs any necessary serialization on self.data, and returns the result."""
headers = self.data["headers"] if not self.data['headers']:
if "Reply-To" in headers: del self.data['headers'] # don't send empty headers
# Reply-To must be in its own param
reply_to = headers.pop('Reply-To')
self.set_reply_to(parse_address_list([reply_to]))
if len(headers) > 0:
self.data["headers"] = dict(headers) # flatten to normal dict for json serialization
else:
del self.data["headers"] # don't send empty headers
# SendinBlue use different argument's name if we use template functionality # SendinBlue use different argument's name if we use template functionality
if self.template_id: if self.template_id:
@@ -179,8 +172,7 @@ class SendinBluePayload(RequestsPayload):
self.data['replyTo'] = self.email_object(emails[0]) self.data['replyTo'] = self.email_object(emails[0])
def set_extra_headers(self, headers): def set_extra_headers(self, headers):
for key in headers.keys(): self.data['headers'].update(headers)
self.data['headers'][key] = headers[key]
def set_tags(self, tags): def set_tags(self, tags):
if len(tags) > 0: if len(tags) > 0:

View File

@@ -147,7 +147,7 @@ class SparkPostPayload(BasePayload):
def set_extra_headers(self, headers): def set_extra_headers(self, headers):
if headers: if headers:
self.params['custom_headers'] = headers self.params['custom_headers'] = dict(headers) # convert CaseInsensitiveDict to plain dict for SP lib
def set_text_body(self, body): def set_text_body(self, body):
self.params['text'] = body self.params['text'] = body

View File

@@ -318,3 +318,36 @@ class CatchCommonErrorsTests(TestBackendTestCase):
self.message.reply_to = ugettext_lazy("single-reply-to@example.com") self.message.reply_to = ugettext_lazy("single-reply-to@example.com")
with self.assertRaisesMessage(TypeError, '"reply_to" attribute must be a list or other iterable'): with self.assertRaisesMessage(TypeError, '"reply_to" attribute must be a list or other iterable'):
self.message.send() self.message.send()
def flatten_emails(emails):
return [str(email) for email in emails]
class SpecialHeaderTests(TestBackendTestCase):
"""Anymail should handle special extra_headers the same way Django does"""
def test_reply_to(self):
"""Django allows message.reply_to and message.extra_headers['Reply-To'], and the latter takes precedence"""
self.message.reply_to = ["attr@example.com"]
self.message.extra_headers = {"X-Extra": "extra"}
self.message.send()
params = self.get_send_params()
self.assertEqual(flatten_emails(params['reply_to']), ["attr@example.com"])
self.assertEqual(params['extra_headers'], {"X-Extra": "extra"})
self.message.reply_to = None
self.message.extra_headers = {"Reply-To": "header@example.com", "X-Extra": "extra"}
self.message.send()
params = self.get_send_params()
self.assertEqual(flatten_emails(params['reply_to']), ["header@example.com"])
self.assertEqual(params['extra_headers'], {"X-Extra": "extra"}) # Reply-To no longer there
# If both are supplied, the header wins (to match Django EmailMessage.message() behavior).
# Also, header names are case-insensitive.
self.message.reply_to = ["attr@example.com"]
self.message.extra_headers = {"REPLY-to": "header@example.com", "X-Extra": "extra"}
self.message.send()
params = self.get_send_params()
self.assertEqual(flatten_emails(params['reply_to']), ["header@example.com"])
self.assertEqual(params['extra_headers'], {"X-Extra": "extra"}) # Reply-To no longer there

View File

@@ -135,8 +135,8 @@ class SparkPostBackendStandardEmailTests(SparkPostBackendMockAPITestCase):
self.assertEqual(params['recipients'], ['to1@example.com', 'Also To <to2@example.com>']) self.assertEqual(params['recipients'], ['to1@example.com', 'Also To <to2@example.com>'])
self.assertEqual(params['bcc'], ['bcc1@example.com', 'Also BCC <bcc2@example.com>']) self.assertEqual(params['bcc'], ['bcc1@example.com', 'Also BCC <bcc2@example.com>'])
self.assertEqual(params['cc'], ['cc1@example.com', 'Also CC <cc2@example.com>']) self.assertEqual(params['cc'], ['cc1@example.com', 'Also CC <cc2@example.com>'])
self.assertEqual(params['reply_to'], 'another@example.com')
self.assertEqual(params['custom_headers'], { self.assertEqual(params['custom_headers'], {
'Reply-To': 'another@example.com',
'X-MyHeader': 'my value', 'X-MyHeader': 'my value',
'Message-ID': 'mycustommsgid@example.com'}) 'Message-ID': 'mycustommsgid@example.com'})