From 73e8637002639e565938d3f205bf46e7f1dbd6a8 Mon Sep 17 00:00:00 2001 From: Jamie Phan Date: Sat, 17 Feb 2024 13:38:07 +1100 Subject: [PATCH] gh-113812: Allow DatagramTransport.sendto to send empty data (#115199) Also include the UDP packet header sizes (8 bytes per packet) in the buffer size reported to the flow control subsystem. --- Doc/library/asyncio-protocol.rst | 5 +++++ Doc/whatsnew/3.13.rst | 6 +++++ Lib/asyncio/proactor_events.py | 5 +---- Lib/asyncio/selector_events.py | 4 +--- Lib/asyncio/transports.py | 2 ++ Lib/test/test_asyncio/test_proactor_events.py | 22 ++++++++++++++----- Lib/test/test_asyncio/test_selector_events.py | 19 ++++++++++++---- ...-02-09-12-22-47.gh-issue-113812.wOraaG.rst | 3 +++ 8 files changed, 50 insertions(+), 16 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-02-09-12-22-47.gh-issue-113812.wOraaG.rst diff --git a/Doc/library/asyncio-protocol.rst b/Doc/library/asyncio-protocol.rst index ecd8cdc709a..7c08d65f26b 100644 --- a/Doc/library/asyncio-protocol.rst +++ b/Doc/library/asyncio-protocol.rst @@ -362,6 +362,11 @@ Datagram Transports This method does not block; it buffers the data and arranges for it to be sent out asynchronously. + .. versionchanged:: 3.13 + This method can be called with an empty bytes object to send a + zero-length datagram. The buffer size calculation used for flow + control is also updated to account for the datagram header. + .. method:: DatagramTransport.abort() Close the transport immediately, without waiting for pending diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 1e0764144a2..7c6a2af2875 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -218,6 +218,12 @@ asyncio the Unix socket when the server is closed. (Contributed by Pierre Ossman in :gh:`111246`.) +* :meth:`asyncio.DatagramTransport.sendto` will now send zero-length + datagrams if called with an empty bytes object. The transport flow + control also now accounts for the datagram header when calculating + the buffer size. + (Contributed by Jamie Phan in :gh:`115199`.) + copy ---- diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index 1e2a730cf36..a512db6367b 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -487,9 +487,6 @@ class _ProactorDatagramTransport(_ProactorBasePipeTransport, raise TypeError('data argument must be bytes-like object (%r)', type(data)) - if not data: - return - if self._address is not None and addr not in (None, self._address): raise ValueError( f'Invalid address: must be None or {self._address}') @@ -502,7 +499,7 @@ class _ProactorDatagramTransport(_ProactorBasePipeTransport, # Ensure that what we buffer is immutable. self._buffer.append((bytes(data), addr)) - self._buffer_size += len(data) + self._buffer_size += len(data) + 8 # include header bytes if self._write_fut is None: # No current write operations are active, kick one off diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py index 10fbdd76e93..8e888d26ea0 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -1241,8 +1241,6 @@ class _SelectorDatagramTransport(_SelectorTransport, transports.DatagramTranspor if not isinstance(data, (bytes, bytearray, memoryview)): raise TypeError(f'data argument must be a bytes-like object, ' f'not {type(data).__name__!r}') - if not data: - return if self._address: if addr not in (None, self._address): @@ -1278,7 +1276,7 @@ class _SelectorDatagramTransport(_SelectorTransport, transports.DatagramTranspor # Ensure that what we buffer is immutable. self._buffer.append((bytes(data), addr)) - self._buffer_size += len(data) + self._buffer_size += len(data) + 8 # include header bytes self._maybe_pause_protocol() def _sendto_ready(self): diff --git a/Lib/asyncio/transports.py b/Lib/asyncio/transports.py index 30fd41d49af..34c7ad44ffd 100644 --- a/Lib/asyncio/transports.py +++ b/Lib/asyncio/transports.py @@ -181,6 +181,8 @@ class DatagramTransport(BaseTransport): to be sent out asynchronously. addr is target socket address. If addr is None use target address pointed on transport creation. + If data is an empty bytes object a zero-length datagram will be + sent. """ raise NotImplementedError diff --git a/Lib/test/test_asyncio/test_proactor_events.py b/Lib/test/test_asyncio/test_proactor_events.py index c42856e578b..fcaa2f6ade2 100644 --- a/Lib/test/test_asyncio/test_proactor_events.py +++ b/Lib/test/test_asyncio/test_proactor_events.py @@ -585,11 +585,10 @@ class ProactorDatagramTransportTests(test_utils.TestCase): def test_sendto_no_data(self): transport = self.datagram_transport() - transport._buffer.append((b'data', ('0.0.0.0', 12345))) - transport.sendto(b'', ()) - self.assertFalse(self.sock.sendto.called) - self.assertEqual( - [(b'data', ('0.0.0.0', 12345))], list(transport._buffer)) + transport.sendto(b'', ('0.0.0.0', 1234)) + self.assertTrue(self.proactor.sendto.called) + self.proactor.sendto.assert_called_with( + self.sock, b'', addr=('0.0.0.0', 1234)) def test_sendto_buffer(self): transport = self.datagram_transport() @@ -628,6 +627,19 @@ class ProactorDatagramTransportTests(test_utils.TestCase): list(transport._buffer)) self.assertIsInstance(transport._buffer[1][0], bytes) + def test_sendto_buffer_nodata(self): + data2 = b'' + transport = self.datagram_transport() + transport._buffer.append((b'data1', ('0.0.0.0', 12345))) + transport._write_fut = object() + transport.sendto(data2, ('0.0.0.0', 12345)) + self.assertFalse(self.proactor.sendto.called) + self.assertEqual( + [(b'data1', ('0.0.0.0', 12345)), + (b'', ('0.0.0.0', 12345))], + list(transport._buffer)) + self.assertIsInstance(transport._buffer[1][0], bytes) + @mock.patch('asyncio.proactor_events.logger') def test_sendto_exception(self, m_log): data = b'data' diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py index c22b780b5ed..aaeda33dd0c 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -1280,11 +1280,10 @@ class SelectorDatagramTransportTests(test_utils.TestCase): def test_sendto_no_data(self): transport = self.datagram_transport() - transport._buffer.append((b'data', ('0.0.0.0', 12345))) - transport.sendto(b'', ()) - self.assertFalse(self.sock.sendto.called) + transport.sendto(b'', ('0.0.0.0', 1234)) + self.assertTrue(self.sock.sendto.called) self.assertEqual( - [(b'data', ('0.0.0.0', 12345))], list(transport._buffer)) + self.sock.sendto.call_args[0], (b'', ('0.0.0.0', 1234))) def test_sendto_buffer(self): transport = self.datagram_transport() @@ -1320,6 +1319,18 @@ class SelectorDatagramTransportTests(test_utils.TestCase): list(transport._buffer)) self.assertIsInstance(transport._buffer[1][0], bytes) + def test_sendto_buffer_nodata(self): + data2 = b'' + transport = self.datagram_transport() + transport._buffer.append((b'data1', ('0.0.0.0', 12345))) + transport.sendto(data2, ('0.0.0.0', 12345)) + self.assertFalse(self.sock.sendto.called) + self.assertEqual( + [(b'data1', ('0.0.0.0', 12345)), + (b'', ('0.0.0.0', 12345))], + list(transport._buffer)) + self.assertIsInstance(transport._buffer[1][0], bytes) + def test_sendto_tryagain(self): data = b'data' diff --git a/Misc/NEWS.d/next/Library/2024-02-09-12-22-47.gh-issue-113812.wOraaG.rst b/Misc/NEWS.d/next/Library/2024-02-09-12-22-47.gh-issue-113812.wOraaG.rst new file mode 100644 index 00000000000..7ef7bc891cd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-02-09-12-22-47.gh-issue-113812.wOraaG.rst @@ -0,0 +1,3 @@ +:meth:`DatagramTransport.sendto` will now send zero-length datagrams if +called with an empty bytes object. The transport flow control also now +accounts for the datagram header when calculating the buffer size.