+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>
-- 
https://code.launchpad.net/~thumper/launchpad/mail-header-oops/+merge/52156
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~thumper/launchpad/mail-header-oops into lp:launchpad.

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to