sklarsa / django-sendgrid-v5

An implementation of Django's EmailBackend compatible with sendgrid-python v5+
MIT License
323 stars 54 forks source link

UnicodeDecodeError when encoding attachment in Python 2.7 #29

Closed mick88 closed 6 years ago

mick88 commented 6 years ago

I am getting an error:

UnicodeDecodeError: 'ascii' codec can't decode byte 0xd0 in position 0: ordinal not in range(128)
  File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\django\core\mail\message.py", line 348, in send
    return self.get_connection(fail_silently).send_messages([self])
  File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\sendgrid_backend\mail.py", line 63, in send_messages
    data = self._build_sg_mail(msg)
  File "C:\Users\Mick\Desktop\Projects\django\meg-forms\lib\site-packages\sendgrid_backend\mail.py", line 161, in _build_sg_mail
    content = content.encode()

Relevant code:

attachment.filename = filename
# todo: Read content if stream?
if isinstance(content, str):
    # line 161:
    content = content.encode()
attachment.content = base64.b64encode(content).decode()
attachment.type = mimetype

the attachment is an xls document and the first byte us 0xd0. This crashes sendgrid, because b'\xd0'.encode() produces an error. Ordinarily encode() encodes unicode into a byte string, but in the code above, it only executes for strings which are already bytestrings (Python 2.7) due to the condition, which makes no sense to me. Does it have anything to do with Python 3 compatibility?

Environment: Python 2.7.9 django-sendgrid-v5==0.6.88 sendgrid==5.3.0

sklarsa commented 6 years ago

My gut is that it's a python 2/3 compatibility issue. If you would like to make a PR, I'd be more than happy to review. Please let me know if you plan on doing so, otherwise it will be a few days before I can take a closer look.

mick88 commented 6 years ago

I'll give it a try tomorrow, I've worked with python 2/3 compatible code before, but I used future library.

sklarsa commented 6 years ago

Thanks!! If you want to import __future__, I have no problem with that! I have automated tests that run against various django and python versions so we should be able to check if everything works

On Thu, May 31, 2018 at 12:30 PM, Michal Dabski notifications@github.com wrote:

I'll give it a try tomorrow, I've worked with python 2/3 compatible code before, but I used future library.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sklarsa/django-sendgrid-v5/issues/29#issuecomment-393592575, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1xRZDVIgcyYCJpX1HEcdHRcvJH53VMks5t4BrDgaJpZM4UVTju .

mick88 commented 6 years ago

Replicated the issue in a test, added future library to import Python 3 str. Test passing on my machine. Need to test if it fixes the issue in my project.

mick88 commented 6 years ago

Yes, it fixes my issue on Python 2.7. Noted that tests are failing - fixed by adding future to dev-requirements.txt

mick88 commented 6 years ago

@sklarsa the issue is fixed now. I recommend using future library in other parts of the project to ensure python version compatibility. For instance:

if sys.version_info >= (3.0, 0.0):
    basestring = str

can be replaced with:

from past.builtins import basestring