#13433 closed (fixed)
EmailMessage mangles body text
Reported by: | Leo Shklovskii | Owned by: | Leo Shklovskii |
---|---|---|---|
Component: | Core (Mail) | Version: | 1.2-beta |
Severity: | Keywords: | mangle_from_ | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
This is a fun one. If I do the following code in Django:
from django.core.mail import EmailMessage message = EmailMessage('blah', 'From puppies','bob@hope.com', ['bob@hope.com']) message.send()
The message that appears on the other end has this in the body:
>From puppies
A From
at the beginning of any lines in the body gets a >
prepended to it.
Digging deep into Python's innards reveals that this is a somewhat esoteric protection in case you're writing out Unix mailbox files. The specific issue is here: http://docs.python.org/release/2.6.2/library/email.generator.html#email.generator.Generator and involves the mangle_from_
flag.
The hugely more likely case is that if you're using Django's EmailMessage
class you're sending emails rather than writing Unix mailbox files and are running into this bug that way.
One proposed solution would be to override the __str()__
methods on django.core.mail.SafeMIMEText
and django.core.mail.SafeMIMEMultipart
. That method by default calls as_string()
which in turn creates a Generator
with mangle_from_
set to True
- see email.message.Message
. If the implementation in the Django classes looked something like this, it would work fine:
def __str__(self): from email.generator import Generator fp = StringIO() g = Generator(fp, mangle_from_ = False) g.flatten(self, unixfrom=self.unixfrom) return fp.getvalue()
Happy to do patches/test/docs as appropriate but I'd like to have a core committer take a look at it and bless the approach (or punt the bug to 1.3/wontfix/etc...)
Attachments (1)
Change History (15)
comment:1 by , 15 years ago
milestone: | 1.2 |
---|---|
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 15 years ago
Triage Stage: | Design decision needed → Accepted |
---|
It's not pretty, but I really don't see much of an alternative, either. My only question is whether overriding str is the right place; given that as_string() is the affected interface, it seem like that should be the method that is fixed. However, that's just based on an initial inspection; I'm willing to be convinced that I'm wrong on that one.
comment:3 by , 15 years ago
Looking at the Django email backend code, they all seem to call as_string() directly (not implicitly converting via str), so I'd agree with russellm here that as_string() would be the correct place to fix this bug.
comment:4 by , 15 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Patch attached. A few notes:
- I've cleaned up some text & comment formatting in the doctest.
- Some of the doctests were not running as intended since they did not use the
<BLANKLINE>
syntax to indicate blank lines in the intended output (added in Python 2.4, so fine for Django)
comment:5 by , 15 years ago
milestone: | → 1.3 |
---|
comment:7 by , 14 years ago
@ramiro converting all of the tests to unit tests seems like a fairly high bar for a bug fix like this. What about setting this one to ready to checkin (if that's the only issue) and then opening up a new bug to convert these doctests into unit tests?
comment:8 by , 14 years ago
@Leo - one of the major accomplishments of Django 1.3 has been to purge doctests from our codebase. We're not going to start introducing new doctests into our codebase at this point.
comment:9 by , 14 years ago
Sorry, I'm unclear, are you asking me to convert all of the existing doctests to unit tests? or just the new one that I wrote?
comment:13 by , 14 years ago
Just a heads up, sorry for not having a full patch and documentation for this but I just noticed.
This patch breaks in Python 2.4.
In 2.4:
from email.generator import Generator
have to be:
from email.Generator import Generator
in order to work.
The problem seems to be that all modules have been renamed according to PEP 8 standards in version 4 of the mail package in Python, as stated here: http://docs.python.org/library/email.html#package-history although it says to be compatible with Python 2.3 to 2.5.
We're in a feature freeze, so the 1.2 milestone is reserved for targeted features or bugs caused by 1.2 added features.
The issue seems valid (it doesn't seem like we need
mangle_from_
) but I'd bring up the implementation approaches on the django-dev group.