Merge lp:~thumper/launchpad/fix-dud-referer into lp:launchpad

[Tim Penhey]

[r=stevenk][bug=752218] Fix the UnicodeDecodeError when the referer is encoded and the resource is not found.

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.

[Steve Kowalik (stevenk)]

review: Approve (code)

[Robert Collins (lifeless)]

perhaps

if type(referer) != unicode:
    referer = referer.decode('utf8', errors='replace')

instead, which will replace the bad data but leave the rest intact.

=== 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:41:54 +0000
@@ -209,7 +209,15 @@
         """
         referrer = self.request.get('HTTP_REFERER')
         if referrer:
-            return referrer
+            # Since this is going to be included in the page template it will
+            # be coerced into unicode.  The byte string representation
+            # 'should' be ascii, but often it isn't.  The only use for this is
+            # to show a link back to the referring site, so we can't use
+            # replace or ignore.  Best to just pretent it doesn't exist.
+            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:41:54 +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