Mailgun: disable non-ASCII attachment filename workaround when not needed

urllib3 v1.25 fixes non-ASCII filenames in multipart form data to be
RFC 5758 compliant by default, so our earlier workaround is no longer
needed. Disable the workaround if we detect that Requests is using a
fixed version of urllib3.

Closes #157

Author: medmunds

.travis.yml

@@ -52,6 +52,9 @@ matrix:
     - { env: TOXENV=django22-py37-none, python: 3.7 }
     - { env: TOXENV=django22-py37-amazon_ses, python: 3.7 }
     - { env: TOXENV=django22-py37-sparkpost, python: 3.7 }
+    # Test some specific older package versions
+    - { env: TOXENV=django111-py27-all-old_urllib3, python: 3.7 }
+    - { env: TOXENV=django22-py37-all-old_urllib3, python: 3.7 }
 
   allow_failures:
     - env: TOXENV=djangoMaster-py37-all

CHANGELOG.rst

@@ -38,6 +38,14 @@ Features
   (Thanks `@anstosa`_.)
 
 
+Other
+~~~~~
+
+* **Mailgun:** Disable Anymail's workaround for a Requests/urllib3 issue with non-ASCII
+  attachment filenames when a newer version of urllib3--which fixes the problem--is
+  installed. (Workaround was added in Anymail v4.3; fix appears in urllib3 v1.25.)
+
+
 v6.1
 ----
 

anymail/backends/mailgun.py

@@ -11,6 +11,21 @@
 from .base_requests import AnymailRequestsBackend, RequestsPayload
 
 
+# Feature-detect whether requests (urllib3) correctly uses RFC 7578 encoding for non-
+# ASCII filenames in Content-Disposition headers. (This was fixed in urllib3 v1.25.)
+# See MailgunPayload.get_request_params for info (and a workaround on older versions).
+# (Note: when this workaround is removed, please also remove the "old_urllib3" tox envs.)
+def is_requests_rfc_5758_compliant():
+    request = Request(method='POST', url='https://www.example.com',
+                      files=[('attachment', (u'\N{NOT SIGN}.txt', 'test', 'text/plain'))])
+    prepared = request.prepare()
+    form_data = prepared.body  # bytes
+    return b'filename*=' not in form_data
+
+
+REQUESTS_IS_RFC_7578_COMPLIANT = is_requests_rfc_5758_compliant()
+
+
 class EmailBackend(AnymailRequestsBackend):
     """
     Mailgun API Email Backend
@@ -94,7 +109,7 @@ def get_request_params(self, api_url):
         non_ascii_filenames = [filename
                                for (field, (filename, content, mimetype)) in params["files"]
                                if filename is not None and not isascii(filename)]
-        if non_ascii_filenames:
+        if non_ascii_filenames and not REQUESTS_IS_RFC_7578_COMPLIANT:
             # Workaround https://github.com/requests/requests/issues/4652:
             # Mailgun expects RFC 7578 compliant multipart/form-data, and is confused
             # by Requests/urllib3's improper use of RFC 2231 encoded filename parameters

tests/mock_requests_backend.py

@@ -115,6 +115,13 @@ def get_api_call_auth(self, required=True):
         """Returns the auth sent to the mock ESP API"""
         return self.get_api_call_arg('auth', required)
 
+    def get_api_prepared_request(self):
+        """Returns the PreparedRequest that would have been sent"""
+        (args, kwargs) = self.mock_request.call_args
+        kwargs.pop('timeout', None)  # Session-only param
+        request = requests.Request(**kwargs)
+        return request.prepare()
+
     def assert_esp_not_called(self, msg=None):
         if self.mock_request.called:
             raise AssertionError(msg or "ESP API was called and shouldn't have been")

tests/test_mailgun_backend.py

@@ -182,12 +182,20 @@ def test_unicode_attachment_correctly_decoded(self):
         self.message.send()
 
         # Verify the RFC 7578 compliance workaround has kicked in:
-        data = self.get_api_call_data().decode("utf-8")
+        data = self.get_api_call_data()
+        if isinstance(data, dict):  # workaround not needed or used (but let's double check actual request)
+            workaround = False
+            prepared = self.get_api_prepared_request()
+            data = prepared.body
+        else:
+            workaround = True
+        data = data.decode("utf-8").replace("\r\n", "\n")
         self.assertNotIn("filename*=", data)  # No RFC 2231 encoding
         self.assertIn(u'Content-Disposition: form-data; name="attachment"; filename="Une pièce jointe.html"', data)
 
-        files = self.get_api_call_files(required=False)
-        self.assertFalse(files)  # files should have been moved to formdata body
+        if workaround:
+            files = self.get_api_call_files(required=False)
+            self.assertFalse(files)  # files should have been moved to formdata body
 
     def test_rfc_7578_compliance(self):
         # Check some corner cases in the workaround that undoes RFC 2231 multipart/form-data encoding...
@@ -214,7 +222,12 @@ def test_rfc_7578_compliance(self):
         self.message.attach(u"besked med vedhæftede filer", forwarded_message, "message/rfc822")
         self.message.send()
 
-        data = self.get_api_call_data().decode("utf-8").replace("\r\n", "\n")
+        data = self.get_api_call_data()
+        if isinstance(data, dict):  # workaround not needed or used (but let's double check actual request)
+            prepared = self.get_api_prepared_request()
+            data = prepared.body
+        data = data.decode("utf-8").replace("\r\n", "\n")
+
         # Top-level attachment (in form-data) should have RFC 7578 filename (raw Unicode):
         self.assertIn(
             u'Content-Disposition: form-data; name="attachment"; filename="besked med vedhæftede filer"', data)

tox.ini

@@ -15,6 +15,9 @@ envlist =
     djangoMaster-py{36,37}-all
     # ... then partial installation (limit extras):
     django22-py37-{none,amazon_ses,sparkpost}
+    # ... then older versions of some dependencies:
+    django111-py27-all-old_urllib3
+    django22-py37-all-old_urllib3
 
 [testenv]
 deps =
@@ -23,6 +26,7 @@ deps =
     django21: django~=2.1.0
     django22: django~=2.2.0
     djangoMaster: https://github.com/django/django/tarball/master
+    old_urllib3: urllib3<1.25
     # testing dependencies (duplicates setup.py tests_require, less optional extras):
     mock
 extras =