Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#23063 closed Bug (fixed)

send email 1.6.5 OK, 1.7c1 malformed packet in wireshark

Reported by: contact@… Owned by: nobody
Component: Core (Mail) Version: 1.7
Severity: Normal Keywords: send_mail, smtp, malformed, packet
Cc: Florian Apolloner, ainslie.dejour@… Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hello,

I'm working on a 1.7c1 application with Django, and my emails are not sent.

My settings are:
EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend'
EMAIL_HOST = 'ns0.ovh.net'
EMAIL_PORT = 587
EMAIL_HOST_USER = #USER#
EMAIL_HOST_PASSWORD = #PASSWORD#
EMAIL_USE_SSL = False

I tested my account with telnet, and smtplib, and everything goes smooth.

The code I run:

mail.send_mail('susu', 'body body', from_email, [to_email])

Under 1.6.5, the email is sent. Under wireshark, the data package is good.
Under 1.7c1, the email is not sent. I open my wireshark, and look at the data packet. It shows that the packet is malformed.

If I dump via filebased backend, there's no difference except the timestamp and message-id between the two versions.

I copy you my packet dump from wireshark:
http://wikisend.com/download/667092/wireshark_smtp_packet_1-6-5_good.txt
http://wikisend.com/download/569376/wireshark_smtp_packet_1-7-1_bad.txt

Attachments (4)

wireshark_smtp_packet_1-6-5_good.txt (54.3 KB) - added by contact@… 4 years ago.
Good dump of package sniffing from Django 1.6.5
wireshark_smtp_packet_1-7-1_bad.txt (53.5 KB) - added by contact@… 4 years ago.
Dump of package malformed sniffing from Django 1.7 c1
20140727-152731-139769070023792.log (339 bytes) - added by aRkadeFR 4 years ago.
Email to send
packet_malformed_smtp.pcapng (2.9 KB) - added by aRkadeFR 4 years ago.
Dump pcapng malformed packet smtp

Download all attachments as: .zip

Change History (50)

Changed 4 years ago by contact@…

Good dump of package sniffing from Django 1.6.5

Changed 4 years ago by contact@…

Dump of package malformed sniffing from Django 1.7 c1

comment:1 Changed 4 years ago by anonymous

Type: UncategorizedBug

comment:2 Changed 4 years ago by Tim Graham

Could you bisect to determine the commit that introduced the regression?

comment:3 Changed 4 years ago by aRkadeFR

I upgraded my packages via pip and aptitude, and I always have malformed packet on every version of django if the body message = "test body".

If I add some \n in the message, for example: msg = "test message body \n\n " , I dont have anymore the malformed packet, but the SMTP refuse my message.

I'm gonna check again on my other computer if I still can send email with django, and then bisect.

comment:4 Changed 4 years ago by speijnik

Resolution: needsinfo
Status: newclosed

I had a quick look at the dumps.

Any chance you could provide us with the TCP conversation only, without the lower layers included?
I doubt the problem is caused by something lower than TCP. Even though unlikely, if that's the case, the problem might not be related to Django at all.

Also, the files the file backend generated might be useful as well.

Changed 4 years ago by aRkadeFR

Email to send

Changed 4 years ago by aRkadeFR

Dump pcapng malformed packet smtp

comment:5 Changed 4 years ago by aRkadeFR

I just attached a dump pcapng and the dump email in file type from the file backend.
I tried to change of version of django to the 1.6.5 but still bugging on my setup.
Python 3.2 or 3.4. smtplib builtin python I assume.

comment:6 Changed 4 years ago by aRkadeFR

Resolution: needsinfoinvalid

I just changed of smtp server, going to google and not OVH, and the malformed packet is not here anymore.
It works like a charm.
Thanks anyway

comment:7 Changed 4 years ago by Ainslied

Resolution: invalid
Status: closednew
Version: 1.7-rc-11.7

Hello,

First it is the first time I use this tool, so I'm really sorry if I don't use it properly.
Secondly, English is not my natural language, so sorry for the errors.

