Skip to content

gh-155389: Return bytes from _pyio.BytesIO.peek() - #155390

Merged
serhiy-storchaka merged 2 commits into
python:mainfrom
fedonman:fix-pyio-bytesio-peek-bytes
Aug 16, 2026
Merged

gh-155389: Return bytes from _pyio.BytesIO.peek()#155390
serhiy-storchaka merged 2 commits into
python:mainfrom
fedonman:fix-pyio-bytesio-peek-bytes

Conversation

@fedonman

@fedonman fedonman commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

_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.

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.
@cmaloney

cmaloney commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

cc: @marcelm @vstinner

Comment thread Lib/_pyio.py Outdated
size = size_index()

with self._lock:
if size < 1:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the size piece here needs to be in the lock

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks. Only the slicing is under the lock now.

Comment thread Lib/_pyio.py
@aisk

aisk commented Aug 8, 2026

Copy link
Copy Markdown
Member

It's better to add a test to check if TypeError is raised for peek(1.5).

The size normalisation touches no shared state, so it does not need the lock.
@cmaloney

Copy link
Copy Markdown
Contributor

for news, falls under the existing news add in: 72cad14. This is just refining behavior.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@serhiy-storchaka
serhiy-storchaka merged commit 13aa41f into python:main Aug 16, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants