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/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/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) 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")