Skip to content

Commit 6965755

Browse files
committed
gh-160181: Don't modify a hashed bytes buffer in place in io.BytesIO
Treat the internal buffer as shared once its hash is cached, so the next modification copies it, as gh-158219 did for bytearray.
1 parent ab5187c commit 6965755

3 files changed

Lines changed: 62 additions & 4 deletions

File tree

‎Lib/test/test_io/test_memoryio.py‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,52 @@ def test_cow_mutable(self):
10541054
memio = self.ioclass(ba)
10551055
self.assertEqual(sys.getrefcount(ba), old_rc)
10561056

1057+
def test_cow_hashed(self):
1058+
# A buffer which was exposed to Python code and hashed must not be
1059+
# modified in place, even if BytesIO holds the only reference to it:
1060+
# it would keep the cached hash of its old contents.
1061+
def write(memio):
1062+
memio.seek(0)
1063+
memio.write(b"efgh")
1064+
def getbuffer(memio):
1065+
with memio.getbuffer() as view:
1066+
view[:] = b"efgh"
1067+
def truncate(memio):
1068+
memio.truncate(2)
1069+
memio.seek(0)
1070+
memio.write(b"efgh")
1071+
def getvalue(memio):
1072+
return memio.getvalue()
1073+
def read(memio):
1074+
memio.seek(0)
1075+
return memio.read()
1076+
def peek(memio):
1077+
memio.seek(0)
1078+
return memio.peek(4)
1079+
for mutation in write, getbuffer, truncate:
1080+
for expose in getvalue, read, peek:
1081+
with self.subTest(mutation=mutation.__name__,
1082+
expose=expose.__name__):
1083+
memio = self.ioclass()
1084+
memio.write(b"abcd")
1085+
# Shrink the overallocated buffer to the exact size,
1086+
# so read() and peek() can return it without copying.
1087+
memio.getvalue()
1088+
hash(expose(memio))
1089+
mutation(memio)
1090+
self.assertEqual(memio.getvalue(), b"efgh")
1091+
self.assertEqual(hash(memio.getvalue()), hash(b"efgh"))
1092+
self.assertIn(memio.getvalue(), {b"efgh"})
1093+
1094+
with self.subTest(mutation=mutation.__name__, expose="__init__"):
1095+
initial = bytes(bytearray(b"abcd"))
1096+
hash(initial)
1097+
memio = self.ioclass(initial)
1098+
del initial
1099+
mutation(memio)
1100+
self.assertEqual(memio.getvalue(), b"efgh")
1101+
self.assertEqual(hash(memio.getvalue()), hash(b"efgh"))
1102+
10571103
def test_write_with_export(self):
10581104
memio = self.ioclass(b"abcd")
10591105
memio.seek(2)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix :meth:`io.BytesIO.getvalue` sometimes returning a :class:`bytes` object
2+
with the hash of earlier contents after the buffer was written to, which made
3+
:class:`dict` and :class:`set` lookups with it fail.

‎Modules/_io/bytesio.c‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ typedef struct {
3939
#define bytesiobuf_CAST(op) ((bytesiobuf *)(op))
4040

4141
/* The bytesio object can be in three states:
42-
* Py_REFCNT(buf) == 1, exports == 0.
43-
* Py_REFCNT(buf) > 1. exports == 0,
42+
* Py_REFCNT(buf) == 1, the hash of buf is not cached, exports == 0.
43+
* Py_REFCNT(buf) > 1 or the hash of buf is cached. exports == 0,
4444
first modification or export causes the internal buffer copying.
4545
* exports > 0. Any modifications are forbidden. Every exported buffer
4646
keeps a reference to buf, so it outlives closing of the bytesio object.
@@ -77,10 +77,19 @@ check_exports(bytesio *self)
7777
return NULL; \
7878
}
7979

80+
/* If the hash value is cached, the internal bytes object was already used in
81+
Python somehow (ex: as a dictionary key). Modifying it in place would leave
82+
the hash of the old contents, even if nothing else references it anymore. */
83+
#define HASHED_BUF(self) \
84+
(_PyBytes_GET_CACHED_HASH((PyBytesObject *)(self)->buf) != -1)
85+
8086
#ifdef Py_GIL_DISABLED
81-
#define SHARED_BUF(self) ((self)->buf_shared || !_PyObject_IsUniquelyReferenced((self)->buf))
87+
#define SHARED_BUF(self) \
88+
((self)->buf_shared || !_PyObject_IsUniquelyReferenced((self)->buf) \
89+
|| HASHED_BUF(self))
8290
#else
83-
#define SHARED_BUF(self) (!_PyObject_IsUniquelyReferenced((self)->buf))
91+
#define SHARED_BUF(self) \
92+
(!_PyObject_IsUniquelyReferenced((self)->buf) || HASHED_BUF(self))
8493
#endif
8594

8695
static inline void

0 commit comments

Comments
 (0)