I reopen this thread because I have exactly the same problem.
OVH is a professional provider massively used is europe and I can't use an other one as google to send my emails.
So changing for gmail is not a well solution that correct the problem.

I have not tested with the 1.6.5 version of django, but I can compare a mail sended with python and the other one sended with django.
There is a problem with django.

I send you here the content of the well formed (sended with python) and the not well formed IMF packet

Well formed :

0000   f4 ca e5 4a a8 1f 9c 4e 36 b6 4f 24 08 00 45 00  ...J...N6.O$..E.
0010   00 fb 4a 28 40 00 80 06 f7 49 c0 a8 01 14 d5 ba  ..J(@....I......
0020   21 14 e9 1b 02 4b 7e 07 0a 74 7e 22 d6 7d 50 18  !....K~..t~".}P.
0030   f9 d1 33 ea 00 00 43 6f 6e 74 65 6e 74 2d 54 79  ..3...Content-Ty
0040   70 65 3a 20 74 65 78 74 2f 70 6c 61 69 6e 3b 20  pe: text/plain; 
0050   63 68 61 72 73 65 74 3d 22 75 73 2d 61 73 63 69  charset="us-asci
0060   69 22 0d 0a 4d 49 4d 45 2d 56 65 72 73 69 6f 6e  i"..MIME-Version
0070   3a 20 31 2e 30 0d 0a 43 6f 6e 74 65 6e 74 2d 54  : 1.0..Content-T
0080   72 61 6e 73 66 65 72 2d 45 6e 63 6f 64 69 6e 67  ransfer-Encoding
0090   3a 20 37 62 69 74 0d 0a 53 75 62 6a 65 63 74 3a  : 7bit..Subject:
00a0   20 48 65 6c 6c 6f 0d 0a 46 72 6f 6d 3a 20 77 65   Hello..From: we
00b0   62 6d 61 73 74 65 72 40 xx xx xx xx xx xx xx xx  bmaster@xxxxxxxx
00c0   xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx  xxxxxxxxxxxxxxxx
00d0   xx xx xx xx 0d 0a 54 6f 3a 20 xx xx xx xx xx xx  xxxx..To: xxxxxx
00e0   xx xx xx xx xx xx xx xx 40 67 6d 61 69 6c 2e 63  xxxxxxxx@gmail.c
00f0   6f 6d 0d 0a 0d 0a 42 6f 64 79 20 67 6f 65 73 20  om....Body goes 
0100   68 65 72 65 0d 0a 2e 0d 0a                       here.....

TEXT ONLY :

JN6O$EJ(@I!K~
t~"}P3Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Hello
From: webmaster@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
To: xxxxxxxxxxxxxx@gmail.com

Body goes here
.

Malformed :

0000   f4 ca e5 4a a8 1f 9c 4e 36 b6 4f 24 08 00 45 00  ...J...N6.O$..E.
0010   01 46 49 e9 40 00 80 06 f7 3d c0 a8 01 14 d5 ba  .FI.@....=......
0020   21 14 e9 14 02 4b 58 c0 97 78 2e c3 40 84 50 18  !....KX..x..@.P.
0030   f9 d1 4a a3 00 00 4d 49 4d 45 2d 56 65 72 73 69  ..J...MIME-Versi
0040   6f 6e 3a 20 31 2e 30 0a 43 6f 6e 74 65 6e 74 2d  on: 1.0.Content-
0050   54 79 70 65 3a 20 74 65 78 74 2f 70 6c 61 69 6e  Type: text/plain
0060   3b 20 63 68 61 72 73 65 74 3d 22 75 74 66 2d 38  ; charset="utf-8
0070   22 0a 43 6f 6e 74 65 6e 74 2d 54 72 61 6e 73 66  ".Content-Transf
0080   65 72 2d 45 6e 63 6f 64 69 6e 67 3a 20 37 62 69  er-Encoding: 7bi
0090   74 0a 53 75 62 6a 65 63 74 3a 20 48 65 6c 6c 6f  t.Subject: Hello
00a0   0a 46 72 6f 6d 3a 20 77 65 62 6d 61 73 74 65 72  .From: webmaster
00b0   40 xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx  @xxxxxxxxxxxxxxx
00c0   xx xx xx xx xx xx xx xx xx xx xx xx xx 0a 54 6f  xxxxxxxxxxxxx.To
00d0   3a 20 xx xx xx xx xx xx xx xx xx xx xx xx xx xx  : xxxxxxxxxxxxxx
00e0   40 67 6d 61 69 6c 2e 63 6f 6d 0a 44 61 74 65 3a  @gmail.com.Date:
00f0   20 57 65 64 2c 20 30 38 20 4f 63 74 20 32 30 31   Wed, 08 Oct 201
0100   34 20 31 36 3a 33 30 3a 32 30 20 2d 30 30 30 30  4 16:30:20 -0000
0110   0a 4d 65 73 73 61 67 65 2d 49 44 3a 20 3c 32 30  .Message-ID: <20
0120   31 34 31 30 30 38 31 36 33 30 32 30 2e 38 34 32  141008163020.842
0130   34 2e 31 38 35 36 35 40 4a 41 58 32 33 30 3e 0a  4.18565@JAX230>.
0140   0a 42 6f 64 79 20 67 6f 65 73 20 68 65 72 65 0d  .Body goes here.
0150   0a 2e 0d 0a                                      ....

TEXT ONLY :

JN6O$EFI@=!KXx.@PJMIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Subject: Hello
From: webmaster@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
To: xxxxxxxxxxxxxx@gmail.com
Date: Wed, 08 Oct 2014 16:30:20 -0000
Message-ID: <20141008163020.8424.18565@JAX230>

Body goes here
.

Thanks !!

comment:8 Changed 4 years ago by Tim Graham

Could you bisect to determine the commit that introduced the issue?

comment:9 Changed 4 years ago by Ainslied

Ok, can you tell me how?
I tried this :
pip uninstall django
pip install django=1.6.5

but I get an error in the brother when I load a http page...
do you have a list of other version I can try whith this method?

comment:10 Changed 4 years ago by Ainslied

It's look like there is a probleme with the end of line :
well formed : endline = 0D0A
malformed : endline = 0A only

...

comment:11 Changed 4 years ago by Ainslied

I tryed pip install django==1.6.7 but I have an error.
python manage.py runserver is ok
but on my brother, I get : "A server error occurred. Please contact the administrator."

comment:12 Changed 4 years ago by Ainslied

Trying 1.6.7, I get this in the console :

Quit the server with CTRL-BREAK.
Traceback (most recent call last):
  File "C:\JA\dev\bin\Python34-win32\lib\site-packages\django\utils\module_loadi
ng.py", line 28, in import_by_path
    attr = getattr(module, class_name)
AttributeError: 'module' object has no attribute 'SessionAuthenticationMiddlewar
e'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\JA\dev\bin\Python34-win32\lib\wsgiref\handlers.py", line 137, in run
    self.result = application(self.environ, self.start_response)
  File "C:\JA\dev\bin\Python34-win32\lib\site-packages\django\contrib\staticfile
s\handlers.py", line 67, in __call__
    return self.application(environ, start_response)
  File "C:\JA\dev\bin\Python34-win32\lib\site-packages\django\core\handlers\wsgi
.py", line 187, in __call__
    self.load_middleware()
  File "C:\JA\dev\bin\Python34-win32\lib\site-packages\django\core\handlers\base
.py", line 45, in load_middleware
    mw_class = import_by_path(middleware_path)
  File "C:\JA\dev\bin\Python34-win32\lib\site-packages\django\utils\module_loadi
ng.py", line 31, in import_by_path
    error_prefix, module_path, class_name))
