From b3d98e89b74dcda7ce72cca1f13bfa13280b81ae Mon Sep 17 00:00:00 2001 From: Martin Teichmann Date: Tue, 9 Sep 2025 15:53:43 +0200 Subject: [PATCH] remove unnecessary and buggy type checks in asyncio Within asyncio, several `write`, `send` and `sendto` methods contained a check whether the `data` parameter is one of the *right* types, complaining that the data should be bytes-like. This error message is very misleading if one hands over a byte-like object that happens not to be one of the *right* types. Usually, these kinds of tests are not necessary at all, as the code the data is handed to will complain itself if the type is wrong. But in the case of the mentioned methods, the data may be put into a buffer if the data cannot be sent immediately. Any type errors will only be raised much later, when the buffer is finally sent. Interestingly, the test is incorrect as well: any memoryview is considered *right*, although it may be passed to functions that cannot deal with non-contiguous memoryviews, in which case the all the problems that the test should protect from reappear. There are several options one can go forward: * one could update the documentation to reflect the fact that not all bytes-like objects can be passed, but only special ones. This would be unfortunate as the underlying code actually can deal with all bytes-like data. * actually test whether the data is bytes-like. This is unfortunately not easy. The correct test would be to check whether the data can be parsed as by PyArg_Parse with a y* format. I am not aware of a simple test like this. * Remove the test. In this case one should assure that only bytes-like data will be put into the buffers if the data cannot be sent immediately. For simplicity I opted for the last option. One should note another problem here: if we accept objects like memoryview or bytearray (which we currently do), the user may modify the data after-the-fact, which will lead to weird, unexpected behavior. This could be mitigated by always copying the data. This is done in some of the modified methods, but not all, most likely for performance reasons. I think it would be beneficial to deal with this problem in a systematic way, but this is beyond the scope of this patch. --- Lib/asyncio/proactor_events.py | 11 ++--------- Lib/asyncio/selector_events.py | 13 +++---------- Lib/asyncio/sslproto.py | 6 ++---- Lib/test/test_asyncio/test_selector_events.py | 3 +++ 4 files changed, 10 insertions(+), 23 deletions(-) diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index f404273c3ae5c1..acf3657c6ac9f4 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -336,10 +336,6 @@ def __init__(self, *args, **kw): self._empty_waiter = None def write(self, data): - if not isinstance(data, (bytes, bytearray, memoryview)): - raise TypeError( - f"data argument must be a bytes-like object, " - f"not {type(data).__name__}") if self._eof_written: raise RuntimeError('write_eof() already called') if self._empty_waiter is not None: @@ -485,10 +481,6 @@ def abort(self): self._force_close(None) def sendto(self, data, addr=None): - if not isinstance(data, (bytes, bytearray, memoryview)): - raise TypeError('data argument must be bytes-like object (%r)', - type(data)) - if self._address is not None and addr not in (None, self._address): raise ValueError( f'Invalid address: must be None or {self._address}') @@ -500,7 +492,8 @@ def sendto(self, data, addr=None): return # Ensure that what we buffer is immutable. - self._buffer.append((bytes(data), addr)) + data = bytes(data) + self._buffer.append((data, addr)) self._buffer_size += len(data) + self._header_size if self._write_fut is None: diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py index a7e27ccf0aa856..bfa4f886169f7a 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -1049,9 +1049,6 @@ def _read_ready__on_eof(self): self.close() def write(self, data): - 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 self._eof: raise RuntimeError('Cannot call write() after write_eof()') if self._empty_waiter is not None: @@ -1071,7 +1068,7 @@ def write(self, data): n = self._sock.send(data) except (BlockingIOError, InterruptedError): pass - except (SystemExit, KeyboardInterrupt): + except (TypeError, SystemExit, KeyboardInterrupt): raise except BaseException as exc: self._fatal_error(exc, 'Fatal write error on socket transport') @@ -1084,7 +1081,7 @@ def write(self, data): self._loop._add_writer(self._sock_fd, self._write_ready) # Add it to the buffer. - self._buffer.append(data) + self._buffer.append(bytes(data)) self._maybe_pause_protocol() def _get_sendmsg_buffer(self): @@ -1255,10 +1252,6 @@ def _read_ready(self): self._protocol.datagram_received(data, addr) def sendto(self, data, addr=None): - 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 self._address: if addr not in (None, self._address): raise ValueError( @@ -1284,7 +1277,7 @@ def sendto(self, data, addr=None): except OSError as exc: self._protocol.error_received(exc) return - except (SystemExit, KeyboardInterrupt): + except (TypeError, SystemExit, KeyboardInterrupt): raise except BaseException as exc: self._fatal_error( diff --git a/Lib/asyncio/sslproto.py b/Lib/asyncio/sslproto.py index 74c5f0d5ca0609..6c18fbefe59a2f 100644 --- a/Lib/asyncio/sslproto.py +++ b/Lib/asyncio/sslproto.py @@ -214,12 +214,10 @@ def write(self, data): This does not block; it buffers the data and arranges for it to be sent out asynchronously. """ - if not isinstance(data, (bytes, bytearray, memoryview)): - raise TypeError(f"data: expecting a bytes-like instance, " - f"got {type(data).__name__}") if not data: return - self._ssl_protocol._write_appdata((data,)) + # Ensure that what we buffer is immutable. + self._ssl_protocol._write_appdata((bytes(data),)) def writelines(self, list_of_data): """Write a list (or any iterable) of data bytes to the transport. diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py index 4bb5d4fb816a9a..59f260053a8970 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -1499,6 +1499,9 @@ def test_sendto_error_received_connected(self): def test_sendto_str(self): transport = self.datagram_transport() + def sendto(data, addr): + bytes(data) # raises TypeError + self.sock.sendto = sendto self.assertRaises(TypeError, transport.sendto, 'str', ()) def test_sendto_connected_addr(self):