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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2011-02-11 17:38 |
Revised patch using BaseException.
|
msg128411 - (view) |
Author: SilentGhost (SilentGhost) * |
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) * |
Date: 2011-02-11 18:08 |
Woops. Thanks for catching that. Will fix before commit.
|
msg128432 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-02-11 22:48 |
Committed in r88403.
|
msg128433 - (view) |
Author: R. David Murray (r.david.murray) * |
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) * |
Date: 2011-02-12 07:25 |
Thanks!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:12 | admin | set | github: 55325 |
2011-02-12 07:25:53 | georg.brandl | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128438 |
2011-02-11 23:04:47 | r.david.murray | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128433 |
2011-02-11 22:48:34 | r.david.murray | set | status: open -> closed nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128432
resolution: fixed stage: patch review -> resolved |
2011-02-11 18:08:22 | r.david.murray | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128413 |
2011-02-11 18:01:23 | SilentGhost | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128411 |
2011-02-11 17:38:48 | r.david.murray | set | files:
+ mailbox_cleanup2.patch nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128408
|
2011-02-11 14:37:28 | r.david.murray | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128393 |
2011-02-11 14:17:16 | r.david.murray | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128389 |
2011-02-11 07:55:22 | georg.brandl | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128366 |
2011-02-11 00:28:13 | r.david.murray | set | files:
+ 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:53 | r.david.murray | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128340 |
2011-02-07 12:56:40 | georg.brandl | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128112 |
2011-02-06 16:38:59 | sdaoden | set | nosy:
georg.brandl, r.david.murray, SilentGhost, sdaoden messages:
+ msg128068 |
2011-02-05 18:54:39 | r.david.murray | set | nosy:
+ georg.brandl messages:
+ msg128011
|
2011-02-05 12:09:42 | sdaoden | set | files:
+ fp_mailbox.py nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127983
|
2011-02-05 02:26:06 | r.david.murray | set | nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127970 |
2011-02-04 21:18:19 | sdaoden | set | nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127941 |
2011-02-04 19:56:04 | sdaoden | set | nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127928 |
2011-02-04 19:34:41 | sdaoden | set | nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127925 |
2011-02-04 19:24:32 | sdaoden | set | nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127923 |
2011-02-04 19:22:30 | sdaoden | set | nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127921 |
2011-02-04 14:01:05 | r.david.murray | set | nosy:
r.david.murray, SilentGhost, sdaoden messages:
+ msg127891 |
2011-02-04 13:56:44 | r.david.murray | set | assignee: r.david.murray nosy:
r.david.murray, SilentGhost, sdaoden |
2011-02-04 13:56:35 | r.david.murray | set | nosy:
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:34 | SilentGhost | set | nosy:
+ SilentGhost messages:
+ msg127885
|
2011-02-04 11:54:13 | vstinner | set | nosy:
+ r.david.murray
|
2011-02-04 11:50:51 | sdaoden | set | title: mailbox and email errors -> (mailbox and) email (errors) -> patch |
2011-02-04 11:49:34 | sdaoden | set | files:
- email_mbox.txt |
2011-02-04 11:48:55 | sdaoden | set | files:
+ email_header.patch
messages:
+ msg127884 keywords:
+ patch |
2011-02-04 10:45:50 | sdaoden | set | messages:
+ msg127882 |
2011-02-04 10:39:47 | sdaoden | create | |