Code review comment for ubuntu-dev-tools:import-bug-fix

Revision history for this message
Mattia Rizzolo (mapreri) wrote :

Right, I think your analysis is correct.

However I think a more formally appropriate fix would be to iterate through the payloads (in case of a multipart message), gather all the inline pieces of text/plain, concatenate them and use the resulting string. Mails with multiple inline text/plain parts are incredibly rare, but I've seen submitters of Debian bugs doing that to interleave descriptions to attachments.

See an example here: https://bugs.debian.org/981577

Anyway, rather than that `if isinstance()` check, you should be using .is_multipart(), followed by checking with .get_content_type() to verify it's really text/plain.
Everything while looping through .walk().

At the very very least, I'd rather the code gave up if it detected a too complex mail, erroring out saying so.

review: Needs Fixing

« Back to merge proposal