Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#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: UI/UX:


This is a fun one. If I do the following code in Django:

from django.core.mail import EmailMessage
message = EmailMessage('blah', 'From puppies','', [''])

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: 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)

bug13433.patch (7.6 KB) - added by Leo Shklovskii 7 years ago.
Patch against r13040

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by Chris Beaven

milestone: 1.2
Triage Stage: UnreviewedDesign decision needed

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.

comment:2 Changed 7 years ago by Russell Keith-Magee

Triage Stage: Design decision neededAccepted

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 Changed 7 years ago by Greg Wogan-Browne

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.

Changed 7 years ago by Leo Shklovskii

Attachment: bug13433.patch added

Patch against r13040

comment:4 Changed 7 years ago by Leo Shklovskii

Has patch: set
Owner: changed from nobody to Leo Shklovskii
Status: newassigned

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 Changed 7 years ago by Leo Shklovskii

milestone: 1.3

comment:6 Changed 6 years ago by Ramiro Morales

Patch needs improvement: set

Tests need to be converted to unit tests.

comment:7 Changed 6 years ago by Leo Shklovskii

@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 Changed 6 years ago by Russell Keith-Magee

@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 Changed 6 years ago by Leo Shklovskii

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:10 Changed 6 years ago by Greg Wogan-Browne

All the existing tests have already been converted.

comment:11 Changed 6 years ago by Ramiro Morales

Resolution: fixed
Status: assignedclosed

In [15669]:

Fixed #13433 -- Changed default behavior of Django email message wrappers to not mangle lines starting with 'From '. Thanks Leo for the report and patch.

comment:12 Changed 6 years ago by Ramiro Morales

In [15670]:

[1.2.X] Fixed #13433 -- Changed default behavior of Django email message wrappers to not mangle lines starting with 'From '. Thanks Leo for the report and patch.

Backport of [15669] from trunk.

comment:13 in reply to:  12 Changed 6 years ago by dobcey

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: although it says to be compatible with Python 2.3 to 2.5.

Version 0, edited 6 years ago by dobcey (next)

comment:14 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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