Rewrite url_concat function using PythonStandardLibrary

diff --git a/tornado/httputil.py b/tornado/httputil.py
index 21842caa22b59ac811012d67906f0a2377210060..d299cfc455a6443524a84252649ca49477f4dc79 100644 (file)
--- a/tornado/httputil.py
+++ b/tornado/httputil.py
@@ -38,11 +38,12 @@ from tornado.util import ObjectDict, PY3
 if PY3:
     import http.cookies as Cookie
     from http.client import responses
-    from urllib.parse import urlencode
+    from urllib.parse import urlencode, urlparse, urlunparse, parse_qsl
 else:
     import Cookie
     from httplib import responses
     from urllib import urlencode
+    from urlparse import urlparse, urlunparse, parse_qsl
 
 
 # responses is unused in this file, but we re-export it to other files.
@@ -599,11 +600,26 @@ def url_concat(url, args):
     >>> url_concat("http://example.com/foo?a=b", [("c", "d"), ("c", "d2")])
     'http://example.com/foo?a=b&c=d&c=d2'
     """
-    if not args:
-        return url
-    if url[-1] not in ('?', '&'):
-        url += '&' if ('?' in url) else '?'
-    return url + urlencode(args)
+    parsed_url = urlparse(url)
+    if isinstance(args, dict):
+        parsed_query = parse_qsl(parsed_url.query, keep_blank_values=True)
+        parsed_query.extend(args.items())
+    elif isinstance(args, list) or isinstance(args, tuple):
+        parsed_query = parse_qsl(parsed_url.query, keep_blank_values=True)
+        parsed_query.extend(args)
+    else:
+        err = "'args' parameter should be dict, list or tuple. Not {0}".format(
+            type(args))
+        raise TypeError(err)
+    final_query = urlencode(parsed_query)
+    url = urlunparse((
+        parsed_url[0],
+        parsed_url[1],
+        parsed_url[2],
+        parsed_url[3],
+        final_query,
+        parsed_url[5]))
+    return url
 
 
 class HTTPFile(ObjectDict):

diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py
index 3eb104d1ce9296e617c12c33afa06cf1e9d688b1..72f4c48bb9bb123b67ed5c7a6bef92fae2815fa1 100644 (file)
--- a/tornado/test/httputil_test.py
+++ b/tornado/test/httputil_test.py
@@ -43,14 +43,14 @@ class TestUrlConcat(unittest.TestCase):
             "https://localhost/path?x",
             [('y', 'y'), ('z', 'z')],
         )
-        self.assertEqual(url, "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")
+        self.assertEqual(url, "https://localhost/path?x=&y=y&z=z")
 
     def test_url_concat_mult_params(self):
         url = url_concat(
@@ -66,6 +66,34 @@ class TestUrlConcat(unittest.TestCase):
         )
         self.assertEqual(url, "https://localhost/path?r=1&t=2")
 
+    def test_url_concat_with_frag(self):
+        url = url_concat(
+            "https://localhost/path#tab",
+            [('y', 'y')],
+        )
+        self.assertEqual(url, "https://localhost/path?y=y#tab")
+
+    def test_url_concat_multi_same_params(self):
+        url = url_concat(
+            "https://localhost/path",
+            [('y', 'y1'), ('y', 'y2')],
+        )
+        self.assertEqual(url, "https://localhost/path?y=y1&y=y2")
+
+    def test_url_concat_multi_same_query_params(self):
+        url = url_concat(
+            "https://localhost/path?r=1&r=2",
+            [('y', 'y')],
+        )
+        self.assertEqual(url, "https://localhost/path?r=1&r=2&y=y")
+
+    def test_url_concat_dict_params(self):
+        url = url_concat(
+            "https://localhost/path",
+            dict(y='y'),
+        )
+        self.assertEqual(url, "https://localhost/path?y=y")
+
 
 class MultipartFormDataTest(unittest.TestCase):
     def test_file_upload(self):