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
4 changes: 4 additions & 0 deletions Doc/library/sqlite3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1760,6 +1760,10 @@ Blob objects

.. versionadded:: 3.11

.. versionchanged:: next
:class:`Blob` now supports negative-step slices
(e.g. ``blob[9:0:-2]``) for both reading and writing.

A :class:`Blob` instance is a :term:`file-like object`
that can read and write data in an SQLite :abbr:`BLOB (Binary Large OBject)`.
Call :func:`len(blob) <len>` to get the size (number of bytes) of the blob.
Expand Down
8 changes: 8 additions & 0 deletions Doc/whatsnew/3.16.rst
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,14 @@ shlex
a string, even if it is already safe for a shell without being quoted.
(Contributed by Jay Berry in :gh:`148846`.)

sqlite3
-------

* :class:`sqlite3.Blob` now supports negative-step slices for reading and
writing (e.g. ``blob[9:0:-2]``). Previously, such slices would raise
:exc:`SystemError` or :exc:`ValueError`.
(Contributed by Jiseok CHOI in :gh:`150449`.)

xml
---

Expand Down
31 changes: 31 additions & 0 deletions Lib/test/test_sqlite3/test_dbapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,16 @@ def test_blob_get_slice_negative_index(self):
def test_blob_get_slice_with_skip(self):
self.assertEqual(self.blob[0:10:2], b"ti lb")

def test_blob_get_slice_with_negative_step(self):
# gh-150449: negative-step slices must not crash
self.assertEqual(self.blob[9:0:-2], self.data[9:0:-2])
self.assertEqual(self.blob[9::-2], self.data[9::-2])
self.assertEqual(self.blob[::-1], self.data[::-1])
# When start <= stop with a negative step the slice is empty; this
# must return b"" rather than crashing or raising an exception.
self.assertEqual(self.blob[3:8:-1], self.data[3:8:-1]) # b""
self.assertEqual(self.blob[5:5:-1], self.data[5:5:-1]) # b""

def test_blob_set_slice(self):
self.blob[0:5] = b"12345"
expected = b"12345" + self.data[5:]
Expand Down Expand Up @@ -1418,6 +1428,27 @@ def test_blob_set_slice_with_skip(self):
expected = b"1h2s3b4o5 " + self.data[10:]
self.assertEqual(actual, expected)

def test_blob_set_slice_with_negative_step(self):
# gh-150449: negative-step slice assignment must not crash
expected = bytearray(self.data)
expected[9:0:-2] = b"12345"
self.blob[9:0:-2] = b"12345"
actual = self.cx.execute("select b from test").fetchone()[0]
self.assertEqual(actual, bytes(expected))

# Also verify a slice that includes index 0
expected2 = bytearray(self.data)
expected2[9::-2] = b"12345"
self.blob[9::-2] = b"12345"
actual2 = self.cx.execute("select b from test").fetchone()[0]
self.assertEqual(actual2, bytes(expected2))

# When start <= stop with a negative step the slice is empty;
# assigning b"" to it must be a no-op (blob contents unchanged).
state_before = bytes(self.blob[:])
self.blob[3:8:-1] = b""
self.assertEqual(bytes(self.blob[:]), state_before)

def test_blob_mapping_invalid_index_type(self):
msg = "indices must be integers"
with self.assertRaisesRegex(TypeError, msg):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:class:`sqlite3.Blob` now supports negative-step slices for reading and
writing (e.g. ``blob[9:0:-2]``). Previously, such slices would raise
:exc:`SystemError` or :exc:`ValueError`.
35 changes: 27 additions & 8 deletions Modules/_sqlite/blob.c
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,14 @@ subscript_slice(pysqlite_Blob *self, PyObject *item)
return read_multiple(self, len, start);
}

PyObject *blob = read_multiple(self, stop - start, start);
// Compute the contiguous blob region covering all slice elements, then
// copy each element using the standard size_t-cursor pattern that handles
// both positive and negative steps via unsigned arithmetic.
Py_ssize_t last = start + (len - 1) * step;
Py_ssize_t read_offset = Py_MIN(start, last);
Py_ssize_t read_length = Py_ABS(start - last) + 1;

PyObject *blob = read_multiple(self, read_length, read_offset);
if (blob == NULL) {
return NULL;
}
Expand All @@ -456,10 +463,12 @@ subscript_slice(pysqlite_Blob *self, PyObject *item)
return NULL;
}
char *res_buf = PyBytesWriter_GetData(writer);

char *blob_buf = PyBytes_AS_STRING(blob);
for (Py_ssize_t i = 0, j = 0; i < len; i++, j += step) {
res_buf[i] = blob_buf[j];

size_t cur;
Py_ssize_t i;
for (cur = (size_t)start, i = 0; i < len; cur += (size_t)step, i++) {
res_buf[i] = blob_buf[(Py_ssize_t)cur - read_offset];
Comment on lines +468 to +471
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOC why all those casts? start and atep are already ssize values, and then we recast cur to ssize_t. Why not having ssize_t everywhere? there should not be any issues with signed comparisons as both operands are signed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSIZE_MAX + SSIZE_MAX = integer overflow.

The code is more complicated with signed integers.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right. If we can add a test case for that it would be good but if it is too complex to do it (by mocking some stuff), let's forget it. I am keeping the comment unresolved so that future readers will remember

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth adding an inline comment here to explain why size_t is used instead of Py_ssize_t? Something like:

// Use size_t for 'cur' so that negative steps work via well-defined
// unsigned wrap-around arithmetic. A signed accumulator would risk
// undefined behaviour when start or step is near SSIZE_MAX.

}
Py_DECREF(blob);
return PyBytesWriter_Finish(writer);
Expand Down Expand Up @@ -553,13 +562,23 @@ ass_subscript_slice(pysqlite_Blob *self, PyObject *item, PyObject *value)
rc = inner_write(self, vbuf.buf, len, start);
}
else {
PyObject *blob_bytes = read_multiple(self, stop - start, start);
// Compute the contiguous blob region covering all slice elements, then
// update each element using the standard size_t-cursor pattern that
// handles both positive and negative steps via unsigned arithmetic.
Py_ssize_t last = start + (len - 1) * step;
Py_ssize_t write_offset = Py_MIN(start, last);
Py_ssize_t write_length = Py_ABS(start - last) + 1;
PyObject *blob_bytes = read_multiple(self, write_length, write_offset);
if (blob_bytes != NULL) {
char *blob_buf = PyBytes_AS_STRING(blob_bytes);
for (Py_ssize_t i = 0, j = 0; i < len; i++, j += step) {
blob_buf[j] = ((char *)vbuf.buf)[i];
size_t cur;
Py_ssize_t i;
for (cur = (size_t)start, i = 0; i < len;
cur += (size_t)step, i++) {
blob_buf[(Py_ssize_t)cur - write_offset] =
Comment thread
picnixz marked this conversation as resolved.
((char *)vbuf.buf)[i];
}
rc = inner_write(self, blob_buf, stop - start, start);
rc = inner_write(self, blob_buf, write_length, write_offset);
Py_DECREF(blob_bytes);
}
}
Expand Down
Loading