Merge lp:~thumper/launchpad/fix-unicode-path-oops into lp:launchpad

[Tim Penhey]

[r=lifeless][bug=200572]

This branch catches a couple of unicode issues.

One is where we were trying to look for something in storm with a potentially bad byte ordering, and the second was when we got a NotFoundError, the rendering of the page checked for a webservice request, which tryied to UTF-8 decode the string, which again failed.

[Robert Collins (lifeless)]

We shouldn't really be raising a NotFound there *either*, but this is an improvement.

review: Approve

[Tim Penhey (thumper)]

Why not?

[Robert Collins (lifeless)]

On Fri, Apr 8, 2011 at 1:40 PM, Tim Penhey <email address hidden> wrote:
> Why not?

Because we shouldn't be raising an OOPS for any 404 unless its coming
from launchpad/a canonical property which we can fix the url on.

There is a separate bug for that, so it doesn't need to affect your work today.

-Rob

=== modified file 'lib/canonical/launchpad/doc/renamed-view.txt'
--- lib/canonical/launchpad/doc/renamed-view.txt	2010-11-09 23:38:44 +0000
+++ lib/canonical/launchpad/doc/renamed-view.txt	2011-04-10 23:56:45 +0000
@@ -89,7 +89,7 @@
     >>> view.publishTraverse(request, u'foo')
     Traceback (most recent call last):
      ...
-    NotFound: Object: u'foo', name: <Distribution 'Ubuntu' (ubuntu)>
+    NotFound: Object: <Distribution 'Ubuntu' (ubuntu)>, name: u'foo'
 
 
 == Registering from ZCML ==

=== modified file 'lib/canonical/launchpad/helpers.py'
--- lib/canonical/launchpad/helpers.py	2011-03-30 23:37:11 +0000
+++ lib/canonical/launchpad/helpers.py	2011-04-10 23:56:45 +0000
@@ -482,26 +482,6 @@
     return open(fullpath).read()
 
 
-def is_ascii_only(string):
-    """Ensure that the string contains only ASCII characters.
-
-        >>> is_ascii_only(u'ascii only')
-        True
-        >>> is_ascii_only('ascii only')
-        True
-        >>> is_ascii_only('\xf4')
-        False
-        >>> is_ascii_only(u'\xf4')
-        False
-    """
-    try:
-        string.encode('ascii')
-    except UnicodeError:
-        return False
-    else:
-        return True
-
-
 def truncate_text(text, max_length):
     """Return a version of string no longer than max_length characters.
 

=== modified file 'lib/canonical/launchpad/rest/configuration.py'
--- lib/canonical/launchpad/rest/configuration.py	2011-03-22 11:54:10 +0000
+++ lib/canonical/launchpad/rest/configuration.py	2011-04-10 23:56:45 +0000
@@ -60,6 +60,10 @@
 
     def createRequest(self, body_instream, environ):
         """See `IWebServiceConfiguration`."""
+        # The request is going to try to decode the 'PATH_INFO' using utf-8,
+        # so if it is currently unicode, encode it.
+        if isinstance(environ.get('PATH_INFO'), unicode):
+            environ['PATH_INFO'] = environ['PATH_INFO'].encode('utf-8')
         request = WebServiceClientRequest(body_instream, environ)
         request.setPublication(WebServicePublication(None))
         return request

=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
--- lib/canonical/launchpad/webapp/publisher.py	2011-03-21 21:33:25 +0000
+++ lib/canonical/launchpad/webapp/publisher.py	2011-04-10 23:56:45 +0000
@@ -70,6 +70,7 @@
 from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.lazr.utils import get_current_browser_request
 from lp.app.errors import NotFoundError
+from lp.services.encoding import is_ascii_only
 
 # HTTP Status code constants - define as appropriate.
 HTTP_MOVED_PERMANENTLY = 301
@@ -657,6 +658,10 @@
         This needs moving into the publication component, once it has been
         refactored.
         """
