From 28773465e699b4a9a2e6411de80fb06d2652d3e3 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 13 Feb 2014 09:24:37 +0100 Subject: [PATCH] ayncio, Tulip issue 129: BaseEventLoop.sock_connect() now raises an error if the address is not resolved (hostname instead of an IP address) for AF_INET and AF_INET6 address families. --- Doc/library/asyncio-eventloop.rst | 6 ++++++ Lib/asyncio/base_events.py | 25 +++++++++++++++++++++++++ Lib/asyncio/proactor_events.py | 9 ++++++++- Lib/asyncio/selector_events.py | 20 ++++++++------------ Lib/test/test_asyncio/test_events.py | 12 ++++++++++++ 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst index c9721b46c0..7760fcb168 100644 --- a/Doc/library/asyncio-eventloop.rst +++ b/Doc/library/asyncio-eventloop.rst @@ -366,6 +366,12 @@ Low-level socket operations Connect to a remote socket at *address*. + The *address* must be already resolved to avoid the trap of hanging the + entire event loop when the address requires doing a DNS lookup. For + example, it must be an IP address, not an hostname, for + :py:data:`~socket.AF_INET` and :py:data:`~socket.AF_INET6` address families. + Use :meth:`getaddrinfo` to resolve the hostname asynchronously. + This method returns a :ref:`coroutine object `. .. seealso:: diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 7d120424df..3bbf6b5466 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -41,6 +41,31 @@ class _StopError(BaseException): """Raised to stop the event loop.""" +def _check_resolved_address(sock, address): + # Ensure that the address is already resolved to avoid the trap of hanging + # the entire event loop when the address requires doing a DNS lookup. + family = sock.family + if family not in (socket.AF_INET, socket.AF_INET6): + return + + host, port = address + type_mask = 0 + if hasattr(socket, 'SOCK_NONBLOCK'): + type_mask |= socket.SOCK_NONBLOCK + if hasattr(socket, 'SOCK_CLOEXEC'): + type_mask |= socket.SOCK_CLOEXEC + # Use getaddrinfo(AI_NUMERICHOST) to ensure that the address is + # already resolved. + try: + socket.getaddrinfo(host, port, + family=family, + type=(sock.type & ~type_mask), + proto=sock.proto, + flags=socket.AI_NUMERICHOST) + except socket.gaierror as err: + raise ValueError("address must be resolved (IP address), got %r: %s" + % (address, err)) + def _raise_stop_error(*args): raise _StopError diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index 74566b2ec9..5de4d3d691 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -404,7 +404,14 @@ class BaseProactorEventLoop(base_events.BaseEventLoop): return self._proactor.send(sock, data) def sock_connect(self, sock, address): - return self._proactor.connect(sock, address) + try: + base_events._check_resolved_address(sock, address) + except ValueError as err: + fut = futures.Future(loop=self) + fut.set_exception(err) + return fut + else: + return self._proactor.connect(sock, address) def sock_accept(self, sock): return self._proactor.accept(sock) diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py index 14231c5fb8..10b0257910 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -208,6 +208,8 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop): return fut def _sock_recv(self, fut, registered, sock, n): + # _sock_recv() can add itself as an I/O callback if the operation can't + # be done immediatly. Don't use it directly, call sock_recv(). fd = sock.fileno() if registered: # Remove the callback early. It should be rare that the @@ -260,22 +262,16 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop): def sock_connect(self, sock, address): """XXX""" - # That address better not require a lookup! We're not calling - # self.getaddrinfo() for you here. But verifying this is - # complicated; the socket module doesn't have a pattern for - # IPv6 addresses (there are too many forms, apparently). fut = futures.Future(loop=self) - self._sock_connect(fut, False, sock, address) + try: + base_events._check_resolved_address(sock, address) + except ValueError as err: + fut.set_exception(err) + else: + self._sock_connect(fut, False, sock, address) return fut def _sock_connect(self, fut, registered, sock, address): - # TODO: Use getaddrinfo() to look up the address, to avoid the - # trap of hanging the entire event loop when the address - # requires doing a DNS lookup. (OTOH, the caller should - # already have done this, so it would be nice if we could - # easily tell whether the address needs looking up or not. I - # know how to do this for IPv4, but IPv6 addresses have many - # syntaxes.) fd = sock.fileno() if registered: self.remove_writer(fd) diff --git a/Lib/test/test_asyncio/test_events.py b/Lib/test/test_asyncio/test_events.py index d5d667a327..4300ddd0ff 100644 --- a/Lib/test/test_asyncio/test_events.py +++ b/Lib/test/test_asyncio/test_events.py @@ -1191,6 +1191,18 @@ class EventLoopTestsMixin: {'clock_resolution': self.loop._clock_resolution, 'selector': self.loop._selector.__class__.__name__}) + def test_sock_connect_address(self): + address = ('www.python.org', 80) + for family in (socket.AF_INET, socket.AF_INET6): + for sock_type in (socket.SOCK_STREAM, socket.SOCK_DGRAM): + sock = socket.socket(family, sock_type) + with sock: + connect = self.loop.sock_connect(sock, address) + with self.assertRaises(ValueError) as cm: + self.loop.run_until_complete(connect) + self.assertIn('address must be resolved', + str(cm.exception)) + class SubprocessTestsMixin: -- 2.40.0