diff --git a/.flake8 b/.flake8 index 18c7216..3a7da2c 100644 --- a/.flake8 +++ b/.flake8 @@ -13,5 +13,7 @@ ignore = # flake8 and black disagree about # W503 line break before binary operator # E203 whitespace before ':' - W503,E203 + # E701/E704 multiple statements on one line + # https://black.readthedocs.io/en/latest/guides/using_black_with_other_tools.html#labels-why-pycodestyle-warnings + W503,E203,E701,E704 doctests = true diff --git a/debian/changelog b/debian/changelog index 327a11f..202de20 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,54 @@ +python-tornado (6.4.2-3) unstable; urgency=medium + + * Team upload. + * Increase timeout on riscv64 to fix FTBFS issue. (Closes: #1106130) + + -- Bo YU Thu, 22 May 2025 21:16:28 +0800 + +python-tornado (6.4.2-2) unstable; urgency=medium + + * Team upload. + * CVE-2025-47287: httputil: Raise errors instead of logging in + multipart/form-data parsing (closes: #1105886). + + -- Colin Watson Sun, 18 May 2025 16:43:40 +0100 + +python-tornado (6.4.2-1) unstable; urgency=medium + + * Team upload. + * New upstream release: + - CVE-2024-52804: Parsing of the cookie header is now much more + efficient. The older algorithm sometimes had quadratic performance + which allowed for a denial-of-service attack in which the server would + spend excessive CPU time parsing cookies and block the event loop + (closes: #1088112). + + -- Colin Watson Fri, 29 Nov 2024 13:09:50 +0000 + +python-tornado (6.4.1-3) unstable; urgency=medium + + * Team upload. + * Remove dependency on old singledispatch backport + * Disable on test that fails on Salsa CI but not locally + + -- Alexandre Detiste Wed, 25 Sep 2024 10:15:25 +0200 + +python-tornado (6.4.1-2) unstable; urgency=medium + + * Team upload. + * Fix tests with Twisted 24.7.0 (closes: #1078411). + * Fix tests with openssl >= 3.3.1-5 (see discussion in #965041). + + -- Colin Watson Mon, 19 Aug 2024 17:06:53 +0100 + +python-tornado (6.4.1-1) unstable; urgency=medium + + * Remove hurd patch - obsolete according to upstream + * Package new upstream. + * Drop obsolete patch for empty AsyncTestCase + + -- Julien Puydt Thu, 13 Jun 2024 17:28:38 +0200 + python-tornado (6.4.0-2) unstable; urgency=medium * Team upload. diff --git a/debian/control b/debian/control index 8eed658..e48299d 100644 --- a/debian/control +++ b/debian/control @@ -13,7 +13,6 @@ Build-Depends: ca-certificates, python3-doc, python3-pycurl, python3-setuptools, - python3-singledispatch, python3-sphinx, python3-sphinx-rtd-theme, python3-sphinxcontrib-asyncio, diff --git a/debian/patches/0006-Use-local-objects.inv-for-intersphinx-mapping.patch b/debian/patches/0006-Use-local-objects.inv-for-intersphinx-mapping.patch index 8381b0b..7f28a7a 100644 --- a/debian/patches/0006-Use-local-objects.inv-for-intersphinx-mapping.patch +++ b/debian/patches/0006-Use-local-objects.inv-for-intersphinx-mapping.patch @@ -9,11 +9,9 @@ Forwarded: not-needed docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -diff --git a/docs/conf.py b/docs/conf.py -index 424d844..8016853 100644 ---- a/docs/conf.py -+++ b/docs/conf.py -@@ -84,7 +84,7 @@ latex_documents = [ +--- python-tornado.orig/docs/conf.py ++++ python-tornado/docs/conf.py +@@ -84,7 +84,7 @@ ) ] diff --git a/debian/patches/0007-Higher-test_gc-timeout.patch b/debian/patches/0007-Higher-test_gc-timeout.patch index f377dd2..2152d72 100644 --- a/debian/patches/0007-Higher-test_gc-timeout.patch +++ b/debian/patches/0007-Higher-test_gc-timeout.patch @@ -7,11 +7,9 @@ Forwarded: not-needed tornado/test/gen_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -diff --git a/tornado/test/gen_test.py b/tornado/test/gen_test.py -index c17bf65..e2678c9 100644 ---- a/tornado/test/gen_test.py -+++ b/tornado/test/gen_test.py -@@ -967,7 +967,10 @@ class RunnerGCTest(AsyncTestCase): +--- python-tornado.orig/tornado/test/gen_test.py ++++ python-tornado/tornado/test/gen_test.py +@@ -967,7 +967,10 @@ self.io_loop.add_callback(callback) yield fut diff --git a/debian/patches/0007-allow-to-instantiate-an-empty-AsyncTestCase.patch b/debian/patches/0007-allow-to-instantiate-an-empty-AsyncTestCase.patch deleted file mode 100644 index 7e42ce7..0000000 --- a/debian/patches/0007-allow-to-instantiate-an-empty-AsyncTestCase.patch +++ /dev/null @@ -1,69 +0,0 @@ -From: Ran Benita -Date: Sun, 28 Apr 2024 14:17:54 +0300 -Subject: allow to instantiate an empty AsyncTestCase - -`unittest.TestCase` has a feature where it allows instantiating -`MyTestClass()` with the default method name `runTest` even if a -`runTest` method doesn't actually exist. This is documented in -`TestCase`'s docs under "Changed in version 3.2"[0]. - -Since version 8.2, pytest relies on this, and started breaking on -Tornado's `AsyncTestCase`[1]. - -Change `AsyncTestCase` to allow empty instatiation, by matching the -upstream code. - -[0] https://docs.python.org/3/library/unittest.html#unittest.TestCase -[1] https://github.com/pytest-dev/pytest/issues/12263 - -Origin: upstream, https://github.com/tornadoweb/tornado/pull/3374 -Bug: https://github.com/pytest-dev/pytest/issues/12263 -Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1072153 ---- - tornado/test/testing_test.py | 9 +++++++++ - tornado/testing.py | 12 +++++++++++- - 2 files changed, 20 insertions(+), 1 deletion(-) - -diff --git a/tornado/test/testing_test.py b/tornado/test/testing_test.py -index 0429fee..8e2b8db 100644 ---- a/tornado/test/testing_test.py -+++ b/tornado/test/testing_test.py -@@ -61,6 +61,15 @@ class AsyncTestCaseTest(AsyncTestCase): - self.io_loop.add_timeout(self.io_loop.time() + 0.2, self.stop) - self.wait(timeout=0.4) - -+ def test_empty_instantation_is_allowed(self): -+ """ -+ Test that empty instatiation of an AsyncTestCase is allowed. -+ -+ unittest.TestCase docs guarantee this working, and pytest's unittest -+ support relies on it. -+ """ -+ AsyncTestCaseTest() -+ - - class LeakTest(AsyncTestCase): - def tearDown(self): -diff --git a/tornado/testing.py b/tornado/testing.py -index bdbff87..9455411 100644 ---- a/tornado/testing.py -+++ b/tornado/testing.py -@@ -177,7 +177,17 @@ class AsyncTestCase(unittest.TestCase): - # the test will silently be ignored because nothing will consume - # the generator. Replace the test method with a wrapper that will - # make sure it's not an undecorated generator. -- setattr(self, methodName, _TestMethodWrapper(getattr(self, methodName))) -+ try: -+ test_method = getattr(self, methodName) -+ except AttributeError: -+ if methodName != "runTest": -+ # We allow instantiation with no explicit method name -+ # but not an *incorrect* or missing method name. -+ raise ValueError( -+ "no such test method in %s: %s" % (self.__class__, methodName) -+ ) -+ else: -+ setattr(self, methodName, _TestMethodWrapper(test_method)) - - # Not used in this class itself, but used by @gen_test - self._test_generator = None # type: Optional[Union[Generator, Coroutine]] diff --git a/debian/patches/CVE-2025-47287.patch b/debian/patches/CVE-2025-47287.patch new file mode 100644 index 0000000..209722e --- /dev/null +++ b/debian/patches/CVE-2025-47287.patch @@ -0,0 +1,231 @@ +From: Ben Darnell +Date: Thu, 8 May 2025 13:29:43 -0400 +Subject: httputil: Raise errors instead of logging in multipart/form-data + parsing + +We used to continue after logging an error, which allowed repeated +errors to spam the logs. The error raised here will still be logged, +but only once per request, consistent with other error handling in +Tornado. + +Origin: backport, https://github.com/tornadoweb/tornado/pull/3497 +Bug-Debian: https://bugs.debian.org/1105886 +Last-Update: 2025-05-18 +--- + tornado/httputil.py | 30 +++++++++++------------------- + tornado/test/httpserver_test.py | 4 ++-- + tornado/test/httputil_test.py | 13 ++++++++----- + tornado/web.py | 17 +++++++++++++---- + 4 files changed, 34 insertions(+), 30 deletions(-) + +diff --git a/tornado/httputil.py b/tornado/httputil.py +index ebdc805..090a977 100644 +--- a/tornado/httputil.py ++++ b/tornado/httputil.py +@@ -34,7 +34,6 @@ import unicodedata + from urllib.parse import urlencode, urlparse, urlunparse, parse_qsl + + from tornado.escape import native_str, parse_qs_bytes, utf8 +-from tornado.log import gen_log + from tornado.util import ObjectDict, unicode_type + + +@@ -762,25 +761,22 @@ def parse_body_arguments( + """ + if content_type.startswith("application/x-www-form-urlencoded"): + if headers and "Content-Encoding" in headers: +- gen_log.warning( +- "Unsupported Content-Encoding: %s", headers["Content-Encoding"] ++ raise HTTPInputError( ++ "Unsupported Content-Encoding: %s" % headers["Content-Encoding"] + ) +- return + try: + # 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 = {} ++ raise HTTPInputError("Invalid x-www-form-urlencoded body: %s" % e) from e + for name, values in uri_arguments.items(): + if values: + arguments.setdefault(name, []).extend(values) + elif content_type.startswith("multipart/form-data"): + if headers and "Content-Encoding" in headers: +- gen_log.warning( +- "Unsupported Content-Encoding: %s", headers["Content-Encoding"] ++ raise HTTPInputError( ++ "Unsupported Content-Encoding: %s" % headers["Content-Encoding"] + ) +- return + try: + fields = content_type.split(";") + for field in fields: +@@ -789,9 +785,9 @@ def parse_body_arguments( + parse_multipart_form_data(utf8(v), body, arguments, files) + break + else: +- raise ValueError("multipart boundary not found") ++ raise HTTPInputError("multipart boundary not found") + except Exception as e: +- gen_log.warning("Invalid multipart/form-data: %s", e) ++ raise HTTPInputError("Invalid multipart/form-data: %s" % e) from e + + + def parse_multipart_form_data( +@@ -820,26 +816,22 @@ def parse_multipart_form_data( + boundary = boundary[1:-1] + final_boundary_index = data.rfind(b"--" + boundary + b"--") + if final_boundary_index == -1: +- gen_log.warning("Invalid multipart/form-data: no final boundary") +- return ++ raise HTTPInputError("Invalid multipart/form-data: no final boundary found") + parts = data[:final_boundary_index].split(b"--" + boundary + b"\r\n") + for part in parts: + if not part: + continue + eoh = part.find(b"\r\n\r\n") + if eoh == -1: +- gen_log.warning("multipart/form-data missing headers") +- continue ++ raise HTTPInputError("multipart/form-data missing headers") + headers = HTTPHeaders.parse(part[:eoh].decode("utf-8")) + disp_header = headers.get("Content-Disposition", "") + disposition, disp_params = _parse_header(disp_header) + if disposition != "form-data" or not part.endswith(b"\r\n"): +- gen_log.warning("Invalid multipart/form-data") +- continue ++ raise HTTPInputError("Invalid multipart/form-data") + value = part[eoh + 4 : -2] + if not disp_params.get("name"): +- gen_log.warning("multipart/form-data value missing name") +- continue ++ raise HTTPInputError("multipart/form-data missing name") + name = disp_params["name"] + if disp_params.get("filename"): + ctype = headers.get("Content-Type", "application/unknown") +diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py +index 0b29a39..5d5fb13 100644 +--- a/tornado/test/httpserver_test.py ++++ b/tornado/test/httpserver_test.py +@@ -1131,9 +1131,9 @@ class GzipUnsupportedTest(GzipBaseTest, AsyncHTTPTestCase): + # Gzip support is opt-in; without it the server fails to parse + # the body (but parsing form bodies is currently just a log message, + # not a fatal error). +- with ExpectLog(gen_log, "Unsupported Content-Encoding"): ++ with ExpectLog(gen_log, ".*Unsupported Content-Encoding"): + response = self.post_gzip("foo=bar") +- self.assertEqual(json_decode(response.body), {}) ++ self.assertEqual(response.code, 400) + + + class StreamingChunkSizeTest(AsyncHTTPTestCase): +diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py +index 975900a..9494d0c 100644 +--- a/tornado/test/httputil_test.py ++++ b/tornado/test/httputil_test.py +@@ -12,7 +12,6 @@ from tornado.httputil import ( + ) + from tornado.escape import utf8, native_str + from tornado.log import gen_log +-from tornado.testing import ExpectLog + from tornado.test.util import ignore_deprecation + + import copy +@@ -195,7 +194,9 @@ Foo + b"\n", b"\r\n" + ) + args, files = form_data_args() +- with ExpectLog(gen_log, "multipart/form-data missing headers"): ++ with self.assertRaises( ++ HTTPInputError, msg="multipart/form-data missing headers" ++ ): + parse_multipart_form_data(b"1234", data, args, files) + self.assertEqual(files, {}) + +@@ -209,7 +210,7 @@ Foo + b"\n", b"\r\n" + ) + args, files = form_data_args() +- with ExpectLog(gen_log, "Invalid multipart/form-data"): ++ with self.assertRaises(HTTPInputError, msg="Invalid multipart/form-data"): + parse_multipart_form_data(b"1234", data, args, files) + self.assertEqual(files, {}) + +@@ -222,7 +223,7 @@ Foo--1234--""".replace( + b"\n", b"\r\n" + ) + args, files = form_data_args() +- with ExpectLog(gen_log, "Invalid multipart/form-data"): ++ with self.assertRaises(HTTPInputError, msg="Invalid multipart/form-data"): + parse_multipart_form_data(b"1234", data, args, files) + self.assertEqual(files, {}) + +@@ -236,7 +237,9 @@ Foo + b"\n", b"\r\n" + ) + args, files = form_data_args() +- with ExpectLog(gen_log, "multipart/form-data value missing name"): ++ with self.assertRaises( ++ HTTPInputError, msg="multipart/form-data value missing name" ++ ): + parse_multipart_form_data(b"1234", data, args, files) + self.assertEqual(files, {}) + +diff --git a/tornado/web.py b/tornado/web.py +index 0393964..8ec5601 100644 +--- a/tornado/web.py ++++ b/tornado/web.py +@@ -1751,6 +1751,14 @@ class RequestHandler(object): + try: + if self.request.method not in self.SUPPORTED_METHODS: + raise HTTPError(405) ++ ++ # If we're not in stream_request_body mode, this is the place where we parse the body. ++ if not _has_stream_request_body(self.__class__): ++ try: ++ self.request._parse_body() ++ except httputil.HTTPInputError as e: ++ raise HTTPError(400, "Invalid body: %s" % e) from e ++ + self.path_args = [self.decode_argument(arg) for arg in args] + self.path_kwargs = dict( + (k, self.decode_argument(v, name=k)) for (k, v) in kwargs.items() +@@ -1941,7 +1949,7 @@ def _has_stream_request_body(cls: Type[RequestHandler]) -> bool: + + + def removeslash( +- method: Callable[..., Optional[Awaitable[None]]] ++ method: Callable[..., Optional[Awaitable[None]]], + ) -> Callable[..., Optional[Awaitable[None]]]: + """Use this decorator to remove trailing slashes from the request path. + +@@ -1970,7 +1978,7 @@ def removeslash( + + + def addslash( +- method: Callable[..., Optional[Awaitable[None]]] ++ method: Callable[..., Optional[Awaitable[None]]], + ) -> Callable[..., Optional[Awaitable[None]]]: + """Use this decorator to add a missing trailing slash to the request path. + +@@ -2394,8 +2402,9 @@ class _HandlerDelegate(httputil.HTTPMessageDelegate): + if self.stream_request_body: + future_set_result_unless_cancelled(self.request._body_future, None) + else: ++ # Note that the body gets parsed in RequestHandler._execute so it can be in ++ # the right exception handler scope. + self.request.body = b"".join(self.chunks) +- self.request._parse_body() + self.execute() + + def on_connection_close(self) -> None: +@@ -3267,7 +3276,7 @@ class GZipContentEncoding(OutputTransform): + + + def authenticated( +- method: Callable[..., Optional[Awaitable[None]]] ++ method: Callable[..., Optional[Awaitable[None]]], + ) -> Callable[..., Optional[Awaitable[None]]]: + """Decorate methods with this to require that the user be logged in. + diff --git a/debian/patches/disable-should-be-failing-test.patch b/debian/patches/disable-should-be-failing-test.patch new file mode 100644 index 0000000..70d86b2 --- /dev/null +++ b/debian/patches/disable-should-be-failing-test.patch @@ -0,0 +1,10 @@ +--- a/tornado/test/tcpclient_test.py ++++ b/tornado/test/tcpclient_test.py +@@ -155,6 +155,7 @@ + self.do_test_connect(socket.AF_INET, "127.0.0.1", source_ip="127.0.0.1") + + @skipIfNonUnix ++ @unittest.skip('failing on Salsa CI') + def test_source_port_fail(self): + """Fail when trying to use source port 1.""" + if getpass.getuser() == "root": diff --git a/debian/patches/fix-ftbfs-on-hurd.patch b/debian/patches/fix-ftbfs-on-hurd.patch deleted file mode 100644 index 94cc52e..0000000 --- a/debian/patches/fix-ftbfs-on-hurd.patch +++ /dev/null @@ -1,27 +0,0 @@ -From: Mattia Rizzolo -Date: Sat, 21 May 2016 21:55:27 +0000 -Subject: skip UnixSocketTest on hurd, - as unix sockets with SO_REUSEADDR are not supported there - - A little discussion about unix sockets with SO_REUSEADDR can be found on - https://lists.gnu.org/archive/html/bug-hurd/2016-01/msg00039.html - -Forwarded: https://github.com/tornadoweb/tornado/issues/3290 -Patch-Name: fix-ftbfs-on-hurd.patch ---- - tornado/test/httpserver_test.py | 2 ++ - 1 file changed, 2 insertions(+) - -diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py -index 1faf63f..438f97e 100644 ---- a/tornado/test/httpserver_test.py -+++ b/tornado/test/httpserver_test.py -@@ -749,6 +749,8 @@ class ManualProtocolTest(HandlerBaseTestCase): - not hasattr(socket, "AF_UNIX") or sys.platform == "cygwin", - "unix sockets not supported on this platform", - ) -+@unittest.skipIf(sys.platform == 'gnu0', -+ "unix sockets with SO_REUSEADDR not supported on this platform") - class UnixSocketTest(AsyncTestCase): - """HTTPServers can listen on Unix sockets too. - diff --git a/debian/patches/ignoreuserwarning.patch b/debian/patches/ignoreuserwarning.patch index 43907b4..eafb325 100644 --- a/debian/patches/ignoreuserwarning.patch +++ b/debian/patches/ignoreuserwarning.patch @@ -10,11 +10,9 @@ Forwarded: not-needed tornado/test/runtests.py | 1 + 1 file changed, 1 insertion(+) -diff --git a/tornado/test/runtests.py b/tornado/test/runtests.py -index f35b372..e627e8a 100644 ---- a/tornado/test/runtests.py -+++ b/tornado/test/runtests.py -@@ -121,6 +121,7 @@ def main(): +--- python-tornado.orig/tornado/test/runtests.py ++++ python-tornado/tornado/test/runtests.py +@@ -121,6 +121,7 @@ # setuptools sometimes gives ImportWarnings about things that are on # sys.path even if they're not being used. warnings.filterwarnings("ignore", category=ImportWarning) diff --git a/debian/patches/increase-timeout-rv64.patch b/debian/patches/increase-timeout-rv64.patch new file mode 100644 index 0000000..4381ced --- /dev/null +++ b/debian/patches/increase-timeout-rv64.patch @@ -0,0 +1,29 @@ +Description: increase timeout on riscv64 +Author: Bo YU +Bug: https://bugs.debian.org/1106130 +Forwarded: not-needed +Last-Update: 2025-05-19 +--- +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ +--- a/tornado/test/autoreload_test.py ++++ b/tornado/test/autoreload_test.py +@@ -7,6 +7,7 @@ + import textwrap + import time + import unittest ++import platform + + + class AutoreloadTest(unittest.TestCase): +@@ -91,7 +92,10 @@ + for i in range(40): + if p.poll() is not None: + break +- time.sleep(0.1) ++ if platform.machine() == "riscv64": ++ time.sleep(1) ++ else: ++ time.sleep(0.1) + else: + p.kill() + raise Exception("subprocess failed to terminate") diff --git a/debian/patches/pythonpath-autoreload-test.patch b/debian/patches/pythonpath-autoreload-test.patch index 36366e7..00000b9 100644 --- a/debian/patches/pythonpath-autoreload-test.patch +++ b/debian/patches/pythonpath-autoreload-test.patch @@ -10,11 +10,9 @@ Forwarded: https://github.com/tornadoweb/tornado/pull/3358 tornado/test/autoreload_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -diff --git a/tornado/test/autoreload_test.py b/tornado/test/autoreload_test.py -index 5675faa..8bbca0f 100644 ---- a/tornado/test/autoreload_test.py -+++ b/tornado/test/autoreload_test.py -@@ -75,7 +75,7 @@ class AutoreloadTest(unittest.TestCase): +--- python-tornado.orig/tornado/test/autoreload_test.py ++++ python-tornado/tornado/test/autoreload_test.py +@@ -75,7 +75,7 @@ # application pythonpath = os.getcwd() if "PYTHONPATH" in os.environ: diff --git a/debian/patches/series b/debian/patches/series index cb5dc3c..d9af7fc 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,7 +1,8 @@ disable-domain-tests.patch ignoreuserwarning.patch -fix-ftbfs-on-hurd.patch 0006-Use-local-objects.inv-for-intersphinx-mapping.patch 0007-Higher-test_gc-timeout.patch pythonpath-autoreload-test.patch -0007-allow-to-instantiate-an-empty-AsyncTestCase.patch +disable-should-be-failing-test.patch +CVE-2025-47287.patch +increase-timeout-rv64.patch diff --git a/debian/rules b/debian/rules index cee8f7a..61964c7 100755 --- a/debian/rules +++ b/debian/rules @@ -9,6 +9,8 @@ export NO_NETWORK=1 export ASYNC_TEST_TIMEOUT=30 # py3 tests is failling without this export HOME=/tmp +# See discussion in https://bugs.debian.org/965041. +export CRYPTOGRAPHY_OPENSSL_NO_LEGACY=1 %: dh $@ --with python3,sphinxdoc --buildsystem=pybuild diff --git a/debian/tests/python3 b/debian/tests/python3 index c0f35e6..9f2a812 100755 --- a/debian/tests/python3 +++ b/debian/tests/python3 @@ -8,6 +8,8 @@ PYS=${PYS:-"$(py3versions -s 2>/dev/null)"} cd "$AUTOPKGTEST_TMP" # subprocess tests get unexpected output if $HOME/.python_history is not readable export HOME=$AUTOPKGTEST_TMP +# See discussion in https://bugs.debian.org/965041. +export CRYPTOGRAPHY_OPENSSL_NO_LEGACY=1 for py in $PYS; do echo "=== $py ===" diff --git a/demos/blog/blog.py b/demos/blog/blog.py index bd0c5b3..e6e23f8 100755 --- a/demos/blog/blog.py +++ b/demos/blog/blog.py @@ -40,13 +40,13 @@ class NoResultError(Exception): async def maybe_create_tables(db): try: - with (await db.cursor()) as cur: + with await db.cursor() as cur: await cur.execute("SELECT COUNT(*) FROM entries LIMIT 1") await cur.fetchone() except psycopg2.ProgrammingError: with open("schema.sql") as f: schema = f.read() - with (await db.cursor()) as cur: + with await db.cursor() as cur: await cur.execute(schema) @@ -89,7 +89,7 @@ async def execute(self, stmt, *args): Must be called with ``await self.execute(...)`` """ - with (await self.application.db.cursor()) as cur: + with await self.application.db.cursor() as cur: await cur.execute(stmt, args) async def query(self, stmt, *args): @@ -103,7 +103,7 @@ async def query(self, stmt, *args): for row in await self.query(...) """ - with (await self.application.db.cursor()) as cur: + with await self.application.db.cursor() as cur: await cur.execute(stmt, args) return [self.row_to_obj(row, cur) for row in await cur.fetchall()] diff --git a/demos/google_auth/main.py b/demos/google_auth/main.py index 06dd3b5..40cdd7a 100644 --- a/demos/google_auth/main.py +++ b/demos/google_auth/main.py @@ -10,6 +10,7 @@ - Run this file with `python main.py --config_file=main.cfg` - Visit "http://localhost:8888" in your browser. """ + import asyncio import json import tornado diff --git a/docs/releases.rst b/docs/releases.rst index da8dd59..5c7a106 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,6 +4,8 @@ Release notes .. toctree:: :maxdepth: 2 + releases/v6.4.2 + releases/v6.4.1 releases/v6.4.0 releases/v6.3.3 releases/v6.3.2 diff --git a/docs/releases/v6.4.1.rst b/docs/releases/v6.4.1.rst new file mode 100644 index 0000000..8d72b2b --- /dev/null +++ b/docs/releases/v6.4.1.rst @@ -0,0 +1,41 @@ +What's new in Tornado 6.4.1 +=========================== + +Jun 6, 2024 +----------- + +Security Improvements +~~~~~~~~~~~~~~~~~~~~~ + +- Parsing of the ``Transfer-Encoding`` header is now stricter. Unexpected transfer-encoding values + were previously ignored and treated as the HTTP/1.0 default of read-until-close. This can lead to + framing issues with certain proxies. We now treat any unexpected value as an error. +- Handling of whitespace in headers now matches the RFC more closely. Only space and tab characters + are treated as whitespace and stripped from the beginning and end of header values. Other unicode + whitespace characters are now left alone. This could also lead to framing issues with certain + proxies. +- ``tornado.curl_httpclient`` now prohibits carriage return and linefeed headers in HTTP headers + (matching the behavior of ``simple_httpclient``). These characters could be used for header + injection or request smuggling if untrusted data were used in headers. + +General Changes +~~~~~~~~~~~~~~~ + +`tornado.iostream` +~~~~~~~~~~~~~~~~~~ + +- `.SSLIOStream` now understands changes to error codes from OpenSSL 3.2. The main result of this + change is to reduce the noise in the logs for certain errors. + +``tornado.simple_httpclient`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- ``simple_httpclient`` now prohibits carriage return characters in HTTP headers. It had previously + prohibited only linefeed characters. + +`tornado.testing` +~~~~~~~~~~~~~~~~~ + +- `.AsyncTestCase` subclasses can now be instantiated without being associated with a test + method. This improves compatibility with test discovery in Pytest 8.2. + diff --git a/docs/releases/v6.4.2.rst b/docs/releases/v6.4.2.rst new file mode 100644 index 0000000..0dc567d --- /dev/null +++ b/docs/releases/v6.4.2.rst @@ -0,0 +1,12 @@ +What's new in Tornado 6.4.2 +=========================== + +Nov 21, 2024 +------------ + +Security Improvements +~~~~~~~~~~~~~~~~~~~~~ + +- Parsing of the cookie header is now much more efficient. The older algorithm sometimes had + quadratic performance which allowed for a denial-of-service attack in which the server would spend + excessive CPU time parsing cookies and block the event loop. This change fixes CVE-2024-7592. \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index ce09865..9118bdf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,7 +8,7 @@ alabaster==0.7.13 # via sphinx babel==2.11.0 # via sphinx -black==22.12.0 +black==24.4.2 # via -r requirements.in build==0.10.0 # via pip-tools @@ -38,11 +38,11 @@ filelock==3.12.0 # virtualenv flake8==6.0.0 # via -r requirements.in -idna==3.4 +idna==3.7 # via requests imagesize==1.4.1 # via sphinx -jinja2==3.1.2 +jinja2==3.1.4 # via sphinx markupsafe==2.1.2 # via jinja2 @@ -56,6 +56,7 @@ mypy-extensions==0.4.3 # mypy packaging==23.1 # via + # black # build # pyproject-api # sphinx @@ -83,7 +84,7 @@ pyproject-hooks==1.0.0 # via build pytz==2022.7.1 # via babel -requests==2.31.0 +requests==2.32.2 # via sphinx snowballstemmer==2.2.0 # via sphinx diff --git a/tornado/__init__.py b/tornado/__init__.py index a0ae714..91e4cde 100644 --- a/tornado/__init__.py +++ b/tornado/__init__.py @@ -22,8 +22,8 @@ # is zero for an official release, positive for a development branch, # or negative for a release candidate or beta (after the base version # number has been incremented) -version = "6.4" -version_info = (6, 4, 0, 0) +version = "6.4.2" +version_info = (6, 4, 2, 0) import importlib import typing diff --git a/tornado/concurrent.py b/tornado/concurrent.py index 86bbd70..5047c53 100644 --- a/tornado/concurrent.py +++ b/tornado/concurrent.py @@ -118,6 +118,7 @@ def foo(self): The ``callback`` argument was removed. """ + # Fully type-checking decorators is tricky, and this one is # discouraged anyway so it doesn't have all the generic magic. def run_on_executor_decorator(fn: Callable) -> Callable[..., Future]: diff --git a/tornado/curl_httpclient.py b/tornado/curl_httpclient.py index 23320e4..397c3a9 100644 --- a/tornado/curl_httpclient.py +++ b/tornado/curl_httpclient.py @@ -19,6 +19,7 @@ import functools import logging import pycurl +import re import threading import time from io import BytesIO @@ -44,6 +45,8 @@ curl_log = logging.getLogger("tornado.curl_httpclient") +CR_OR_LF_RE = re.compile(b"\r|\n") + class CurlAsyncHTTPClient(AsyncHTTPClient): def initialize( # type: ignore @@ -347,14 +350,15 @@ def _curl_setup_request( if "Pragma" not in request.headers: request.headers["Pragma"] = "" - curl.setopt( - pycurl.HTTPHEADER, - [ - b"%s: %s" - % (native_str(k).encode("ASCII"), native_str(v).encode("ISO8859-1")) - for k, v in request.headers.get_all() - ], - ) + encoded_headers = [ + b"%s: %s" + % (native_str(k).encode("ASCII"), native_str(v).encode("ISO8859-1")) + for k, v in request.headers.get_all() + ] + for line in encoded_headers: + if CR_OR_LF_RE.search(line): + raise ValueError("Illegal characters in header (CR or LF): %r" % line) + curl.setopt(pycurl.HTTPHEADER, encoded_headers) curl.setopt( pycurl.HEADERFUNCTION, diff --git a/tornado/gen.py b/tornado/gen.py index dab4fd0..0e3c7a6 100644 --- a/tornado/gen.py +++ b/tornado/gen.py @@ -66,6 +66,7 @@ def get(self): via ``singledispatch``. """ + import asyncio import builtins import collections @@ -165,13 +166,11 @@ def _fake_ctx_run(f: Callable[..., _T], *args: Any, **kw: Any) -> _T: @overload def coroutine( func: Callable[..., "Generator[Any, Any, _T]"] -) -> Callable[..., "Future[_T]"]: - ... +) -> Callable[..., "Future[_T]"]: ... @overload -def coroutine(func: Callable[..., _T]) -> Callable[..., "Future[_T]"]: - ... +def coroutine(func: Callable[..., _T]) -> Callable[..., "Future[_T]"]: ... def coroutine( diff --git a/tornado/http1connection.py b/tornado/http1connection.py index ca50e8f..1a23f5c 100644 --- a/tornado/http1connection.py +++ b/tornado/http1connection.py @@ -38,6 +38,8 @@ from typing import cast, Optional, Type, Awaitable, Callable, Union, Tuple +CR_OR_LF_RE = re.compile(b"\r|\n") + class _QuietException(Exception): def __init__(self) -> None: @@ -389,14 +391,11 @@ def write_headers( self._request_start_line = start_line lines.append(utf8("%s %s HTTP/1.1" % (start_line[0], start_line[1]))) # Client requests with a non-empty body must have either a - # Content-Length or a Transfer-Encoding. + # Content-Length or a Transfer-Encoding. If Content-Length is not + # present we'll add our Transfer-Encoding below. self._chunking_output = ( start_line.method in ("POST", "PUT", "PATCH") and "Content-Length" not in headers - and ( - "Transfer-Encoding" not in headers - or headers["Transfer-Encoding"] == "chunked" - ) ) else: assert isinstance(start_line, httputil.ResponseStartLine) @@ -418,9 +417,6 @@ def write_headers( and (start_line.code < 100 or start_line.code >= 200) # No need to chunk the output if a Content-Length is specified. and "Content-Length" not in headers - # Applications are discouraged from touching Transfer-Encoding, - # but if they do, leave it alone. - and "Transfer-Encoding" not in headers ) # If connection to a 1.1 client will be closed, inform client if ( @@ -453,8 +449,8 @@ def write_headers( ) lines.extend(line.encode("latin1") for line in header_lines) for line in lines: - if b"\n" in line: - raise ValueError("Newline in header: " + repr(line)) + if CR_OR_LF_RE.search(line): + raise ValueError("Illegal characters (CR or LF) in header: %r" % line) future = None if self.stream.closed(): future = self._write_future = Future() @@ -560,7 +556,7 @@ def _can_keep_alive( return connection_header != "close" elif ( "Content-Length" in headers - or headers.get("Transfer-Encoding", "").lower() == "chunked" + or is_transfer_encoding_chunked(headers) or getattr(start_line, "method", None) in ("HEAD", "GET") ): # start_line may be a request or response start line; only @@ -598,13 +594,6 @@ def _read_body( delegate: httputil.HTTPMessageDelegate, ) -> Optional[Awaitable[None]]: if "Content-Length" in headers: - if "Transfer-Encoding" in headers: - # Response cannot contain both Content-Length and - # Transfer-Encoding headers. - # http://tools.ietf.org/html/rfc7230#section-3.3.3 - raise httputil.HTTPInputError( - "Response with both Transfer-Encoding and Content-Length" - ) if "," in headers["Content-Length"]: # Proxies sometimes cause Content-Length headers to get # duplicated. If all the values are identical then we can @@ -631,20 +620,22 @@ def _read_body( else: content_length = None + is_chunked = is_transfer_encoding_chunked(headers) + if code == 204: # This response code is not allowed to have a non-empty body, # and has an implicit length of zero instead of read-until-close. # http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.3 - if "Transfer-Encoding" in headers or content_length not in (None, 0): + if is_chunked or content_length not in (None, 0): raise httputil.HTTPInputError( "Response with code %d should not have body" % code ) content_length = 0 + if is_chunked: + return self._read_chunked_body(delegate) if content_length is not None: return self._read_fixed_body(content_length, delegate) - if headers.get("Transfer-Encoding", "").lower() == "chunked": - return self._read_chunked_body(delegate) if self.is_client: return self._read_body_until_close(delegate) return None @@ -863,3 +854,33 @@ def parse_hex_int(s: str) -> int: if HEXDIGITS.fullmatch(s) is None: raise ValueError("not a hexadecimal integer: %r" % s) return int(s, 16) + + +def is_transfer_encoding_chunked(headers: httputil.HTTPHeaders) -> bool: + """Returns true if the headers specify Transfer-Encoding: chunked. + + Raise httputil.HTTPInputError if any other transfer encoding is used. + """ + # Note that transfer-encoding is an area in which postel's law can lead + # us astray. If a proxy and a backend server are liberal in what they accept, + # but accept slightly different things, this can lead to mismatched framing + # and request smuggling issues. Therefore we are as strict as possible here + # (even technically going beyond the requirements of the RFCs: a value of + # ",chunked" is legal but doesn't appear in practice for legitimate traffic) + if "Transfer-Encoding" not in headers: + return False + if "Content-Length" in headers: + # Message cannot contain both Content-Length and + # Transfer-Encoding headers. + # http://tools.ietf.org/html/rfc7230#section-3.3.3 + raise httputil.HTTPInputError( + "Message with both Transfer-Encoding and Content-Length" + ) + if headers["Transfer-Encoding"].lower() == "chunked": + return True + # We do not support any transfer-encodings other than chunked, and we do not + # expect to add any support because the concept of transfer-encoding has + # been removed in HTTP/2. + raise httputil.HTTPInputError( + "Unsupported Transfer-Encoding %s" % headers["Transfer-Encoding"] + ) diff --git a/tornado/httputil.py b/tornado/httputil.py index b21d804..ebdc805 100644 --- a/tornado/httputil.py +++ b/tornado/httputil.py @@ -62,6 +62,9 @@ from asyncio import Future # noqa: F401 import unittest # noqa: F401 +# To be used with str.strip() and related methods. +HTTP_WHITESPACE = " \t" + @lru_cache(1000) def _normalize_header(name: str) -> str: @@ -171,7 +174,7 @@ def parse_line(self, line: str) -> None: # continuation of a multi-line header if self._last_key is None: raise HTTPInputError("first header line cannot start with whitespace") - new_part = " " + line.lstrip() + new_part = " " + line.lstrip(HTTP_WHITESPACE) self._as_list[self._last_key][-1] += new_part self._dict[self._last_key] += new_part else: @@ -179,7 +182,7 @@ def parse_line(self, line: str) -> None: name, value = line.split(":", 1) except ValueError: raise HTTPInputError("no colon in header line") - self.add(name, value.strip()) + self.add(name, value.strip(HTTP_WHITESPACE)) @classmethod def parse(cls, headers: str) -> "HTTPHeaders": @@ -1054,15 +1057,20 @@ def qs_to_qsl(qs: Dict[str, List[AnyStr]]) -> Iterable[Tuple[str, AnyStr]]: yield (k, v) -_OctalPatt = re.compile(r"\\[0-3][0-7][0-7]") -_QuotePatt = re.compile(r"[\\].") -_nulljoin = "".join +_unquote_sub = re.compile(r"\\(?:([0-3][0-7][0-7])|(.))").sub + + +def _unquote_replace(m: re.Match) -> str: + if m[1]: + return chr(int(m[1], 8)) + else: + return m[2] def _unquote_cookie(s: str) -> str: """Handle double quotes and escaping in cookie values. - This method is copied verbatim from the Python 3.5 standard + This method is copied verbatim from the Python 3.13 standard library (http.cookies._unquote) so we don't have to depend on non-public interfaces. """ @@ -1083,30 +1091,7 @@ def _unquote_cookie(s: str) -> str: # \012 --> \n # \" --> " # - i = 0 - n = len(s) - res = [] - while 0 <= i < n: - o_match = _OctalPatt.search(s, i) - q_match = _QuotePatt.search(s, i) - if not o_match and not q_match: # Neither matched - res.append(s[i:]) - break - # else: - j = k = -1 - if o_match: - j = o_match.start(0) - if q_match: - k = q_match.start(0) - if q_match and (not o_match or k < j): # QuotePatt matched - res.append(s[i:k]) - res.append(s[k + 1]) - i = k + 2 - else: # OctalPatt matched - res.append(s[i:j]) - res.append(chr(int(s[j + 1 : j + 4], 8))) - i = j + 4 - return _nulljoin(res) + return _unquote_sub(_unquote_replace, s) def parse_cookie(cookie: str) -> Dict[str, str]: diff --git a/tornado/iostream.py b/tornado/iostream.py index bd001ae..ee57759 100644 --- a/tornado/iostream.py +++ b/tornado/iostream.py @@ -1374,7 +1374,7 @@ def _do_ssl_handshake(self) -> None: return elif err.args[0] in (ssl.SSL_ERROR_EOF, ssl.SSL_ERROR_ZERO_RETURN): return self.close(exc_info=err) - elif err.args[0] == ssl.SSL_ERROR_SSL: + elif err.args[0] in (ssl.SSL_ERROR_SSL, ssl.SSL_ERROR_SYSCALL): try: peer = self.socket.getpeername() except Exception: diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index 2460863..5b2d4dc 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -429,9 +429,9 @@ async def run(self) -> None: self.request.method == "POST" and "Content-Type" not in self.request.headers ): - self.request.headers[ - "Content-Type" - ] = "application/x-www-form-urlencoded" + self.request.headers["Content-Type"] = ( + "application/x-www-form-urlencoded" + ) if self.request.decompress_response: self.request.headers["Accept-Encoding"] = "gzip" req_path = (self.parsed.path or "/") + ( diff --git a/tornado/test/__main__.py b/tornado/test/__main__.py index 430c895..890bd50 100644 --- a/tornado/test/__main__.py +++ b/tornado/test/__main__.py @@ -2,6 +2,7 @@ This only works in python 2.7+. """ + from tornado.test.runtests import all, main # tornado.testing.main autodiscovery relies on 'all' being present in diff --git a/tornado/test/escape_test.py b/tornado/test/escape_test.py index 6bd2ae7..3115a19 100644 --- a/tornado/test/escape_test.py +++ b/tornado/test/escape_test.py @@ -194,9 +194,11 @@ ( "www.external-link.com and www.internal-link.com/blogs extra", { - "extra_params": lambda href: 'class="internal"' - if href.startswith("http://www.internal-link.com") - else 'rel="nofollow" class="external"' + "extra_params": lambda href: ( + 'class="internal"' + if href.startswith("http://www.internal-link.com") + else 'rel="nofollow" class="external"' + ) }, 'www.external-link.com' # noqa: E501 ' and www.internal-link.com/blogs extra', # noqa: E501 diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index 31a1916..17291f8 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -725,6 +725,22 @@ def test_error_after_cancel(self): if el.logged_stack: break + def test_header_crlf(self): + # Ensure that the client doesn't allow CRLF injection in headers. RFC 9112 section 2.2 + # prohibits a bare CR specifically and "a recipient MAY recognize a single LF as a line + # terminator" so we check each character separately as well as the (redundant) CRLF pair. + for header, name in [ + ("foo\rbar:", "cr"), + ("foo\nbar:", "lf"), + ("foo\r\nbar:", "crlf"), + ]: + with self.subTest(name=name, position="value"): + with self.assertRaises(ValueError): + self.fetch("/hello", headers={"foo": header}) + with self.subTest(name=name, position="key"): + with self.assertRaises(ValueError): + self.fetch("/hello", headers={header: "foo"}) + class RequestProxyTest(unittest.TestCase): def test_request_set(self): diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py index 1faf63f..0b29a39 100644 --- a/tornado/test/httpserver_test.py +++ b/tornado/test/httpserver_test.py @@ -581,6 +581,76 @@ def test_chunked_request_body_invalid_size(self): ) self.assertEqual(400, start_line.code) + def test_chunked_request_body_duplicate_header(self): + # Repeated Transfer-Encoding headers should be an error (and not confuse + # the chunked-encoding detection to mess up framing). + self.stream.write( + b"""\ +POST /echo HTTP/1.1 +Transfer-Encoding: chunked +Transfer-encoding: chunked + +2 +ok +0 + +""" + ) + with ExpectLog( + gen_log, + ".*Unsupported Transfer-Encoding chunked,chunked", + level=logging.INFO, + ): + start_line, headers, response = self.io_loop.run_sync( + lambda: read_stream_body(self.stream) + ) + self.assertEqual(400, start_line.code) + + def test_chunked_request_body_unsupported_transfer_encoding(self): + # We don't support transfer-encodings other than chunked. + self.stream.write( + b"""\ +POST /echo HTTP/1.1 +Transfer-Encoding: gzip, chunked + +2 +ok +0 + +""" + ) + with ExpectLog( + gen_log, ".*Unsupported Transfer-Encoding gzip, chunked", level=logging.INFO + ): + start_line, headers, response = self.io_loop.run_sync( + lambda: read_stream_body(self.stream) + ) + self.assertEqual(400, start_line.code) + + def test_chunked_request_body_transfer_encoding_and_content_length(self): + # Transfer-encoding and content-length are mutually exclusive + self.stream.write( + b"""\ +POST /echo HTTP/1.1 +Transfer-Encoding: chunked +Content-Length: 2 + +2 +ok +0 + +""" + ) + with ExpectLog( + gen_log, + ".*Message with both Transfer-Encoding and Content-Length", + level=logging.INFO, + ): + start_line, headers, response = self.io_loop.run_sync( + lambda: read_stream_body(self.stream) + ) + self.assertEqual(400, start_line.code) + @gen_test def test_invalid_content_length(self): # HTTP only allows decimal digits in content-length. Make sure we don't diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py index aa9b6ee..975900a 100644 --- a/tornado/test/httputil_test.py +++ b/tornado/test/httputil_test.py @@ -334,6 +334,25 @@ def test_unicode_newlines(self): gen_log.warning("failed while trying %r in %s", newline, encoding) raise + def test_unicode_whitespace(self): + # Only tabs and spaces are to be stripped according to the HTTP standard. + # Other unicode whitespace is to be left as-is. In the context of headers, + # this specifically means the whitespace characters falling within the + # latin1 charset. + whitespace = [ + (" ", True), # SPACE + ("\t", True), # TAB + ("\u00a0", False), # NON-BREAKING SPACE + ("\u0085", False), # NEXT LINE + ] + for c, stripped in whitespace: + headers = HTTPHeaders.parse("Transfer-Encoding: %schunked" % c) + if stripped: + expected = [("Transfer-Encoding", "chunked")] + else: + expected = [("Transfer-Encoding", "%schunked" % c)] + self.assertEqual(expected, list(headers.get_all())) + def test_optional_cr(self): # Both CRLF and LF should be accepted as separators. CR should not be # part of the data when followed by LF, but it is a normal char @@ -541,3 +560,49 @@ def test_invalid_cookies(self): self.assertEqual( parse_cookie(" = b ; ; = ; c = ; "), {"": "b", "c": ""} ) + + def test_unquote(self): + # Copied from + # https://github.com/python/cpython/blob/dc7a2b6522ec7af41282bc34f405bee9b306d611/Lib/test/test_http_cookies.py#L62 + cases = [ + (r'a="b=\""', 'b="'), + (r'a="b=\\"', "b=\\"), + (r'a="b=\="', "b=="), + (r'a="b=\n"', "b=n"), + (r'a="b=\042"', 'b="'), + (r'a="b=\134"', "b=\\"), + (r'a="b=\377"', "b=\xff"), + (r'a="b=\400"', "b=400"), + (r'a="b=\42"', "b=42"), + (r'a="b=\\042"', "b=\\042"), + (r'a="b=\\134"', "b=\\134"), + (r'a="b=\\\""', 'b=\\"'), + (r'a="b=\\\042"', 'b=\\"'), + (r'a="b=\134\""', 'b=\\"'), + (r'a="b=\134\042"', 'b=\\"'), + ] + for encoded, decoded in cases: + with self.subTest(encoded): + c = parse_cookie(encoded) + self.assertEqual(c["a"], decoded) + + def test_unquote_large(self): + # Adapted from + # https://github.com/python/cpython/blob/dc7a2b6522ec7af41282bc34f405bee9b306d611/Lib/test/test_http_cookies.py#L87 + # Modified from that test because we handle semicolons differently from the stdlib. + # + # This is a performance regression test: prior to improvements in Tornado 6.4.2, this test + # would take over a minute with n= 100k. Now it runs in tens of milliseconds. + n = 100000 + for encoded in r"\\", r"\134": + with self.subTest(encoded): + start = time.time() + data = 'a="b=' + encoded * n + '"' + value = parse_cookie(data)["a"] + end = time.time() + self.assertEqual(value[:3], "b=\\") + self.assertEqual(value[-3:], "\\\\\\") + self.assertEqual(len(value), n + 2) + + # Very loose performance check to avoid false positives + self.assertLess(end - start, 1, "Test took too long") diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py index 9485afe..d07438a 100644 --- a/tornado/test/ioloop_test.py +++ b/tornado/test/ioloop_test.py @@ -261,6 +261,7 @@ def test_close_file_object(self): the object should be closed (by IOLoop.close(all_fds=True), not just the fd. """ + # Use a socket since they are supported by IOLoop on all platforms. # Unfortunately, sockets don't support the .closed attribute for # inspecting their close status, so we must use a wrapper. diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py index 62bd483..593f81f 100644 --- a/tornado/test/simple_httpclient_test.py +++ b/tornado/test/simple_httpclient_test.py @@ -828,7 +828,7 @@ def test_chunked_with_content_length(self): with ExpectLog( gen_log, ( - "Malformed HTTP message from None: Response " + "Malformed HTTP message from None: Message " "with both Transfer-Encoding and Content-Length" ), level=logging.INFO, diff --git a/tornado/test/testing_test.py b/tornado/test/testing_test.py index 0429fee..4432bb1 100644 --- a/tornado/test/testing_test.py +++ b/tornado/test/testing_test.py @@ -5,7 +5,6 @@ from tornado.web import Application import asyncio import contextlib -import inspect import gc import os import platform @@ -118,7 +117,11 @@ def tearDown(self): super().tearDown() -class AsyncTestCaseWrapperTest(unittest.TestCase): +class AsyncTestCaseReturnAssertionsTest(unittest.TestCase): + # These tests verify that tests that return non-None values (without being decorated with + # @gen_test) raise errors instead of incorrectly succeeding. These tests should be removed or + # updated when the _callTestMethod method is removed from AsyncTestCase (the same checks will + # still happen, but they'll be performed in the stdlib as DeprecationWarnings) def test_undecorated_generator(self): class Test(AsyncTestCase): def test_gen(self): @@ -135,7 +138,10 @@ def test_gen(self): "pypy destructor warnings cannot be silenced", ) @unittest.skipIf( - sys.version_info >= (3, 12), "py312 has its own check for test case returns" + # This check actually exists in 3.11 but it changed in 3.12 in a way that breaks + # this test. + sys.version_info >= (3, 12), + "py312 has its own check for test case returns", ) def test_undecorated_coroutine(self): class Test(AsyncTestCase): @@ -176,17 +182,6 @@ def test_other_return(self): self.assertEqual(len(result.errors), 1) self.assertIn("Return value from test method ignored", result.errors[0][1]) - def test_unwrap(self): - class Test(AsyncTestCase): - def test_foo(self): - pass - - test = Test("test_foo") - self.assertIs( - inspect.unwrap(test.test_foo), - test.test_foo.orig_method, # type: ignore[attr-defined] - ) - class SetUpTearDownTest(unittest.TestCase): def test_set_up_tear_down(self): diff --git a/tornado/test/twisted_test.py b/tornado/test/twisted_test.py index 7f983a7..36a541a 100644 --- a/tornado/test/twisted_test.py +++ b/tornado/test/twisted_test.py @@ -18,10 +18,7 @@ from tornado.testing import AsyncTestCase, gen_test try: - from twisted.internet.defer import ( # type: ignore - inlineCallbacks, - returnValue, - ) + from twisted.internet.defer import inlineCallbacks # type: ignore have_twisted = True except ImportError: @@ -43,7 +40,7 @@ def fn(): # inlineCallbacks doesn't work with regular functions; # must have a yield even if it's unreachable. yield - returnValue(42) + return 42 res = yield fn() self.assertEqual(res, 42) diff --git a/tornado/testing.py b/tornado/testing.py index bdbff87..4c33b3e 100644 --- a/tornado/testing.py +++ b/tornado/testing.py @@ -84,39 +84,6 @@ def get_async_test_timeout() -> float: return 5 -class _TestMethodWrapper(object): - """Wraps a test method to raise an error if it returns a value. - - This is mainly used to detect undecorated generators (if a test - method yields it must use a decorator to consume the generator), - but will also detect other kinds of return values (these are not - necessarily errors, but we alert anyway since there is no good - reason to return a value from a test). - """ - - def __init__(self, orig_method: Callable) -> None: - self.orig_method = orig_method - self.__wrapped__ = orig_method - - def __call__(self, *args: Any, **kwargs: Any) -> None: - result = self.orig_method(*args, **kwargs) - if isinstance(result, Generator) or inspect.iscoroutine(result): - raise TypeError( - "Generator and coroutine test methods should be" - " decorated with tornado.testing.gen_test" - ) - elif result is not None: - raise ValueError("Return value from test method ignored: %r" % result) - - def __getattr__(self, name: str) -> Any: - """Proxy all unknown attributes to the original method. - - This is important for some of the decorators in the `unittest` - module, such as `unittest.skipIf`. - """ - return getattr(self.orig_method, name) - - class AsyncTestCase(unittest.TestCase): """`~unittest.TestCase` subclass for testing `.IOLoop`-based asynchronous code. @@ -173,12 +140,6 @@ def __init__(self, methodName: str = "runTest") -> None: self.__stop_args = None # type: Any self.__timeout = None # type: Optional[object] - # It's easy to forget the @gen_test decorator, but if you do - # the test will silently be ignored because nothing will consume - # the generator. Replace the test method with a wrapper that will - # make sure it's not an undecorated generator. - setattr(self, methodName, _TestMethodWrapper(getattr(self, methodName))) - # Not used in this class itself, but used by @gen_test self._test_generator = None # type: Optional[Union[Generator, Coroutine]] @@ -289,6 +250,30 @@ def run( self.__rethrow() return ret + def _callTestMethod(self, method: Callable) -> None: + """Run the given test method, raising an error if it returns non-None. + + Failure to decorate asynchronous test methods with ``@gen_test`` can lead to tests + incorrectly passing. + + Remove this override when Python 3.10 support is dropped. This check (in the form of a + DeprecationWarning) became a part of the standard library in 3.11. + + Note that ``_callTestMethod`` is not documented as a public interface. However, it is + present in all supported versions of Python (3.8+), and if it goes away in the future that's + OK because we can just remove this override as noted above. + """ + # Calling super()._callTestMethod would hide the return value, even in python 3.8-3.10 + # where the check isn't being done for us. + result = method() + if isinstance(result, Generator) or inspect.iscoroutine(result): + raise TypeError( + "Generator and coroutine test methods should be" + " decorated with tornado.testing.gen_test" + ) + elif result is not None: + raise ValueError("Return value from test method ignored: %r" % result) + def stop(self, _arg: Any = None, **kwargs: Any) -> None: """Stops the `.IOLoop`, causing one pending (or future) call to `wait()` to return. diff --git a/tornado/websocket.py b/tornado/websocket.py index fbfd700..8f0e0ae 100644 --- a/tornado/websocket.py +++ b/tornado/websocket.py @@ -1392,9 +1392,9 @@ def __init__( # from the server). # TODO: set server parameters for deflate extension # if requested in self.compression_options. - request.headers[ - "Sec-WebSocket-Extensions" - ] = "permessage-deflate; client_max_window_bits" + request.headers["Sec-WebSocket-Extensions"] = ( + "permessage-deflate; client_max_window_bits" + ) # Websocket connection is currently unable to follow redirects request.follow_redirects = False