search

brack3t / Djrill

[INACTIVE/UNMAINTAINED] Djrill is an email backend and new message class for Django users that want to take advantage of the Mandrill transactional email service from MailChimp.
BSD 3-Clause "New" or "Revised" License
319 stars 64 forks source link

Sender email address as ugettext_lazy object breaks Djrill #87

Closed spheroid closed 9 years ago

spheroid commented 9 years ago

Defining sender email as ugettext_lazy object breaks the Djrill backend:

File "/opt/lib/python2.7/site-packages/djrill/mail/backends/djrill.py", line 68, in send_messages
    sent = self._send(message)
File "/opt/lib/python2.7/site-packages/djrill/mail/backends/djrill.py", line 85, in _send
    msg_dict = self._build_standard_message_dict(message)
File "/opt/lib/python2.7/site-packages/djrill/mail/backends/djrill.py", line 141, in _build_standard_message_dict
    sender = sanitize_address(message.from_email, message.encoding)
File "/opt/lib/python2.7/site-packages/django/core/mail/message.py", line 105, in sanitize_address
    nm, addr = addr
ValueError: too many values to unpack

Forcing the value to be evaluated before it is being broken into pieces seems to fix the issue.

medmunds commented 9 years ago

Could you provide an example where this fails? (This change would need a test case.)

Djrill call's Django's own sanitize_address here, in the same way that Django's smtp.EmailBackend does. So if there's a problem with using a lazy string as the to-address, it probably also exists in Django.

Hmm... It looks like Django's sanitize_address already tries to call force_text on the address (and this code hasn't changed since at least Django 1.3) -- so Djrill shouldn't need to force_text as well. Maybe the test for "is-string-like" in Django's sanitize_address is wrong?

medmunds commented 9 years ago

Yeah, this seems to be a bug in Django core mail. I think it would be better fixed there, rather than working around it in Djrill.

Reported as Django #24416. Patch in django/django#4211.

In the meantime, if this is impacting your use of Djrill, I'd suggest calling force_text on the sender address at the point where you're building the EmailMessage object.

spheroid commented 9 years ago

I was looking at that line of code as well, but didn't feel bold enough to blame it on them. :-) I worked around the problem in the meantime by extending the Djrill backend with custom one that does this. I'm unable to control all the places email gets sent (Django error handler for instance) so the code relies on DEFAULT_FROM_EMAIL which needs to be lazy.

Thank you.