Merge lp:~matsubara/launchpad/bug-504124-oops-handling-email into lp:launchpad

[Diogo Matsubara]

This branch fixes bug 504124 by using do_paranoid_envelope_to_validation() function to assert the parsed email addresses from the email message are sane. In case the addresses are not sane (i.e. spam), email is deleted from the mailbox and an OOPS is not logged. This fixes the top OOPS report from lpnet OOPS summary.

= Tests =

bin/test test_incoming

[Graham Binns (gmb)]

review: Approve (code)

[Curtis Hovey (sinzui)]

Thanks for providing this fix. You have my RC to land this branch into db-devel. We will use your revision as the release branch.

review: Approve (rc)

=== added file 'lib/canonical/launchpad/mail/ftests/emails/invalid-to-header.txt'
--- lib/canonical/launchpad/mail/ftests/emails/invalid-to-header.txt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/mail/ftests/emails/invalid-to-header.txt	2010-06-01 01:38:34 +0000
@@ -0,0 +1,23 @@
+From: "Fonti Hanna" <[e-mail]>
+To: <<[e-mail]>,
+	<[e-mail]>,
+	<[e-mail]>,
+	<[e-mail]>,
+	<[e-mail]>,
+	<[e-mail]>,
+	<[e-mail]>,
+	<[e-mail]>,
+	<[e-mail]>>
+Subject: (no subject)
+Date: Sat, 29 May 2010 10:13:18 +0800
+Message-ID: <01caff17$8ee25a10$b28c2872@manager>
+MIME-Version: 1.0
+Content-Type: multipart/alternative;
+	boundary="----=_NextPart_000_000E_01CAFF17.8EE25A10"
+X-Priority: 3 (Normal)
+X-MSMail-Priority: Normal
+X-Mailer: Microsoft Outlook IMO, Build 9.0.2416 (9.0.2910.0)
+X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2670
+Importance: Normal
+
+Some spam with invalid To: header.

=== modified file 'lib/canonical/launchpad/mail/incoming.py'
--- lib/canonical/launchpad/mail/incoming.py	2010-03-12 12:20:35 +0000
+++ lib/canonical/launchpad/mail/incoming.py	2010-06-01 01:38:34 +0000
@@ -29,6 +29,7 @@
 from canonical.launchpad.webapp.interaction import setupInteraction
 from canonical.launchpad.mail.commands import get_error_message
 from canonical.launchpad.mail.handlers import mail_handlers
+from lp.services.mail.sendmail import do_paranoid_envelope_to_validation
 from lp.services.mail.signedmessage import signed_message_from_string
 from canonical.launchpad.mailnotification import (
     send_process_error_notification)
@@ -273,6 +274,14 @@
                 names_addresses = getaddresses(to + cc)
                 addresses = [addr for name, addr in names_addresses]
 
+                try:
+                    do_paranoid_envelope_to_validation(addresses)
+                except AssertionError, e:
+                    _handle_error(
+                        "Invalid email address: %s" % e,
+                        file_alias_url, notify=False)
+                    continue
+
                 handler = None
                 for email_addr in addresses:
                     user, domain = email_addr.split('@')

=== modified file 'lib/canonical/launchpad/mail/tests/test_incoming.py'
--- lib/canonical/launchpad/mail/tests/test_incoming.py	2010-04-12 06:21:37 +0000
+++ lib/canonical/launchpad/mail/tests/test_incoming.py	2010-06-01 01:38:34 +0000
@@ -1,16 +1,19 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import os
 import unittest
 
 import transaction
 from zope.testing.doctest import DocTestSuite
 
+from canonical.launchpad.mail.ftests.helpers import testmails_path
 from canonical.launchpad.mail.incoming import handleMail, MailErrorUtility
 from canonical.testing import LaunchpadZopelessLayer
 from lp.testing import TestCaseWithFactory
 from lp.testing.mail_helpers import pop_notifications
 from lp.services.mail.sendmail import MailController
+from lp.services.mail.stub import TestMailer
 
 
 class TestIncoming(TestCaseWithFactory):
@@ -52,6 +55,24 @@
             "Error message:\n\nSignature couldn't be verified: No data",
             body)
 
+    def test_invalid_to_addresses(self):
+        """Invalid To: header should not be handled as an OOPS."""
+        raw_mail = open(os.path.join(
+            testmails_path, 'invalid-to-header.txt')).read()
+        # Due to the way handleMail works, even if we pass a valid To header
+        # to the TestMailer, as we're doing here, it falls back to parse all
+        # To and CC headers from the raw_mail. Also, TestMailer is used here
+        # because MailController won't send an email with a broken To: header.
+        TestMailer().send("[e-mail]", "[e-mail]", raw_mail)
+        error_utility = MailErrorUtility()
+        old_oops = error_utility.getLastOopsReport()
+        handleMail()
+        current_oops = error_utility.getLastOopsReport()
+        if old_oops is None:
+            self.assertIs(None, current_oops)
+        else:
+            self.assertEqual(old_oops.id, current_oops.id)
+
 
 def test_suite():
     suite = unittest.TestSuite()

=== modified file 'lib/lp/services/mail/sendmail.py'
--- lib/lp/services/mail/sendmail.py	2010-04-30 15:45:43 +0000
+++ lib/lp/services/mail/sendmail.py	2010-06-01 01:38:34 +0000
@@ -16,6 +16,7 @@
 
 __all__ = [
     'append_footer',
+    'do_paranoid_envelope_to_validation',
     'format_address',
     'format_address_for_person',
     'get_msgid',