[Python-checkins] cpython (3.4): ayncio, Tulip issue 129: BaseEventLoop.sock_connect() now raises an error if

[larry.hastings]

http://hg.python.org/cpython/rev/b4c75a9f1541
changeset:   89708:b4c75a9f1541
branch:      3.4
user:        Victor Stinner <victor.stinner at gmail.com>
date:        Thu Feb 13 09:24:37 2014 +0100
summary:
  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.

files:
  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
--- a/Doc/library/asyncio-eventloop.rst
+++ b/Doc/library/asyncio-eventloop.rst
@@ -366,6 +366,12 @@
 
    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 <coroutine>`.
 
    .. seealso::
diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py
--- a/Lib/asyncio/base_events.py
+++ b/Lib/asyncio/base_events.py
@@ -41,6 +41,31 @@
     """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
--- a/Lib/asyncio/proactor_events.py
+++ b/Lib/asyncio/proactor_events.py
@@ -404,7 +404,14 @@
         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
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -208,6 +208,8 @@
         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 @@
 
     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
--- a/Lib/test/test_asyncio/test_events.py
+++ b/Lib/test/test_asyncio/test_events.py
@@ -1191,6 +1191,18 @@
             {'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:
 

-- 
Repository URL: http://hg.python.org/cpython