django.core.exceptions.ImproperlyConfigured: Module "django.contrib.auth.middlew
are" does not define a "SessionAuthenticationMiddleware" attribute/class
[09/Oct/2014 00:14:26] "POST /signup/ HTTP/1.1" 500 59

comment:13 Changed 4 years ago by Tim Graham

SessionAuthenticationMiddleware is new in Django 1.7. You will need to remove it from your settings file when testing on Django 1.6.

To bisect, first install the development version of Django, then follow a tutorial on using git bisect.

comment:14 Changed 4 years ago by Ainslied

ok
I have the same problem with 1.6.7, 1.6.5, 1.6 and 1.5...

An idea why the endline is 0A with django and 0D0A with python?

comment:15 Changed 4 years ago by Ainslied

Editing django\core\mail\backends\smtp.py line 115 :

message.as_bytes() -> message.as_string()

solved the problem.

Now the message seems to respect the "IMF" (internet message format) standards. Wireshark doesn't tag the packet as Malformed and the provider ovh accepts the message and the message is well sended.

In the doc of python, as_string() is used as well..
(see https://docs.python.org/2/library/email-examples.html )

Is there a reason why using as_bytes() here?

comment:16 Changed 4 years ago by Tim Graham

The last commit for that code is [5dfd824d].

comment:17 Changed 4 years ago by Ainslied

Yes, I found the same :
http://gitelephant.cypresslab.net/django/commit/5dfd824d38ec7d1f695494e46d603e89cae68661

So, what do you think? which one is good?

It is sure that here on my local machine, this version using as_bytes() introduces a bug and as_string() corrects it..

comment:18 Changed 4 years ago by Tim Graham

Cc: Florian Apolloner added

Not sure, but looking at that commit it seems like there's some backwards compatibility concerns to switching from bytes to string. I'll add Florian to CC.

comment:19 Changed 4 years ago by Florian Apolloner

message.as_string is broken beyond repair, so we can't use that. Interestingly in my tests both variants produce LF instead of CRLF, so I have yet to figure out where the CRLF is coming from. That said if Google accepts the mail it can't be that bad, so OVH should get their stuff together :þ

comment:20 Changed 4 years ago by Aymeric Augustin

This looks like a duplicate of #23490, which has a more straightforward description and fewer tcpdumps :-)

comment:21 Changed 4 years ago by Florian Apolloner

Does this fix it:

diff --git a/django/core/mail/message.py b/django/core/mail/message.py
index da9891f..961d1a8 100644
--- a/django/core/mail/message.py
+++ b/django/core/mail/message.py
@@ -149,7 +149,7 @@ class MIMEMixin():
             """
             fp = six.BytesIO()
             g = generator.BytesGenerator(fp, mangle_from_=False)
-            g.flatten(self, unixfrom=unixfrom)
+            g.flatten(self, unixfrom=unixfrom, linesep='\r\n')
             return fp.getvalue()
 
 
Last edited 4 years ago by Florian Apolloner (previous) (diff)

comment:22 Changed 4 years ago by Florian Apolloner

What I don't understand is why this is only an issue with Python3, this should be a problem on every Python version -- or did I missread that report somehow.

comment:23 Changed 4 years ago by Florian Apolloner

@augustin:

The easiest way is to inject it when we instanciate the Generator. But I'm wondering if it wouldn't be safer to set it on the message, to avoid the same kind of bugs in other code paths.

Yes, but policy objects are relatively new (as is the linesep argument), so this would only work on python 3.3 onwards. So essentially we'd have to fix it in both instances.

comment:24 Changed 4 years ago by Florian Apolloner

Oh, I found something else: https://bugs.php.net/bug.php?id=15841 -- so for a sendmail backend we should probably use LF instead of CRLF? If that is indeed the case, this might get a bit ugly.

comment:25 Changed 4 years ago by Ainslied

@apollo13 > I can't make the test for the moment (probably in about 10 hours)

Everybody :
It's look like IMF packet require CRLF and only US-ASCII in the header and in the body.
I found an clear explaination of IMF format here : http://www.digitalpreservation.gov/formats/fdd/fdd000393.shtml
and the full documentation is here : http://tools.ietf.org/html/rfc5322
I don't really understand what you have to do if you want send utf8 chars in the body, but anyway, it is not the real issue here.. (the issue is about CRLF not sended)

comment:26 Changed 4 years ago by Ainslied

@appollo13 : can you tell more about this assertion ? :

message.as_string is broken beyond repair

comment:27 Changed 4 years ago by Florian Apolloner

as_string implies text/unicode on python 3. There is no way to know how this should get encoded for wire transfers (especially if you have multipart mime messages). This is also the reason why it broke on python 3 in the first place cause it produced quoted printable mails.

Last edited 4 years ago by Florian Apolloner (previous) (diff)

comment:28 Changed 4 years ago by Ainslied

about the "fix" I have to try (comment:21) adding linesep='\r\n' to flatten

The doc of flatten says :

Optional linesep specifies the line separator character used to terminate lines in the output.
If specified it overrides the value specified by the Generatoror msg‘s policy.

In the doc of email.message, I see the reference to the policy, and in the doc of policy, I can read there is a specific (and new?) policy for SMTP email.policy.SMTP including linesep set to \r\n

Using Python without Django, the msg looks well formated (so with the good policy, or with a good policy hook?), but not with django. I can't really figure out why..

comment:29 Changed 4 years ago by Florian Apolloner

Using Python without Django, the msg looks well formated (so with the good policy, or with a good policy hook?), but not with django. I can't really figure out why..

Can you provide the exact python code you are using? Cause by default the compat policy is used which certainly produces newlines and not CRLF.

comment:30 Changed 4 years ago by Florian Apolloner

Triage Stage: UnreviewedAccepted

comment:31 Changed 4 years ago by Ainslied

Sure, here is the code :

import smtplib
from email.mime.text import MIMEText

msg = MIMEText('Body goes here')
msg['Subject'] = 'Hello'
msg['From'] = 'webmaster@xxxxxxxxxxxxxxxxxxxxxxxxxxxx'
msg['To'] = 'xxxxxxxxxxxxxx@gmail.com'
s = smtplib.SMTP('SSL0.OVH.NET', 587, timeout = 4000)
s.login('webmaster@xxxxxxxxxxxxxxxxxxxxxxxxxxxx', 'xxxxxxxx')
s.sendmail('webmaster@xxxxxxxxxxxxxxxxxxxxxxxxxxxx', ['xxxxxxxxxxxxxx@gmail.com'], msg.as_string())
s.quit()

It seems, I could not use as_string() nor as_byte() as shown in this official exemple :

import smtplib
from email.mime.text import MIMEText

fp = open(textfile, 'rb')
msg = MIMEText(fp.read())
fp.close()

msg['Subject'] = 'The contents of %s' % textfile
msg['From'] = me
msg['To'] = you

s = smtplib.SMTP('localhost')
s.send_message(msg)
s.quit()

-> Anyway the SMTP.sendmail part of the doc seems to be the most important to explain our issue :

SMTP.sendmail(from_addr, to_addrs, msg, ...)
The required arguments are ..., and a message string.
msg may be a string containing characters in the ASCII range, or a byte string.
A string is encoded to bytes using the ascii codec, and lone \r and \n characters are converted to \r\n characters.
A byte string is not modified.

-> A byte string is not modified.

That seems to expain why it works with python and not with the Django code.
So it seems to be not because of the policy but because of the "byte string" transmitted to the sendmail function.

comment:32 Changed 4 years ago by Florian Apolloner

Good catch, I found the relevant snippets in the python 2.7 codebase too, so all we need is to fix bytes on Python3.x for SMTP. Reading the php bug I linked, I think the best solution is to fix this just for the SMTP backend.

comment:33 Changed 4 years ago by Florian Apolloner

New patch:

diff --git a/django/core/mail/backends/smtp.py b/django/core/mail/backends/smtp.py
index 4c41eb8..620168e 100644
--- a/django/core/mail/backends/smtp.py
+++ b/django/core/mail/backends/smtp.py
@@ -120,7 +120,7 @@ class EmailBackend(BaseEmailBackend):
                       for addr in email_message.recipients()]
         message = email_message.message()
         try:
-            self.connection.sendmail(from_email, recipients, message.as_bytes())
+            self.connection.sendmail(from_email, recipients, message.as_bytes(linesep='\r\n'))
         except smtplib.SMTPException:
             if not self.fail_silently:
                 raise
diff --git a/django/core/mail/message.py b/django/core/mail/message.py
index da9891f..63b0014 100644
--- a/django/core/mail/message.py
+++ b/django/core/mail/message.py
@@ -123,7 +123,7 @@ def sanitize_address(addr, encoding):
 
 
 class MIMEMixin():
-    def as_string(self, unixfrom=False):
+    def as_string(self, unixfrom=False, linesep='\n'):
         """Return the entire formatted message as a string.
         Optional `unixfrom' when True, means include the Unix From_ envelope
         header.
@@ -133,13 +133,16 @@ class MIMEMixin():
         """
         fp = six.StringIO()
         g = generator.Generator(fp, mangle_from_=False)
-        g.flatten(self, unixfrom=unixfrom)
+        if six.PY2:
+            g.flatten(self, unixfrom=unixfrom)
+        else:
+            g.flatten(self, unixfrom=unixfrom, linesep=linesep)
         return fp.getvalue()
 
     if six.PY2:
         as_bytes = as_string
     else:
-        def as_bytes(self, unixfrom=False):
+        def as_bytes(self, unixfrom=False, linesep='\n'):
             """Return the entire formatted message as bytes.
             Optional `unixfrom' when True, means include the Unix From_ envelope
             header.
@@ -149,7 +152,7 @@ class MIMEMixin():
             """
             fp = six.BytesIO()
             g = generator.BytesGenerator(fp, mangle_from_=False)
-            g.flatten(self, unixfrom=unixfrom)
+            g.flatten(self, unixfrom=unixfrom, linesep=linesep)
             return fp.getvalue()

This is the most backwardscompatible approach I could come up with.

Last edited 4 years ago by Florian Apolloner (previous) (diff)

comment:34 Changed 4 years ago by Florian Apolloner

Attached a PR https://github.com/django/django/pull/3329 -- let's see what jenkins says.

comment:35 Changed 4 years ago by Ainslied

Is Jenkins the bot?

comment:36 Changed 4 years ago by Ainslied

Do you think it is necessary to add a test and test that the packet is well formed?
I can then try to send a message with a \n in the body.
Normaly this is not a well formed packet.

comment:37 Changed 4 years ago by aRkadeFR

Thanks for the patch @apollo13, it seems OK to me.

@ainslied: Jenkins is the CI (continuous integration tests platform)

comment:38 Changed 4 years ago by Ainslied

Thank you for the patch.
I'll try in few hours

comment:39 Changed 4 years ago by Ainslied

Ok, well done, it works fine.. !

sending "Body \n goes here" works well too.. :
both python and django send "0d 0a" for this "\n" in the body

I tried also this : "äâùèBody \n goes here"
and it works well.

Thank you !!!

comment:40 Changed 4 years ago by Ainslied

Cc: ainslie.dejour@… added

comment:41 Changed 4 years ago by Collin Anderson

In answer to your question earlier, I bet a test would be much appreciated.

Last edited 4 years ago by Collin Anderson (previous) (diff)

comment:42 Changed 4 years ago by Ainslied

Hi collinanderson.

You mean a method in a test.py ?
I just begin in python and django but it is something I can think about for a bit later yes (when I ll be a little more experimented)..

comment:43 Changed 4 years ago by Florian Apolloner

I updated the PR with a test, but it is a bit ugly -- the whole smtplib module is a bit annoying ;)

comment:44 Changed 4 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:45 Changed 4 years ago by Florian Apolloner <florian@…>

Resolution: fixed
Status: newclosed

In 8d789449c7861b8cf8b10d244f26d9d354989aaf:

Fixed #23063 -- Convert \n and \r to \r\n when using the SMTP backend as per RFC.

comment:46 Changed 4 years ago by Florian Apolloner <florian@…>

In bc13a08f89b4a9b7980588b013f358b2721c42a6:

[1.7.x] Fixed #23063 -- Convert \n and \r to \r\n when using the SMTP backend as per RFC.

Backport of 8d789449c7861b8cf8b10d244f26d9d354989aaf from master.

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