From 63deaa5b70108ef441c57728322da6b4321db4fc Mon Sep 17 00:00:00 2001 From: Vincent Michel Date: Tue, 7 May 2019 19:18:49 +0200 Subject: bpo-31922: Do not connect UDP sockets when broadcast is allowed (GH-423) *Moved from python/asyncio#493.* This PR fixes issue python/asyncio#480, as explained in [this comment](https://github.com/python/asyncio/issues/480#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 --- Lib/asyncio/base_events.py | 3 ++- Lib/asyncio/selector_events.py | 17 +++++++++++------ Lib/test/test_asyncio/test_base_events.py | 17 +++++++++++++++++ Lib/test/test_asyncio/test_selector_events.py | 1 + .../Library/2018-05-30-01-05-50.bpo-31922.fobsXJ.rst | 3 +++ 5 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-05-30-01-05-50.bpo-31922.fobsXJ.rst diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index c58906f..9613ac2 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -1306,7 +1306,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 93b6889..2996821 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -587,7 +587,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() @@ -976,9 +979,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: @@ -989,7 +994,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) @@ -1012,7 +1017,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 c245c47..25420b2 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -1586,6 +1586,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 d0d171a..bf721b0 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -1065,6 +1065,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 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. -- cgit v0.12