Created on 2018-09-05 14:44 by schlatterbeck, last changed 2018-09-27 11:51 by joseph_myers.
|msg6239||Author: [hidden] (schlatterbeck)||Date: 2018-09-05 14:44|
Tested with python2.7, not python3: The mail parsing in mailgw.py in class RoundupMessage (which now inherits from email.message.Message) relies on recursively parsing mails using the method extract_content. After the unpacking step this fails because get_payload returns a email.message.Message object, not a RoundupMessage, so subsequent recursive calls fail because the object has no extract_content method. The traceback (for an attachment of type message/rfc822 but any multipart mail currently fails) is given below. Looks like we currently don't have any multipart examples in the mail gateway tests? Traceback (most recent call last): File "/usr/local/lib/python2.7/dist-packages/roundup/mailgw.py", line 1450, in handle_Message return self.handle_message(message) File "/usr/local/lib/python2.7/dist-packages/roundup/mailgw.py", line 1521, in handle_message return self._handle_message(message) File "/usr/local/lib/python2.7/dist-packages/roundup/mailgw.py", line 1533, in _handle_message nodeid = self.parsed_message.parse () File "/usr/local/lib/python2.7/dist-packages/roundup/mailgw.py", line 1196, in parse ret = method() File "/usr/local/lib/python2.7/dist-packages/roundup/mailgw.py", line 1005, in get_content_and_attachments html2text=html2text ) File "/usr/local/lib/python2.7/dist-packages/roundup/mailgw.py", line 309, in extract_content new_content, new_attach, html_part = part.extract_content( AttributeError: Message instance has no attribute 'extract_content'
|msg6249||Author: [hidden] (joseph_myers)||Date: 2018-09-16 13:59|
Please see if the patch I just checked in fixes this (it certainly fixes a more or less identical backtrace I observed testing a multipart message (with a single part) in mailbox input to roundup-mailgw when fixing that for Python 3).
|msg6256||Author: [hidden] (schlatterbeck)||Date: 2018-09-25 14:41|
Works now. What worries me: We *do* have tests that test multipart mails, see test/test_mailgw.py all tests that use self.multipart_msg, e.g., testMultipartKeepAlternatives Did these tests fail with the bug still present? I would expect them to fail but that the bug got undetected worries me. Thanks for fixing this! Ralf
|msg6259||Author: [hidden] (joseph_myers)||Date: 2018-09-25 15:46|
I believe the tests passed with the bug present. My fix was specifically in MailGW.do_mailbox - that is, the input method for one particular way of putting received emails into Roundup (the other methods had smaller fixes to use message_from_bytes / message_from_binary_file, although POP and IMAP could do with testing that they actually work with Python 3). The tests in test_mailgw.py don't test the external input methods, just the MailGW.main method, and MailGW.main was calling email.message_from_file with RoundupMessage passed as the second argument before my patch (so probably unaffected for Python 2 but probably having Python 3 issues with mixed character set 8-bit-encoded attachments before my patch). Generally the tests operate at the Python API level and are weak on external input/output interfaces (which is also an area where Python 3 issues are likely and so manual testing for those is important).
|msg6263||Author: [hidden] (joseph_myers)||Date: 2018-09-27 11:51|
Note that https://codecov.io/gh/roundup-tracker/roundup/src/master/roundup/mailgw.py shows the lack of test coverage in do_mailbox. While it's certainly useful to improve test coverage for code currently lacking it, this is one place among many that lacks coverage, so I don't think it's useful to keep this issue open for it.
|2018-09-27 11:51:41||joseph_myers||set||status: open -> fixed|
messages: + msg6263
|2018-09-25 15:46:58||joseph_myers||set||messages: + msg6259|
|2018-09-25 14:41:40||schlatterbeck||set||status: new -> open|
messages: + msg6256
messages: + msg6249
|2018-09-05 14:47:38||schlatterbeck||set||keywords: + python2|