launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/fix-dud-referer into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #752218 in Launchpad itself: "Referer header with non-ascii oopses on not found page"
  https://bugs.launchpad.net/launchpad/+bug/752218

For more details, see:
https://code.launchpad.net/~thumper/launchpad/fix-dud-referer/+merge/56508

I was investigating bug 44919, and looking at OOPS-979B2903, OOPS-1921EC1104 and OOPS-979C3111.

The not found page was failing to render due to a unicode decode error on the referer. This is different to the unicode decode error on bug 44919 which as to do with form data.

I managed to replicate the error first.  It would oops twice, once for the not found, and a second time when trying to render the page with the bad referer.

The simplest fix was to ignore the referer if it doesn't convert nicely to unicode.
-- 
https://code.launchpad.net/~thumper/launchpad/fix-dud-referer/+merge/56508
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/fix-dud-referer into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/error.py'
--- lib/canonical/launchpad/webapp/error.py	2010-12-17 19:16:54 +0000
+++ lib/canonical/launchpad/webapp/error.py	2011-04-06 05:21:36 +0000
@@ -209,7 +209,10 @@
         """
         referrer = self.request.get('HTTP_REFERER')
         if referrer:
-            return referrer
+            try:
+                return unicode(referrer)
+            except UnicodeDecodeError:
+                return None
         else:
             return None
 

=== modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py'
--- lib/canonical/launchpad/webapp/tests/test_publication.py	2011-02-04 14:41:18 +0000
+++ lib/canonical/launchpad/webapp/tests/test_publication.py	2011-04-06 05:21:36 +0000
@@ -21,12 +21,11 @@
 from storm.zope.interfaces import IZStorm
 from zope.component import getUtility
 from zope.error.interfaces import IErrorReportingUtility
-from zope.interface import (
-    directlyProvides,
-    noLongerProvides,
+from zope.interface import directlyProvides
+from zope.publisher.interfaces import (
+    NotFound,
+    Retry,
     )
-from zope.publisher.interfaces import Retry
-from zope.publisher.interfaces.browser import IBrowserRequest
 
 from canonical.config import dbconfig
 from canonical.launchpad.database.emailaddress import EmailAddress
@@ -440,6 +439,22 @@
         maybe_block_offsite_form_post(request)
 
 
+class TestEncodedReferer(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_not_found(self):
+        # The only oops should be a NotFound.
+        browser = self.getUserBrowser()
+        browser.addHeader('Referer', '/whut\xe7foo')
+        self.assertRaises(
+            NotFound,
+            browser.open,
+            'http://launchpad.dev/missing')
+        self.assertEqual(1, len(self.oopses))
+        self.assertEqual('NotFound', self.oopses[0].type)
+
+
 def test_suite():
     suite = unittest.TestLoader().loadTestsFromName(__name__)
     return suite