Merge lp:~salgado/launchpad/fix-unicode-error-on-login into lp:launchpad/db-devel

Proposed by Guilherme Salgado
Status: Merged
Merged at revision: not available
Proposed branch: lp:~salgado/launchpad/fix-unicode-error-on-login
Merge into: lp:launchpad/db-devel
Diff against target: 54 lines (+18/-1)
2 files modified
lib/canonical/launchpad/webapp/login.py (+2/-1)
lib/canonical/launchpad/webapp/tests/test_login.py (+16/-0)
To merge this branch: bzr merge lp:~salgado/launchpad/fix-unicode-error-on-login
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) release-critical Approve
Michael Nelson (community) code Approve
Review via email: mp+20656@code.launchpad.net

Description of the change

Fix OpenIDLogin to not OOPS when the query string has non-ascii chars.

Just utf8-encoding the value before passing it to urllib.urlencode()
seems to do the trick.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

I'm not sure why the diff hasn't updated yet, but I checked this locally, and it all looks good. Thanks Guilherme.

review: Approve (code)
Revision history for this message
Guilherme Salgado (salgado) wrote :

http://paste.ubuntu.com/388337/ has the diff, in case it never shows up here.

Revision history for this message
Francis J. Lacoste (flacoste) wrote :

Land it on db-devel.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/webapp/login.py'
--- lib/canonical/launchpad/webapp/login.py 2010-03-02 15:03:43 +0000
+++ lib/canonical/launchpad/webapp/login.py 2010-03-04 17:21:54 +0000
@@ -194,7 +194,8 @@
194 # they started the login process (i.e. the current URL without the194 # they started the login process (i.e. the current URL without the
195 # '+login' bit). To do that we encode that URL as a query arg in the195 # '+login' bit). To do that we encode that URL as a query arg in the
196 # return_to URL passed to the OpenID Provider196 # return_to URL passed to the OpenID Provider
197 starting_url = urllib.urlencode([('starting_url', self.starting_url)])197 starting_url = urllib.urlencode(
198 [('starting_url', self.starting_url.encode('utf-8'))])
198 trust_root = allvhosts.configs['mainsite'].rooturl199 trust_root = allvhosts.configs['mainsite'].rooturl
199 return_to = urlappend(trust_root, '+openid-callback')200 return_to = urlappend(trust_root, '+openid-callback')
200 return_to = "%s?%s" % (return_to, starting_url)201 return_to = "%s?%s" % (return_to, starting_url)
201202
=== modified file 'lib/canonical/launchpad/webapp/tests/test_login.py'
--- lib/canonical/launchpad/webapp/tests/test_login.py 2010-03-02 15:03:43 +0000
+++ lib/canonical/launchpad/webapp/tests/test_login.py 2010-03-04 17:21:54 +0000
@@ -307,6 +307,7 @@
307307
308class FakeOpenIDRequest:308class FakeOpenIDRequest:
309 extensions = None309 extensions = None
310 return_to = None
310311
311 def addExtension(self, extension):312 def addExtension(self, extension):
312 if self.extensions is None:313 if self.extensions is None:
@@ -318,6 +319,7 @@
318 return False319 return False
319320
320 def htmlMarkup(self, trust_root, return_to):321 def htmlMarkup(self, trust_root, return_to):
322 self.return_to = return_to
321 return None323 return None
322324
323325
@@ -334,6 +336,20 @@
334class TestOpenIDLogin(TestCaseWithFactory):336class TestOpenIDLogin(TestCaseWithFactory):
335 layer = DatabaseFunctionalLayer337 layer = DatabaseFunctionalLayer
336338
339 def test_return_to_with_non_ascii_chars(self):
340 # Sometimes the +login link will have non-ascii characters in the
341 # query string, and we need to include those in the return_to URL that
342 # we pass to the OpenID provider, so we must utf-encode them.
343 request = LaunchpadTestRequest(
344 form={'non_ascii_field': 'subproc\xc3\xa9s'})
345 # This is a hack to make the request.getURL(1) call issued by the view
346 # not raise an IndexError.
347 request._app_names = ['foo']
348 view = StubbedOpenIDLogin(object(), request)
349 view()
350 self.assertIn(
351 'non_ascii_field%3Dsubproc%C3%A9s', view.openid_request.return_to)
352
337 def test_sreg_fields(self):353 def test_sreg_fields(self):
338 # We request the user's email address and Full Name (through the SREG354 # We request the user's email address and Full Name (through the SREG
339 # extension) to the OpenID provider so that we can automatically355 # extension) to the OpenID provider so that we can automatically

Subscribers

People subscribed via source and target branches

to status/vote changes: