Skip to content

Commit fd657b0

Browse files
authored
gh-160181: Don't modify a hashed bytes buffer in place in io.BytesIO (GH-160182)
Treat the internal buffer as shared once its hash is cached, so the next modification copies it, as gh-158219 did for bytearray. ANT-2026-IZN6JY41
1 parent badd5d5 commit fd657b0

3 files changed

Lines changed: 63 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
@@ -1076,6 +1076,52 @@ def test_cow_mutable(self):
10761076
memio = self.ioclass(ba)
10771077
self.assertEqual(sys.getrefcount(ba), old_rc)
10781078

1079+
def test_cow_hashed(self):
1080+
# A buffer which was exposed to Python code and hashed must not be
1081+
# modified in place, even if BytesIO holds the only reference to it:
1082+
# it would keep the cached hash of its old contents.
1083+
def write(memio):
1084+
memio.seek(0)
1085+
memio.write(b"efgh")
1086+
def getbuffer(memio):
1087+
with memio.getbuffer() as view:
1088+
view[:] = b"efgh"
1089+
def truncate(memio):
1090+
memio.truncate(2)
1091+
memio.seek(0)
1092+
memio.write(b"efgh")
1093+
def getvalue(memio):
1094+
return memio.getvalue()
1095+
def read(memio):
1096+
memio.seek(0)
1097+
return memio.read()
1098+
def peek(memio):
1099+
memio.seek(0)
1100+
return memio.peek(4)
1101+
for mutation in write, getbuffer, truncate:
1102+
for expose in getvalue, read, peek:
1103+
with self.subTest(mutation=mutation.__name__,
1104+
expose=expose.__name__):
1105+
memio = self.ioclass()
1106+
memio.write(b"abcd")
1107+
# Shrink the overallocated buffer to the exact size,
1108+
# so read() and peek() can return it without copying.
1109+
memio.getvalue()
1110+
hash(expose(memio))
1111+
mutation(memio)
1112+
self.assertEqual(memio.getvalue(), b"efgh")
1113+
self.assertEqual(hash(memio.getvalue()), hash(b"efgh"))
1114+
self.assertIn(memio.getvalue(), {b"efgh"})
1115+
1116+
with self.subTest(mutation=mutation.__name__, expose="__init__"):
1117+
initial = bytes(bytearray(b"abcd"))
1118+
hash(initial)
1119+
memio = self.ioclass(initial)
1120+
del initial
1121+
mutation(memio)
1122+
self.assertEqual(memio.getvalue(), b"efgh")
1123+
self.assertEqual(hash(memio.getvalue()), hash(b"efgh"))
1124+
10791125
def test_write_with_export(self):
10801126
memio = self.ioclass(b"abcd")
10811127
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: 14 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
@@ -269,6 +278,7 @@ write_bytes_lock_held(bytesio *self, PyObject *b)
269278
goto done;
270279
}
271280
}
281+
assert(_PyBytes_IsMutable(self->buf));
272282

273283
if (self->pos > self->string_size) {
274284
/* In case of overseek, pad with null bytes the buffer region between

0 commit comments

Comments
 (0)