Skip to content

Commit 71c2d07

Browse files
gh-155702: Fix sqlite3.Blob slice assignment with a step
It patched the bytes object read from the blob, which for a single byte is an immortal singleton, so that the value of that byte was changed in the whole process. Read into a plain buffer instead. The raw read is factored out of read_multiple() into inner_read().
1 parent f056212 commit 71c2d07

3 files changed

Lines changed: 56 additions & 17 deletions

File tree

Lib/test/test_sqlite3/test_dbapi.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1396,6 +1396,18 @@ def test_blob_set_slice(self):
13961396
actual = self.cx.execute("select b from test").fetchone()[0]
13971397
self.assertEqual(actual, expected)
13981398

1399+
def test_blob_set_slice_with_step_keeps_bytes_intact(self):
1400+
# The buffer used for the read-patch-write cycle must not be the
1401+
# bytes object read from the blob: for a single byte it is an
1402+
# immortal singleton.
1403+
old_byte = self.data[5]
1404+
self.blob[5:6:2] = b"\xab"
1405+
self.assertEqual(bytes([old_byte])[0], old_byte)
1406+
self.assertEqual(self.blob[5:6], b"\xab")
1407+
expected = self.data[:5] + b"\xab" + self.data[6:]
1408+
actual = self.cx.execute("select b from test").fetchone()[0]
1409+
self.assertEqual(actual, expected)
1410+
13991411
def test_blob_set_empty_slice(self):
14001412
self.blob[0:0] = b""
14011413
self.assertEqual(self.blob[:], self.data)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix :class:`sqlite3.Blob` slice assignment with a step.
2+
It patched the bytes object read from the blob,
3+
which for a single byte is an immortal singleton,
4+
so that the value of that byte was changed in the whole process.

Modules/_sqlite/blob.c

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -139,26 +139,35 @@ read_single(pysqlite_Blob *self, Py_ssize_t offset)
139139
return PyLong_FromUnsignedLong((unsigned long)buf);
140140
}
141141

142-
static PyObject *
143-
read_multiple(pysqlite_Blob *self, Py_ssize_t length, Py_ssize_t offset)
142+
static int
143+
inner_read(pysqlite_Blob *self, char *buf, Py_ssize_t length,
144+
Py_ssize_t offset)
144145
{
145146
assert(length <= sqlite3_blob_bytes(self->blob));
146147
assert(offset < sqlite3_blob_bytes(self->blob));
147148

148-
PyBytesWriter *writer = PyBytesWriter_Create(length);
149-
if (writer == NULL) {
150-
return NULL;
151-
}
152-
char *raw_buffer = PyBytesWriter_GetData(writer);
153-
154149
int rc;
155150
Py_BEGIN_ALLOW_THREADS
156-
rc = sqlite3_blob_read(self->blob, raw_buffer, (int)length, (int)offset);
151+
rc = sqlite3_blob_read(self->blob, buf, (int)length, (int)offset);
157152
Py_END_ALLOW_THREADS
158153

159154
if (rc != SQLITE_OK) {
160-
PyBytesWriter_Discard(writer);
161155
blob_seterror(self, rc);
156+
return -1;
157+
}
158+
return 0;
159+
}
160+
161+
static PyObject *
162+
read_multiple(pysqlite_Blob *self, Py_ssize_t length, Py_ssize_t offset)
163+
{
164+
PyBytesWriter *writer = PyBytesWriter_Create(length);
165+
if (writer == NULL) {
166+
return NULL;
167+
}
168+
169+
if (inner_read(self, PyBytesWriter_GetData(writer), length, offset) < 0) {
170+
PyBytesWriter_Discard(writer);
162171
return NULL;
163172
}
164173
return PyBytesWriter_Finish(writer);
@@ -553,14 +562,28 @@ ass_subscript_slice(pysqlite_Blob *self, PyObject *item, PyObject *value)
553562
rc = inner_write(self, vbuf.buf, len, start);
554563
}
555564
else {
556-
PyObject *blob_bytes = read_multiple(self, stop - start, start);
557-
if (blob_bytes != NULL) {
558-
char *blob_buf = PyBytes_AS_STRING(blob_bytes);
559-
for (Py_ssize_t i = 0, j = 0; i < len; i++, j += step) {
560-
blob_buf[j] = ((char *)vbuf.buf)[i];
565+
/* Read the affected region, patch it and write it back. The
566+
object returned by read_multiple() cannot be used as the buffer,
567+
because for a single byte it is an immortal singleton. */
568+
Py_ssize_t length = stop - start;
569+
if (length <= 0) {
570+
/* start > stop for a negative step; see gh-150449. */
571+
PyErr_SetString(PyExc_ValueError, "size must be >= 0");
572+
}
573+
else {
574+
char *buf = PyMem_Malloc(length);
575+
if (buf == NULL) {
576+
PyErr_NoMemory();
577+
}
578+
else {
579+
if (inner_read(self, buf, length, start) == 0) {
580+
for (Py_ssize_t i = 0, j = 0; i < len; i++, j += step) {
581+
buf[j] = ((char *)vbuf.buf)[i];
582+
}
583+
rc = inner_write(self, buf, length, start);
584+
}
585+
PyMem_Free(buf);
561586
}
562-
rc = inner_write(self, blob_buf, stop - start, start);
563-
Py_DECREF(blob_bytes);
564587
}
565588
}
566589
PyBuffer_Release(&vbuf);

0 commit comments

Comments
 (0)