From 7dd25f4f4a767d0f26e5be0d0e4e2bc874cf0973 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Sun, 5 Jul 2026 12:06:30 +0000 Subject: [PATCH 1/3] web, websocket: format ValueError messages in raise These two raise statements pass extra positional arguments to `ValueError` but use only the format string as the first argument, so the result is a tuple of (format_string, arg) instead of the formatted message. str(exc) then shows the literal format string rather than the intended text. Use string formatting to produce a single formatted message, matching the pattern used everywhere else in the codebase. Affected paths: - web.RequestHandler.xsrf_token: unknown xsrf cookie version %d - WebSocketProtocol13._process_server_headers: unsupported extension %r --- tornado/web.py | 2 +- tornado/websocket.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tornado/web.py b/tornado/web.py index b572beac0..723058caf 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -1564,7 +1564,7 @@ def xsrf_token(self) -> bytes: ] ) else: - raise ValueError("unknown xsrf cookie version %d", output_version) + raise ValueError("unknown xsrf cookie version %d" % output_version) if version is None: if self.current_user and "expires_days" not in cookie_kwargs: cookie_kwargs["expires_days"] = 30 diff --git a/tornado/websocket.py b/tornado/websocket.py index 4e4ad23ef..4922d63dd 100644 --- a/tornado/websocket.py +++ b/tornado/websocket.py @@ -993,7 +993,7 @@ def _process_server_headers( if ext[0] == "permessage-deflate" and self._compression_options is not None: self._create_compressors("client", ext[1]) else: - raise ValueError("unsupported extension %r", ext) + raise ValueError("unsupported extension %r" % ext) self.selected_subprotocol = headers.get("Sec-WebSocket-Protocol", None) From 878154e4a0e120823c03bdf35023ad06415c0b2f Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Sun, 5 Jul 2026 17:42:40 +0000 Subject: [PATCH 2/3] netutil, simple_httpclient: format the ValueError messages in raise bind_unix_socket raised ValueError('File %s exists and is not a socket', file) and _HTTPConnection.run raised ValueError('unsupported auth_mode %s', mode). Both pass a literal format string and the value as separate arguments, so str(exc) renders the tuple repr with the format string and the value tucked into e.args[1] rather than producing a usable error message. Replace with the formatted single-argument form so the offending file path or mode name is actually visible to a user reading the traceback or a log line. Two regression tests: - tornado/test/netutil_test.py::TestBindUnixSocket:: test_existing_non_socket_file_message_includes_path creates a regular file where bind_unix_socket expects a socket, captures the leaked fd from the error path, and asserts the path and 'exists and is not a socket' appear in str(exc) with a single-arg ValueError. - tornado/test/httpclient_test.py::HTTPClientCommonTestCase:: test_unsupported_auth_mode_message_format fetches /auth with auth_mode= 'asdf' against the simple client and asserts the 'asdf' literal appears in the raised ValueError and '%s' does not. --- tornado/netutil.py | 2 +- tornado/simple_httpclient.py | 2 +- tornado/test/httpclient_test.py | 26 +++++++++++++++++++ tornado/test/netutil_test.py | 45 +++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 2 deletions(-) diff --git a/tornado/netutil.py b/tornado/netutil.py index d9e722eff..dc1957773 100644 --- a/tornado/netutil.py +++ b/tornado/netutil.py @@ -221,7 +221,7 @@ def bind_unix_socket( if stat.S_ISSOCK(st.st_mode): os.remove(file) else: - raise ValueError("File %s exists and is not a socket", file) + raise ValueError("File %s exists and is not a socket" % file) sock.bind(file) os.chmod(file, mode) else: diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index b8e4d8c93..051f21475 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -385,7 +385,7 @@ async def run(self) -> None: if username is not None: assert password is not None if self.request.auth_mode not in (None, "basic"): - raise ValueError("unsupported auth_mode %s", self.request.auth_mode) + raise ValueError("unsupported auth_mode %s" % self.request.auth_mode) self.request.headers["Authorization"] = "Basic " + _unicode( base64.b64encode( httputil.encode_username_password(username, password) diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index 1540ac3c2..aec66470b 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -322,6 +322,32 @@ def test_unsupported_auth_mode(self): raise_error=True, ) + def test_unsupported_auth_mode_message_format(self): + # The "unsupported auth_mode" ValueError used to be raised with a + # two-arg form (ValueError("unsupported auth_mode %s", mode)) so + # str(e) rendered the literal format string with the mode tucked + # into e.args[1]. The simple client now formats the message before + # raising, so a user reading the traceback or log line sees the + # actual offending mode. Only assert the format on the simple + # client; the curl client wraps the same condition in an HTTPError + # so we skip the check there. + client_cls = self.http_client.__class__ + if not client_cls.__name__.startswith("Simple"): + return + with ExpectLog(gen_log, "uncaught exception", required=False): + with self.assertRaises(ValueError) as cm: + self.fetch( + "/auth", + auth_username="Aladdin", + auth_password="open sesame", + auth_mode="asdf", + raise_error=True, + ) + self.assertIn("asdf", str(cm.exception)) + self.assertNotIn("%s", str(cm.exception)) + self.assertEqual(len(cm.exception.args), 1) + self.assertIsInstance(cm.exception.args[0], str) + def test_follow_redirect(self): response = self.fetch("/countdown/2", follow_redirects=False) self.assertEqual(302, response.code) diff --git a/tornado/test/netutil_test.py b/tornado/test/netutil_test.py index 912f8228c..c7723b005 100644 --- a/tornado/test/netutil_test.py +++ b/tornado/test/netutil_test.py @@ -1,7 +1,9 @@ import errno +import os import signal import socket import sys +import tempfile import time import typing import unittest @@ -211,3 +213,46 @@ def test_reuse_port(self): sock.close() for sock in sockets: sock.close() + + +@unittest.skipIf(not hasattr(socket, "AF_UNIX"), "AF_UNIX is not supported") +class TestBindUnixSocket(unittest.TestCase): + def test_existing_non_socket_file_message_includes_path(self) -> None: + from tornado.netutil import bind_unix_socket + + path = os.path.join( + tempfile.mkdtemp(), + "tornado-bind-unix-not-a-socket", + ) + self.addCleanup(os.rmdir, os.path.dirname(path)) + with open(path, "w") as f: + f.write("not a socket") + self.addCleanup(os.remove, path) + # bind_unix_socket creates the AF_UNIX socket before it inspects the + # path, so on the "exists and is not a socket" error path the socket fd + # is leaked. Monkey-patch the constructor to capture and close it. + leaked_socks: list[socket.socket] = [] + original_socket = socket.socket + + def capturing_socket(*args: typing.Any, **kwargs: typing.Any) -> socket.socket: + s = original_socket(*args, **kwargs) + leaked_socks.append(s) + return s + + socket.socket = capturing_socket # type: ignore[assignment] + self.addCleanup(setattr, socket, "socket", original_socket) + try: + with self.assertRaises(ValueError) as cm: + bind_unix_socket(path) + finally: + for s in leaked_socks: + s.close() + # The exception message must be a single, formatted string that names + # the offending path. Pre-fix it was the two-arg form + # ValueError("File %s exists and is not a socket", file) which made + # str(e) render the literal format string instead of the path. + self.assertIn(path, str(cm.exception)) + self.assertIn("exists and is not a socket", str(cm.exception)) + self.assertNotIn("%s", str(cm.exception)) + self.assertEqual(len(cm.exception.args), 1) + self.assertIsInstance(cm.exception.args[0], str) From 1b8687e0edbac21eb0dd12493b9a48967ef522e9 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Sun, 5 Jul 2026 21:27:31 +0000 Subject: [PATCH 3/3] web: format the stream_request_body TypeError message with the class stream_request_body and _has_stream_request_body used the two-argument form raise TypeError("expected subclass of RequestHandler, got %r", cls) which stores the format string and class as a tuple in e.args. str(e) became the tuple repr "('expected subclass of RequestHandler, got %r', )" with the literal "%r" placeholder visible and the class tucked into e.args[1]. The user-visible message did not mention RequestHandler or the class, and any assertRaisesRegex over the formatted string failed. Format the message before raising so str(e) is a single, formatted string that names both the expected type and the offending class. Add StreamRequestBodyTypeErrorTest with two regression guards that assert the formatted message contains the class name, does not contain the "%r" placeholder, and that e.args[0] is the same string as str(e). On pre-fix code both fail with the literal "%r" placeholder visible. --- tornado/test/web_test.py | 34 ++++++++++++++++++++++++++++++++++ tornado/web.py | 4 ++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index 36049e26b..2bbfc0b11 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -16,6 +16,7 @@ from io import BytesIO from tornado import gen, locale +import tornado.web from tornado.concurrent import Future from tornado.escape import ( json_decode, @@ -3438,3 +3439,36 @@ def test_accept_language_ignore(self): def test_accept_language_invalid(self): response = self.fetch("/", headers={"Accept-Language": "fr-FR;q=-1"}) self.assertEqual(response.headers["Content-Language"], "en-US") + + +class StreamRequestBodyTypeErrorTest(unittest.TestCase): + def test_stream_request_body_rejects_non_handler(self): + # stream_request_body and _has_stream_request_body used to call + # ``raise TypeError("expected subclass of RequestHandler, got %r", cls)`` + # with the format string as a separate argument. That puts the literal + # ``"%r"`` placeholder and the class object into ``e.args`` as a tuple + # rather than formatting the message, so ``str(e)`` was the tuple repr + # ``"('expected subclass of RequestHandler, got %r', )"`` and + # ``e.args[0]`` did not even mention ``RequestHandler``. + class NotAHandler: + pass + + with self.assertRaises(TypeError) as cm: + stream_request_body(NotAHandler) + msg = str(cm.exception) + self.assertIn("expected subclass of RequestHandler", msg) + self.assertIn("NotAHandler", msg) + self.assertNotIn("%r", msg) + self.assertEqual(cm.exception.args[0], msg) + + def test_has_stream_request_body_rejects_non_handler(self): + class NotAHandler: + pass + + with self.assertRaises(TypeError) as cm: + tornado.web._has_stream_request_body(NotAHandler) + msg = str(cm.exception) + self.assertIn("expected subclass of RequestHandler", msg) + self.assertIn("NotAHandler", msg) + self.assertNotIn("%r", msg) + self.assertEqual(cm.exception.args[0], msg) diff --git a/tornado/web.py b/tornado/web.py index 723058caf..69d173d77 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -2001,14 +2001,14 @@ def stream_request_body(cls: type[_RequestHandlerType]) -> type[_RequestHandlerT for example usage. """ # noqa: E501 if not issubclass(cls, RequestHandler): - raise TypeError("expected subclass of RequestHandler, got %r", cls) + raise TypeError("expected subclass of RequestHandler, got %r" % cls) cls._stream_request_body = True return cls def _has_stream_request_body(cls: type[RequestHandler]) -> bool: if not issubclass(cls, RequestHandler): - raise TypeError("expected subclass of RequestHandler, got %r", cls) + raise TypeError("expected subclass of RequestHandler, got %r" % cls) return cls._stream_request_body