Mailgun: Better error message for invalid sender domains

Try to catch cases where Mailgun will return HTTP 200-OK with the
text "Mailgun Magnificent API" rather than sending the email.

See #144.

CHANGELOG.rst

@@ -42,6 +42,9 @@ Breaking changes
 Fixes
 ~~~~~
 
+* **Mailgun:** Better error message for invalid sender domains (that caused a cryptic
+  "Mailgun API response 200: OK Mailgun Magnificent API" error in earlier releases).
+
 * **Postmark:** Don't error if a message is sent with only Cc and/or Bcc recipients
   (but no To addresses). Also, `message.anymail_status.recipients[email]` now includes
   send status for Cc and Bcc recipients. (Thanks to `@ailionx`_ for reporting the error.)

anymail/backends/mailgun.py

@@ -1,5 +1,6 @@
 from datetime import datetime
 from email.utils import encode_rfc2231
+from six.moves.urllib.parse import quote_plus
 
 from requests import Request
 
@@ -79,7 +80,12 @@ class MailgunPayload(RequestsPayload):
                                "Either provide valid `from_email`, "
                                "or set `message.esp_extra={'sender_domain': 'example.com'}`",
                                backend=self.backend, email_message=self.message, payload=self)
-        return "%s/messages" % self.sender_domain
+        if '/' in self.sender_domain or '%' in self.sender_domain:
+            # Mailgun returns a cryptic 200-OK "Mailgun Magnificent API" response
+            # if '/' (or even %-encoded '/') confuses it about the API endpoint.
+            raise AnymailError("Invalid sender domain '%s'" % self.sender_domain,
+                               backend=self.backend, email_message=self.message, payload=self)
+        return "%s/messages" % quote_plus(self.sender_domain)
 
     def get_request_params(self, api_url):
         params = super(MailgunPayload, self).get_request_params(api_url)

tests/test_mailgun_backend.py

@@ -20,7 +20,7 @@ from django.test import SimpleTestCase, override_settings, tag
 from django.utils.timezone import get_fixed_timezone, override as override_current_timezone
 
 from anymail.exceptions import (
-    AnymailAPIError, AnymailInvalidAddress,
+    AnymailError, AnymailAPIError, AnymailInvalidAddress,
     AnymailRequestsAPIError, AnymailUnsupportedFeature)
 from anymail.message import attach_inline_image_file
 
@@ -507,6 +507,23 @@ class MailgunBackendAnymailFeatureTests(MailgunBackendMockAPITestCase):
         self.message.send()
         self.assert_esp_called('/mg.example.com/messages')  # setting overrides from_email
 
+    def test_invalid_sender_domain(self):
+        # Make sure we won't construct an invalid API endpoint like
+        # `https://api.mailgun.net/v3/example.com/INVALID/messages`
+        # (which returns a cryptic 200-OK "Mailgun Magnificent API" response).
+        self.message.from_email = "<from@example.com/invalid>"
+        with self.assertRaisesMessage(AnymailError,
+                                      "Invalid sender domain 'example.com/invalid'"):
+            self.message.send()
+
+    @override_settings(ANYMAIL_MAILGUN_SENDER_DOMAIN='example.com%2Finvalid')
+    def test_invalid_sender_domain_setting(self):
+        # See previous test. Also, note that Mailgun unquotes % encoding *before*
+        # extracting the sender domain (so %2f is just as bad as '/')
+        with self.assertRaisesMessage(AnymailError,
+                                      "Invalid sender domain 'example.com%2Finvalid'"):
+            self.message.send()
+
     def test_default_omits_options(self):
         """Make sure by default we don't send any ESP-specific options.