Repository: tornadoweb/tornado
Pull request #2735 (2019-08-29 03:06:09)

parse_body_arguments: allow incomplete url-escaping
support x-www-form-urlencoded body with values consisting of
encoded bytes which are not url-encoded into ascii
(as long as there is no ambiguous `&` or `%` or `=`)

it seems other web frameworks often support this

inspired by #2733 and #2572 

ploxiln: suggestions for making the tests more concise and elegant would be welcome :)

Patch:
diff --git a/tornado/escape.py b/tornado/escape.py
index b0ec3323..8d2d6bc8 100644
--- a/tornado/escape.py
+++ b/tornado/escape.py
@@ -145,10 +145,10 @@ def url_unescape(  # noqa: F811
 
 
 def parse_qs_bytes(
-    qs: str, keep_blank_values: bool = False, strict_parsing: bool = False
+    qs: Union[str, bytes], keep_blank_values: bool = False, strict_parsing: bool = False
 ) -> Dict[str, List[bytes]]:
-    """Parses a query string like urlparse.parse_qs, but returns the
-    values as byte strings.
+    """Parses a query string like urlparse.parse_qs,
+    but takes bytes and returns the values as byte strings.
 
     Keys still become type str (interpreted as latin1 in python3!)
     because it's too painful to keep them as byte strings in
@@ -156,6 +156,8 @@ def parse_qs_bytes(
     """
     # This is gross, but python3 doesn't give us another way.
     # Latin1 is the universal donor of character encodings.
+    if isinstance(qs, bytes):
+        qs = qs.decode("latin1")
     result = urllib.parse.parse_qs(
         qs, keep_blank_values, strict_parsing, encoding="latin1", errors="strict"
     )
diff --git a/tornado/httputil.py b/tornado/httputil.py
index 26a6c440..758d30d9 100644
--- a/tornado/httputil.py
+++ b/tornado/httputil.py
@@ -783,7 +783,8 @@ def parse_body_arguments(
             )
             return
         try:
-            uri_arguments = parse_qs_bytes(native_str(body), keep_blank_values=True)
+            # real charset decoding will happen in RequestHandler.decode_argument()
+            uri_arguments = parse_qs_bytes(body, keep_blank_values=True)
         except Exception as e:
             gen_log.warning("Invalid x-www-form-urlencoded body: %s", e)
             uri_arguments = {}


Test patch:
diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py
index 2a8b6a5b..4ad3ce7e 100644
--- a/tornado/test/httpserver_test.py
+++ b/tornado/test/httpserver_test.py
@@ -41,6 +41,7 @@ import ssl
 import sys
 import tempfile
 import unittest
+import urllib.parse
 from io import BytesIO
 
 import typing
@@ -378,6 +379,19 @@ class TypeCheckHandler(RequestHandler):
             self.errors[name] = "expected %s, got %s" % (expected_type, actual_type)
 
 
+class PostEchoHandler(RequestHandler):
+    def post(self, *path_args):
+        self.write(dict(echo=self.get_argument("data")))
+
+
+class PostEchoGBKHandler(PostEchoHandler):
+    def decode_argument(self, value, name=None):
+        try:
+            return value.decode("gbk")
+        except Exception:
+            raise HTTPError(400, "invalid gbk bytes: %r" % value)
+
+
 class HTTPServerTest(AsyncHTTPTestCase):
     def get_app(self):
         return Application(
@@ -385,6 +399,8 @@ class HTTPServerTest(AsyncHTTPTestCase):
                 ("/echo", EchoHandler),
                 ("/typecheck", TypeCheckHandler),
                 ("//doubleslash", EchoHandler),
+                ("/post_utf8", PostEchoHandler),
+                ("/post_gbk", PostEchoGBKHandler),
             ]
         )
 
@@ -423,18 +439,22 @@ class HTTPServerTest(AsyncHTTPTestCase):
         self.assertEqual(200, response.code)
         self.assertEqual(json_decode(response.body), {})
 
-    def test_malformed_body(self):
-        # parse_qs is pretty forgiving, but it will fail on python 3
-        # if the data is not utf8.
-        with ExpectLog(gen_log, "Invalid x-www-form-urlencoded body"):
-            response = self.fetch(
-                "/echo",
-                method="POST",
-                headers={"Content-Type": "application/x-www-form-urlencoded"},
-                body=b"\xe9",
-            )
-        self.assertEqual(200, response.code)
-        self.assertEqual(b"{}", response.body)
+    def test_post_encodings(self):
+        headers = {"Content-Type": "application/x-www-form-urlencoded"}
+        uni_text = "chinese: \u5f20\u4e09"
+        for enc in ("utf8", "gbk"):
+            for quote in (True, False):
+                with self.subTest(enc=enc, quote=quote):
+                    bin_text = uni_text.encode(enc)
+                    if quote:
+                        bin_text = urllib.parse.quote(bin_text).encode("ascii")
+                    response = self.fetch(
+                        "/post_" + enc,
+                        method="POST",
+                        headers=headers,
+                        body=(b"data=" + bin_text),
+                    )
+                    self.assertEqual(json_decode(response.body), {"echo": uni_text})
 
 
 class HTTPServerRawTest(AsyncHTTPTestCase):
@@ -943,7 +963,7 @@ class GzipBaseTest(object):
 
     def test_uncompressed(self):
         response = self.fetch("/", method="POST", body="foo=bar")
-        self.assertEquals(json_decode(response.body), {u"foo": [u"bar"]})
+        self.assertEqual(json_decode(response.body), {u"foo": [u"bar"]})
 
 
 class GzipTest(GzipBaseTest, AsyncHTTPTestCase):
@@ -952,7 +972,7 @@ class GzipTest(GzipBaseTest, AsyncHTTPTestCase):
 
     def test_gzip(self):
         response = self.post_gzip("foo=bar")
-        self.assertEquals(json_decode(response.body), {u"foo": [u"bar"]})
+        self.assertEqual(json_decode(response.body), {u"foo": [u"bar"]})
 
 
 class GzipUnsupportedTest(GzipBaseTest, AsyncHTTPTestCase):
@@ -962,7 +982,7 @@ class GzipUnsupportedTest(GzipBaseTest, AsyncHTTPTestCase):
         # not a fatal error).
         with ExpectLog(gen_log, "Unsupported Content-Encoding"):
             response = self.post_gzip("foo=bar")
-        self.assertEquals(json_decode(response.body), {})
+        self.assertEqual(json_decode(response.body), {})
 
 
 class StreamingChunkSizeTest(AsyncHTTPTestCase):
diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py
index c094a09e..2ebd9060 100644
--- a/tornado/test/httputil_test.py
+++ b/tornado/test/httputil_test.py
@@ -388,7 +388,7 @@ Foo: even
         headers.add("Foo", "2")
         headers.add("Foo", "3")
         headers2 = HTTPHeaders.parse(str(headers))
-        self.assertEquals(headers, headers2)
+        self.assertEqual(headers, headers2)
 
 
 class FormatTimestampTest(unittest.TestCase):
diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py
index bb9ca119..2933ea50 100644
--- a/tornado/test/simple_httpclient_test.py
+++ b/tornado/test/simple_httpclient_test.py
@@ -389,7 +389,7 @@ class SimpleHTTPClientTestMixin(object):
         if response.body == b"HTTP/1 required":
             self.skipTest("requires HTTP/1.x")
         else:
-            self.assertEquals(b"hello", response.body)
+            self.assertEqual(b"hello", response.body)
 
     def sync_body_producer(self, write):
         write(b"1234")
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index d43d46bc..5908710a 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -2284,7 +2284,7 @@ class StreamingRequestBodyTest(WebTestCase):
         self.data = Future()
         stream.write(b"4\r\nqwer\r\n")
         data = yield self.data
-        self.assertEquals(data, b"qwer")
+        self.assertEqual(data, b"qwer")
         stream.write(b"0\r\n\r\n")
         yield self.finished
         data = yield stream.read_until_close()