diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index c53262e358b48ea..3f6560869f34ac9 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -4925,6 +4925,31 @@ def test_load_from_and_dump_to_file(self): unpickled = self.load(stream) self.assertEqual(unpickled, data) + def test_load_from_file_returning_bytes_subclass(self): + # gh-158841: read() returning a bytes subclass must not crash + # when the payload is large enough to need several chunked reads. + class MyBytes(bytes): + pass + + class Reader: + def __init__(self, data): + self.data = data + self.pos = 0 + def read(self, n): + chunk = self.data[self.pos:self.pos + n] + self.pos += len(chunk) + return MyBytes(chunk) + def readline(self): + end = self.data.find(b'\n', self.pos) + end = len(self.data) if end < 0 else end + 1 + line = self.data[self.pos:end] + self.pos = end + return line + + obj = {'v': 'B' * (3 * 1024 * 1024)} + data = self.dumps(obj, protocol=4) + self.assertEqual(self.load(Reader(data)), obj) + def test_highest_protocol(self): # Of course this needs to be changed when HIGHEST_PROTOCOL changes. self.assertEqual(pickle.HIGHEST_PROTOCOL, 5) diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index 0f53d915c63dd31..b8e11ffad77f167 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -244,6 +244,9 @@ def test_resize(self): """Test _PyBytes_Resize()""" _resize = _testcapi.bytes_resize + class BytesSubclass(bytes): + pass + def assert_is_fresh_copy(result, refcnt, is_new_obj): self.assertEqual(refcnt, 1) self.assertTrue(is_new_obj) @@ -275,6 +278,20 @@ def resize(obj, size, new, compute_hash=False): self.assertEqual(refcnt, 1) self.assertFalse(sys._is_immortal(result)) + if not new: + # Given a subclass of bytes, _resize always returns a new copy + # or empty singleton + sub = BytesSubclass(obj) + sub_result, refcnt, is_new_obj = _resize( + sub, size, new, compute_hash=compute_hash) + + if size == 0: + self.assertIs(sub_result, b'') + self.assertTrue(sys._is_immortal(sub_result)) + self.assertTrue(is_new_obj) + else: + assert_is_fresh_copy(sub_result, refcnt, is_new_obj) + return result for new in True, False: diff --git a/Misc/NEWS.d/next/C_API/2026-10-09-14-10-47.gh-issue-158846.tpi7gn.rst b/Misc/NEWS.d/next/C_API/2026-10-09-14-10-47.gh-issue-158846.tpi7gn.rst new file mode 100644 index 000000000000000..1398747d735a077 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-10-09-14-10-47.gh-issue-158846.tpi7gn.rst @@ -0,0 +1,3 @@ +When the :c:func:`_PyBytes_Resize` function is called with a subclass of +:py:class:`bytes`, it now skips optimization (which could lead to crashes), +and returns a new bytes object. diff --git a/Misc/NEWS.d/next/Library/2026-10-05-14-30-00.gh-issue-158841.pK3vNd.rst b/Misc/NEWS.d/next/Library/2026-10-05-14-30-00.gh-issue-158841.pK3vNd.rst new file mode 100644 index 000000000000000..eda6d7daefa48d8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-05-14-30-00.gh-issue-158841.pK3vNd.rst @@ -0,0 +1,3 @@ +Fix a crash in :func:`pickle.load` and :class:`pickle.Unpickler` when the +``read()`` method of the file object returns an instance of a :class:`bytes` +subclass and the requested size is large. diff --git a/Modules/_testcapi/bytes.c b/Modules/_testcapi/bytes.c index 605df203d6fc9a4..11e216e7c6c3b95 100644 --- a/Modules/_testcapi/bytes.c +++ b/Modules/_testcapi/bytes.c @@ -28,7 +28,7 @@ bytes_resize(PyObject *Py_UNUSED(module), PyObject *args, PyObject *kwargs) NULLABLE(obj); if (new) { assert(obj != NULL); - assert(PyBytes_CheckExact(obj)); + assert(PyBytes_Check(obj)); PyObject *newobj = PyBytes_FromStringAndSize(NULL, PyBytes_Size(obj)); if (newobj == NULL) { return NULL; @@ -56,6 +56,7 @@ bytes_resize(PyObject *Py_UNUSED(module), PyObject *args, PyObject *kwargs) assert(obj != NULL); } + assert(PyBytes_CheckExact(obj)); Py_ssize_t refcnt = Py_REFCNT(obj); return Py_BuildValue("Nnp", obj, refcnt, obj != old_obj); } diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 1917055d25198f1..c354b001e317747 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3457,7 +3457,7 @@ bytes_resize_inplace(PyObject **pv, Py_ssize_t newsize) } -/* The following function breaks the notion that bytes are immutable: +/* The _PyBytes_Resize function breaks the notion that bytes are immutable: it changes the size of a bytes object. You can think of it as creating a new bytes object and destroying the old one, only more efficiently. @@ -3467,8 +3467,39 @@ bytes_resize_inplace(PyObject **pv, Py_ssize_t newsize) returned, and the value in *pv may or may not be the same as on input. As always, an extra byte is allocated for a trailing \0 byte (newsize does *not* include that), and a trailing \0 byte is stored. + + The result is always of the exact bytes type. + If the input is "fresh" (_PyBytes_IsMutable is true), the result will + be empty or "fresh". */ +// Return a copy. Called when the existing object cannot be reused. +static int +bytes_resize_newobject(PyObject **pv, Py_ssize_t newsize) +{ + PyObject *v = *pv; + if (newsize == 0) { + *pv = bytes_get_empty(); + assert(*pv != NULL); + } + else { + Py_ssize_t oldsize = PyBytes_GET_SIZE(v); + // Allocate and then copy so we don't get a shared immortal + // one-character singleton! + PyObject *result = bytes_alloc(newsize); + if (!result) { + return -1; + } + + memcpy(PyBytes_AS_STRING(result), PyBytes_AS_STRING(v), + Py_MIN(oldsize, newsize)); + *pv = result; + assert(_PyBytes_IsMutable(*pv)); + } + Py_DECREF(v); + return 0; +} + // Similar to _PyBytes_Resize(), but leaves the object unchanged on error. int _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize) @@ -3480,6 +3511,10 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize) return -1; } + if (!PyBytes_CheckExact(v)) { + return bytes_resize_newobject(pv, newsize); + } + Py_ssize_t oldsize = PyBytes_GET_SIZE(v); if (oldsize == newsize) { // Leave the object unchanged if the new size is the same as the old @@ -3501,7 +3536,8 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize) } if (newsize == 0) { - *pv = bytes_get_empty(); // cannot fail + *pv = bytes_get_empty(); + assert(*pv != NULL); Py_DECREF(v); return 0; } @@ -3510,19 +3546,7 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize) // Return a copy if the hash value was already computed || get_ob_shash((PyBytesObject *)v) != -1) { - // Allocate and then copy so we don't get a shared immortal - // one-character singleton! - PyObject *result = bytes_alloc(newsize); - if (!result) { - return -1; - } - - memcpy(PyBytes_AS_STRING(result), PyBytes_AS_STRING(v), - Py_MIN(oldsize, newsize)); - *pv = result; - Py_DECREF(v); - assert(_PyBytes_IsMutable(*pv)); - return 0; + return bytes_resize_newobject(pv, newsize); } return bytes_resize_inplace(pv, newsize); @@ -3534,9 +3558,7 @@ _PyBytes_Resize(PyObject **pv, Py_ssize_t newsize) { int res = _PyBytes_ResizeKeepOnError(pv, newsize); if (res < 0) { - PyObject *v = *pv; - *pv = NULL; - Py_DECREF(v); + Py_CLEAR(*pv); } return res; }