From aeda8b8988d708795e7283c4501d5ef0b6cfde08 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sat, 12 Sep 2026 07:30:56 +0200 Subject: [PATCH 1/4] gh-157242: Add more PyBytesWriter tests (#157352) * Fix PyBytesWriter_Grow() check on the size argument. * Fix undefined behavior in _PyBytes_ResizeKeepOnError(): raise OverflowError on integer overflow. * Add write(), grow() and get_data() methods to the _testcapi wrapper of PyBytesWriter. * Remove the string argument from _testcapi.PyBytesWriter() and resize() method. * Add tests on PyBytesWriter_Resize() invalid sizes. (cherry picked from commit 63d4398be879e346ef68f899115f86cdb9fa555c) --- Lib/test/test_capi/test_bytes.py | 172 ++++++++++++++++++++++++------- Modules/_testcapi/bytes.c | 110 ++++++++++++++------ Objects/bytesobject.c | 24 ++++- 3 files changed, 232 insertions(+), 74 deletions(-) diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index 9ee200191de9308..eee7a9e3bbf97f0 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -318,6 +318,10 @@ def test_join(self): class BaseWriterTest: RESULT_TYPE = NotImplementedError + SMALL_BUFFER = 11 # bytes + assert SMALL_BUFFER < _testcapi.PyBytesWriter_small_buffer + LARGE_BUFFER = _testcapi.PyBytesWriter_small_buffer + 17 # bytes + NEW_BYTE = b'\xff' def create_writer(self, alloc=0, string=b''): raise NotImplementedError @@ -334,23 +338,60 @@ def test_create(self): self.assertEqual(writer.get_size(), 0) self.bytes_equal(writer.finish(), b'') - writer = self.create_writer(3, b'abc') + writer = self.create_writer(3) + writer.write(0, b'abc') self.assertEqual(writer.get_size(), 3) self.bytes_equal(writer.finish(), b'abc') + @unittest.skipUnless(support.Py_DEBUG, 'need Py_DEBUG') + def test_get_data(self): + # Test PyBytesWriter_GetData() + writer = self.create_writer(6) + NEW_BYTE = self.NEW_BYTE + self.assertEqual(writer.get_data(), NEW_BYTE * 6) + writer.write(0, b'abc') + self.assertEqual(writer.get_data(), b'abc' + NEW_BYTE * 3) + writer.write(3, b'123') + self.assertEqual(writer.get_data(), b'abc123') + + # Switch from small buffer to large buffer + small, large = self.SMALL_BUFFER, self.LARGE_BUFFER + writer = self.create_writer(small) + self.assertEqual(writer.get_data(), NEW_BYTE * small) + writer.write(0, b's' * small) + self.assertEqual(writer.get_data(), b's' * small) + writer.resize(large) + self.assertEqual(writer.get_data(), b's' * small + NEW_BYTE * (large - small)) + writer.write(small, b'L' * (large - small)) + self.assertEqual(writer.get_data(), b's' * small + b'L' * (large - small)) + + # Resize large buffer + small, large = self.SMALL_BUFFER, self.LARGE_BUFFER + writer = self.create_writer(large) + self.assertEqual(writer.get_data(), NEW_BYTE * large) + writer.write(0, b'L' * large) + self.assertEqual(writer.get_data(), b'L' * large) + writer.resize(large + 10) + self.assertEqual(writer.get_data(), b'L' * large + NEW_BYTE * 10) + writer.write(large, b'#' * 10) + self.assertEqual(writer.get_data(), b'L' * large + b'#' * 10) + def test_finish_with_size(self): # Test PyBytesWriter_FinishWithSize() - writer = self.create_writer(10, b'abcdef') + writer = self.create_writer(10) + writer.write(0, b'abc123') self.assertEqual(writer.get_size(), 10) self.bytes_equal(writer.finish_with_size(3), b'abc') # Error if the size is negative - writer = self.create_writer(3, b'abc') + writer = self.create_writer(3, ) + writer.write(0, b'abc') with self.assertRaises(ValueError): writer.finish_with_size(-3) # Error if the requested size is larger than the allocated size - writer = self.create_writer(3, b'abc') + writer = self.create_writer(3) + writer.write(0, b'abc') with self.assertRaises(ValueError): writer.finish_with_size(4) @@ -368,44 +409,92 @@ def test_write_bytes(self): def test_resize(self): # Test PyBytesWriter_Resize() writer = self.create_writer() - writer.resize(len(b'number=123456'), b'number=123456') - writer.resize(len(b'number=123456'), b'') - self.assertEqual(writer.get_size(), len(b'number=123456')) - self.bytes_equal(writer.finish(), b'number=123456') + writer.resize(len(b'hello')) + writer.write(0, b'hello') + self.assertEqual(writer.finish(), b'hello') writer = self.create_writer() - writer.resize(0, b'') - writer.resize(len(b'number=123456'), b'number=123456') - self.bytes_equal(writer.finish(), b'number=123456') - + writer.resize(0) # noop + writer.resize(len(b'number')) + writer.write(0, b'number') + writer.resize(len(b'number=')) + writer.write(len(b'number'), b'=') + writer.resize(len(b'number=123'), ) + writer.write(len(b'number='), b'123') + writer.resize(len(b'number=123')) # noop + self.assertEqual(writer.finish(), b'number=123') + + # Switch from small buffer to large buffer writer = self.create_writer() - writer.resize(len(b'number='), b'number=') - writer.resize(len(b'number=123456'), b'123456') - self.bytes_equal(writer.finish(), b'number=123456') + small, large = self.SMALL_BUFFER, self.LARGE_BUFFER + writer.resize(small) + writer.write(0, b's' * small) + writer.resize(large) + writer.write(small, b'L' * (large - small)) + self.assertEqual(writer.finish(), + b's' * small + b'L' * (large - small)) + + # invalid size + for size in (self.SMALL_BUFFER, self.LARGE_BUFFER): + with self.subTest(size=size): + writer = self.create_writer() + writer.write_bytes(b'x' * size, -1) + with self.assertRaisesRegex(ValueError, 'size must be >= 0'): + writer.resize(-1) + with self.assertRaises((MemoryError, OverflowError)): + writer.resize(_testcapi.PY_SSIZE_T_MAX) + self.assertEqual(writer.finish(), b'x' * size) + + def test_grow(self): + # Test PyBytesWriter_Grow() + writer = self.create_writer(0) + writer.grow(len(b'number=123')) + writer.write(0, b'number=123') + self.assertEqual(writer.finish(), b'number=123') writer = self.create_writer() - writer.resize(len(b'number='), b'number=') - writer.resize(len(b'number='), b'') - writer.resize(len(b'number=123456'), b'123456') - self.bytes_equal(writer.finish(), b'number=123456') - + writer.grow(0) # noop + writer.grow(len(b'number')) + writer.write(0, b'number') + writer.grow(len(b'=')) + writer.write(len(b'number'), b'=') + writer.grow(len(b'123'), ) + writer.write(len(b'number='), b'123') + writer.grow(0) # noop + self.assertEqual(writer.finish(), b'number=123') + + # Switch from small buffer to large buffer writer = self.create_writer() - writer.resize(len(b'number'), b'number') - writer.resize(len(b'number='), b'=') - writer.resize(len(b'number=123'), b'123') - writer.resize(len(b'number=123456'), b'456') - self.bytes_equal(writer.finish(), b'number=123456') + small, large = self.SMALL_BUFFER, self.LARGE_BUFFER + writer.grow(small) + writer.write(0, b's' * small) + writer.grow(large - small) + writer.write(small, b'L' * (large - small)) + self.assertEqual(writer.finish(), + b's' * small + b'L' * (large - small)) + + # invalid size + for size in (self.SMALL_BUFFER, self.LARGE_BUFFER): + with self.subTest(size=size): + writer = self.create_writer() + writer.write_bytes(b'x' * size, -1) + with self.assertRaisesRegex(ValueError, 'size must be >= 0'): + writer.grow(-1) + with self.assertRaises(MemoryError): + writer.grow(_testcapi.PY_SSIZE_T_MAX) + self.assertEqual(writer.finish(), b'x' * size) @support.nomemtest def test_resize_error(self): - small_buffer = _testcapi.PyBytesWriter_small_buffer - init = b'x' * (small_buffer * 2) - writer = self.create_writer(len(init), init) + # Test PyBytesWriter_Resize() error + init = b'x' * self.LARGE_BUFFER + writer = self.create_writer(len(init)) + writer.write(0, init) size = len(init) + 100 try: with self.assertRaises(MemoryError): _testcapi.set_nomemory(0) - writer.resize(size, b'') + writer.resize(size) finally: _testcapi.remove_mem_hooks() suffix = b'still working' @@ -443,36 +532,41 @@ def test_format_s_memory_error(self): finally: _testcapi.remove_mem_hooks() writer.write_bytes(b'after', -1) - self.bytes_equal(writer.finish(), b'after') class BytesWriterTest(BaseWriterTest, unittest.TestCase): RESULT_TYPE = bytes - def create_writer(self, alloc=0, string=b''): + def create_writer(self, size=0): # Test PyBytesWriter_Create() - return _testcapi.PyBytesWriter(alloc, string, 0) + return _testcapi.PyBytesWriter(size, 0) # Only PyBytesWriter_Create() returns singletons def test_singletons(self): empty = b'' singletons = {ch: bytes((ch,)) for ch in range(256)} - small_buffer = _testcapi.PyBytesWriter_small_buffer writer = self.create_writer() self.assertIs(writer.finish(), empty) - # Test writer larger than small_buffer + # Large buffer writer = self.create_writer() - unused_text = b'x' * (small_buffer * 2) + unused_text = b'x' * self.LARGE_BUFFER writer.write_bytes(unused_text, len(unused_text)) self.assertIs(writer.finish_with_size(0), empty) + # Large buffer with resize + writer = self.create_writer() + unused_text = b'x' * self.LARGE_BUFFER + writer.write_bytes(unused_text, len(unused_text)) + writer.resize(0) + self.assertIs(writer.finish(), empty) + for ch in range(256): - text = bytes((ch,)) + byte = bytes((ch,)) writer = self.create_writer() - writer.write_bytes(text, 1) + writer.write_bytes(byte, len(byte)) self.assertIs(writer.finish(), singletons[ch]) def test_example_abc(self): @@ -488,9 +582,9 @@ def test_example_highlevel(self): class ByteArrayWriterTest(BaseWriterTest, unittest.TestCase): RESULT_TYPE = bytearray - def create_writer(self, alloc=0, string=b''): + def create_writer(self, size=0): # Test private _PyBytesWriter_CreateByteArray() - return _testcapi.PyBytesWriter(alloc, string, 1) + return _testcapi.PyBytesWriter(size, 1) if __name__ == "__main__": diff --git a/Modules/_testcapi/bytes.c b/Modules/_testcapi/bytes.c index 984a857a83db11b..0c3b323aca7b8ef 100644 --- a/Modules/_testcapi/bytes.c +++ b/Modules/_testcapi/bytes.c @@ -87,34 +87,25 @@ writer_init(PyObject *self_raw, PyObject *args, PyObject *kwargs) return -1; } - Py_ssize_t alloc; - char *str; - Py_ssize_t str_size; - int use_bytearray; - if (!PyArg_ParseTuple(args, "ny#i", - &alloc, &str, &str_size, &use_bytearray)) { + Py_ssize_t size; + int use_bytearray = 0; + if (!PyArg_ParseTuple(args, "n|i", &size, &use_bytearray)) { return -1; } WriterObject *self = (WriterObject *)self_raw; - if (self->writer) { - PyBytesWriter_Discard(self->writer); - } + PyBytesWriter_Discard(self->writer); + if (use_bytearray) { - self->writer = _PyBytesWriter_CreateByteArray(alloc); + self->writer = _PyBytesWriter_CreateByteArray(size); } else { - self->writer = PyBytesWriter_Create(alloc); + self->writer = PyBytesWriter_Create(size); } if (self->writer == NULL) { return -1; } - if (str_size) { - char *buf = PyBytesWriter_GetData(self->writer); - memcpy(buf, str, str_size); - } - return 0; } @@ -143,6 +134,32 @@ writer_check(WriterObject *self) } +static PyObject* +writer_write(PyObject *self_raw, PyObject *args) +{ + WriterObject *self = (WriterObject *)self_raw; + if (writer_check(self) < 0) { + return NULL; + } + + Py_ssize_t pos, size; + char *str; + if (!PyArg_ParseTuple(args, "ny#", &pos, &str, &size)) { + return NULL; + } + + if (pos < 0 || (pos + size) > PyBytesWriter_GetSize(self->writer)) { + PyErr_SetString(PyExc_ValueError, "invalid position or size"); + return NULL; + } + + char *data = PyBytesWriter_GetData(self->writer); + memcpy(data + pos, str, size); + + Py_RETURN_NONE; +} + + static PyObject* writer_write_bytes(PyObject *self_raw, PyObject *args) { @@ -185,6 +202,7 @@ writer_format_i(PyObject *self_raw, PyObject *args) } +// PyBytesWriter_Resize static PyObject* writer_format_s(PyObject *self_raw, PyObject *args) { @@ -215,27 +233,52 @@ writer_resize(PyObject *self_raw, PyObject *args) } Py_ssize_t size; - char *str; - Py_ssize_t str_size; - if (!PyArg_ParseTuple(args, - "ny#", - &size, &str, &str_size)) { + if (!PyArg_ParseTuple(args, "n", &size)) { return NULL; } - assert(size >= str_size); - Py_ssize_t pos = PyBytesWriter_GetSize(self->writer); if (PyBytesWriter_Resize(self->writer, size) < 0) { return NULL; } + Py_RETURN_NONE; +} + + +// Test PyBytesWriter_Grow() +static PyObject* +writer_grow(PyObject *self_raw, PyObject *args) +{ + WriterObject *self = (WriterObject *)self_raw; + if (writer_check(self) < 0) { + return NULL; + } - char *buf = PyBytesWriter_GetData(self->writer); - memcpy(buf + pos, str, str_size); + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "n", &size)) { + return NULL; + } + if (PyBytesWriter_Grow(self->writer, size) < 0) { + return NULL; + } Py_RETURN_NONE; } +static PyObject* +writer_get_data(PyObject *self_raw, PyObject *Py_UNUSED(args)) +{ + WriterObject *self = (WriterObject *)self_raw; + if (writer_check(self) < 0) { + return NULL; + } + + const char *data = PyBytesWriter_GetData(self->writer); + Py_ssize_t size = PyBytesWriter_GetSize(self->writer); + return PyBytes_FromStringAndSize(data, size); +} + + static PyObject* writer_get_size(PyObject *self_raw, PyObject *Py_UNUSED(args)) { @@ -244,8 +287,8 @@ writer_get_size(PyObject *self_raw, PyObject *Py_UNUSED(args)) return NULL; } - Py_ssize_t alloc = PyBytesWriter_GetSize(self->writer); - return PyLong_FromSsize_t(alloc); + Py_ssize_t size = PyBytesWriter_GetSize(self->writer); + return PyLong_FromSsize_t(size); } @@ -283,10 +326,13 @@ writer_finish_with_size(PyObject *self_raw, PyObject *args) static PyMethodDef writer_methods[] = { + {"write", _PyCFunction_CAST(writer_write), METH_VARARGS}, {"write_bytes", _PyCFunction_CAST(writer_write_bytes), METH_VARARGS}, {"format_i", _PyCFunction_CAST(writer_format_i), METH_VARARGS}, {"format_s", _PyCFunction_CAST(writer_format_s), METH_VARARGS}, {"resize", _PyCFunction_CAST(writer_resize), METH_VARARGS}, + {"grow", _PyCFunction_CAST(writer_grow), METH_VARARGS}, + {"get_data", _PyCFunction_CAST(writer_get_data), METH_NOARGS}, {"get_size", _PyCFunction_CAST(writer_get_size), METH_NOARGS}, {"finish", _PyCFunction_CAST(writer_finish), METH_NOARGS}, {"finish_with_size", _PyCFunction_CAST(writer_finish_with_size), METH_VARARGS}, @@ -335,8 +381,9 @@ byteswriter_resize(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) char *buf = PyBytesWriter_GetData(writer); // Write some bytes - memcpy(buf, "Hello ", strlen("Hello ")); - buf += strlen("Hello "); + const char *hello = "Hello "; + memcpy(buf, hello, strlen(hello)); + buf += strlen(hello); // Allocate 10 more bytes buf = PyBytesWriter_GrowAndUpdatePointer(writer, 10, buf); @@ -346,8 +393,9 @@ byteswriter_resize(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) } // Write more bytes - memcpy(buf, "World", strlen("World")); - buf += strlen("World"); + const char *world = "World"; + memcpy(buf, world, strlen(world)); + buf += strlen(world); // Truncate to the exact size and create a bytes object return PyBytesWriter_FinishWithPointer(writer, buf); diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 233f9dc3d63bbc3..492fb6bc6663566 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3386,6 +3386,12 @@ _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize) } assert(v != bytes_get_empty()); + if ((size_t)newsize > (size_t)PY_SSIZE_T_MAX - PyBytesObject_SIZE) { + PyErr_SetString(PyExc_OverflowError, + "byte string is too large"); + return -1; + } + #ifdef Py_TRACE_REFS _Py_ForgetReference(v); #endif @@ -3610,6 +3616,8 @@ _PyBytes_Repeat(char* dest, Py_ssize_t len_dest, // --- PyBytesWriter API ----------------------------------------------------- +#define PyBytesWrite_NEW_BYTE 0xff + static inline char* byteswriter_data(PyBytesWriter *writer) { @@ -3647,6 +3655,7 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t size, int resize) Py_ssize_t old_allocated = byteswriter_allocated(writer); if (size <= old_allocated) { + // Do not shrink the buffer before PyBytesWriter_FinishWithSize() return 0; } @@ -3663,6 +3672,7 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t size, int resize) } } else { + // Can raise MemoryError or OverflowError if (_PyBytes_ResizeKeepOnError(&writer->obj, size)) { assert(writer->obj != NULL); return -1; @@ -3698,7 +3708,7 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t size, int resize) #ifdef Py_DEBUG Py_ssize_t allocated = byteswriter_allocated(writer); if (resize && allocated > old_allocated) { - memset(byteswriter_data(writer) + old_allocated, 0xff, + memset(byteswriter_data(writer) + old_allocated, PyBytesWrite_NEW_BYTE, allocated - old_allocated); } #endif @@ -3736,7 +3746,8 @@ byteswriter_create(Py_ssize_t size, int use_bytearray) writer->size = size; } #ifdef Py_DEBUG - memset(byteswriter_data(writer), 0xff, byteswriter_allocated(writer)); + memset(byteswriter_data(writer), PyBytesWrite_NEW_BYTE, + byteswriter_allocated(writer)); #endif return writer; } @@ -3877,10 +3888,15 @@ _PyBytesWriter_ResizeAndUpdatePointer(PyBytesWriter *writer, Py_ssize_t size, int PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t size) { - if (size < 0 && writer->size + size < 0) { - PyErr_SetString(PyExc_ValueError, "invalid size"); + if (size < 0) { + PyErr_SetString(PyExc_ValueError, "size must be >= 0"); return -1; } + if (size == 0) { + // Nothing to do + return 0; + } + if (size > PY_SSIZE_T_MAX - writer->size) { PyErr_NoMemory(); return -1; From 4d7ed4c49bf6be56bd1b0a7f30fb2a55d6d66f2b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sat, 12 Sep 2026 22:12:45 +0200 Subject: [PATCH 2/4] gh-157242: Allow negative grow in PyBytesWriter_Grow() (#157375) PyBytesWriter_Resize() doesn't need to call byteswriter_resize() if the buffer is already large enough. (cherry picked from commit bab97c599f7c66b6b7b718a1c03a4bf4520b504c) --- Doc/c-api/bytes.rst | 2 +- Lib/test/test_capi/test_bytes.py | 22 +++++++++++++++++++-- Objects/bytesobject.c | 33 ++++++++++++++++++++------------ 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/Doc/c-api/bytes.rst b/Doc/c-api/bytes.rst index 516e8b0bbfa856b..77b698009bf132c 100644 --- a/Doc/c-api/bytes.rst +++ b/Doc/c-api/bytes.rst @@ -436,7 +436,7 @@ Low-level API On success, return ``0``. On error, set an exception and return ``-1``. - *size* can be negative to shrink the writer. + *grow* can be negative to shrink the writer. .. c:function:: void* PyBytesWriter_GrowAndUpdatePointer(PyBytesWriter *writer, Py_ssize_t size, void *buf) diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index eee7a9e3bbf97f0..a4c19a451c7ce6b 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -463,6 +463,24 @@ def test_grow(self): writer.grow(0) # noop self.assertEqual(writer.finish(), b'number=123') + for size in (self.SMALL_BUFFER, self.LARGE_BUFFER): + with self.subTest(size=size): + # Truncate the last byte + data = b'x' * size + writer = self.create_writer(size) + writer.write(0, data) + self.assertEqual(writer.get_data(), data) + writer.grow(-1) + self.assertEqual(writer.get_data(), data[:-1]) + self.assertEqual(writer.finish(), data[:-1]) + + # Make the buffer empty + writer = self.create_writer(size) + writer.write(0, data) + writer.grow(-size) + self.assertEqual(writer.get_data(), b'') + self.assertEqual(writer.finish(), b'') + # Switch from small buffer to large buffer writer = self.create_writer() small, large = self.SMALL_BUFFER, self.LARGE_BUFFER @@ -478,8 +496,8 @@ def test_grow(self): with self.subTest(size=size): writer = self.create_writer() writer.write_bytes(b'x' * size, -1) - with self.assertRaisesRegex(ValueError, 'size must be >= 0'): - writer.grow(-1) + with self.assertRaisesRegex(ValueError, 'invalid size'): + writer.grow(-size - 1) with self.assertRaises(MemoryError): writer.grow(_testcapi.PY_SSIZE_T_MAX) self.assertEqual(writer.finish(), b'x' * size) diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 492fb6bc6663566..6e15a9195c583b3 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3865,8 +3865,13 @@ PyBytesWriter_Resize(PyBytesWriter *writer, Py_ssize_t size) PyErr_SetString(PyExc_ValueError, "size must be >= 0"); return -1; } - if (byteswriter_resize(writer, size, 1) < 0) { - return -1; + if (writer->size < size) { + if (byteswriter_resize(writer, size, 1) < 0) { + return -1; + } + } + else { + // The buffer is already large enough. Never shrink the buffer. } writer->size = size; return 0; @@ -3886,22 +3891,26 @@ _PyBytesWriter_ResizeAndUpdatePointer(PyBytesWriter *writer, Py_ssize_t size, int -PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t size) +PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t grow) { - if (size < 0) { - PyErr_SetString(PyExc_ValueError, "size must be >= 0"); - return -1; - } - if (size == 0) { + if (grow == 0) { // Nothing to do return 0; } - if (size > PY_SSIZE_T_MAX - writer->size) { - PyErr_NoMemory(); - return -1; + if (grow >= 0) { + if (grow > PY_SSIZE_T_MAX - writer->size) { + PyErr_NoMemory(); + return -1; + } + } + else { + if (writer->size + grow < 0) { + PyErr_SetString(PyExc_ValueError, "invalid size"); + return -1; + } } - size = writer->size + size; + Py_ssize_t size = writer->size + grow; if (byteswriter_resize(writer, size, 1) < 0) { return -1; From 9092e346e4925247747771a5a238e645cbde9514 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 18 Sep 2026 04:43:38 +0200 Subject: [PATCH 3/4] gh-157242: Add byteswriter_check_consistency() (#157723) Add byteswriter_check_consistency() to check that the bytes/bytearray object can be mutated. It is called by all PyBytesWriter functions. * Add test_thread to PyBytesWriter tests. * _PyBytes_IsMutable() now checks _PyObject_IsUniquelyReferenced(). * Clarify thread safety in PyBytesWriter documentation. (cherry picked from commit e2ff498cc03886b73fca0429ee745b04646ad8e0) --- Doc/c-api/bytes.rst | 4 +- Lib/test/test_capi/test_bytes.py | 42 ++++++++++++++++ Objects/bytesobject.c | 84 +++++++++++++++++++++++++++++++- 3 files changed, 127 insertions(+), 3 deletions(-) diff --git a/Doc/c-api/bytes.rst b/Doc/c-api/bytes.rst index 77b698009bf132c..3b995de6eb34f5f 100644 --- a/Doc/c-api/bytes.rst +++ b/Doc/c-api/bytes.rst @@ -299,8 +299,8 @@ object. A bytes writer instance. - The API is **not thread safe**: a writer should only be used by a single - thread at the same time. + The API is **not thread safe**. To share a writer with multiple threads, a + critical section or a lock is needed. The instance must be destroyed by :c:func:`PyBytesWriter_Finish` on success, or :c:func:`PyBytesWriter_Discard` on error. diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index a4c19a451c7ce6b..4ccbf18c1b00437 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -1,7 +1,11 @@ import sys +import textwrap +import threading import unittest from test import support from test.support import import_helper +from test.support import threading_helper +from test.support.script_helper import assert_python_failure _testlimitedcapi = import_helper.import_module('_testlimitedcapi') _testcapi = import_helper.import_module('_testcapi') @@ -434,6 +438,14 @@ def test_resize(self): self.assertEqual(writer.finish(), b's' * small + b'L' * (large - small)) + # Make sure that it's possible to write after a resize to zero + # when a bytes/bytearray object is allocated. + writer = self.create_writer() + writer.resize(self.LARGE_BUFFER) + writer.resize(0) + writer.write_bytes(b'abc', 3) + self.assertEqual(writer.finish(), b'abc') + # invalid size for size in (self.SMALL_BUFFER, self.LARGE_BUFFER): with self.subTest(size=size): @@ -551,6 +563,36 @@ def test_format_s_memory_error(self): _testcapi.remove_mem_hooks() writer.write_bytes(b'after', -1) + @threading_helper.requires_working_threading() + def test_thread(self): + # PyBytesWriter can be used by multiple threads: it's up to the caller + # to implement a lock to prevent concurrent accesses. + writer = self.create_writer(0) + size = None + data = None + + def thread_func(writer, LARGE_BUFFER): + nonlocal size, data + + # create a bytes object for the buffer + writer.write_bytes(b'x' * LARGE_BUFFER, LARGE_BUFFER) + + # so we can check a write with a bytes object + writer.write_bytes(b'yz', 2) + writer.format_i(b'i=%i', 5) + writer.resize(10) + data = writer.get_data() + size = writer.get_size() + + thread = threading.Thread(target=thread_func, + args=(writer, self.LARGE_BUFFER)) + thread.start() + threading_helper.join_thread(thread) + + self.assertEqual(size, 10) + self.assertEqual(data, b'x' * 10) + self.assertEqual(writer.finish(), b'x' * 10) + class BytesWriterTest(BaseWriterTest, unittest.TestCase): RESULT_TYPE = bytes diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 6e15a9195c583b3..559f59af184b5c4 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -37,7 +37,7 @@ static Py_ssize_t _PyBytesWriter_ResizeToAllocated(PyBytesWriter *writer); #define CHARACTERS _Py_SINGLETON(bytes_characters) #define CHARACTER(ch) \ - ((PyBytesObject *)&(CHARACTERS[ch])); + ((PyBytesObject *)&(CHARACTERS[ch])) #define EMPTY (&_Py_SINGLETON(bytes_empty)) @@ -3324,6 +3324,33 @@ PyBytes_ConcatAndDel(PyObject **pv, PyObject *w) } +#ifndef NDEBUG +// Make sure that a bytes object can still be mutated. +// +// Usage: assert(_PyBytes_IsMutable(obj)). +int +_PyBytes_IsMutable(PyObject *self) +{ + assert(PyBytes_Check(self)); + // Do not use _PyObject_IsUniquelyReferenced(): this function is called + // by bytearray and PyBytesWriter which can be used by multiple threads. + assert(Py_REFCNT(self) == 1); + assert(!_Py_IsImmortal(self)); + + // Check that the object is not a singleton + Py_ssize_t size = PyBytes_GET_SIZE(self); + if (size == 0) { + assert(self != bytes_get_empty()); + } + else if (size == 1) { + unsigned char ch = PyBytes_AS_STRING(self)[0]; + assert(self != (PyObject*)CHARACTER(ch)); + } + return 1; +} +#endif + + /* The following 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 @@ -3640,6 +3667,33 @@ byteswriter_allocated(PyBytesWriter *writer) } +#ifndef NDEBUG +static int +byteswriter_check_consistency(PyBytesWriter *writer) +{ + PyObject *obj = writer->obj; + if (obj != NULL) { + if (writer->use_bytearray) { + assert(PyByteArray_CheckExact(obj)); + // Do not use _PyObject_IsUniquelyReferenced(): the caller can have + // its own lock to prevent a writer from being used by two threads + // at the same time. + assert(Py_REFCNT(obj) == 1); + PyByteArrayObject *bytearray = (PyByteArrayObject*)obj; + obj = bytearray->ob_bytes_object; + assert(obj != NULL); + } + + // Code adapted from _PyBytes_IsMutable() + assert(PyBytes_CheckExact(obj)); + assert(_PyBytes_IsMutable(obj)); + assert((size_t)PyBytes_GET_SIZE(obj) > sizeof(writer->small_buffer)); + } + return 1; +} +#endif + + #ifdef MS_WINDOWS /* On Windows, overallocate by 50% is the best factor */ # define OVERALLOCATE_FACTOR 2 @@ -3668,6 +3722,7 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t size, int resize) if (writer->obj != NULL) { if (writer->use_bytearray) { if (PyByteArray_Resize(writer->obj, size)) { + assert(byteswriter_check_consistency(writer)); return -1; } } @@ -3675,6 +3730,7 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t size, int resize) // Can raise MemoryError or OverflowError if (_PyBytes_ResizeKeepOnError(&writer->obj, size)) { assert(writer->obj != NULL); + assert(byteswriter_check_consistency(writer)); return -1; } } @@ -3749,6 +3805,7 @@ byteswriter_create(Py_ssize_t size, int use_bytearray) memset(byteswriter_data(writer), PyBytesWrite_NEW_BYTE, byteswriter_allocated(writer)); #endif + assert(byteswriter_check_consistency(writer)); return writer; } @@ -3772,6 +3829,8 @@ PyBytesWriter_Discard(PyBytesWriter *writer) return; } + assert(byteswriter_check_consistency(writer)); + Py_XDECREF(writer->obj); _Py_FREELIST_FREE(bytes_writers, writer, PyMem_Free); } @@ -3780,6 +3839,8 @@ PyBytesWriter_Discard(PyBytesWriter *writer) PyObject* PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size) { + assert(byteswriter_check_consistency(writer)); + // Check for negative size here to raise ValueError in all cases, rather // than having a different exception depending on the code path. For // example, _PyBytes_Resize() raises SystemError on negative size. @@ -3821,6 +3882,13 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size) else { result = PyBytes_FromStringAndSize(writer->small_buffer, size); } + +#ifdef Py_DEBUG + // Reset the writer, so byteswriter_check_consistency() doesn't fail + // in PyBytesWriter_Discard(). + writer->size = 0; +#endif + PyBytesWriter_Discard(writer); return result; @@ -3847,6 +3915,8 @@ PyBytesWriter_FinishWithPointer(PyBytesWriter *writer, void *buf) void* PyBytesWriter_GetData(PyBytesWriter *writer) { + assert(byteswriter_check_consistency(writer)); + return byteswriter_data(writer); } @@ -3854,6 +3924,8 @@ PyBytesWriter_GetData(PyBytesWriter *writer) Py_ssize_t PyBytesWriter_GetSize(PyBytesWriter *writer) { + assert(byteswriter_check_consistency(writer)); + return _PyBytesWriter_GetSize(writer); } @@ -3861,6 +3933,8 @@ PyBytesWriter_GetSize(PyBytesWriter *writer) int PyBytesWriter_Resize(PyBytesWriter *writer, Py_ssize_t size) { + assert(byteswriter_check_consistency(writer)); + if (size < 0) { PyErr_SetString(PyExc_ValueError, "size must be >= 0"); return -1; @@ -3874,6 +3948,7 @@ PyBytesWriter_Resize(PyBytesWriter *writer, Py_ssize_t size) // The buffer is already large enough. Never shrink the buffer. } writer->size = size; + assert(byteswriter_check_consistency(writer)); return 0; } @@ -3893,6 +3968,8 @@ _PyBytesWriter_ResizeAndUpdatePointer(PyBytesWriter *writer, Py_ssize_t size, int PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t grow) { + assert(byteswriter_check_consistency(writer)); + if (grow == 0) { // Nothing to do return 0; @@ -3916,6 +3993,7 @@ PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t grow) return -1; } writer->size = size; + assert(byteswriter_check_consistency(writer)); return 0; } @@ -3951,6 +4029,8 @@ PyBytesWriter_WriteBytes(PyBytesWriter *writer, } char *buf = byteswriter_data(writer); memcpy(buf + pos, bytes, size); + + assert(byteswriter_check_consistency(writer)); return 0; } @@ -3988,5 +4068,7 @@ _PyBytesWriter_ResizeToAllocated(PyBytesWriter *writer) { Py_ssize_t allocated = byteswriter_allocated(writer); writer->size = allocated; + + assert(byteswriter_check_consistency(writer)); return allocated; } From 0c93671ff40cf012d0d29e1cc807e00bbe91199a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 16 Oct 2026 17:28:23 +0200 Subject: [PATCH 4/4] Run prek --all-files --- Lib/test/test_capi/test_bytes.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index 4ccbf18c1b00437..05c7f90eec7cbc0 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -1,11 +1,9 @@ import sys -import textwrap import threading import unittest from test import support from test.support import import_helper from test.support import threading_helper -from test.support.script_helper import assert_python_failure _testlimitedcapi = import_helper.import_module('_testlimitedcapi') _testcapi = import_helper.import_module('_testcapi')