gh-92019: Make sqlite3.Blob indexing conform with the norm (#92020)

- get index now returns an int
- set index now requires an int in range(0, 256)

Resolves #92019

Author: erlend-aasland

Doc/includes/sqlite3/blob.py

@@ -9,8 +9,8 @@
     blob.write(b"hello, ")
     blob.write(b"world.")
     # Modify the first and last bytes of our blob
-    blob[0] = b"H"
-    blob[-1] = b"!"
+    blob[0] = ord("H")
+    blob[-1] = ord("!")
 
 # Read the contents of our blob
 with con.blobopen("test", "blob_col", 1) as blob:

Lib/test/test_sqlite3/test_dbapi.py

@@ -1173,38 +1173,29 @@ def test_blob_length(self):
         self.assertEqual(len(self.blob), 50)
 
     def test_blob_get_item(self):
-        self.assertEqual(self.blob[5], b"b")
-        self.assertEqual(self.blob[6], b"l")
-        self.assertEqual(self.blob[7], b"o")
-        self.assertEqual(self.blob[8], b"b")
-        self.assertEqual(self.blob[-1], b"!")
+        self.assertEqual(self.blob[5], ord("b"))
+        self.assertEqual(self.blob[6], ord("l"))
+        self.assertEqual(self.blob[7], ord("o"))
+        self.assertEqual(self.blob[8], ord("b"))
+        self.assertEqual(self.blob[-1], ord("!"))
 
     def test_blob_set_item(self):
-        self.blob[0] = b"b"
+        self.blob[0] = ord("b")
         expected = b"b" + self.data[1:]
         actual = self.cx.execute("select b from test").fetchone()[0]
         self.assertEqual(actual, expected)
 
     def test_blob_set_item_with_offset(self):
         self.blob.seek(0, SEEK_END)
         self.assertEqual(self.blob.read(), b"")  # verify that we're at EOB
-        self.blob[0] = b"T"
-        self.blob[-1] = b"."
+        self.blob[0] = ord("T")
+        self.blob[-1] = ord(".")
         self.blob.seek(0, SEEK_SET)
         expected = b"This blob data string is exactly fifty bytes long."
         self.assertEqual(self.blob.read(), expected)
 
-    def test_blob_set_buffer_object(self):
+    def test_blob_set_slice_buffer_object(self):
         from array import array
-        self.blob[0] = memoryview(b"1")
-        self.assertEqual(self.blob[0], b"1")
-
-        self.blob[1] = bytearray(b"2")
-        self.assertEqual(self.blob[1], b"2")
-
-        self.blob[2] = array("b", [4])
-        self.assertEqual(self.blob[2], b"\x04")
-
         self.blob[0:5] = memoryview(b"12345")
         self.assertEqual(self.blob[0:5], b"12345")
 
@@ -1215,8 +1206,8 @@ def test_blob_set_buffer_object(self):
         self.assertEqual(self.blob[0:5], b"\x01\x02\x03\x04\x05")
 
     def test_blob_set_item_negative_index(self):
-        self.blob[-1] = b"z"
-        self.assertEqual(self.blob[-1], b"z")
+        self.blob[-1] = 255
+        self.assertEqual(self.blob[-1], 255)
 
     def test_blob_get_slice(self):
         self.assertEqual(self.blob[5:14], b"blob data")
@@ -1264,13 +1255,29 @@ def test_blob_get_item_error(self):
         with self.assertRaisesRegex(IndexError, "cannot fit 'int'"):
             self.blob[ULLONG_MAX]
 
+        # Provoke read error
+        self.cx.execute("update test set b='aaaa' where rowid=1")
+        with self.assertRaises(sqlite.OperationalError):
+            self.blob[0]
+
     def test_blob_set_item_error(self):
-        with self.assertRaisesRegex(ValueError, "must be a single byte"):
+        with self.assertRaisesRegex(TypeError, "cannot be interpreted"):
             self.blob[0] = b"multiple"
+        with self.assertRaisesRegex(TypeError, "cannot be interpreted"):
+            self.blob[0] = b"1"
+        with self.assertRaisesRegex(TypeError, "cannot be interpreted"):
+            self.blob[0] = bytearray(b"1")
         with self.assertRaisesRegex(TypeError, "doesn't support.*deletion"):
             del self.blob[0]
         with self.assertRaisesRegex(IndexError, "Blob index out of range"):
-            self.blob[1000] = b"a"
+            self.blob[1000] = 0
+        with self.assertRaisesRegex(ValueError, "must be in range"):
+            self.blob[0] = -1
+        with self.assertRaisesRegex(ValueError, "must be in range"):
+            self.blob[0] = 256
+        # Overflow errors are overridden with ValueError
+        with self.assertRaisesRegex(ValueError, "must be in range"):
+            self.blob[0] = 2**65
 
     def test_blob_set_slice_error(self):
         with self.assertRaisesRegex(IndexError, "wrong size"):

Modules/_sqlite/blob.c

@@ -120,10 +120,26 @@ blob_seterror(pysqlite_Blob *self, int rc)
 }
 
 static PyObject *
