Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#26802 closed Bug (fixed)

Sending mails with attachment results in 'bytes' object has no attribute 'encode'

Reported by: Brandl Owned by: nobody
Component: Core (Mail) Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When trying to send a EMail with attachment, it always failed with the following Exception:

 'bytes' object has no attribute 'encode'

At first I thought this is a bug in django-post-office or a duplicate of this bug:

https://code.djangoproject.com/ticket/24623

But I'm running the newest Version of Django(1.9.7) and django-post_office(2.0.7) on Python Python 3.5.1

Here is the trace:

File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/post_office/models.py", line 119, in dispatch
    self.email_message(connection=connection).send()
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/message.py", line 292, in send
    return self.get_connection(fail_silently).send_messages([self])
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/backends/smtp.py", line 107, in send_messages
    sent = self._send(message)
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/backends/smtp.py", line 121, in _send
    message = email_message.message()
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/message.py", line 256, in message
    msg = self._create_message(msg)
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/message.py", line 344, in _create_message
    return self._create_attachments(msg)
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/message.py", line 357, in _create_attachments
    msg.attach(self._create_attachment(*attachment))
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/message.py", line 399, in _create_attachment
    attachment = self._create_mime_attachment(content, mimetype)
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/message.py", line 370, in _create_mime_attachment
    attachment = SafeMIMEText(content, subtype, encoding)
  File "/home/me/Projects/MyProject/.env/lib/python3.5/site-packages/django/core/mail/message.py", line 171, in __init__
    MIMEText.__init__(self, _text, _subtype, None)
  File "/usr/lib64/python3.5/email/mime/text.py", line 34, in __init__
    _text.encode('us-ascii')
AttributeError: 'bytes' object has no attribute 'encode'

As it turns out:

        if _charset == 'utf-8':
            # Unfortunately, Python < 3.5 doesn't support setting a Charset instance
            # as MIMEText init parameter (http://bugs.python.org/issue16324).
            # We do it manually and trigger re-encoding of the payload.
            MIMEText.__init__(self, _text, _subtype, 'utf-8')

instead of

MIMEText.__init__(self, _text, _subtype, None)

fixes the bug, but I'm not sure if that's a clean solution.

Change History (13)

comment:1 by Tim Graham, 8 years ago

Can you please give steps to reproduce? Ideally, a test case for tests/mail/tests.py.

comment:2 by Claude Paroz, 8 years ago

Resolution: needsinfo
Status: newclosed

I'm surprised that bytes are given as text input.

in reply to:  2 comment:3 by Brandl, 8 years ago

from post_office import mail
mail.send(
    'some@email.com',
    'some@email.com',
    subject='My email',
    message='Hi there!',
    attachments={
        'manage.py': 'manage.py',
    },
    priority='now',
)

As I said, the exception happens while using a third-party library, but it's basically a wrapper around the Django core mail functionality:
https://github.com/ui/django-post_office/blob/master/post_office/models.py#L95

comment:4 by Tim Graham, 8 years ago

Could you please give steps to reproduce without a third-party library to confirm that it's not a bug there?

comment:5 by Claude Paroz, 8 years ago

Resolution: needsinfo
Status: closednew

I have been able to reproduce. This happens because post-office attach files with binary content (using FileField.read()) whatever the mime type (see for example how Django distinguish file read mode for text-based attachments in EmailMessage.attach_file).

I have not made my mind yet if and how Django should safeguard against such issues...

Test to reproduce:

    def test_attach_text_as_bytes(self):
        msg = EmailMessage('subject', 'body', 'from@example.com', ['to@example.com'])
        file_path = os.path.join(os.path.dirname(upath(__file__)), 'attachments', 'file.txt')
        with open(file_path, mode='rb') as fh:
            msg.attach('file.txt', fh.read())
        msg.send()

comment:6 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

Accepting for further investigation/thought.

comment:7 by Brandl, 8 years ago

That may be naive, but am I really the first one, who encounters a problem with saving something in a FileField and then sending it per Mail?

So this is where the file gets stored:

if isinstance(content, string_types):
    # `content` is a filename - try to open the file
    opened_file = open(content, 'rb')
    content = File(opened_file)

attachment = Attachment()
attachment.file.save(filename, content=content, save=True)

This how it gets attached to the mail:

msg.attach(attachment.name, attachment.file.read())

At the moment the Django offers two methods of attaching files:

    def attach_file(self, path, mimetype=None):

and

    def attach(self, filename=None, content=None, mimetype=None):

The first one has the benefit of some sophisticated Mimetype guessing, when reading the file, but does not allow to supply a filename, so when the attachment names get hashed, this would destroy the name information. The other one does allow this, but seemingly throws an exception, when provided with a binary file and no Mimetype.

So what are my/our options here? Of course me and others, who encounter the same problem, would need to replicate the functionality of attach_file, but since the implementation is by no means straight forward, I would prefer Django would provide a convenience method for this, maybe even in the Django FileField.

Since the Django FileField also has a path attribute, I could also utilize the attach_file(), method, but then I would love to have a way of overriding the file name.

Or maybe I forgot something more simple than that? Also I wonder, why my quick fix:

MIMEText.__init__(self, _text, _subtype, 'utf-8')

is solving this more complicated problem and in which cases that fix would still cause an exception?

comment:8 by Claude Paroz, 8 years ago

The reason we are using None instead of utf-8 as the charset is that with utf-8, Python will take its default 'utf-8' Charset instance which does BASE64 body encoding. And we don't want to use that body encoding, we use either Quoted-printable or None at all (that was [ececbe77ff573707d8f25084018e66ee07f820fd] and recently [836d475afefecd643d5e7f44027d7209df3ac690]).

We could easily fix this in Python 3.5 by using a real Charset instance instead of 'utf-8'). Python 2.7 is not affected because there is no charset sniffing with encode(). We are left with Python 3.4, which we could special-case and decode the text before passing it to MIMEText.__init__. I'll suggest a patch.

comment:9 by Claude Paroz, 8 years ago

Has patch: set

Unfortunately, I'm just realizing that the fix proposed to Python in http://bugs.python.org/issue16324 only partially fixes the issue, as the Charset instance isn't pass to the set_payload() as is. We'll have to keep the workaround for some more years :-(.
Patch updated.

comment:10 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Claude Paroz <claude@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 04b7b28:

Fixed #26802 -- Prevented crash when attaching bytes as text message

Thanks Tim Graham for the review.

comment:12 by Harris Lapiroff, 7 years ago

What version of Django is this set to be merged into? I ran into this bug on 1.10.3 and 1.10.4.

comment:13 by Claude Paroz, 7 years ago

This was not backported to 1.10, so you'll have to wait for Django 1.11.

Note: See TracTickets for help on using tickets.
Back to Top