bpo-32788: Better error handling in sqlite3. (GH-3723)

Propagate unexpected errors (like MemoryError and KeyboardInterrupt) to user.

Lib/sqlite3/test/types.py

@@ -102,10 +102,16 @@ class DeclTypesTests(unittest.TestCase):
         def __str__(self):
             return "<%s>" % self.val
 
+    class BadConform:
+        def __init__(self, exc):
+            self.exc = exc
+        def __conform__(self, protocol):
+            raise self.exc
+
     def setUp(self):
         self.con = sqlite.connect(":memory:", detect_types=sqlite.PARSE_DECLTYPES)
         self.cur = self.con.cursor()
-        self.cur.execute("create table test(i int, s str, f float, b bool, u unicode, foo foo, bin blob, n1 number, n2 number(5))")
+        self.cur.execute("create table test(i int, s str, f float, b bool, u unicode, foo foo, bin blob, n1 number, n2 number(5), bad bad)")
 
         # override float, make them always return the same number
         sqlite.converters["FLOAT"] = lambda x: 47.2
@@ -113,6 +119,7 @@ class DeclTypesTests(unittest.TestCase):
         # and implement two custom ones
         sqlite.converters["BOOL"] = lambda x: bool(int(x))
         sqlite.converters["FOO"] = DeclTypesTests.Foo
+        sqlite.converters["BAD"] = DeclTypesTests.BadConform
         sqlite.converters["WRONG"] = lambda x: "WRONG"
         sqlite.converters["NUMBER"] = float
 
@@ -120,6 +127,8 @@ class DeclTypesTests(unittest.TestCase):
         del sqlite.converters["FLOAT"]
         del sqlite.converters["BOOL"]
         del sqlite.converters["FOO"]
+        del sqlite.converters["BAD"]
+        del sqlite.converters["WRONG"]
         del sqlite.converters["NUMBER"]
         self.cur.close()
         self.con.close()
@@ -159,13 +168,13 @@ class DeclTypesTests(unittest.TestCase):
         self.cur.execute("insert into test(b) values (?)", (False,))
         self.cur.execute("select b from test")
         row = self.cur.fetchone()
-        self.assertEqual(row[0], False)
+        self.assertIs(row[0], False)
 
         self.cur.execute("delete from test")
         self.cur.execute("insert into test(b) values (?)", (True,))
         self.cur.execute("select b from test")
         row = self.cur.fetchone()
-        self.assertEqual(row[0], True)
+        self.assertIs(row[0], True)
 
     def CheckUnicode(self):
         # default
@@ -182,6 +191,19 @@ class DeclTypesTests(unittest.TestCase):
         row = self.cur.fetchone()
         self.assertEqual(row[0], val)
 
+    def CheckErrorInConform(self):
+        val = DeclTypesTests.BadConform(TypeError)
+        with self.assertRaises(sqlite.InterfaceError):
+            self.cur.execute("insert into test(bad) values (?)", (val,))
+        with self.assertRaises(sqlite.InterfaceError):
+            self.cur.execute("insert into test(bad) values (:val)", {"val": val})
+
+        val = DeclTypesTests.BadConform(KeyboardInterrupt)
+        with self.assertRaises(KeyboardInterrupt):
+            self.cur.execute("insert into test(bad) values (?)", (val,))
+        with self.assertRaises(KeyboardInterrupt):
+            self.cur.execute("insert into test(bad) values (:val)", {"val": val})
+
     def CheckUnsupportedSeq(self):
         class Bar: pass
         val = Bar()

Misc/NEWS.d/next/Library/2018-07-23-12-20-02.bpo-32788.R2jSiK.rst

+Errors other than :exc:`TypeError` raised in methods ``__adapt__()`` and
+``__conform__()`` in the :mod:`sqlite3` module are now propagated to the
+user.

Modules/_sqlite/cache.c

@@ -119,7 +119,7 @@ PyObject* pysqlite_cache_get(pysqlite_Cache* self, PyObject* args)
     pysqlite_Node* ptr;
     PyObject* data;
 
