ioloop: reject zero/negative timedelta in PeriodicCallback constructor

tornado/ioloop.py

@@ -897,11 +897,12 @@ def __init__(
     ) -> None:
         self.callback = callback
         if isinstance(callback_time, datetime.timedelta):
-            self.callback_time = callback_time / datetime.timedelta(milliseconds=1)
+            callback_time_ms = callback_time / datetime.timedelta(milliseconds=1)
         else:
-            if callback_time <= 0:
-                raise ValueError("Periodic callback must have a positive callback_time")
-            self.callback_time = callback_time
+            callback_time_ms = float(callback_time)
+        if callback_time_ms <= 0:
+            raise ValueError("Periodic callback must have a positive callback_time")
+        self.callback_time = callback_time_ms
         self.jitter = jitter
         self._running = False
         self._timeout: object = None

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:

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)

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)

tornado/test/ioloop_test.py

@@ -715,6 +715,21 @@ def test_timedelta(self):
         expected_callback_time = 83000
         self.assertEqual(pc.callback_time, expected_callback_time)
 
+    def test_timedelta_zero_raises(self):
+        # Zero timedelta previously fell through the numeric-only check
+        # and produced callback_time = 0.0, which the start() loop would
+        # re-schedule immediately. Now it raises ValueError up front.
+        with self.assertRaises(ValueError) as cm:
+            PeriodicCallback(lambda: None, datetime.timedelta(0))
+        self.assertIn("positive", str(cm.exception))
+
+    def test_timedelta_negative_raises(self):
+        # Negative timedelta previously produced a negative callback_time
+        # (no error at construction time), which silently broke scheduling.
+        with self.assertRaises(ValueError) as cm:
+            PeriodicCallback(lambda: None, datetime.timedelta(microseconds=-1))
+        self.assertIn("positive", str(cm.exception))
+
 
 class TestPeriodicCallbackAsync(AsyncTestCase):
     def test_periodic_plain(self):

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)

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', <class>)"`` 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)

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
 
 

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)