Roundup Tracker - Issues

Issue 2551000

classification
Title: MailGW currently broken for multipart messages (python2.7)
Type: crash Severity: normal
Components: Mail interface Versions: devel
process
Status: fixed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: joseph_myers, schlatterbeck
Priority: high Keywords: python2

Created on 2018-09-05 14:44 by schlatterbeck, last changed 2018-09-27 11:51 by joseph_myers.

Messages
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.
History
Date User Action Args
2018-09-27 11:51:41joseph_myerssetstatus: open -> fixed
resolution: fixed
messages: + msg6263
2018-09-25 15:46:58joseph_myerssetmessages: + msg6259
2018-09-25 14:41:40schlatterbecksetstatus: new -> open
messages: + msg6256
2018-09-16 13:59:33joseph_myerssetnosy: + joseph_myers
messages: + msg6249
2018-09-05 14:47:38schlatterbecksetkeywords: + python2
2018-09-05 14:44:37schlatterbeckcreate