+        # Launchpad only produces ascii URLs.  If the name is not ascii, we
+        # can say nothing is found here.
+        if not is_ascii_only(name):
+            raise NotFound(self.context, name)
         nextobj = self._publishTraverse(request, name)
         getUtility(IOpenLaunchBag).add(nextobj)
         return nextobj
@@ -839,7 +844,7 @@
 
     def publishTraverse(self, request, name):
         """See zope.publisher.interfaces.browser.IBrowserPublisher."""
-        raise NotFound(name, self.context)
+        raise NotFound(self.context, name)
 
     def browserDefault(self, request):
         """See zope.publisher.interfaces.browser.IBrowserPublisher."""

=== modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py'
--- lib/canonical/launchpad/webapp/tests/test_publication.py	2011-04-06 05:13:57 +0000
+++ lib/canonical/launchpad/webapp/tests/test_publication.py	2011-04-10 23:56:45 +0000
@@ -455,6 +455,16 @@
         self.assertEqual('NotFound', self.oopses[0].type)
 
 
-def test_suite():
-    suite = unittest.TestLoader().loadTestsFromName(__name__)
-    return suite
+class TestUnicodePath(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_non_ascii_url(self):
+        # The only oops should be a NotFound.
+        browser = self.getUserBrowser()
+        self.assertRaises(
+            NotFound,
+            browser.open,
+            'http://launchpad.dev/%ED%B4%B5')
+        self.assertEqual(1, len(self.oopses))
+        self.assertEqual('NotFound', self.oopses[0].type)

=== modified file 'lib/lp/services/encoding.py'
--- lib/lp/services/encoding.py	2011-02-17 17:02:54 +0000
+++ lib/lp/services/encoding.py	2011-04-10 23:56:45 +0000
@@ -8,6 +8,7 @@
     'ascii_smash',
     'escape_nonascii_uniquely',
     'guess',
+    'is_ascii_only',
     ]
 
 import re
@@ -387,3 +388,23 @@
     def quote(match):
         return '\\x%x' % ord(match.group(0))
     return nonascii_regex.sub(quote, bogus_string)
+
+
+def is_ascii_only(string):
+    """Ensure that the string contains only ASCII characters.
+
+        >>> is_ascii_only(u'ascii only')
+        True
+        >>> is_ascii_only('ascii only')
+        True
+        >>> is_ascii_only('\xf4')
+        False
+        >>> is_ascii_only(u'\xf4')
+        False
+    """
+    try:
+        string.encode('ascii')
+    except UnicodeError:
+        return False
+    else:
+        return True

=== modified file 'lib/lp/services/mail/sendmail.py'
--- lib/lp/services/mail/sendmail.py	2010-12-14 18:54:38 +0000
+++ lib/lp/services/mail/sendmail.py	2011-04-10 23:56:45 +0000
@@ -50,9 +50,9 @@
 from zope.sendmail.interfaces import IMailDelivery
 
 from canonical.config import config
+from canonical.lp import isZopeless
 from lp.app import versioninfo
-from canonical.launchpad.helpers import is_ascii_only
-from canonical.lp import isZopeless
+from lp.services.encoding import is_ascii_only
 from lp.services.mail.stub import TestMailer
 from lp.services.timeline.requesttimeline import get_request_timeline
 

=== modified file 'lib/lp/services/mail/tests/test_sendmail.py'
--- lib/lp/services/mail/tests/test_sendmail.py	2010-09-04 05:37:08 +0000
+++ lib/lp/services/mail/tests/test_sendmail.py	2011-04-10 23:56:45 +0000
@@ -7,7 +7,7 @@
 from email.Message import Message
 import unittest
 
-from canonical.launchpad.helpers import is_ascii_only
+from lp.services.encoding import is_ascii_only
 from lp.services.mail import sendmail
 from lp.services.mail.sendmail import MailController
 from lp.testing import TestCase