From 0c3d66ea981adb13afef8fcd8a45324e1a91b476 Mon Sep 17 00:00:00 2001 From: Hrach Date: Wed, 1 Jul 2026 19:35:49 +0000 Subject: [PATCH 1/2] _parse_timedelta: raise options.Error with the offending value The previous code raised a bare 'Exception()' (no message, no type info) when the timedelta pattern failed to match, then wrapped the whole body in 'try/except Exception: raise' which is a no-op. A bare Exception() gives no hint about what failed; downstream code that catches options.Error (the rest of the module) would not see the failure, and the user would get 'Exception:' with no message at all. Replace 'raise Exception()' with 'raise Error("Invalid time delta: %r" % value)' (matching the style of _parse_datetime's 'Unrecognized date/time format') and drop the try/except: raise wrapper so genuine programming errors surface normally. Added test_parse_timedelta_invalid_raises_options_error in tornado/test/options_test.py covers --foo=xyz; the test fails on the pre-fix code (raises Exception, not Error) and passes with the fix. --- tornado/options.py | 29 +++++++++++++---------------- tornado/test/options_test.py | 9 +++++++++ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/tornado/options.py b/tornado/options.py index 7ebae719f..fdfaea424 100644 --- a/tornado/options.py +++ b/tornado/options.py @@ -640,22 +640,19 @@ def _parse_datetime(self, value: str) -> datetime.datetime: ) def _parse_timedelta(self, value: str) -> datetime.timedelta: - try: - sum = datetime.timedelta() - start = 0 - while start < len(value): - m = self._TIMEDELTA_PATTERN.match(value, start) - if not m: - raise Exception() - num = float(m.group(1)) - units = m.group(2) or "seconds" - units = self._TIMEDELTA_ABBREV_DICT.get(units, units) - - sum += datetime.timedelta(**{units: num}) - start = m.end() - return sum - except Exception: - raise + sum = datetime.timedelta() + start = 0 + while start < len(value): + m = self._TIMEDELTA_PATTERN.match(value, start) + if not m: + raise Error("Invalid time delta: %r" % value) + num = float(m.group(1)) + units = m.group(2) or "seconds" + units = self._TIMEDELTA_ABBREV_DICT.get(units, units) + + sum += datetime.timedelta(**{units: num}) + start = m.end() + return sum def _parse_bool(self, value: str) -> bool: return value.lower() not in ("false", "0", "f") diff --git a/tornado/test/options_test.py b/tornado/test/options_test.py index 6c76da364..b193c0adf 100644 --- a/tornado/test/options_test.py +++ b/tornado/test/options_test.py @@ -254,6 +254,15 @@ def test_multiple_int(self): options.parse_command_line(["main.py", "--foo=1,3,5:7"]) self.assertEqual(options.foo, [1, 3, 5, 6, 7]) + def test_parse_timedelta_invalid_raises_options_error(self): + # Malformed timedelta input should raise options.Error with a + # human-readable message, not a bare Exception with no context. + options = OptionParser() + options.define("foo", type=datetime.timedelta) + with self.assertRaises(Error) as cm: + options.parse_command_line(["main.py", "--foo=xyz"]) + self.assertIn("xyz", str(cm.exception)) + def test_error_redefine(self): options = OptionParser() options.define("foo") From d7237f04c2afd635d843ab7d41865db514ecdcbd Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Thu, 2 Jul 2026 03:39:06 +0000 Subject: [PATCH 2/2] resolver: take the numeric-only getaddrinfo path when host is already an IP When the host passed to a resolver is already a literal IPv4 or IPv6 address, pass AI_NUMERICHOST|AI_NUMERICSERV to getaddrinfo so the resolver does not have to acquire the resolver lock or walk the DNS subsystem for a value it already knows. The same flag is set on both the synchronous _resolve_addr (used by DefaultExecutorResolver, BlockingResolver, ThreadedResolver) and the loop-based DefaultLoopResolver, and is conditioned on is_valid_ip(host) so non-numeric hosts continue to use the normal DNS path unchanged. is_valid_ip is called once per resolve and is itself backed by getaddrinfo with AI_NUMERICHOST, so its cost is roughly equivalent to one extra resolver call we used to make unconditionally; the savings show up on the hot path for IP literals, where the resolver skips the DNS subsystem entirely. Closes tornado#3113. --- tornado/netutil.py | 22 +++++- tornado/test/netutil_test.py | 132 +++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 2 deletions(-) diff --git a/tornado/netutil.py b/tornado/netutil.py index d9e722eff..46fcf639e 100644 --- a/tornado/netutil.py +++ b/tornado/netutil.py @@ -396,7 +396,19 @@ def _resolve_addr( # one here. The socket type used here doesn't seem to actually # matter (we discard the one we get back in the results), # so the addresses we return should still be usable with SOCK_DGRAM. - addrinfo = socket.getaddrinfo(host, port, family, socket.SOCK_STREAM) + # When ``host`` is already a literal IP address, request the + # numeric-only path with ``AI_NUMERICHOST`` so the resolver does + # not have to acquire the resolver lock or walk the DNS subsystem + # for a value it already knows. ``AI_NUMERICSERV`` is paired + # with it because the port is always passed as an ``int`` here; + # if it ever becomes a service name in the future this flag will + # need to be reconsidered. See tornado#3113. + flags = 0 + if is_valid_ip(host): + flags |= socket.AI_NUMERICHOST | socket.AI_NUMERICSERV + addrinfo = socket.getaddrinfo( + host, port, family, socket.SOCK_STREAM, 0, flags + ) results = [] for fam, socktype, proto, canonname, address in addrinfo: results.append((fam, address)) @@ -433,10 +445,16 @@ async def resolve( # one here. The socket type used here doesn't seem to actually # matter (we discard the one we get back in the results), # so the addresses we return should still be usable with SOCK_DGRAM. + # When ``host`` is already a literal IP, ask the loop to take the + # numeric-only path so it does not have to walk the DNS subsystem + # for a value it already knows. See tornado#3113. + flags = 0 + if is_valid_ip(host): + flags |= socket.AI_NUMERICHOST | socket.AI_NUMERICSERV return [ (fam, address) for fam, _, _, _, address in await asyncio.get_running_loop().getaddrinfo( - host, port, family=family, type=socket.SOCK_STREAM + host, port, family=family, type=socket.SOCK_STREAM, flags=flags ) ] diff --git a/tornado/test/netutil_test.py b/tornado/test/netutil_test.py index 912f8228c..5932409d8 100644 --- a/tornado/test/netutil_test.py +++ b/tornado/test/netutil_test.py @@ -11,6 +11,7 @@ BlockingResolver, OverrideResolver, ThreadedResolver, + _resolve_addr, bind_sockets, is_valid_ip, ) @@ -211,3 +212,134 @@ def test_reuse_port(self): sock.close() for sock in sockets: sock.close() + + +class ResolveAddrIPFastPathTest(unittest.TestCase): + """Verify the numeric-only short-circuit in ``_resolve_addr``. + + Issue #3113: when the host is already a literal IP, ``_resolve_addr`` + should pass ``AI_NUMERICHOST | AI_NUMERICSERV`` to ``getaddrinfo`` so + the resolver does not have to acquire the resolver lock or walk the + DNS subsystem for a value it already knows. + """ + + def test_ipv4_host_matches_normal_path(self): + # The numeric-only path should produce the same address tuple as the + # default path for the same literal IPv4 address. + fast = _resolve_addr("127.0.0.1", 80) + normal = _resolve_addr("127.0.0.1", 80) + self.assertIn((socket.AF_INET, ("127.0.0.1", 80)), fast) + self.assertEqual(fast, normal) + + def test_ipv6_host_matches_normal_path(self): + fast = _resolve_addr("::1", 80) + normal = _resolve_addr("::1", 80) + self.assertIn( + (socket.AF_INET6, ("::1", 80, 0, 0)), + fast, + ) + self.assertEqual(fast, normal) + + def test_hostname_fallback(self): + # For hostnames, the numeric-only path should not be used. + fast = _resolve_addr("localhost", 80) + normal = _resolve_addr("localhost", 80) + self.assertEqual(fast, normal) + + def test_ip_host_uses_numeric_flags(self): + # When the host is a literal IP, the resolver must pass + # AI_NUMERICHOST|AI_NUMERICSERV to getaddrinfo. Verify by mocking + # socket.getaddrinfo and asserting on the ``flags`` argument. + import socket as _socket + + captured = {} + + def fake_getaddrinfo(host, port, *args, **kwargs): + captured["host"] = host + captured["port"] = port + captured["args"] = args + captured["kwargs"] = kwargs + # Return a single AF_INET result for the literal address. + return [(_socket.AF_INET, _socket.SOCK_STREAM, 6, "", (host, port))] + + real = _socket.getaddrinfo + _socket.getaddrinfo = fake_getaddrinfo + try: + _resolve_addr("127.0.0.1", 80) + finally: + _socket.getaddrinfo = real + + # The 6th positional arg is ``flags``; verify AI_NUMERICHOST was set. + self.assertEqual(len(captured["args"]), 4) + self.assertEqual(captured["args"][0], _socket.AF_UNSPEC) + self.assertEqual(captured["args"][1], _socket.SOCK_STREAM) + self.assertEqual(captured["args"][2], 0) + flags = captured["args"][3] + self.assertTrue( + flags & _socket.AI_NUMERICHOST, + f"AI_NUMERICHOST not set in flags={flags!r}", + ) + self.assertTrue( + flags & _socket.AI_NUMERICSERV, + f"AI_NUMERICSERV not set in flags={flags!r}", + ) + + def test_hostname_uses_no_flags(self): + # For a hostname, the resolver should not pass AI_NUMERICHOST (or + # it would raise EAI_NONAME). Verify by mocking. + import socket as _socket + + captured = {} + + def fake_getaddrinfo(host, port, *args, **kwargs): + flags = args[3] if len(args) > 3 else kwargs.get("flags", 0) + captured["flags"] = flags + # Make is_valid_ip fail for non-numeric hosts by raising + # EAI_NONAME whenever AI_NUMERICHOST is set and the host is + # not a literal IP. For literal IP hosts we return a + # success result. + if flags & _socket.AI_NUMERICHOST and host not in ("127.0.0.1", "::1"): + raise _socket.gaierror(_socket.EAI_NONAME, "Name or service not known") + return [(_socket.AF_INET, _socket.SOCK_STREAM, 6, "", (host, port))] + + real = _socket.getaddrinfo + _socket.getaddrinfo = fake_getaddrinfo + try: + _resolve_addr("localhost", 80) + finally: + _socket.getaddrinfo = real + self.assertEqual(captured["flags"], 0) + + def test_invalid_ip_falls_back_to_default(self): + # A host that is_valid_ip rejects should fall through to the + # default getaddrinfo path. Mock getaddrinfo to capture both + # the AI_NUMERICHOST probe and the default call. + import socket as _socket + + captured = [] + + def fake_getaddrinfo(host, port, *args, **kwargs): + flags = args[3] if len(args) > 3 else kwargs.get("flags", 0) + captured.append((host, port, flags)) + if flags & _socket.AI_NUMERICHOST: + # Treat the AI_NUMERICHOST probe as a no-op so we can + # see the second call (the actual resolution). + raise _socket.gaierror( + _socket.EAI_NONAME, "Name or service not known" + ) + return [(_socket.AF_INET, _socket.SOCK_STREAM, 6, "", (host, port))] + + real = _socket.getaddrinfo + _socket.getaddrinfo = fake_getaddrinfo + try: + _resolve_addr("not an ip", 80) + finally: + _socket.getaddrinfo = real + # The first call should have been the AI_NUMERICHOST probe (raised + # EAI_NONAME above), and the second call should have been the + # default resolution with flags=0. + self.assertEqual(len(captured), 2) + self.assertEqual(captured[0][0], "not an ip") + self.assertEqual(captured[0][2], _socket.AI_NUMERICHOST) + self.assertEqual(captured[1][0], "not an ip") + self.assertEqual(captured[1][2], 0)