Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Include/internal/pycore_bytesobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ PyAPI_FUNC(PyObject *) _PyBytes_Repeat(PyObject *self, Py_ssize_t n);
*/
#define _PyBytesObject_SIZE (offsetof(PyBytesObject, ob_sval) + 1)

extern int _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize);

/* --- PyBytesWriter ------------------------------------------------------ */

struct PyBytesWriter {
Expand Down
28 changes: 28 additions & 0 deletions Lib/test/test_bytes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1555,6 +1555,34 @@ def test_resize(self):
self.assertRaises(MemoryError, bytearray().resize, sys.maxsize)
self.assertRaises(MemoryError, bytearray(1000).resize, sys.maxsize)

def test_resize_error(self):
# gh-157242: If bytearray.resize() fails (memory allocation failure),
# the bytearray must be left unchanged.
_testcapi = import_helper.import_module('_testcapi')

# Simple bytearray
data = b'some data'
ba = bytearray(data)
try:
with self.assertRaises(MemoryError):
_testcapi.set_nomemory(0)
ba.resize(1024)
finally:
_testcapi.remove_mem_hooks()
self.assertEqual(ba, bytearray(data))

# bytearray with non-zero logical start
ba = bytearray(b'0123456789')
expected = ba[3:]
del ba[:3]
try:
with self.assertRaises(MemoryError):
_testcapi.set_nomemory(0)
ba.resize(1024)
finally:
_testcapi.remove_mem_hooks()
self.assertEqual(ba, expected)

def test_take_bytes(self):
ba = bytearray(b'ab')
self.assertEqual(ba.take_bytes(), b'ab')
Expand Down
15 changes: 15 additions & 0 deletions Lib/test/test_capi/test_bytes.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,21 @@ def test_resize(self):
writer.resize(len(b'number=123456'), b'456')
self.assertEqual(writer.finish(), self.result_type(b'number=123456'))

def test_resize_error(self):
small_buffer = _testcapi.PyBytesWriter_small_buffer
init = b'x' * (small_buffer * 2)
writer = self.create_writer(len(init), init)
size = len(init) + 100
try:
with self.assertRaises(MemoryError):
_testcapi.set_nomemory(0)
writer.resize(size, b'')
finally:
_testcapi.remove_mem_hooks()
suffix = b'still working'
writer.write_bytes(suffix, -1)
self.assertEqual(writer.finish(), self.result_type(init + suffix))

def test_format_i(self):
# Test PyBytesWriter_Format()
writer = self.create_writer()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :c:func:`PyBytesWriter_Resize` to handle properly memory allocation
failure. Leave the writer unchanged on error, instead of leaving it in an
inconsistent state. Patch by Victor Stinner.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
If :meth:`bytearray.resize` or :meth:`bytearray.take_bytes` fails, leave the
:class:`bytearray` unchanged, instead of clearing it. Patch by Victor
Stinner.
16 changes: 6 additions & 10 deletions Objects/bytearrayobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,20 +289,19 @@ bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
_PyBytes_Resize will do a malloc + memcpy internally. */
memmove(obj->ob_bytes, obj->ob_start,
Py_MIN(requested_size, Py_SIZE(self)));
obj->ob_start = obj->ob_bytes;
}

int ret = _PyBytes_Resize(&obj->ob_bytes_object, alloc);
if (ret == -1) {
obj->ob_bytes_object = Py_GetConstant(Py_CONSTANT_EMPTY_BYTES);
size = alloc = 0;
if (_PyBytes_ResizeKeepOnError(&obj->ob_bytes_object, alloc) < 0) {
Comment thread
vstinner marked this conversation as resolved.
return -1;
}

bytearray_reinit_from_bytes(obj, size, alloc);
if (alloc != size) {
/* Add mid-buffer null; end provided by bytes. */
obj->ob_bytes[size] = '\0';
}

return ret;
return 0;
}

int
Expand Down Expand Up @@ -1637,10 +1636,7 @@ bytearray_take_bytes_impl(PyByteArrayObject *self, PyObject *n)
self->ob_start = self->ob_bytes;
}

