diff --git a/tornado/auth.py b/tornado/auth.py index cb11db0f..574bb55f 100644 --- a/tornado/auth.py +++ b/tornado/auth.py @@ -58,6 +58,7 @@ import uuid from tornado import httpclient from tornado import escape +from tornado.httputil import url_concat from tornado.ioloop import IOLoop from tornado.util import bytes_type @@ -388,8 +389,8 @@ class OAuth2Mixin(object): "client_id": client_id } if extra_params: args.update(extra_params) - self.redirect(self._OAUTH_AUTHORIZE_URL + - urllib.urlencode(args)) + self.redirect( + url_concat(self._OAUTH_AUTHORIZE_URL, args)) def _oauth_request_token_url(self, redirect_uri= None, client_id = None, client_secret=None, code=None, @@ -402,7 +403,7 @@ class OAuth2Mixin(object): client_secret=client_secret, ) if extra_params: args.update(extra_params) - return url + urllib.urlencode(args) + return url_concat(url, args) class TwitterMixin(OAuthMixin): """Twitter OAuth authentication. diff --git a/tornado/httputil.py b/tornado/httputil.py index d40ebc6a..b33ea33e 100755 --- a/tornado/httputil.py +++ b/tornado/httputil.py @@ -16,6 +16,7 @@ """HTTP utility code shared by clients and servers.""" +import urllib import re class HTTPHeaders(dict): @@ -148,6 +149,18 @@ class HTTPHeaders(dict): return normalized +def url_concat(url, args): + """Concatenate url and argument dictionary regardless of whether + url has existing query parameters. + + >>> url_concat("http://example.com/foo?a=b", dict(c="d")) + 'http://example.com/foo?a=b&c=d' + """ + if url[-1] not in ('?', '&'): + url += '&' if ('?' in url) else '?' + return url + urllib.urlencode(args) + + def doctests(): import doctest return doctest.DocTestSuite() diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py new file mode 100644 index 00000000..abad065e --- /dev/null +++ b/tornado/test/httputil_test.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python + +from tornado.httputil import url_concat +import unittest + + +class TestUrlConcat(unittest.TestCase): + + def test_url_concat_no_query_params(self): + url = url_concat( + "https://localhost/path", + {'y':'y', 'z':'z'}, + ) + self.assertEqual(url, "https://localhost/path?y=y&z=z") + + def test_url_concat_encode_args(self): + url = url_concat( + "https://localhost/path", + {'y':'/y', 'z':'z'}, + ) + self.assertEqual(url, "https://localhost/path?y=%2Fy&z=z") + + def test_url_concat_trailing_q(self): + url = url_concat( + "https://localhost/path?", + {'y':'y', 'z':'z'}, + ) + self.assertEqual(url, "https://localhost/path?y=y&z=z") + + def test_url_concat_q_with_no_trailing_amp(self): + url = url_concat( + "https://localhost/path?x", + {'y':'y', 'z':'z'}, + ) + self.assertEqual(url, "https://localhost/path?x&y=y&z=z") + + def test_url_concat_trailing_amp(self): + url = url_concat( + "https://localhost/path?x&", + {'y':'y', 'z':'z'}, + ) + self.assertEqual(url, "https://localhost/path?x&y=y&z=z") + + def test_url_concat_mult_params(self): + url = url_concat( + "https://localhost/path?a=1&b=2", + {'y':'y', 'z':'z'}, + ) + self.assertEqual(url, "https://localhost/path?a=1&b=2&y=y&z=z") diff --git a/tornado/test/runtests.py b/tornado/test/runtests.py index 78e5d5e7..70e5db25 100755 --- a/tornado/test/runtests.py +++ b/tornado/test/runtests.py @@ -7,6 +7,7 @@ TEST_MODULES = [ 'tornado.util.doctests', 'tornado.test.escape_test', 'tornado.test.httpserver_test', + 'tornado.test.httputil_test', 'tornado.test.ioloop_test', 'tornado.test.iostream_test', 'tornado.test.simple_httpclient_test',