-inner_read(pysqlite_Blob *self, Py_ssize_t length, Py_ssize_t offset)
+read_single(pysqlite_Blob *self, Py_ssize_t offset)
+{
+    unsigned char buf = 0;
+    int rc;
+    Py_BEGIN_ALLOW_THREADS
+    rc = sqlite3_blob_read(self->blob, (void *)&buf, 1, (int)offset);
+    Py_END_ALLOW_THREADS
+
+    if (rc != SQLITE_OK) {
+        blob_seterror(self, rc);
+        return NULL;
+    }
+    return PyLong_FromUnsignedLong((unsigned long)buf);
+}
+
+static PyObject *
+read_multiple(pysqlite_Blob *self, Py_ssize_t length, Py_ssize_t offset)
 {
     assert(length <= sqlite3_blob_bytes(self->blob));
-    assert(offset <= sqlite3_blob_bytes(self->blob));
+    assert(offset < sqlite3_blob_bytes(self->blob));
 
     PyObject *buffer = PyBytes_FromStringAndSize(NULL, length);
     if (buffer == NULL) {
@@ -175,7 +191,12 @@ blob_read_impl(pysqlite_Blob *self, int length)
         length = max_read_len;
     }
 
-    PyObject *buffer = inner_read(self, length, self->offset);
+    assert(length >= 0);
+    if (length == 0) {
+        return PyBytes_FromStringAndSize(NULL, 0);
+    }
+
+    PyObject *buffer = read_multiple(self, length, self->offset);
     if (buffer == NULL) {
         return NULL;
     }
@@ -387,7 +408,7 @@ subscript_index(pysqlite_Blob *self, PyObject *item)
     if (i < 0) {
         return NULL;
     }
-    return inner_read(self, 1, i);
+    return read_single(self, i);
 }
 
 static int
@@ -411,9 +432,9 @@ subscript_slice(pysqlite_Blob *self, PyObject *item)
     }
 
     if (step == 1) {
-        return inner_read(self, len, start);
+        return read_multiple(self, len, start);
     }
-    PyObject *blob = inner_read(self, stop - start, start);
+    PyObject *blob = read_multiple(self, stop - start, start);
     if (blob == NULL) {
         return NULL;
     }
@@ -455,24 +476,29 @@ ass_subscript_index(pysqlite_Blob *self, PyObject *item, PyObject *value)
                         "Blob doesn't support item deletion");
         return -1;
     }
+    if (!PyLong_Check(value)) {
+        PyErr_Format(PyExc_TypeError,
+                     "'%s' object cannot be interpreted as an integer",
+                     Py_TYPE(value)->tp_name);
+        return -1;
+    }
     Py_ssize_t i = get_subscript_index(self, item);
     if (i < 0) {
         return -1;
     }
 
-    Py_buffer vbuf;
-    if (PyObject_GetBuffer(value, &vbuf, PyBUF_SIMPLE) < 0) {
-        return -1;
+    long val = PyLong_AsLong(value);
+    if (val == -1 && PyErr_Occurred()) {
+        PyErr_Clear();
+        val = -1;
     }
-    int rc = -1;
-    if (vbuf.len != 1) {
-        PyErr_SetString(PyExc_ValueError, "Blob assignment must be a single byte");
-    }
-    else {
-        rc = inner_write(self, (const char *)vbuf.buf, 1, i);
+    if (val < 0 || val > 255) {
+        PyErr_SetString(PyExc_ValueError, "byte must be in range(0, 256)");
+        return -1;
     }
-    PyBuffer_Release(&vbuf);
-    return rc;
+    // Downcast to avoid endianness problems.
+    unsigned char byte = (unsigned char)val;
+    return inner_write(self, (const void *)&byte, 1, i);
 }
 
 static int
@@ -507,7 +533,7 @@ ass_subscript_slice(pysqlite_Blob *self, PyObject *item, PyObject *value)
         rc = inner_write(self, vbuf.buf, len, start);
     }
     else {
-        PyObject *blob_bytes = inner_read(self, stop - start, start);
+        PyObject *blob_bytes = read_multiple(self, stop - start, start);
         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) {