gh-155389: Return bytes from _pyio.BytesIO.peek() - #155390
Conversation
Sorry, something went wrong.
peek() returned a slice of the internal bytearray, where read() converts with take_bytes(). It also did not coerce its size through __index__ and did not hold the lock while slicing, both of which read() and the C implementation do.
Sorry, something went wrong.
| size = size_index() | ||
|
|
||
| with self._lock: | ||
| if size < 1: |
There was a problem hiding this comment.
I don't think the size piece here needs to be in the lock
Sorry, something went wrong.
There was a problem hiding this comment.
Done, thanks. Only the slicing is under the lock now.
Sorry, something went wrong.
|
It's better to add a test to check if TypeError is raised for peek(1.5). |
Sorry, something went wrong.
The size normalisation touches no shared state, so it does not need the lock.
|
for news, falls under the existing news add in: 72cad14. This is just refining behavior. |
Sorry, something went wrong.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
LGTM. 👍
Sorry, something went wrong.
_pyio.BytesIO.peek() returned a slice of the internal bytearray, so it gave a bytearray where the C version and the docs say bytes. It also did not accept an __index__-able size, and did not take the lock while slicing.
It now does what read() does: coerce the size, take the lock, convert with take_bytes().
test_peek only used assertEqual, and bytearray(b'1') == b'1', so it passed either way. It now checks the type and an __index__ size, and fails without the change.
BytesIO.peek() is new in 3.16, so there is no NEWS entry.