Title: mailbox.mbox writes without empty line after each message
Type: behavior Stage: resolved
Components: email, Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: barry, lilydjwg, petri.lehtinen, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2012-06-29 05:47 by lilydjwg, last changed 2012-10-26 11:54 by petri.lehtinen. This issue is now closed.

File name Uploaded Description Edit
mbox lilydjwg, 2012-06-29 14:21 A sample mbox file
mbox2 lilydjwg, 2012-07-04 05:55 An updated sample mbox file
newmail lilydjwg, 2012-07-04 05:57 A new mail to append to the mbox2 lilydjwg, 2012-07-04 05:58 The script to reproduce the issue
issue15222_v1_0001_end_test_msg_template_in_newline.patch petri.lehtinen, 2012-08-25 18:27
issue15222_v1_0002_insert_blank_after_each_message_in_mbox.patch petri.lehtinen, 2012-08-25 18:28
issue15222_v2_0002_insert_blank_after_each_message_in_mbox.patch petri.lehtinen, 2012-09-13 18:07 review
Messages (19)
msg164313 - (view) Author: lilydjwg (lilydjwg) * Date: 2012-06-29 05:47
I find that when mbox writes mails back, it loses the last end-of-line, making appending new mails to the mbox becomes incorrect.

I'm using Linux. In _singlefileMailbox.flush(), when writing the mbox, it loses the last byte ('\n') at the end of each message (because the position from '_toc' is inclusive), and mbox._pre_message_hook() adds it back, but only between two messages. So the last message ends without a '\n'.
msg164324 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-06-29 11:31
I cannot reproduce this on 3.2 or 2.7. My mailboxes always have an ending newline if the last message also has it. Only if the last message doesn't end in a newline, there's no newline in the end of the mbox. Furthermore, if a message doesn't end with a newline, there's no blank line between that message and the next.
msg164329 - (view) Author: lilydjwg (lilydjwg) * Date: 2012-06-29 14:21
I think I got something wrong. It seems that it only happens when the last message is deleted.

I've also made up a sample mbox attached. The code to reproduce:

from mailbox import mbox
mb = mbox('mbox')
del mb[len(mb)-1]
msg164350 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-06-29 19:22
Since there's no empty line between the content of mail 2 and the "From " line of mail 3, the body of mail 2 isn't really terminated by a newline. Reading the message confirms this:

>>> import mailbox
>>> mbox = mailbox.mbox('mbox')
>>> mbox[1].get_payload()
'The mail content of mail 2...'

However, according to RFC 4155 this is a bug, as each message in a mbox must be terminated by an empty line.
msg164368 - (view) Author: lilydjwg (lilydjwg) * Date: 2012-06-30 05:22
Hi, Petri Lehtinen. That's a bug of mine. That mbox is made up by me for demo, not procmail which has the newline. However, a newline is indeed lost and in some way it does corrupt my real mbox.
msg164405 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-06-30 18:21
I'm afraid that's not enough details to do any fixing. I would need a concrete way to reproduce the issue.
msg164638 - (view) Author: lilydjwg (lilydjwg) * Date: 2012-07-04 05:55
Hi, I have figured it out.

The 'mbox2' file should be in correct format now.
Run './' once to delete the last mail.
'cat newmail >> mbox2' to append a new mail to that mbox.
Run './' again.

Now, 'mbox2' doesn't have a newline at the end. (And I think a correct mbox should end with a blank line?)
msg164672 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-07-05 04:26
Well, by appending newmail to the mailbox, you effectively break the next-to-last message by not inserting a newline after it. This can also be achieved by adding a message that doesn't end in a newline to the mbox (using the mailbox module).

The real bug is that mailbox doesn't insert an empty line after each message, which it should do even if the message doesn't end in a newline. This breaks appending new messages (either by cat message >> mbox or by itself), and it might also break mbox parsing in other programs that search for starts of messages using "\n\nFrom ".
msg164829 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-07-07 12:14
MDAs blindly write their message to the end of the user's mail spool file. So, if there's no newline at the end, the mailbox gets corrupted (the new message is "joined" with the previous one).

I tested with Postfix, but this probably happens for other MDA's too.
msg169149 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-08-25 18:27
Attaching two patches.

Patch 1 changes the common message template in mailbox tests to end in a newline. If this wasn't done, more or less all common mailbox tests would have to be special cased for mbox.

Patch 2 adds a single blank line after each message in mbox. A single blank line is added if the added message doesn't end in a newline or ends in a single newline. When read back, a single newline is added to the end o the message if it wasn't there (and this is the reason why patch 1 is needed).
msg169689 - (view) Author: lilydjwg (lilydjwg) * Date: 2012-09-02 10:03
The last patch works fine here, thanks!
msg170063 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-09-08 20:04
Looks like reitveld doesn't recognize your patch format, Petri.

