launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-456616 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #456616 in Launchpad itself: "IntegrityError triggered while adding an affected project to a bug report"
  https://bugs.launchpad.net/launchpad/+bug/456616

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-456616/+merge/53556

make_bugtracker_name didn't ensure that the returned name was compatible with valid_name, letting an email address with an invalid character (eg. foo_bar@xxxxxxxxxxx) cause an OOPS.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-456616/+merge/53556
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-456616 into lp:launchpad.

=== modified file 'lib/lp/bugs/doc/bugtracker.txt'
--- lib/lp/bugs/doc/bugtracker.txt	2010-10-27 20:00:54 +0000
+++ lib/lp/bugs/doc/bugtracker.txt	2011-03-16 02:09:27 +0000
@@ -101,38 +101,6 @@
 Auto-creating bug trackers
 --------------------------
 
-The bugtracker module contains a method, make_bugtracker_name() which
-can be used to generate a bug tracker name when a bugtracker is
-automatically created. make_bugtracker_name() accepts a single
-parameter, uri, which is used as the basis for the auto-generated name.
-
-Passing in a URI will produce a name based on the host name part of
-that URL.
-
-    >>> from lp.bugs.model.bugtracker import (
-    ...     make_bugtracker_name)
-    >>> make_bugtracker_name('http://bugs.example.com/shrubbery')
-    'auto-bugs.example.com'
-
-Passing in a mailto: URI will produce a name based on the local part
-of the email address.
-
-    >>> make_bugtracker_name('mailto:foo.bar@xxxxxxxxxxxxx')
-    'auto-foo.bar'
-
-Similarly, there is also make_bugtracker_title(), which does something
-similar, but for the title.
-
-    >>> from lp.bugs.model.bugtracker import (
-    ...     make_bugtracker_title)
-    >>> make_bugtracker_title('http://bugs.example.com/shrubbery')
-    'bugs.example.com/shrubbery'
-
-For email addresses, it uses the first part of the domain too.
-
-    >>> make_bugtracker_title('mailto:foo.bar@xxxxxxxxxxxxx')
-    'Email to foo.bar@canonical'
-
 The IBugTrackerSet interface provides a method, ensureBugTracker(),
 which will retrieve or create a bug tracker for the parameters passed to
 it. If this method is not passed a name parameter when it creates a new

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2011-02-24 15:30:54 +0000
+++ lib/lp/bugs/model/bugtracker.py	2011-03-16 02:09:27 +0000
@@ -155,7 +155,7 @@
     else:
         base_name = base_uri.host
 
-    return 'auto-%s' % base_name
+    return 'auto-%s' % sanitize_name(base_name)
 
 
 def make_bugtracker_title(uri):

=== modified file 'lib/lp/bugs/tests/test_bugtracker.py'
--- lib/lp/bugs/tests/test_bugtracker.py	2011-02-08 21:01:56 +0000
+++ lib/lp/bugs/tests/test_bugtracker.py	2011-03-16 02:09:27 +0000
@@ -39,12 +39,17 @@
     BugTrackerType,
     IBugTracker,
     )
-from lp.bugs.model.bugtracker import BugTrackerSet
+from lp.bugs.model.bugtracker import (
+    BugTrackerSet,
+    make_bugtracker_name,
+    make_bugtracker_title,
+    )
 from lp.bugs.tests.externalbugtracker import UrlLib2TransportTestHandler
 from lp.registry.interfaces.person import IPersonSet
 from lp.testing import (
     login,
     login_person,
+    TestCase,
     TestCaseWithFactory,
     )
 from lp.testing.sampledata import ADMIN_EMAIL
@@ -346,6 +351,39 @@
         self.assertRaises(BugTrackerConnectError, tracker._csv_data)
 
 
+class TestMakeBugtrackerName(TestCase):
+    """Tests for make_bugtracker_name."""
+
+    def test_url(self):
+        self.assertEquals(
+            'auto-bugs.example.com',
+            make_bugtracker_name('http://bugs.example.com/shrubbery'))
+
+    def test_email_address(self):
+        self.assertEquals(
+            'auto-foo.bar',
+            make_bugtracker_name('mailto:foo.bar@xxxxxxxxxxxxx'))
+
+    def test_sanitises_forbidden_characters(self):
+        self.assertEquals(
+            'auto-foobar',
+            make_bugtracker_name('mailto:foo_bar@xxxxxxxxxxxxx'))
+
+
+class TestMakeBugtrackerTitle(TestCase):
+    """Tests for make_bugtracker_title."""
+
+    def test_url(self):
+        self.assertEquals(
+            'bugs.example.com/shrubbery',
+            make_bugtracker_title('http://bugs.example.com/shrubbery'))
+
+    def test_email_address(self):
+        self.assertEquals(
+            'Email to foo.bar@somewhere',
+            make_bugtracker_title('mailto:foo.bar@xxxxxxxxxxxxx'))
+
+
 def test_suite():
     suite = unittest.TestLoader().loadTestsFromName(__name__)
     doctest_suite = DocTestSuite(