launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/fix-unicode-path into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #819841 in Launchpad itself: "OOPs rendering page with non-ascii URL"
  https://bugs.launchpad.net/launchpad/+bug/819841

For more details, see:
https://code.launchpad.net/~abentley/launchpad/fix-unicode-path/+merge/71762

= Summary =
Fix bug 819841: OOPs rendering page with non-ascii URL

== Proposed fix ==
Encode PATH_INFO as utf-8, as expected by sane_environment

== Pre-implementation notes ==
None

== Implementation details ==
None

== Tests ==
bin/test test_servers -t ToBrowser

== Demo and Q/A ==
Go to https://bugs.qastaging.launchpad.net/mahara/+bug/630891%C2%A0/+index

It should display normally, not oops.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/tests/test_distroseriesdifference.py
  lib/lp/answers/browser/tests/test_questiontarget.py
  lib/lp/services/messages/model/message.py
  lib/lp/answers/interfaces/questiontarget.py
  lib/lp/registry/browser/tests/test_distroseries.py
  lib/lp/answers/tests/test_questiontarget.py
  lib/canonical/launchpad/webapp/servers.py
  lib/lp/answers/model/question.py
  lib/lp/registry/model/sourcepackage.py
  lib/lp/registry/model/distroseriesdifference.py
  utilities/sourcedeps.cache
  lib/canonical/launchpad/webapp/tests/test_servers.py
  lib/lp/services/messages/tests/test_message.py
  lib/lp/soyuz/browser/archive.py
-- 
https://code.launchpad.net/~abentley/launchpad/fix-unicode-path/+merge/71762
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/fix-unicode-path into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py	2011-08-10 16:53:27 +0000
+++ lib/canonical/launchpad/webapp/servers.py	2011-08-16 19:14:31 +0000
@@ -703,12 +703,15 @@
 def web_service_request_to_browser_request(webservice_request):
     """Convert a given webservice request into a webapp one.
 
-    Simply overrides 'SERVER_URL' to the 'mainsite', preserving headers and
-    body.
+    Overrides 'SERVER_URL' to the 'mainsite', preserving headers and
+    body.  Encodes PATH_INFO because it is unconditionally decoded by
+    zope.publisher.http.sane_environment.
     """
     body = webservice_request.bodyStream.getCacheStream().read()
     environ = dict(webservice_request.environment)
     environ['SERVER_URL'] = allvhosts.configs['mainsite'].rooturl
+    if 'PATH_INFO' in environ:
+        environ['PATH_INFO'] = environ['PATH_INFO'].encode('utf-8')
     return LaunchpadBrowserRequest(body, environ)
 
 

=== modified file 'lib/canonical/launchpad/webapp/tests/test_servers.py'
--- lib/canonical/launchpad/webapp/tests/test_servers.py	2011-08-10 16:58:22 +0000
+++ lib/canonical/launchpad/webapp/tests/test_servers.py	2011-08-16 19:14:31 +0000
@@ -42,6 +42,7 @@
     WebServicePublication,
     WebServiceRequestPublicationFactory,
     WebServiceTestRequest,
+    web_service_request_to_browser_request,
     )
 from lp.testing import TestCase
 
@@ -278,6 +279,7 @@
 
     def setUp(self):
         super(TestWebServiceRequestTraversal, self).setUp()
+
         # For this test we need to make the URL "/foo" resolve to a
         # resource.  To this end, we'll define a top-level collection
         # named 'foo'.
@@ -557,6 +559,18 @@
             "parameters.")
 
 
+class TestWebServiceRequestToBrowserRequest(WebServiceTestCase):
+
+    def test_unicode_path_info(self):
+        web_service_request = WebServiceTestRequest(
+            PATH_INFO=u'/api/devel\u1234'.encode('utf-8'))
+        browser_request = web_service_request_to_browser_request(
+            web_service_request)
+        self.assertEqual(
+            web_service_request.get('PATH_INFO'),
+            browser_request.get('PATH_INFO'))
+
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(DocTestSuite(