Instead of having the write_line flag, how about doing the newline write in the if body?  A 'lastline=line' at the end of the loop in the buffer loop case is probably less expensive than doing the if test every time through that loop.

Otherwise, it looks good to me.
msg170449 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-09-13 18:07
Attached an updated version of the second patch. David: Is this what you meant? Also, the patch format should now be rietveld compatible :)
msg170451 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-09-13 18:26
Yes, that looks better.

I haven't worked through the rest of the logic in detail, I'll trust your months of cogitation and the tests on that part.
msg171308 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-09-25 19:08
New changeset 6451be0e374a by Petri Lehtinen in branch '2.7':
#15222: test_mailbox: End message template in a newline

New changeset d903d4981e33 by Petri Lehtinen in branch '2.7':
#15222: Insert blank line after each message in mbox mailboxes

New changeset 8adac3f03372 by Petri Lehtinen in branch '3.2':
#15222: test_mailbox: End message template in a newline

New changeset f7615ee43318 by Petri Lehtinen in branch '3.2':
#15222: Insert blank line after each message in mbox mailboxes

New changeset 4b626ccace1a by Petri Lehtinen in branch 'default':
#15222: Merge 3.2
msg171309 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-09-25 19:09
Fixed, thanks!
msg171328 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-09-26 04:44
New changeset a87ab9b0c9e5 by Petri Lehtinen in branch '2.7':
#15222: Fix a test failure on Windows
msg173835 - (view) Author: lilydjwg (lilydjwg) * Date: 2012-10-26 11:29
My system has updated to Python 3.3.0 and this bugs me again :-(
I don't see the changes in Python 3.3.0. So when will this be merged into 3.3?
msg173836 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-10-26 11:54
The fix was committed when 3.3.0 was in release candidate mode, and wasn't considered important enough to be included in 3.3.0 anymore. It will be included in 3.3.1, which is to be released next month.
Date User Action Args
2012-10-26 11:54:09petri.lehtinensetmessages: + msg173836
2012-10-26 11:29:57lilydjwgsetmessages: + msg173835
2012-09-26 04:44:29python-devsetmessages: + msg171328
2012-09-25 19:09:46petri.lehtinensetstatus: open -> closed
messages: + msg171309

keywords: - needs review
resolution: fixed
stage: patch review -> resolved
2012-09-25 19:08:06python-devsetnosy: + python-dev
messages: + msg171308
2012-09-13 18:26:13r.david.murraysetmessages: + msg170451
2012-09-13 18:07:58petri.lehtinensetfiles: + issue15222_v2_0002_insert_blank_after_each_message_in_mbox.patch
2012-09-13 18:07:47petri.lehtinensetfiles: - issue15222_v2_0002_insert_blank_after_each_message_in_mbox.patch
2012-09-13 18:07:05petri.lehtinensetfiles: + issue15222_v2_0002_insert_blank_after_each_message_in_mbox.patch

messages: + msg170449
2012-09-08 20:04:34r.david.murraysetmessages: + msg170063
2012-09-02 10:03:27lilydjwgsetmessages: + msg169689
2012-08-25 18:28:24petri.lehtinensetkeywords: + needs review
files: + issue15222_v1_0002_insert_blank_after_each_message_in_mbox.patch
stage: patch review
2012-08-25 18:27:59petri.lehtinensetfiles: + issue15222_v1_0001_end_test_msg_template_in_newline.patch
keywords: + patch
messages: + msg169149
2012-07-07 12:14:25petri.lehtinensetmessages: + msg164829
2012-07-05 04:26:26petri.lehtinensetnosy: + barry, r.david.murray
messages: + msg164672
components: + email
2012-07-04 05:58:28lilydjwgsetfiles: +
2012-07-04 05:57:59lilydjwgsetfiles: + newmail
2012-07-04 05:55:56lilydjwgsetfiles: + mbox2
status: pending -> open
messages: + msg164638
2012-06-30 18:21:45petri.lehtinensetstatus: open -> pending

messages: + msg164405
2012-06-30 05:22:22lilydjwgsetmessages: + msg164368
2012-06-29 19:22:23petri.lehtinensettitle: mailbox.mbox writes without end-of-line at the file end. -> mailbox.mbox writes without empty line after each message
messages: + msg164350
versions: + Python 2.7, Python 3.3
2012-06-29 14:21:24lilydjwgsetfiles: + mbox
status: pending -> open
messages: + msg164329
2012-06-29 11:31:23petri.lehtinensetstatus: open -> pending
nosy: + petri.lehtinen
messages: + msg164324

2012-06-29 05:47:05lilydjwgcreate