From fd558e904e64c9b57bf7044eb1ec94250ec64922 Mon Sep 17 00:00:00 2001 From: Mike Edmunds Date: Tue, 3 Sep 2019 18:04:27 -0700 Subject: [PATCH] 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 --- .travis.yml | 3 +++ CHANGELOG.rst | 8 ++++++++ anymail/backends/mailgun.py | 17 ++++++++++++++++- tests/mock_requests_backend.py | 7 +++++++ tests/test_mailgun_backend.py | 21 +++++++++++++++++---- tox.ini | 4 ++++ 6 files changed, 55 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 31e6cc3..2f0cdae 100644 --- a/.travis.yml +++ b/.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 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 393fcfc..5c16400 100644 --- a/CHANGELOG.rst +++ b/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 ---- diff --git a/anymail/backends/mailgun.py b/anymail/backends/mailgun.py index 68c2f85..eb4720a 100644 --- a/anymail/backends/mailgun.py +++ b/anymail/backends/mailgun.py @@ -11,6 +11,21 @@ from ..utils import get_anymail_setting, rfc2822date 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 @@ class MailgunPayload(RequestsPayload): 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 diff --git a/tests/mock_requests_backend.py b/tests/mock_requests_backend.py index c6cbe30..96ae76c 100644 --- a/tests/mock_requests_backend.py +++ b/tests/mock_requests_backend.py @@ -115,6 +115,13 @@ class RequestsBackendMockAPITestCase(SimpleTestCase, AnymailTestMixin): """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") diff --git a/tests/test_mailgun_backend.py b/tests/test_mailgun_backend.py index db19a68..48d5af0 100644 --- a/tests/test_mailgun_backend.py +++ b/tests/test_mailgun_backend.py @@ -182,12 +182,20 @@ class MailgunBackendStandardEmailTests(MailgunBackendMockAPITestCase): 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 @@ class MailgunBackendStandardEmailTests(MailgunBackendMockAPITestCase): 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) diff --git a/tox.ini b/tox.ini index 8dd5c65..6e1803e 100644 --- a/tox.ini +++ b/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 =