-    node = (pysqlite_Node*)PyDict_GetItem(self->mapping, key);
+    node = (pysqlite_Node*)PyDict_GetItemWithError(self->mapping, key);
     if (node) {
         /* an entry for this key already exists in the cache */
 
@@ -157,7 +157,11 @@ PyObject* pysqlite_cache_get(pysqlite_Cache* self, PyObject* args)
             }
             ptr->prev = node;
         }
-    } else {
+    }
+    else if (PyErr_Occurred()) {
+        return NULL;
+    }
+    else {
         /* There is no entry for this key in the cache, yet. We'll insert a new
          * entry in the cache, and make space if necessary by throwing the
          * least used item out of the cache. */

Modules/_sqlite/connection.c

@@ -1432,6 +1432,7 @@ finally:
 static PyObject *
 pysqlite_connection_iterdump(pysqlite_Connection* self, PyObject* args)
 {
+    _Py_IDENTIFIER(_iterdump);
     PyObject* retval = NULL;
     PyObject* module = NULL;
     PyObject* module_dict;
@@ -1451,9 +1452,12 @@ pysqlite_connection_iterdump(pysqlite_Connection* self, PyObject* args)
         goto finally;
     }
 
-    pyfn_iterdump = PyDict_GetItemString(module_dict, "_iterdump");
+    pyfn_iterdump = _PyDict_GetItemIdWithError(module_dict, &PyId__iterdump);
     if (!pyfn_iterdump) {
-        PyErr_SetString(pysqlite_OperationalError, "Failed to obtain _iterdump() reference");
+        if (!PyErr_Occurred()) {
+            PyErr_SetString(pysqlite_OperationalError,
+                            "Failed to obtain _iterdump() reference");
+        }
         goto finally;
     }
 

Modules/_sqlite/microprotocols.c

@@ -75,7 +75,9 @@ pysqlite_microprotocols_add(PyTypeObject *type, PyObject *proto, PyObject *cast)
 PyObject *
 pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt)
 {
-    PyObject *adapter, *key;
+    _Py_IDENTIFIER(__adapt__);
+    _Py_IDENTIFIER(__conform__);
+    PyObject *adapter, *key, *adapted;
 
     /* we don't check for exact type conformance as specified in PEP 246
        because the pysqlite_PrepareProtocolType type is abstract and there is no
@@ -86,48 +88,60 @@ pysqlite_microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt)
     if (!key) {
         return NULL;
     }
-    adapter = PyDict_GetItem(psyco_adapters, key);
+    adapter = PyDict_GetItemWithError(psyco_adapters, key);
     Py_DECREF(key);
     if (adapter) {
-        PyObject *adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL);
+        Py_INCREF(adapter);
+        adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL);
+        Py_DECREF(adapter);
         return adapted;
     }
+    if (PyErr_Occurred()) {
+        return NULL;
+    }
 
-    /* try to have the protocol adapt this object*/
-    if (PyObject_HasAttrString(proto, "__adapt__")) {
-        _Py_IDENTIFIER(__adapt__);
-        PyObject *adapted = _PyObject_CallMethodId(proto, &PyId___adapt__, "O", obj);
-
-        if (adapted) {
-            if (adapted != Py_None) {
-                return adapted;
-            } else {
-                Py_DECREF(adapted);
-            }
-        }
+    /* try to have the protocol adapt this object */
+    if (_PyObject_LookupAttrId(proto, &PyId___adapt__, &adapter) < 0) {
+        return NULL;
+    }
+    if (adapter) {
+        adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL);
+        Py_DECREF(adapter);
 
-        if (PyErr_Occurred() && !PyErr_ExceptionMatches(PyExc_TypeError))
-            return NULL;
+        if (adapted == Py_None) {
+            Py_DECREF(adapted);
+        }
+        else if (adapted || !PyErr_ExceptionMatches(PyExc_TypeError)) {
+            return adapted;
+        }
+        else {
+            PyErr_Clear();
+        }
     }
 
     /* and finally try to have the object adapt itself */
-    if (PyObject_HasAttrString(obj, "__conform__")) {
-        _Py_IDENTIFIER(__conform__);
-        PyObject *adapted = _PyObject_CallMethodId(obj, &PyId___conform__,"O", proto);
-
-        if (adapted) {
-            if (adapted != Py_None) {
-                return adapted;
-            } else {
-                Py_DECREF(adapted);
-            }
-        }
+    if (_PyObject_LookupAttrId(obj, &PyId___conform__, &adapter) < 0) {
+        return NULL;
+    }
+    if (adapter) {
+        adapted = PyObject_CallFunctionObjArgs(adapter, proto, NULL);
+        Py_DECREF(adapter);
 
-        if (PyErr_Occurred() && !PyErr_ExceptionMatches(PyExc_TypeError)) {
-            return NULL;
+        if (adapted == Py_None) {
+            Py_DECREF(adapted);
+        }
+        else if (adapted || !PyErr_ExceptionMatches(PyExc_TypeError)) {
+            return adapted;
+        }
+        else {
+            PyErr_Clear();
         }
     }
 
+    if (alt) {
+        Py_INCREF(alt);
+        return alt;
+    }
     /* else set the right exception and return NULL */
     PyErr_SetString(pysqlite_ProgrammingError, "can't adapt");
     return NULL;

Modules/_sqlite/statement.c

@@ -250,12 +250,10 @@ void pysqlite_statement_bind_parameters(pysqlite_Statement* self, PyObject* para
             if (!_need_adapt(current_param)) {
                 adapted = current_param;
             } else {
-                adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, NULL);
-                if (adapted) {
-                    Py_DECREF(current_param);
-                } else {
-                    PyErr_Clear();
-                    adapted = current_param;
+                adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, current_param);
+                Py_DECREF(current_param);
+                if (!adapted) {
+                    return;
                 }
             }
 
@@ -272,6 +270,7 @@ void pysqlite_statement_bind_parameters(pysqlite_Statement* self, PyObject* para
     } else if (PyDict_Check(parameters)) {
         /* parameters passed as dictionary */
         for (i = 1; i <= num_params_needed; i++) {
+            PyObject *binding_name_obj;
             Py_BEGIN_ALLOW_THREADS
             binding_name = sqlite3_bind_parameter_name(self->st, i);
             Py_END_ALLOW_THREADS
@@ -281,26 +280,31 @@ void pysqlite_statement_bind_parameters(pysqlite_Statement* self, PyObject* para
             }
 
             binding_name++; /* skip first char (the colon) */
+            binding_name_obj = PyUnicode_FromString(binding_name);
+            if (!binding_name_obj) {
+                return;
+            }
             if (PyDict_CheckExact(parameters)) {
-                current_param = PyDict_GetItemString(parameters, binding_name);
+                current_param = PyDict_GetItemWithError(parameters, binding_name_obj);
                 Py_XINCREF(current_param);
             } else {
-                current_param = PyMapping_GetItemString(parameters, binding_name);
+                current_param = PyObject_GetItem(parameters, binding_name_obj);
             }
+            Py_DECREF(binding_name_obj);
             if (!current_param) {
-                PyErr_Format(pysqlite_ProgrammingError, "You did not supply a value for binding %d.", i);
+                if (!PyErr_Occurred() || PyErr_ExceptionMatches(PyExc_LookupError)) {
+                    PyErr_Format(pysqlite_ProgrammingError, "You did not supply a value for binding %d.", i);
+                }
                 return;
             }
 
             if (!_need_adapt(current_param)) {
                 adapted = current_param;
             } else {
-                adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, NULL);
-                if (adapted) {
-                    Py_DECREF(current_param);
-                } else {
-                    PyErr_Clear();
-                    adapted = current_param;
+                adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, current_param);
+                Py_DECREF(current_param);
+                if (!adapted) {
+                    return;
                 }
             }