classification
Title: mailbox and email errors
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: r.david.murray Nosy List: SilentGhost, georg.brandl, r.david.murray, sdaoden
Priority: release blocker Keywords: patch

Created on 2011-02-04 10:39 by sdaoden, last changed 2011-02-12 07:25 by georg.brandl. This issue is now closed.

Files
File name Uploaded Description Edit
email_header.patch sdaoden, 2011-02-04 11:48
fp_mailbox.py sdaoden, 2011-02-05 12:09
mailbox_cleanup.patch r.david.murray, 2011-02-11 00:28
mailbox_cleanup2.patch r.david.murray, 2011-02-11 17:38
Messages (27)
msg127880 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 10:39
Following Issue 9124 discussion: it took longer than i thought, but now i could rework my thing and got errors.  I've also tried Lib/test/test_mailbox.py, and that produces 0xA errors and 0xA failures.  I'll attach the entire tracebacks and test output as email_mbox.txt.
My local Py3K repo is at changeset HG:33774ca03c96 (SVN:r88332)
Here are only the exceptions:

==========

| [Opening mailbox "OLD.sdaoden.mbox" as mbox
| * Box contains 8 messages
|   * Dispatching message 1
ERROR: failed to handle box "OLD.sdaoden.mbox"
  Traceback (most recent call last):
    File "./s-postman.py", line 1078, in _walk
      self._do_mailbox(mailbox)
    File "./s-postman.py", line 1088, in _do_mailbox
      Ticket.process_message(msg)
    File "./s-postman.py", line 987, in process_message
      print(msg)
    File "usr/lib/python3.2/email/message.py", line 152, in __str__
      return self.as_string()
    File "usr/lib/python3.2/email/message.py", line 167, in as_string
      g.flatten(self, unixfrom=unixfrom)
    File "usr/lib/python3.2/email/generator.py", line 88, in flatten
      self._write(msg)
    File "usr/lib/python3.2/email/generator.py", line 141, in _write
      self._write_headers(msg)
    File "usr/lib/python3.2/email/generator.py", line 176, in _write_headers
      self.write(header.encode(linesep=self._NL)+self._NL)
    File "usr/lib/python3.2/email/header.py", line 317, in encode
      formatter.feed(lines[0], charset)
  Exception: IndexError: list index out of range
Continue [yY - else no]? n
Exit due to errors as above

===========

PANIC: Box test.mdir: message-add failed, mails may be lost
       FIXME about fetch modus which saves the stuff somewhere
  Traceback (most recent call last):
    File "./s-postman.py", line 739, in add_ticket
      mailbox.add(ticket.message)
    File "usr/lib/python3.2/mailbox.py", line 269, in add
      self._dump_message(message, tmp_file)
    File "usr/lib/python3.2/mailbox.py", line 215, in _dump_message
      gen.flatten(message)
    File "usr/lib/python3.2/email/generator.py", line 88, in flatten
      self._write(msg)
    File "usr/lib/python3.2/email/generator.py", line 141, in _write
      self._write_headers(msg)
    File "usr/lib/python3.2/email/generator.py", line 373, in _write_headers
      self.write(header.encode(linesep=self._NL)+self._NL)
    File "usr/lib/python3.2/email/header.py", line 317, in encode
      formatter.feed(lines[0], charset)
  Exception: IndexError: list index out of range

==========

> Changing Ticket.process_message() to:
>    def process_message(msg):
>        print("Message-Type: ", msg.__name__)

ERROR: failed to handle box "src/s-postman/OLD.sdaoden.mbox"
  Traceback (most recent call last):
    File "./s-postman.py", line 1078, in _walk
      self._do_mailbox(mailbox)
    File "./s-postman.py", line 1088, in _do_mailbox
      Ticket.process_message(msg)
    File "./s-postman.py", line 987, in process_message
      print("Message-Type: ", msg.__name__)
  Exception: AttributeError: 'mboxMessage' object has no attribute '__name__'
Continue [yY - else no]? n
Exit due to errors as above

==========

> And this is about Lib/test/test_mailbox.py

21:30 ~/py3k.hg/Lib/test $ python3 test_mailbox.py > ~/tmp/test.txt

Traceback (most recent call last):
  File "test_mailbox.py", line 2091, in <module>
    test_main()
  File "test_mailbox.py", line 2086, in test_main
    support.run_unittest(*tests)
  File "/usr/lib/python3.2/test/support.py", line 1145, in run_unittest
    _run_suite(suite)
  File "/usr/lib/python3.2/test/support.py", line 1128, in _run_suite
    raise TestFailed(err)
test.support.TestFailed: multiple errors occurred
msg127882 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 10:45
Oops - please be aware that these outputs were saved at the end of a frustrating session.  I'm doing things to show *you* what's passed around and the like, i.e. that the message is indeed a mboxMessage etc.  The error which occurs is always:

    File "usr/lib/python3.2/email/header.py", line 317, in encode
      formatter.feed(lines[0], charset)
msg127884 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 11:48
This simplemost patch (email_header.patch) seems to work at first glance.  It heals *everything* (except for babyl format, see issue 11062).  (I feel a bit like Charlie Chaplin and i think you know what i mean.)
msg127885 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2011-02-04 12:56
The patch would be better to say:

if not lines:
    continue

However, it could be even simpler to do:

for string, charset in self._chunks:
    if not string: continue
msg127890 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-04 13:56
Steffen, I appreciate your testing this.  Your error report doesn't have enough information for me to reproduce the problem.  Can you post a short test case, including a sample email/mailbox file if needed, that reproduces the problem you are seeing?  With a reproducible test case I can fix it quickly, without one I feel like I'm groping in the dark.  I need to be able to to create a unit test that demonstrates the problem in order to check in a fix; having a reproducible test case I can run will also help with writing that.

You also seem to report a failure in test_mailbox.  test_mailbox passes for me and on all the buildbots.  Are you running RC2?  What OS?
msg127891 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-04 14:01
Perhaps your problem with test_mailbox is that you are running the test_mailbox from a checkout, but using an installed python3 that is not RC2, rather than the python3 built from the checkout?
msg127921 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 19:22
You're right, the python3 executable is indeed

20:05 ~/tmp $ python3 -V
Python 3.2rc1+

The relevant .py files (mailbox.py, email/*.py, (email/mime/*.py), test/test_mailbox.py are symlinked to the repo files.
(And, different to this morning - sorry, *sorry*: lunch break - they all point to HG:33774ca03c96 (SVN:r88332) files.
I've removed my patch and this is what happens (only five errors remain).
The traceback:

Traceback (most recent call last):
  File "test_mailbox.py", line 2092, in <module>
    test_main()
  File "test_mailbox.py", line 2087, in test_main
    support.run_unittest(*tests)
  File "/Users/steffen/usr/lib/python3.2/test/support.py", line 1145, in run_unittest
    _run_suite(suite)
  File "/Users/steffen/usr/lib/python3.2/test/support.py", line 1128, in _run_suite
    raise TestFailed(err)
test.support.TestFailed: multiple errors occurred

==========
This is the stripped output:

test_add_binary_file (__main__.TestMaildir) ... ERROR
test_add_binary_file (__main__.TestMbox) ... ERROR
test_add_binary_file (__main__.TestMMDF) ... ERROR
test_add_binary_file (__main__.TestMH) ... ERROR
test_add_binary_file (__main__.TestBabyl) ... ERROR

======================================================================
ERROR: test_add_binary_file (__main__.TestMaildir)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_mailbox.py", line 146, in test_add_binary_file
    sys.exit(1)
SystemExit: 1

======================================================================
ERROR: test_add_binary_file (__main__.TestMbox)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_mailbox.py", line 146, in test_add_binary_file
    sys.exit(1)
SystemExit: 1

======================================================================
ERROR: test_add_binary_file (__main__.TestMMDF)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_mailbox.py", line 146, in test_add_binary_file
    sys.exit(1)
SystemExit: 1

======================================================================
ERROR: test_add_binary_file (__main__.TestMH)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_mailbox.py", line 146, in test_add_binary_file
    sys.exit(1)
SystemExit: 1

======================================================================
ERROR: test_add_binary_file (__main__.TestBabyl)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_mailbox.py", line 146, in test_add_binary_file
    sys.exit(1)
SystemExit: 1

----------------------------------------------------------------------
Ran 327 tests in 8.540s
msg127923 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 19:24
P.S.: it's that messed UNIX with the nice user-experience *G*UI.  And the __pycache__ files state 'cpython-32'.
msg127925 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 19:34
Dear RDM, now i'm really ashamed.  To be absolutely sure that i am up-to-date if diff'd and found that test_mailbox.py contained a sys.exit(1) i've inserted this - it was the lunch break, really!
Forget everything i've posted.  Please close this.  Shame on me.
msg127928 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 19:56
I thought about it.  It's worse.  You did a good job and i was not even able to create symlinks the way it should have been done.  This is a real shame - for me.
msg127941 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-04 21:18
However, as a last peep from me before i'll burn to ashes, and now with all fresh Python 3.2rc2+ all through, i do want to ask a question.

I'm still not finished with my broken thing, so the usual exception still occurs in respect to the malformed spam mail which made its way into Lib/test/test_mailbox.py - this is still my fault.  The thing i want to point out is that the target mailbox will end with these lines after the program fails:

>
>
>
> From MAILER-DAEMON Fri Feb  4 20:17:03 2011<- EOF here

Shouldn't this be suppressed?  Or, otherwise, shouldn't

[> Status: RO]
[> Content-Length: 0]
[> Lines: 0]
>
>

be appended in case of an exception?  The current result does not comply to <http://qmail.org./man/man5/mbox.html> (which is linked by <http://tools.ietf.org/html/rfc4155>).  mutt(1) is still capable to open it (it modifies the mailbox as shown above, including bracket lines), but i don't know if that applies to all mail-clients out there.
msg127970 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-05 02:26
Again, can you provide example input data and a short program that demonstrates the problem?  There's nothing I can reproduce in your report/question.  

As far as I noticed, the caught exceptions all occur at places where no output has yet been done to the mailbox/message.  If you have an example where this is not true, please point it out more specifically.
msg127983 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-05 12:09
Well, here is the long-asked-for 'fp_mailbox.py' test thing.
Note: it generates a 'test.mbox' in CWD!

    print("USAGE: fp_mailbox.py 0|1|2",
          "       0 = use raw UTF-8 string",
          "       1 = use UTF-8 string",
          "       2 = use escaped byte string", sep="\n")

It does not use FeedParser directly, but email.message_from_*.  It's simple minded, but a dumb new python(1) user (like me) does not know otherwise, i guess.  Of course the input is malformed, but, you know, shit happens (you may trust me in respect to that).  I want to point out that .defects is the empty list, even though the input is *not* correctly converted (quopri/base64) and blows mailbox.add later on for test cases 0 and 1 (resulting in corrupted mailbox files).  2 does not cause a traceback and is mailbox.add()ed, but the resulting mail is not standart-conforming either.  For me and my (S-)Postman all of this means that i have to encapsulate all of this with a 'MessageBuilder' layer on top of the python(1) builtin email package.



  I personally will encapsulate the email.* stuff with a MessageBuilder
msg128011 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-05 18:54
Thanks, much easier to communicate when runnable code is involved :) Now I can see what you mean about it writing the From.  I will figure out why and fix it so that the From line is not written.

The traceback from email.generator is unfortunate.  I should have message_from-string reject non-ASCII input with a clear error message like mailbox.add does now, but I didn't think of it.  I will see if the release manager will let me make that change.

For case (2) it is working as designed: faithfully writing the non-conformant message to the mbox.  If you wish to do other sorts of handling of non-conformant email you need to write code to do that.  You can use the email package to make it technically conformant (and ASCII only) by doing something like message_from_string(str(message_from_bytes(non_conformant_message)))  I'm not sure why you'd want to do that, though :)
msg128068 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-02-06 16:38
Thanks for spending your free time, almost alone in such a widespread area like this.
msg128112 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-02-07 12:56
> The traceback from email.generator is unfortunate.  I should have
> message_from-string reject non-ASCII input with a clear error message
> like mailbox.add does now, but I didn't think of it.  I will see if
> the release manager will let me make that change.

Sounds like a good change.
msg128340 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-10 20:55
Upon reflection I don't think my suggested email API change is a good one.  Currently it is possible to create a Message using non-ASCII headers and manipulate that message. The fact that you can't serialize that message is, really, a bug: one can and I think should assume utf-8 as the default encoding when serializing a message.

So rejecting non-ASCII strings in mailbox may also be a questionable decision, but since accepting string at all is really a legacy API (one should use a Message or a binary string), I think it is OK to leave it.  (Why should one not use string for input to mailbox?  Because the reason to use string is to avoid going through Message, but if your message includes non-ASCII it would have to be processed through the bug-fixed Message anyway to serialize it correctly...so you might as well just use Message for input to mailbox).

I hope to have a patch for the mailbox-corruption-if-traceback error soon (before the weekend).
msg128356 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-11 00:28
Here is the promised patch, with tests.  For once writing the patch was harder than writing the tests, but only just :)

I'm not sure all the 100% sure the cleanups will work on all systems (I'm looking at you, Windows), but since any problems will occur in a path where an error is already being raised, it doesn't seem as though it can make things worse.

Looking for review and RM approval.
msg128366 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-02-11 07:55
Hmm, is "except Exception" the right thing to use here?  The "finally" before executed for really all exceptions.

Otherwise, this can go in.
msg128389 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-11 14:17
The finally was doing a _sync_close, which flushes the tmp file and closes it.  The except Exception is checking if any non-BaseException error occurs, *removing* the tmp file, and re-raising the exception.  There's nothing to flush/close in that case.  Then, the else: clause of the exception deals with doing the _sync_close if, eg, we had a keyboard interrupt.  That may or may not leave the mailbox in a sensible state, but it is no worse than the previous behavior.  Certainly for any non-BaseException exception the new code is better; previously you'd most likely wind up with an empty or partially written message tmp file.  I'm open to changing to BaseException, though, if you think that would be better (I can see the argument for that).
msg128393 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-11 14:37
Duh.  After writing all that you'd think I'd have seen my mistake.  That's what reviews are for.  So I guess it should be BaseException, since the most likely one is keyboard interrupt and this would prevent a corrupted mailbox in that case.
msg128408 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-11 17:38
Revised patch using BaseException.
msg128411 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2011-02-11 18:01
compileall.rst diff doesn't seem to belong in that patch.
msg128413 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-11 18:08
Woops.  Thanks for catching that.  Will fix before commit.
msg128432 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-11 22:48
Committed in r88403.
msg128433 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-11 23:04
Decided to backport the fix to 2.7, even though the tests won't backport.  r88406.
msg128438 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-02-12 07:25
Thanks!
History
Date User Action Args
2011-02-12 07:25:53georg.brandlsetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128438
2011-02-11 23:04:47r.david.murraysetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128433
2011-02-11 22:48:34r.david.murraysetstatus: open -> closed
nosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128432

resolution: fixed
stage: patch review -> resolved
2011-02-11 18:08:22r.david.murraysetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128413
2011-02-11 18:01:23SilentGhostsetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128411
2011-02-11 17:38:48r.david.murraysetfiles: + mailbox_cleanup2.patch
nosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128408
2011-02-11 14:37:28r.david.murraysetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128393
2011-02-11 14:17:16r.david.murraysetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128389
2011-02-11 07:55:22georg.brandlsetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128366
2011-02-11 00:28:13r.david.murraysetfiles: + mailbox_cleanup.patch
priority: normal -> release blocker
nosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128356

stage: test needed -> patch review
2011-02-10 20:55:53r.david.murraysetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128340
2011-02-07 12:56:40georg.brandlsetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128112
2011-02-06 16:38:59sdaodensetnosy: georg.brandl, r.david.murray, SilentGhost, sdaoden
messages: + msg128068
2011-02-05 18:54:39r.david.murraysetnosy: + georg.brandl
messages: + msg128011
2011-02-05 12:09:42sdaodensetfiles: + fp_mailbox.py
nosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127983
2011-02-05 02:26:06r.david.murraysetnosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127970
2011-02-04 21:18:19sdaodensetnosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127941
2011-02-04 19:56:04sdaodensetnosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127928
2011-02-04 19:34:41sdaodensetnosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127925
2011-02-04 19:24:32sdaodensetnosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127923
2011-02-04 19:22:30sdaodensetnosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127921
2011-02-04 14:01:05r.david.murraysetnosy: r.david.murray, SilentGhost, sdaoden
messages: + msg127891
2011-02-04 13:56:44r.david.murraysetassignee: r.david.murray
nosy: r.david.murray, SilentGhost, sdaoden
2011-02-04 13:56:35r.david.murraysetnosy: r.david.murray, SilentGhost, sdaoden
title: (mailbox and) email (errors) -> patch -> mailbox and email errors
messages: + msg127890
stage: test needed
2011-02-04 12:56:34SilentGhostsetnosy: + SilentGhost
messages: + msg127885
2011-02-04 11:54:13vstinnersetnosy: + r.david.murray
2011-02-04 11:50:51sdaodensettitle: mailbox and email errors -> (mailbox and) email (errors) -> patch
2011-02-04 11:49:34sdaodensetfiles: - email_mbox.txt
2011-02-04 11:48:55sdaodensetfiles: + email_header.patch

messages: + msg127884
keywords: + patch
2011-02-04 10:45:50sdaodensetmessages: + msg127882
2011-02-04 10:39:47sdaodencreate