launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/nonascii-email into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #747844 in Launchpad itself: "process-mail oopses with TypeError due to non US-ASCII chars in the email address"
  https://bugs.launchpad.net/launchpad/+bug/747844

For more details, see:
https://code.launchpad.net/~abentley/launchpad/nonascii-email/+merge/59671

= Summary =
Fix bug #747844: process-mail oopses with TypeError due to non US-ASCII chars in the email address

== Proposed fix ==
Raise LookupError instead of UnicodeDecodeError.

== Pre-implementation notes ==
None

== Implementation details ==
According to my reading of RFC2822, email addresses may contain only ascii characters.  Therefore, Launchpad should have no such addresses associated with accounts.  Therefore, we can raise a LookupError if someone tries to provide such an address.

Stopped using ensure_unicode because its docstring says "All invokations of ensure_unicode() should eventually be removed."

== Tests ==
bin/test -t test_getByEmail test_account

== Demo and Q/A ==
Send an email to Launchpad with a non-ascii email address.  It should not oops, but should report that it was unable to look up the user.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/database/account.py
  lib/canonical/launchpad/database/tests/test_account.py
-- 
https://code.launchpad.net/~abentley/launchpad/nonascii-email/+merge/59671
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/nonascii-email into lp:launchpad.

=== modified file 'lib/canonical/launchpad/database/account.py'
--- lib/canonical/launchpad/database/account.py	2011-04-21 01:49:49 +0000
+++ lib/canonical/launchpad/database/account.py	2011-05-02 16:01:15 +0000
@@ -25,7 +25,6 @@
 from canonical.database.enumcol import EnumCol
 from canonical.database.sqlbase import SQLBase
 from canonical.launchpad.database.emailaddress import EmailAddress
-from canonical.launchpad.helpers import ensure_unicode
 from canonical.launchpad.interfaces.lpstorm import (
     IMasterObject,
     IMasterStore,
@@ -288,11 +287,17 @@
     def getByEmail(self, email):
         """See `IAccountSet`."""
         store = IStore(Account)
+        try:
+            email = email.decode('US-ASCII')
+        except (UnicodeDecodeError, UnicodeEncodeError):
+            # Non-ascii email addresses are not legal, so assume there are no
+            # matching addresses in Launchpad.
+            raise LookupError(repr(email))
         account = store.find(
             Account,
             EmailAddress.account == Account.id,
             EmailAddress.email.lower()
-                == ensure_unicode(email).strip().lower()).one()
+                == email.strip().lower()).one()
         if account is None:
             raise LookupError(email)
         return account

=== modified file 'lib/canonical/launchpad/database/tests/test_account.py'
--- lib/canonical/launchpad/database/tests/test_account.py	2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/database/tests/test_account.py	2011-05-02 16:01:15 +0000
@@ -8,6 +8,7 @@
 
 import unittest
 
+from testtools.testcase import ExpectedException
 import transaction
 from zope.component import getUtility
 
@@ -96,6 +97,22 @@
             "when importing He-3 from the Moon",
             person.creation_comment)
 
+    def test_getByEmail_non_ascii_bytes(self):
+        """Lookups for non-ascii addresses should raise LookupError.
+
+        This tests the case where input is a bytestring.
+        """
+        with ExpectedException(LookupError, r"'SaraS\\xe1nchez@xxxxxxxxxxx'"):
+            getUtility(IAccountSet).getByEmail('SaraS\xe1nchez@xxxxxxxxxxx')
+
+    def test_getByEmail_non_ascii_unicode(self):
+        """Lookups for non-ascii addresses should raise LookupError.
+
+        This tests the case where input is a unicode string.
+        """
+        with ExpectedException(LookupError, r"u'SaraS\\xe1nchez@.*.net'"):
+            getUtility(IAccountSet).getByEmail(u'SaraS\xe1nchez@xxxxxxxxxxx')
+
 
 class EmailManagementTests(TestCaseWithFactory):
     """Test email account management interfaces for `IAccount`."""