launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/send-host-header-during-getremotebug into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #943261 in Launchpad itself: "Always gets a 404 from the VLC Trac, even when said Trac is online"
  https://bugs.launchpad.net/launchpad/+bug/943261

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/send-host-header-during-getremotebug/+merge/132447

Always send a Host header when we talk to remote bug trackers, so we can make use of virtually hosted bug trackers.

I have refactored some tests and destroyed what I considered pointless comments to claw back LoC.
-- 
https://code.launchpad.net/~stevenk/launchpad/send-host-header-during-getremotebug/+merge/132447
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/send-host-header-during-getremotebug into lp:launchpad.

=== modified file 'lib/lp/bugs/externalbugtracker/base.py'
--- lib/lp/bugs/externalbugtracker/base.py	2012-06-29 08:40:05 +0000
+++ lib/lp/bugs/externalbugtracker/base.py	2012-11-01 04:36:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """External bugtrackers."""
@@ -48,10 +48,6 @@
 BATCH_SIZE_UNLIMITED = 0
 
 
-#
-# Errors.
-#
-
 class BugWatchUpdateError(Exception):
     """Base exception for when we fail to update watches for a tracker."""
 
@@ -96,10 +92,6 @@
     """Launchpad couldn't authenticate with the remote bugtracker."""
 
 
-#
-# Warnings.
-#
-
 class BugWatchUpdateWarning(Exception):
     """An exception representing a warning.
 
@@ -141,10 +133,6 @@
     """Raised when a bug is marked private on the remote bugtracker."""
 
 
-#
-# Everything else.
-#
-
 class ExternalBugTracker:
     """Base class for an external bug tracker."""
 
@@ -238,6 +226,11 @@
         """
         return None
 
+    def _getHeaders(self):
+        # For some reason, bugs.kde.org doesn't allow the regular urllib
+        # user-agent string (Python-urllib/2.x) to access their bugzilla.
+        return {'User-agent': LP_USER_AGENT, 'Host': self.baseurl}
+
     def _fetchPage(self, page, data=None):
         """Fetch a page from the remote server.
 
@@ -250,16 +243,13 @@
 
     def _getPage(self, page):
         """GET the specified page on the remote HTTP server."""
-        # For some reason, bugs.kde.org doesn't allow the regular urllib
-        # user-agent string (Python-urllib/2.x) to access their
-        # bugzilla, so we send our own instead.
-        request = urllib2.Request("%s/%s" % (self.baseurl, page),
-                                  headers={'User-agent': LP_USER_AGENT})
+        request = urllib2.Request(
+            "%s/%s" % (self.baseurl, page), headers=self._getHeaders())
         return self._fetchPage(request).read()
 
     def _post(self, url, data):
         """Post to a given URL."""
-        request = urllib2.Request(url, headers={'User-agent': LP_USER_AGENT})
+        request = urllib2.Request(url, headers=self._getHeaders())
         return self._fetchPage(request, data=data)
 
     def _postPage(self, page, form, repost_on_redirect=False):

=== modified file 'lib/lp/bugs/externalbugtracker/tests/test_externalbugtracker.py'
--- lib/lp/bugs/externalbugtracker/tests/test_externalbugtracker.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/externalbugtracker/tests/test_externalbugtracker.py	2012-11-01 04:36:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the externalbugtracker package."""
@@ -13,6 +13,7 @@
 from lp.bugs.externalbugtracker.base import (
     BugTrackerConnectError,
     ExternalBugTracker,
+    LP_USER_AGENT,
     )
 from lp.bugs.externalbugtracker.debbugs import DebBugs
 from lp.bugs.interfaces.externalbugtracker import (
@@ -86,18 +87,12 @@
         # When either sync_comments or sync_debbugs_comments is False
         # (or both), the Debian Bugs external bug tracker will claim
         # to not support any form of comment syncing.
-        self.pushConfig(
-            'checkwatches', sync_comments=True, sync_debbugs_comments=False)
-        tracker = DebBugs(self.base_url)
-        self.assertFalse(tracker.sync_comments)
-        self.pushConfig(
-            'checkwatches', sync_comments=False, sync_debbugs_comments=True)
-        tracker = DebBugs(self.base_url)
-        self.assertFalse(tracker.sync_comments)
-        self.pushConfig(
-            'checkwatches', sync_comments=False, sync_debbugs_comments=False)
-        tracker = DebBugs(self.base_url)
-        self.assertFalse(tracker.sync_comments)
+        for state in ((True, False), (False, True), (False, False)):
+            self.pushConfig(
+                'checkwatches', sync_comments=state[0],
+                sync_debbugs_comments=state[1])
+            tracker = DebBugs(self.base_url)
+            self.assertFalse(tracker.sync_comments)
 
     def _makeFakePostForm(self, base_url, page=None):
         """Create a fake `urllib2.urlopen` result."""
@@ -168,5 +163,16 @@
         with monkey_patch(urllib2, urlopen=raise404):
             self.assertRaises(
                 BugTrackerConnectError,
-                bugtracker._post,
-                'some-url', {'post-data': 'here'})
+                bugtracker._post, 'some-url', {'post-data': 'here'})
+
+    def test_post_sends_host(self):
+        # When posting, a Host header is sent.
+        base_url = "http://example.com/";
+        bugtracker = ExternalBugTracker(base_url)
+        def assert_headers(request, data):
+            # The ending slash is not sent, so strip it.
+            self.assertContentEqual(
+                [('User-agent', LP_USER_AGENT), ('Host', base_url[:-1])],
+                request.header_items())
+        with monkey_patch(urllib2, urlopen=assert_headers):
+            bugtracker._post('some-url', {'post-data': 'here'})