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) 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 b572beac0..69d173d77 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 @@ -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 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)