Fixed #24623 -- Fixed EmailMessage.attach_file() with text files on Python 3.

Thanks tkrapp for the report and Tim Graham for the review.

8 files changed, 63 insertions, 3 deletions

diff --git a/django/core/mail/message.py b/django/core/mail/message.py
index 9092cce9c1..8725900ed1 100644
--- a/django/core/mail/message.py
+++ b/django/core/mail/message.py
@@ -308,10 +308,36 @@ class EmailMessage(object):
             self.attachments.append((filename, content, mimetype))
 
     def attach_file(self, path, mimetype=None):
-        """Attaches a file from the filesystem."""
+        """
+        Attaches a file from the filesystem.
+
+        The mimetype will be set to the DEFAULT_ATTACHMENT_MIME_TYPE if it is
+        not specified and cannot be guessed or (PY3 only) if it suggests
+        text/* for a binary file.
+        """
         filename = os.path.basename(path)
-        with open(path, 'rb') as f:
-            content = f.read()
+        if not mimetype:
+            mimetype, _ = mimetypes.guess_type(filename)
+            if not mimetype:
+                mimetype = DEFAULT_ATTACHMENT_MIME_TYPE
+        basetype, subtype = mimetype.split('/', 1)
+        read_mode = 'r' if basetype == 'text' else 'rb'
+        content = None
+
+        with open(path, read_mode) as f:
+            try:
+                content = f.read()
+            except UnicodeDecodeError:
+                # If mimetype suggests the file is text but it's actually
+                # binary, read() will raise a UnicodeDecodeError on Python 3.
+                pass
+
+        # If the previous read in text mode failed, try binary mode.
+        if content is None:
+            with open(path, 'rb') as f:
+                content = f.read()
+                mimetype = DEFAULT_ATTACHMENT_MIME_TYPE
+
         self.attach(filename, content, mimetype)
 
     def _create_message(self, msg):
diff --git a/tests/mail/attachments/file.png b/tests/mail/attachments/file.png
new file mode 100644
index 0000000000..85225ca1ae
--- /dev/null
+++ b/tests/mail/attachments/file.png
diff --git a/tests/mail/attachments/file.txt b/tests/mail/attachments/file.txt
new file mode 100644
index 0000000000..b884222da2
--- /dev/null
+++ b/tests/mail/attachments/file.txt
@@ -0,0 +1 @@
+django/django
\ No newline at end of file
diff --git a/tests/mail/attachments/file_png b/tests/mail/attachments/file_png
new file mode 100644
index 0000000000..85225ca1ae
--- /dev/null
+++ b/tests/mail/attachments/file_png
diff --git a/tests/mail/attachments/file_png.txt b/tests/mail/attachments/file_png.txt
new file mode 100644
index 0000000000..85225ca1ae
--- /dev/null
+++ b/tests/mail/attachments/file_png.txt
diff --git a/tests/mail/attachments/file_txt b/tests/mail/attachments/file_txt
new file mode 100644
index 0000000000..b884222da2
--- /dev/null
+++ b/tests/mail/attachments/file_txt
@@ -0,0 +1 @@
+django/django
\ No newline at end of file
diff --git a/tests/mail/attachments/file_txt.png b/tests/mail/attachments/file_txt.png
new file mode 100644
index 0000000000..b884222da2
--- /dev/null
+++ b/tests/mail/attachments/file_txt.png
@@ -0,0 +1 @@
+django/django
\ No newline at end of file
diff --git a/tests/mail/tests.py b/tests/mail/tests.py
index f6eff3bb8f..a54695eb55 100644
--- a/tests/mail/tests.py
+++ b/tests/mail/tests.py
@@ -2,6 +2,7 @@
 from __future__ import unicode_literals
 
 import asyncore
+import mimetypes
 import os
 import shutil
 import smtpd
@@ -20,6 +21,7 @@ from django.core.mail import (
 from django.core.mail.backends import console, dummy, filebased, locmem, smtp
 from django.core.mail.message import BadHeaderError
 from django.test import SimpleTestCase, override_settings
+from django.utils._os import upath
 from django.utils.encoding import force_bytes, force_text
 from django.utils.six import PY3, StringIO, binary_type
 from django.utils.translation import ugettext_lazy
@@ -305,6 +307,35 @@ class MailTests(HeadersCheckMixin, SimpleTestCase):
         payload = message.get_payload()
         self.assertEqual(payload[1].get_filename(), 'une pièce jointe.pdf')
 
+    def test_attach_file(self):
+        """
+        Test attaching a file against different mimetypes and make sure that
+        a file will be attached and sent properly even if an invalid mimetype
+        is specified.
+        """
+        files = (
+            # filename, actual mimetype
+            ('file.txt', 'text/plain'),
+            ('file.png', 'image/png'),
+            ('file_txt', None),
+            ('file_png', None),
+            ('file_txt.png', 'image/png'),
+            ('file_png.txt', 'text/plain'),
+        )
+        test_mimetypes = ['text/plain', 'image/png', None]
+
+        for basename, real_mimetype in files:
+            for mimetype in test_mimetypes:
+                email = EmailMessage('subject', 'body', 'from@example.com', ['to@example.com'])
+                self.assertEqual(mimetypes.guess_type(basename)[0], real_mimetype)
+                self.assertEqual(email.attachments, [])
+                file_path = os.path.join(os.path.dirname(upath(__file__)), 'attachments', basename)
+                email.attach_file(file_path, mimetype=mimetype)
+                self.assertEqual(len(email.attachments), 1)
+                self.assertIn(basename, email.attachments[0])
+                msgs_sent_num = email.send()
+                self.assertEqual(msgs_sent_num, 1)
+
     def test_dummy_backend(self):
         """
         Make sure that dummy backends returns correct number of sent messages