From 883ce6df81bd5392e4d847706b9d4031a4c564ea Mon Sep 17 00:00:00 2001
From: Ping <yuanta11@gmail.com>
Date: Thu, 24 Nov 2016 14:19:11 +0800
Subject: [PATCH] Rewrite url_concat function using PythonStandardLibrary

---
 tornado/httputil.py           | 28 ++++++++++++++++++++++------
 tornado/test/httputil_test.py | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/tornado/httputil.py b/tornado/httputil.py
index 21842caa2..d299cfc45 100644
--- 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 3eb104d1c..72f4c48bb 100644
--- 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):
-- 
2.47.2