bpo-31922: Do not connect UDP sockets when broadcast is allowed (GH-423)

*Moved from python/asyncioGH-493.*

This PR fixes issue python/asyncioGH-480, as explained in [this comment](https://github.com/python/asyncio/issues/480GH-issuecomment-278703828).

The `_SelectorDatagramTransport.sendto` method has to be modified ~~so `_sock.sendto` is used in all cases (because it is tricky to reliably tell if the socket is connected or not). Could that be an issue for connected sockets?~~ *EDIT* ... so `_sock.send` is used only if `_sock` is connected.

It also protects `socket.getsockname` against `OSError` in `_SelectorTransport`. This might happen on Windows if the socket is not connected (e.g. for UDP broadcasting).

https://bugs.python.org/issue31922
(cherry picked from commit 63deaa5b70108ef441c57728322da6b4321db4fc)

Co-authored-by: Vincent Michel <vxgmichel@gmail.com>

diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py
index 65e0529e1d90c8004841019c9657712d93f29225..1252e9dda4ac97575e1ef240d3c9bbd7841c2a1e 100644 (file)
--- a/Lib/asyncio/base_events.py
+++ b/Lib/asyncio/base_events.py
@@ -1229,7 +1229,8 @@ class BaseEventLoop(events.AbstractEventLoop):
                     if local_addr:
                         sock.bind(local_address)
                     if remote_addr:
-                        await self.sock_connect(sock, remote_address)
+                        if not allow_broadcast:
+                            await self.sock_connect(sock, remote_address)
                         r_addr = remote_address
                 except OSError as exc:
                     if sock is not None:

diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py
index d2861d34332a48f96308fa3c7fd014b060a1f600..fa27b3bc23af8412c4833dde232e6726c11d70fd 100644 (file)
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -578,7 +578,10 @@ class _SelectorTransport(transports._FlowControlMixin,
     def __init__(self, loop, sock, protocol, extra=None, server=None):
         super().__init__(extra, loop)
         self._extra['socket'] = sock
-        self._extra['sockname'] = sock.getsockname()
+        try:
+            self._extra['sockname'] = sock.getsockname()
+        except OSError:
+            self._extra['sockname'] = None
         if 'peername' not in self._extra:
             try:
                 self._extra['peername'] = sock.getpeername()
@@ -968,9 +971,11 @@ class _SelectorDatagramTransport(_SelectorTransport):
         if not data:
             return
 
-        if self._address and addr not in (None, self._address):
-            raise ValueError(
-                f'Invalid address: must be None or {self._address}')
+        if self._address:
+            if addr not in (None, self._address):
+                raise ValueError(
+                    f'Invalid address: must be None or {self._address}')
+            addr = self._address
 
         if self._conn_lost and self._address:
             if self._conn_lost >= constants.LOG_THRESHOLD_FOR_CONNLOST_WRITES:
@@ -981,7 +986,7 @@ class _SelectorDatagramTransport(_SelectorTransport):
         if not self._buffer:
             # Attempt to send it right away first.
             try:
-                if self._address:
+                if self._extra['peername']:
                     self._sock.send(data)
                 else:
                     self._sock.sendto(data, addr)
@@ -1004,7 +1009,7 @@ class _SelectorDatagramTransport(_SelectorTransport):
         while self._buffer:
             data, addr = self._buffer.popleft()
             try:
-                if self._address:
+                if self._extra['peername']:
                     self._sock.send(data)
                 else:
                     self._sock.sendto(data, addr)

diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py
index 559ed3ec5279247f44e3c6fac1b15cf38605ab3f..24d3356d159b0276ca769e44cdf45fbb4bf1d798 100644 (file)
--- a/Lib/test/test_asyncio/test_base_events.py
+++ b/Lib/test/test_asyncio/test_base_events.py
@@ -1569,6 +1569,23 @@ class BaseEventLoopWithSelectorTests(test_utils.TestCase):
         self.assertRaises(
             OSError, self.loop.run_until_complete, coro)
 
+    def test_create_datagram_endpoint_allow_broadcast(self):
+        protocol = MyDatagramProto(create_future=True, loop=self.loop)
+        self.loop.sock_connect = sock_connect = mock.Mock()
+        sock_connect.return_value = []
+
+        coro = self.loop.create_datagram_endpoint(
+            lambda: protocol,
+            remote_addr=('127.0.0.1', 0),
+            allow_broadcast=True)
+
+        transport, _ = self.loop.run_until_complete(coro)
+        self.assertFalse(sock_connect.called)
+
+        transport.close()
+        self.loop.run_until_complete(protocol.done)
+        self.assertEqual('CLOSED', protocol.state)
+
     @patch_socket
     def test_create_datagram_endpoint_socket_err(self, m_socket):
         m_socket.getaddrinfo = socket.getaddrinfo

diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py
index e94af28bade37bcd1ea280706ee10cc7f264e10b..2205ee204249bba19d699732ee16d2c87fc69a9a 100644 (file)
--- a/Lib/test/test_asyncio/test_selector_events.py
+++ b/Lib/test/test_asyncio/test_selector_events.py
@@ -1467,6 +1467,7 @@ class SelectorDatagramTransportTests(test_utils.TestCase):
         self.sock.fileno.return_value = 7
 
     def datagram_transport(self, address=None):
+        self.sock.getpeername.side_effect = None if address else OSError
         transport = _SelectorDatagramTransport(self.loop, self.sock,
                                                self.protocol,
                                                address=address)

diff --git a/Misc/NEWS.d/next/Library/2018-05-30-01-05-50.bpo-31922.fobsXJ.rst b/Misc/NEWS.d/next/Library/2018-05-30-01-05-50.bpo-31922.fobsXJ.rst
new file mode 100644 (file)
index 0000000..df3881b
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-05-30-01-05-50.bpo-31922.fobsXJ.rst
@@ -0,0 +1,3 @@
+:meth:`asyncio.AbstractEventLoop.create_datagram_endpoint`:
+Do not connect UDP socket when broadcast is allowed.
+This allows to receive replies after a UDP broadcast.