From 2417cf35733e6ffdbae9e4584095324e993a17c0 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Tue, 11 Jun 2024 12:16:55 -0400 Subject: [PATCH] twisted: Delete TwistedResolver class This class was deprecated and slated for deletion in Tornado 7.0. However, it has become broken due to the adoption of RFC 8482 (which limits the use of the ANY query type in DNS) and it now fails for most domain names (with the exception of localhost). The upstream issue https://github.com/twisted/twisted/issues/10062 has been open for years which is a pretty good sign that no one is depending on this class and it's safe to remove it ahead of schedule. This class was primarily intended to provide thread-free non-blocking DNS resolution. If that is still required, `tornado.platform.caresresolver` is the next best option, although it has its own limitations which differ from TwistedResolver. --- docs/releases/v3.0.0.rst | 4 +- docs/releases/v3.2.0.rst | 2 +- docs/releases/v5.0.0.rst | 2 +- docs/twisted.rst | 26 --------- maint/scripts/test_resolvers.py | 48 +++++++++-------- tornado/netutil.py | 1 - tornado/platform/twisted.py | 96 ++------------------------------- tornado/test/netutil_test.py | 27 ---------- tornado/test/tcpclient_test.py | 2 - 9 files changed, 36 insertions(+), 172 deletions(-) diff --git a/docs/releases/v3.0.0.rst b/docs/releases/v3.0.0.rst index 53c9771f3..ab8e98419 100644 --- a/docs/releases/v3.0.0.rst +++ b/docs/releases/v3.0.0.rst @@ -264,7 +264,7 @@ Multiple modules of three optional dependencies: `~tornado.netutil.ThreadedResolver` using the `concurrent.futures` thread pool, `tornado.platform.caresresolver.CaresResolver` using the ``pycares`` - library, or `tornado.platform.twisted.TwistedResolver` using ``twisted`` + library, or ``tornado.platform.twisted.TwistedResolver`` using ``twisted`` * New function `tornado.netutil.is_valid_ip` returns true if a given string is a valid IP (v4 or v6) address. * `tornado.netutil.bind_sockets` has a new ``flags`` argument that can @@ -314,7 +314,7 @@ Multiple modules * New class ``tornado.platform.twisted.TwistedIOLoop`` allows Tornado code to be run on the Twisted reactor (as opposed to the existing ``TornadoReactor``, which bridges the gap in the other direction). -* New class `tornado.platform.twisted.TwistedResolver` is an asynchronous +* New class ``tornado.platform.twisted.TwistedResolver``` is an asynchronous implementation of the `.Resolver` interface. `tornado.process` diff --git a/docs/releases/v3.2.0.rst b/docs/releases/v3.2.0.rst index f0be99961..d22322423 100644 --- a/docs/releases/v3.2.0.rst +++ b/docs/releases/v3.2.0.rst @@ -125,7 +125,7 @@ New modules `tornado.platform.twisted` ~~~~~~~~~~~~~~~~~~~~~~~~~~ -* `.TwistedResolver` now has better error handling. +* ``.TwistedResolver``` now has better error handling. `tornado.process` ~~~~~~~~~~~~~~~~~ diff --git a/docs/releases/v5.0.0.rst b/docs/releases/v5.0.0.rst index 950b2e173..27346484a 100644 --- a/docs/releases/v5.0.0.rst +++ b/docs/releases/v5.0.0.rst @@ -257,7 +257,7 @@ Other notes `tornado.platform.twisted` ~~~~~~~~~~~~~~~~~~~~~~~~~~ -- The ``io_loop`` arguments to ``TornadoReactor``, `.TwistedResolver`, +- The ``io_loop`` arguments to ``TornadoReactor``, ``TwistedResolver``, and ``tornado.platform.twisted.install`` have been removed. `tornado.process` diff --git a/docs/twisted.rst b/docs/twisted.rst index 5d8fe8fbc..9304032d7 100644 --- a/docs/twisted.rst +++ b/docs/twisted.rst @@ -32,29 +32,3 @@ no effect on native coroutines using ``async def``). implementation was removed in Tornado 6.0.0, this function was removed as well. It was restored in Tornado 6.0.3 using the ``asyncio`` reactor instead. - -Twisted DNS resolver --------------------- - -.. class:: TwistedResolver - - Twisted-based asynchronous resolver. - - This is a non-blocking and non-threaded resolver. It is - recommended only when threads cannot be used, since it has - limitations compared to the standard ``getaddrinfo``-based - `~tornado.netutil.Resolver` and - `~tornado.netutil.DefaultExecutorResolver`. Specifically, it returns at - most one result, and arguments other than ``host`` and ``family`` - are ignored. It may fail to resolve when ``family`` is not - ``socket.AF_UNSPEC``. - - Requires Twisted 12.1 or newer. - - .. versionchanged:: 5.0 - The ``io_loop`` argument (deprecated since version 4.1) has been removed. - - .. deprecated:: 6.2 - This class is deprecated and will be removed in Tornado 7.0. Use the default - thread-based resolver instead. - diff --git a/maint/scripts/test_resolvers.py b/maint/scripts/test_resolvers.py index aea3a61f5..eb382b74c 100755 --- a/maint/scripts/test_resolvers.py +++ b/maint/scripts/test_resolvers.py @@ -1,4 +1,13 @@ #!/usr/bin/env python +"""Basic test for Tornado resolvers. + +Queries real domain names and prints the results from each resolver. +Requires a working internet connection, which is why it's not in a +unit test. + +Will be removed in Tornado 7.0 when the pluggable resolver system is +removed. +""" import pprint import socket @@ -7,18 +16,14 @@ from tornado.ioloop import IOLoop from tornado.netutil import Resolver, ThreadedResolver, DefaultExecutorResolver from tornado.options import parse_command_line, define, options -try: - import twisted -except ImportError: - twisted = None - try: import pycares except ImportError: pycares = None -define('family', default='unspec', - help='Address family to query: unspec, inet, or inet6') +define( + "family", default="unspec", help="Address family to query: unspec, inet, or inet6" +) @gen.coroutine @@ -26,33 +31,34 @@ def main(): args = parse_command_line() if not args: - args = ['localhost', 'www.google.com', - 'www.facebook.com', 'www.dropbox.com'] + args = ["localhost", "www.google.com", "www.facebook.com", "www.dropbox.com"] resolvers = [Resolver(), ThreadedResolver(), DefaultExecutorResolver()] - if twisted is not None: - from tornado.platform.twisted import TwistedResolver - resolvers.append(TwistedResolver()) - if pycares is not None: from tornado.platform.caresresolver import CaresResolver + resolvers.append(CaresResolver()) family = { - 'unspec': socket.AF_UNSPEC, - 'inet': socket.AF_INET, - 'inet6': socket.AF_INET6, + "unspec": socket.AF_UNSPEC, + "inet": socket.AF_INET, + "inet6": socket.AF_INET6, }[options.family] for host in args: - print('Resolving %s' % host) + print("Resolving %s" % host) for resolver in resolvers: - addrinfo = yield resolver.resolve(host, 80, family) - print('%s: %s' % (resolver.__class__.__name__, - pprint.pformat(addrinfo))) + try: + addrinfo = yield resolver.resolve(host, 80, family) + except Exception as e: + print("%s: %s: %s" % (resolver.__class__.__name__, type(e), e)) + else: + print( + "%s: %s" % (resolver.__class__.__name__, pprint.pformat(addrinfo)) + ) print() -if __name__ == '__main__': +if __name__ == "__main__": IOLoop.instance().run_sync(main) diff --git a/tornado/netutil.py b/tornado/netutil.py index 18c91e674..e83afad57 100644 --- a/tornado/netutil.py +++ b/tornado/netutil.py @@ -328,7 +328,6 @@ class Resolver(Configurable): * `tornado.netutil.BlockingResolver` (deprecated) * `tornado.netutil.ThreadedResolver` (deprecated) * `tornado.netutil.OverrideResolver` - * `tornado.platform.twisted.TwistedResolver` (deprecated) * `tornado.platform.caresresolver.CaresResolver` (deprecated) .. versionchanged:: 5.0 diff --git a/tornado/platform/twisted.py b/tornado/platform/twisted.py index 153fe436e..fc57e8db1 100644 --- a/tornado/platform/twisted.py +++ b/tornado/platform/twisted.py @@ -9,103 +9,17 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -"""Bridges between the Twisted package and Tornado. -""" +"""Bridges between the Twisted package and Tornado.""" -import socket import sys -import twisted.internet.abstract # type: ignore -import twisted.internet.asyncioreactor # type: ignore from twisted.internet.defer import Deferred # type: ignore from twisted.python import failure # type: ignore -import twisted.names.cache # type: ignore -import twisted.names.client # type: ignore -import twisted.names.hosts # type: ignore -import twisted.names.resolve # type: ignore - from tornado.concurrent import Future, future_set_exc_info -from tornado.escape import utf8 from tornado import gen -from tornado.netutil import Resolver - -import typing - -if typing.TYPE_CHECKING: - from typing import Generator, Any, List, Tuple # noqa: F401 - - -class TwistedResolver(Resolver): - """Twisted-based asynchronous resolver. - - This is a non-blocking and non-threaded resolver. It is - recommended only when threads cannot be used, since it has - limitations compared to the standard ``getaddrinfo``-based - `~tornado.netutil.Resolver` and - `~tornado.netutil.DefaultExecutorResolver`. Specifically, it returns at - most one result, and arguments other than ``host`` and ``family`` - are ignored. It may fail to resolve when ``family`` is not - ``socket.AF_UNSPEC``. - - Requires Twisted 12.1 or newer. - - .. versionchanged:: 5.0 - The ``io_loop`` argument (deprecated since version 4.1) has been removed. - - .. deprecated:: 6.2 - This class is deprecated and will be removed in Tornado 7.0. Use the default - thread-based resolver instead. - """ - def initialize(self) -> None: - # partial copy of twisted.names.client.createResolver, which doesn't - # allow for a reactor to be passed in. - self.reactor = twisted.internet.asyncioreactor.AsyncioSelectorReactor() - - host_resolver = twisted.names.hosts.Resolver("/etc/hosts") - cache_resolver = twisted.names.cache.CacheResolver(reactor=self.reactor) - real_resolver = twisted.names.client.Resolver( - "/etc/resolv.conf", reactor=self.reactor - ) - self.resolver = twisted.names.resolve.ResolverChain( - [host_resolver, cache_resolver, real_resolver] - ) - - @gen.coroutine - def resolve( - self, host: str, port: int, family: int = 0 - ) -> "Generator[Any, Any, List[Tuple[int, Any]]]": - # getHostByName doesn't accept IP addresses, so if the input - # looks like an IP address just return it immediately. - if twisted.internet.abstract.isIPAddress(host): - resolved = host - resolved_family = socket.AF_INET - elif twisted.internet.abstract.isIPv6Address(host): - resolved = host - resolved_family = socket.AF_INET6 - else: - deferred = self.resolver.getHostByName(utf8(host)) - fut = Future() # type: Future[Any] - deferred.addBoth(fut.set_result) - resolved = yield fut - if isinstance(resolved, failure.Failure): - try: - resolved.raiseException() - except twisted.names.error.DomainError as e: - raise IOError(e) - elif twisted.internet.abstract.isIPAddress(resolved): - resolved_family = socket.AF_INET - elif twisted.internet.abstract.isIPv6Address(resolved): - resolved_family = socket.AF_INET6 - else: - resolved_family = socket.AF_UNSPEC - if family != socket.AF_UNSPEC and family != resolved_family: - raise Exception( - "Requested socket family %d but got %d" % (family, resolved_family) - ) - result = [(typing.cast(int, resolved_family), (resolved, port))] - return result +import typing # noqa: F401 def install() -> None: @@ -127,16 +41,16 @@ def install() -> None: ``asyncio`` reactor instead. """ - from twisted.internet.asyncioreactor import install + from twisted.internet.asyncioreactor import install # type: ignore install() if hasattr(gen.convert_yielded, "register"): - @gen.convert_yielded.register(Deferred) # type: ignore + @gen.convert_yielded.register(Deferred) def _(d: Deferred) -> Future: - f = Future() # type: Future[Any] + f = Future() # type: Future[typing.Any] def errback(failure: failure.Failure) -> None: try: diff --git a/tornado/test/netutil_test.py b/tornado/test/netutil_test.py index b13c5cf08..33a8b7dba 100644 --- a/tornado/test/netutil_test.py +++ b/tornado/test/netutil_test.py @@ -29,14 +29,6 @@ except ImportError: else: from tornado.platform.caresresolver import CaresResolver -try: - import twisted # type: ignore - import twisted.names # type: ignore -except ImportError: - twisted = None -else: - from tornado.platform.twisted import TwistedResolver - class _ResolverTestMixin(object): resolver = None # type: typing.Any @@ -178,25 +170,6 @@ class CaresResolverTest(AsyncTestCase, _ResolverTestMixin): self.resolver = CaresResolver() -# TwistedResolver produces consistent errors in our test cases so we -# could test the regular and error cases in the same class. However, -# in the error cases it appears that cleanup of socket objects is -# handled asynchronously and occasionally results in "unclosed socket" -# warnings if not given time to shut down (and there is no way to -# explicitly shut it down). This makes the test flaky, so we do not -# test error cases here. -@skipIfNoNetwork -@unittest.skipIf(twisted is None, "twisted module not present") -@unittest.skipIf( - getattr(twisted, "__version__", "0.0") < "12.1", "old version of twisted" -) -@unittest.skipIf(sys.platform == "win32", "twisted resolver hangs on windows") -class TwistedResolverTest(AsyncTestCase, _ResolverTestMixin): - def setUp(self): - super().setUp() - self.resolver = TwistedResolver() - - class IsValidIPTest(unittest.TestCase): def test_is_valid_ip(self): self.assertTrue(is_valid_ip("127.0.0.1")) diff --git a/tornado/test/tcpclient_test.py b/tornado/test/tcpclient_test.py index e16479257..7116d2777 100644 --- a/tornado/test/tcpclient_test.py +++ b/tornado/test/tcpclient_test.py @@ -118,8 +118,6 @@ class TCPClientTest(AsyncTestCase): @skipIfNoIPv6 def test_connect_ipv6_dual(self): self.skipIfLocalhostV4() - if Resolver.configured_class().__name__.endswith("TwistedResolver"): - self.skipTest("TwistedResolver does not support multiple addresses") self.do_test_connect(socket.AF_INET6, "localhost") def test_connect_unspec_ipv4(self): -- 2.47.2