From 7dd25f4f4a767d0f26e5be0d0e4e2bc874cf0973 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Sun, 5 Jul 2026 12:06:30 +0000 Subject: [PATCH 1/2] 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/2] 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)