Skip to content

gh-160181: Don't modify a hashed bytes buffer in place in io.BytesIO - #160182

Merged
gpshead merged 2 commits into
python:mainfrom
gpshead:bytesio-getvalue-stale-hash
Oct 23, 2026
Merged

gpshead merged 2 commits into
python:mainfrom
gpshead:bytesio-getvalue-stale-hash

Conversation

@gpshead

@gpshead gpshead commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Treat the internal buffer as shared once its hash is cached, so the next write, getbuffer() export or resize copies it instead of modifying a bytes object that may already be in a set or dict. SHARED_BUF() is the copy-on-write check every modifying path uses, so this covers all of them on both the default and free-threaded builds. The fix for gh-158219 did the same for bytearray.

Fixes #160181

ANT-2026-IZN6JY41

@gpshead gpshead added needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Oct 22, 2026
@gpshead gpshead self-assigned this Oct 22, 2026
@gpshead
gpshead requested a review from cmaloney October 22, 2026 04:26
@gpshead
gpshead marked this pull request as ready for review October 22, 2026 04:26
@gpshead

gpshead commented Oct 22, 2026

Copy link
Copy Markdown
Member Author

The behavior here feels a little sus. This seems better... but bytes. immutable. overwriting. scary.

…tesIO

Treat the internal buffer as shared once its hash is cached, so the next
modification copies it, as pythongh-158219 did for bytearray.
@gpshead
gpshead force-pushed the bytesio-getvalue-stale-hash branch from c720598 to 6965755 Compare October 22, 2026 04:44
@vstinner

Copy link
Copy Markdown
Member

I recently fixed a similar issue in _PyBytes_Resize() if hash value is already computed: see issue gh-158585 and PR gh-158589. I also added _PyBytes_IsMutable() assertion which fails if the hash value has been computed.

@vstinner

Copy link
Copy Markdown
Member

cc @cmaloney

@cmaloney cmaloney left a comment

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.

It would be good to add a _PyBytes_IsMutable assertion in write_bytes_lock_held to validate we've got all the cases here. For the previous fixes to bytearray allowing mutating immutable things found an underlying _PyBytes_Resize issue by adding that shape check.

(resize_unshared_buffer_lock_held doesn't need one because it uses _PyBytes_ResizeKeepOnError which already does)

@gpshead
gpshead enabled auto-merge (squash) October 22, 2026 22:25

@vstinner vstinner 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. I agree that we should not modify a bytes object if its hash value has already been computed. The test seems to cover all cases.

@gpshead
gpshead merged commit fd657b0 into python:main Oct 23, 2026
60 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Oct 23, 2026

Copy link
Copy Markdown

GH-160408 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 23, 2026
@bedevere-app

bedevere-app Bot commented Oct 23, 2026

Copy link
Copy Markdown

GH-160409 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Oct 23, 2026
@gpshead
gpshead deleted the bytesio-getvalue-stale-hash branch October 23, 2026 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

io.BytesIO.getvalue() can return bytes with a stale hash

3 participants