if (_PyBytes_Resize(&self->ob_bytes_object, to_take) == -1) {
assert(self->ob_bytes_object == NULL);
self->ob_bytes_object = Py_GetConstant(Py_CONSTANT_EMPTY_BYTES);
bytearray_reinit_from_bytes(self, 0, 0);
if (_PyBytes_ResizeKeepOnError(&self->ob_bytes_object, to_take) == -1) {
Py_DECREF(remaining);
return NULL;
}
Expand Down
77 changes: 50 additions & 27 deletions Objects/bytesobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -3344,69 +3344,91 @@ PyBytes_ConcatAndDel(PyObject **pv, PyObject *w)
does *not* include that), and a trailing \0 byte is stored.
*/

// Similar to _PyBytes_Resize(), but leaves the object unchanged on error.
int
_PyBytes_Resize(PyObject **pv, Py_ssize_t newsize)
_PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize)
{
PyObject *v;
PyBytesObject *sv;
v = *pv;
PyObject *v = *pv;
PyObject *result;

if (!PyBytes_Check(v) || newsize < 0) {
*pv = 0;
Py_DECREF(v);
PyErr_BadInternalCall();
return -1;
}

Py_ssize_t oldsize = PyBytes_GET_SIZE(v);
if (oldsize == newsize) {
/* return early if newsize equals to v->ob_size */
return 0;
}

if (oldsize == 0) {
*pv = _PyBytes_FromSize(newsize, 0);
result = _PyBytes_FromSize(newsize, 0);
if (result == NULL) {
return -1;
}
*pv = result;
Py_DECREF(v);
return (*pv == NULL) ? -1 : 0;
return 0;
}

if (newsize == 0) {
*pv = bytes_get_empty();
*pv = bytes_get_empty(); // cannot fail
Py_DECREF(v);
return 0;
}

if (!_PyObject_IsUniquelyReferenced(v)) {
// Allocate and then copy so we don't get a shared immortal
// one-character singleton!
*pv = _PyBytes_FromSize(newsize, 0);
if (*pv) {
memcpy(PyBytes_AS_STRING(*pv), PyBytes_AS_STRING(v),
Py_MIN(oldsize, newsize));
result = _PyBytes_FromSize(newsize, 0);
if (!result) {
return -1;
}

memcpy(PyBytes_AS_STRING(result), PyBytes_AS_STRING(v),
Py_MIN(oldsize, newsize));
*pv = result;
Py_DECREF(v);
return (*pv == NULL) ? -1 : 0;
return 0;
}

assert(v != bytes_get_empty());
result = (PyObject *)PyObject_Realloc(v, PyBytesObject_SIZE + newsize);
if (result == NULL) {
PyErr_NoMemory();
return -1;
}

#ifdef Py_TRACE_REFS
_Py_ForgetReference(v);
#endif
_PyReftracerTrack(v, PyRefTracer_DESTROY);
*pv = (PyObject *)
PyObject_Realloc(v, PyBytesObject_SIZE + newsize);
if (*pv == NULL) {
#ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyThreadState_GET());
#endif
PyObject_Free(v);
PyErr_NoMemory();
return -1;
}
_Py_NewReferenceNoTotal(*pv);
sv = (PyBytesObject *) *pv;

v = result;
_Py_NewReferenceNoTotal(v);
PyBytesObject *sv = (PyBytesObject *)v;
Py_SET_SIZE(sv, newsize);
sv->ob_sval[newsize] = '\0';
set_ob_shash(sv, -1); /* invalidate cached hash value */
*pv = v;
return 0;
}


int
_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);
}
return res;
}


/*********************** Bytes Iterator ****************************/

typedef struct {
Expand Down Expand Up @@ -3644,7 +3666,8 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t size, int resize)
}
}
else {
if (_PyBytes_Resize(&writer->obj, size)) {
if (_PyBytes_ResizeKeepOnError(&writer->obj, size)) {
assert(writer->obj != NULL);
return -1;
}
}
Expand Down
Loading