Code review comment for lp:~thumper/launchpad/mail-header-oops

+def extract_addresses(mail, raw_mail, file_alias_url, log):
+ # Extract the domain the mail was sent to. Mails sent to
+ # Launchpad should have an X-Launchpad-Original-To header.

Why not turn that into a docstring? And, maybe add things just saying the types of the parameters and the return code. It's the kind of thing that easily has bugs about passing a string vs a list of strings vs a file etc. (Or, you could use a statically type-checked language. :-P)

+ if ORIGINAL_TO_HEADER in mail:
+ return [mail[ORIGINAL_TO_HEADER]]
+
+ if ORIGINAL_TO_HEADER in raw_mail:
+ # Almost certainly a spam email with a blank line in the email headers
+ log.info('Suspected spam: %s' % file_alias_url)

I really do not see how the comment follows from the condition. How can you even have a blank line in the headers? You can have a blank line that causes things that ought to be headers to be part of the body, but no reasonable mail agent will see them as headers then.

Perhaps this should say something like:

 # Doesn't have an X-Launchpad-Original-To in the headers, but does have one in the body, because of a forwarding loop or attempted spam. See <https://bugs.launchpad.net/launchpad/+bug/701976>