diff --git a/Lib/test/test_sqlite3/test_dbapi.py b/Lib/test/test_sqlite3/test_dbapi.py index 20e39f61e4dedb..447db7e8e20f23 100644 --- a/Lib/test/test_sqlite3/test_dbapi.py +++ b/Lib/test/test_sqlite3/test_dbapi.py @@ -32,7 +32,7 @@ import warnings from test.support import ( - SHORT_TIMEOUT, check_disallow_instantiation, requires_subprocess + SHORT_TIMEOUT, check_disallow_instantiation, requires_subprocess, subTests ) from test.support import gc_collect from test.support import threading_helper, import_helper @@ -728,6 +728,21 @@ def test_database_keyword(self): self.assertEqual(type(cx), sqlite.Connection) +class ParamsCxCloseInIterMany: + def __init__(self, cx): + self.cx = cx + + def __iter__(self): + self.cx.close() + return iter([(1,), (2,), (3,)]) + + +def ParamsCxCloseInNext(cx): + for i in range(10): + cx.close() + yield (i,) + + class CursorTests(unittest.TestCase): def setUp(self): self.cx = sqlite.connect(":memory:") @@ -859,6 +874,31 @@ def __getitem__(slf, x): with self.assertRaises(ZeroDivisionError): self.cu.execute("select name from test where name=?", L()) + def test_execute_use_after_close_with_bind_parameters(self): + # Prevent SIGSEGV when closing the connection while binding parameters. + # + # Internally, the connection's state is checked after bind_parameters(). + # Without this check, we would only be aware of the closed connection + # by calling an sqlite3 function afterwards. However, it is important + # that we report the error before leaving the execute() call. + # + # Regression test for https://github.com/python/cpython/issues/143198. + + class PT: + def __getitem__(self, i): + cx.close() + return 1 + def __len__(self): + return 1 + + cx = sqlite.connect(":memory:") + cx.execute("create table tmp(a number)") + self.addCleanup(cx.close) + cu = cx.cursor() + msg = r"Cannot operate on a closed database\." + with self.assertRaisesRegex(sqlite.ProgrammingError, msg): + cu.execute("insert into tmp(a) values (?)", PT()) + def test_execute_named_param_and_sequence(self): dataset = ( ("select :a", (1,)), @@ -1030,6 +1070,50 @@ def test_execute_many_not_iterable(self): with self.assertRaises(TypeError): self.cu.executemany("insert into test(income) values (?)", 42) + @subTests("params_class", (ParamsCxCloseInIterMany, ParamsCxCloseInNext)) + def test_executemany_use_after_close(self, params_class): + # Prevent SIGSEGV with iterable of parameters closing the connection. + # Regression test for https://github.com/python/cpython/issues/143198. + cx = sqlite.connect(":memory:") + cx.execute("create table tmp(a number)") + self.addCleanup(cx.close) + cu = cx.cursor() + msg = r"Cannot operate on a closed database\." + with self.assertRaisesRegex(sqlite.ProgrammingError, msg): + cu.executemany("insert into tmp(a) values (?)", params_class(cx)) + + @subTests(("j", "n"), ([0, 1], [0, 3], [1, 3], [2, 3])) + @subTests("wtype", (list, lambda x: x)) + def test_executemany_use_after_close_with_bind_parameters(self, j, n, wtype): + # Prevent SIGSEGV when closing the connection while binding parameters. + # + # Internally, the connection's state is checked after bind_parameters(). + # Without this check, we would only be aware of the closed connection + # by calling an sqlite3 function afterwards. However, it is important + # that we report the error before leaving executemany() call. + # + # Regression test for https://github.com/python/cpython/issues/143198. + + cx = sqlite.connect(":memory:") + cx.execute("create table tmp(a number)") + self.addCleanup(cx.close) + + class PT: + def __init__(self, value): + self.value = value + def __getitem__(self, i): + if self.value == j: + cx.close() + return self.value + def __len__(self): + return 1 + + cu = cx.cursor() + msg = r"Cannot operate on a closed database\." + items = iter(wtype(map(PT, range(n)))) + with self.assertRaisesRegex(sqlite.ProgrammingError, msg): + cu.executemany("insert into tmp(a) values (?)", items) + def test_fetch_iter(self): # Optional DB-API extension. self.cu.execute("delete from test") @@ -1711,6 +1795,24 @@ def test_connection_execute(self): result = self.con.execute("select 5").fetchone()[0] self.assertEqual(result, 5, "Basic test of Connection.execute") + def test_connection_execute_use_after_close_with_bind_parameters(self): + # See CursorTests.test_execute_use_after_close_with_bind_parameters(). + + cx = sqlite.connect(":memory:") + cx.execute("create table tmp(a number)") + self.addCleanup(cx.close) + + class PT: + def __getitem__(self, i): + cx.close() + return 1 + def __len__(self): + return 1 + + msg = r"Cannot operate on a closed database\." + with self.assertRaisesRegex(sqlite.ProgrammingError, msg): + cx.execute("insert into tmp(a) values (?)", PT()) + def test_connection_executemany(self): con = self.con con.execute("create table test(foo)") @@ -1719,6 +1821,43 @@ def test_connection_executemany(self): self.assertEqual(result[0][0], 3, "Basic test of Connection.executemany") self.assertEqual(result[1][0], 4, "Basic test of Connection.executemany") + @subTests("params_class", (ParamsCxCloseInIterMany, ParamsCxCloseInNext)) + def test_connection_executemany_use_after_close(self, params_class): + # Prevent SIGSEGV with iterable of parameters closing the connection. + # Regression test for https://github.com/python/cpython/issues/143198. + cx = sqlite.connect(":memory:") + cx.execute("create table tmp(a number)") + self.addCleanup(cx.close) + msg = r"Cannot operate on a closed database\." + with self.assertRaisesRegex(sqlite.ProgrammingError, msg): + cx.executemany("insert into tmp(a) values (?)", params_class(cx)) + + @subTests(("j", "n"), ([0, 1], [0, 3], [1, 3], [2, 3])) + @subTests("wtype", (list, lambda x: x)) + def test_connection_executemany_use_after_close_with_bind_parameters( + self, j, n, wtype, + ): + # See CursorTests.test_executemany_use_after_close_with_bind_parameters(). + + cx = sqlite.connect(":memory:") + cx.execute("create table tmp(a number)") + self.addCleanup(cx.close) + + class PT: + def __init__(self, value): + self.value = value + def __getitem__(self, i): + if self.value == j: + cx.close() + return self.value + def __len__(self): + return 1 + + items = iter(wtype(map(PT, range(n)))) + msg = r"Cannot operate on a closed database\." + with self.assertRaisesRegex(sqlite.ProgrammingError, msg): + cx.executemany("insert into tmp(a) values (?)", items) + def test_connection_executescript(self): con = self.con con.executescript(""" diff --git a/Misc/NEWS.d/next/Library/2025-12-27-10-36-18.gh-issue-143198.DdIHyC.rst b/Misc/NEWS.d/next/Library/2025-12-27-10-36-18.gh-issue-143198.DdIHyC.rst new file mode 100644 index 00000000000000..dd433dbd22104c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-12-27-10-36-18.gh-issue-143198.DdIHyC.rst @@ -0,0 +1,3 @@ +:mod:`sqlite3`: fix crashes in :meth:`Connection.executemany ` +and :meth:`Cursor.executemany ` when iterating over +the query's parameters closes the current connection. Patch by Bénédikt Tran. diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 83ff8e60557c07..9745d92f4e169e 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -149,7 +149,7 @@ static void free_callback_context(callback_context *ctx); static void set_callback_context(callback_context **ctx_pp, callback_context *ctx); static int connection_close(pysqlite_Connection *self); -PyObject *_pysqlite_query_execute(pysqlite_Cursor *, int, PyObject *, PyObject *); +int _pysqlite_query_execute(pysqlite_Cursor *, int, PyObject *, PyObject *); static PyObject * new_statement_cache(pysqlite_Connection *self, pysqlite_state *state, @@ -1853,21 +1853,15 @@ pysqlite_connection_execute_impl(pysqlite_Connection *self, PyObject *sql, PyObject *parameters) /*[clinic end generated code: output=5be05ae01ee17ee4 input=27aa7792681ddba2]*/ { - PyObject* result = 0; - PyObject *cursor = pysqlite_connection_cursor_impl(self, NULL); - if (!cursor) { - goto error; + if (cursor == NULL) { + return NULL; } - - result = _pysqlite_query_execute((pysqlite_Cursor *)cursor, 0, sql, parameters); - if (!result) { - Py_CLEAR(cursor); + int rc = _pysqlite_query_execute((pysqlite_Cursor *)cursor, 0, sql, parameters); + if (rc < 0) { + Py_DECREF(cursor); + return NULL; } - -error: - Py_XDECREF(result); - return cursor; } @@ -1886,21 +1880,15 @@ pysqlite_connection_executemany_impl(pysqlite_Connection *self, PyObject *sql, PyObject *parameters) /*[clinic end generated code: output=776cd2fd20bfe71f input=495be76551d525db]*/ { - PyObject* result = 0; - PyObject *cursor = pysqlite_connection_cursor_impl(self, NULL); - if (!cursor) { - goto error; + if (cursor == NULL) { + return NULL; } - - result = _pysqlite_query_execute((pysqlite_Cursor *)cursor, 1, sql, parameters); - if (!result) { - Py_CLEAR(cursor); + int rc = _pysqlite_query_execute((pysqlite_Cursor *)cursor, 1, sql, parameters); + if (rc < 0) { + Py_DECREF(cursor); + return NULL; } - -error: - Py_XDECREF(result); - return cursor; } diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 4611c9e5e3e437..a487321d54ab56 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -574,14 +574,23 @@ stmt_step(sqlite3_stmt *statement) return rc; } +/* + * Bind the an SQL parameter with a given argument. + * + * The argument must be already be an adapted value. + * + * Return an sqlite3 error code. + */ static int -bind_param(pysqlite_state *state, pysqlite_Statement *self, int pos, - PyObject *parameter) +bind_param(pysqlite_state *state, pysqlite_Connection *conn, + pysqlite_Statement *self, int pos, PyObject *parameter) { int rc = SQLITE_OK; const char *string; Py_ssize_t buflen; parameter_type paramtype; + // Indicate whether 'conn' is safe against conversion's side effects. + bool safe = false; if (parameter == Py_None) { rc = sqlite3_bind_null(self->st, pos); @@ -590,10 +599,13 @@ bind_param(pysqlite_state *state, pysqlite_Statement *self, int pos, if (PyLong_CheckExact(parameter)) { paramtype = TYPE_LONG; + safe = true; } else if (PyFloat_CheckExact(parameter)) { paramtype = TYPE_FLOAT; + safe = true; } else if (PyUnicode_CheckExact(parameter)) { paramtype = TYPE_UNICODE; + safe = true; } else if (PyLong_Check(parameter)) { paramtype = TYPE_LONG; } else if (PyFloat_Check(parameter)) { @@ -604,6 +616,7 @@ bind_param(pysqlite_state *state, pysqlite_Statement *self, int pos, paramtype = TYPE_BUFFER; } else { paramtype = TYPE_UNKNOWN; + safe = true; // there is no conversion } switch (paramtype) { @@ -656,7 +669,7 @@ bind_param(pysqlite_state *state, pysqlite_Statement *self, int pos, } final: - return rc; + return (safe || pysqlite_check_connection(conn)) ? rc : SQLITE_ERROR; } /* returns 0 if the object is one of Python's internal ones that don't need to be adapted */ @@ -675,15 +688,20 @@ need_adapt(pysqlite_state *state, PyObject *obj) } } -static void -bind_parameters(pysqlite_state *state, pysqlite_Statement *self, - PyObject *parameters) +/* + * Bind the SQL parameters for the given values (adapted, if needed). + * + * On error, return -1 with an exception set; otherwise return 0. + */ +static int +bind_parameters(pysqlite_state *state, pysqlite_Connection *conn, + pysqlite_Statement *self, PyObject *parameters) { PyObject* current_param; PyObject* adapted; const char* binding_name; int i; - int rc; + int rc; /* sqlite3 error code */ int num_params_needed; Py_ssize_t num_params; @@ -699,8 +717,11 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, num_params = PyList_GET_SIZE(parameters); } else { num_params = PySequence_Size(parameters); - if (num_params == -1) { - return; + if (num_params == -1 && PyErr_Occurred()) { + return -1; + } + else if (!pysqlite_check_connection(conn)) { + return -1; } } if (num_params != num_params_needed) { @@ -708,7 +729,7 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, "Incorrect number of bindings supplied. The current " "statement uses %d, and there are %zd supplied.", num_params_needed, num_params); - return; + return -1; } for (i = 0; i < num_params; i++) { const char *name = sqlite3_bind_parameter_name(self->st, i+1); @@ -718,7 +739,7 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, "supplied a sequence which requires nameless (qmark) " "placeholders.", i+1, name); - return; + return -1; } if (PyTuple_CheckExact(parameters)) { @@ -729,9 +750,18 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, current_param = Py_XNewRef(item); } else { current_param = PySequence_GetItem(parameters, i); + if (current_param == NULL) { + return -1; + } + else if (!pysqlite_check_connection(conn)) { + Py_DECREF(current_param); + return -1; + } } if (!current_param) { - return; + assert(PyErr_Occurred()); + assert(pysqlite_check_connection(conn)); + return -1; } if (!need_adapt(state, current_param)) { @@ -743,19 +773,26 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, current_param); Py_DECREF(current_param); if (!adapted) { - return; + return -1; + } + else if (!pysqlite_check_connection(conn)) { + Py_DECREF(adapted); + return -1; } } - rc = bind_param(state, self, i + 1, adapted); + rc = bind_param(state, conn, self, i + 1, adapted); Py_DECREF(adapted); + if (!pysqlite_check_connection(conn)) { + return -1; + } if (rc != SQLITE_OK) { PyObject *exc = PyErr_GetRaisedException(); sqlite3 *db = sqlite3_db_handle(self->st); set_error_from_db(state, db); _PyErr_ChainExceptions1(exc); - return; + return -1; } } } else if (PyDict_Check(parameters)) { @@ -768,7 +805,7 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, PyErr_Format(state->ProgrammingError, "Binding %d has no name, but you supplied a " "dictionary (which has only names).", i); - return; + return -1; } binding_name++; /* skip first char (the colon) */ @@ -777,13 +814,17 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, binding_name, ¤t_param); if (found == -1) { - return; + return -1; + } + else if (!pysqlite_check_connection(conn)) { + Py_XDECREF(current_param); + return -1; } else if (found == 0) { PyErr_Format(state->ProgrammingError, "You did not supply a value for binding " "parameter :%s.", binding_name); - return; + return -1; } if (!need_adapt(state, current_param)) { @@ -795,35 +836,49 @@ bind_parameters(pysqlite_state *state, pysqlite_Statement *self, current_param); Py_DECREF(current_param); if (!adapted) { - return; + return -1; + } + else if (!pysqlite_check_connection(conn)) { + Py_DECREF(adapted); + return -1; } } - rc = bind_param(state, self, i, adapted); + rc = bind_param(state, conn, self, i, adapted); Py_DECREF(adapted); + if (!pysqlite_check_connection(conn)) { + return -1; + } if (rc != SQLITE_OK) { PyObject *exc = PyErr_GetRaisedException(); sqlite3 *db = sqlite3_db_handle(self->st); set_error_from_db(state, db); _PyErr_ChainExceptions1(exc); - return; - } + return -1; + } } } else { PyErr_SetString(state->ProgrammingError, "parameters are of unsupported type"); + return -1; } + + return 0; } -PyObject * -_pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation, PyObject* second_argument) -{ - PyObject* parameters_list = NULL; - PyObject* parameters_iter = NULL; - PyObject* parameters = NULL; +/* + * Execute a single prepared statement. + * + * On error, return -1 with an exception set; otherwise return 0. + */ +int +_pysqlite_query_execute(pysqlite_Cursor *self, int multiple, PyObject *operation, PyObject *second_argument) { + PyObject *parameters_list = NULL; + PyObject *parameters_iter = NULL; + PyObject *parameters = NULL; int i; - int rc; + int rc, bind_rc; int numcols; PyObject* column_name; @@ -843,6 +898,9 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation if (!parameters_iter) { goto error; } + else if (!pysqlite_check_connection(self->connection)) { + goto error; + } } } else { parameters_list = PyList_New(0); @@ -920,14 +978,19 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } assert(!sqlite3_stmt_busy(self->statement->st)); - while (1) { - parameters = PyIter_Next(parameters_iter); - if (!parameters) { - break; + while (PyIter_NextItem(parameters_iter, ¶meters)) { + if (parameters == NULL) { + goto error; + } + // PyIter_NextItem() may have a side-effect on the connection's state. + // See: https://github.com/python/cpython/issues/143198. + if (!pysqlite_check_connection(self->connection)) { + goto error; } - bind_parameters(state, self->statement, parameters); - if (PyErr_Occurred()) { + bind_rc = bind_parameters(state, self->connection, self->statement, parameters); + Py_CLEAR(parameters); + if (bind_rc < 0) { goto error; } @@ -942,6 +1005,8 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } } set_error_from_db(state, self->connection->db); + // Here, we may not necessarily have an exception set if we + // do not know how to raise an exception from the error code. goto error; } @@ -955,6 +1020,7 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation Py_BEGIN_ALLOW_THREADS numcols = sqlite3_column_count(self->statement->st); Py_END_ALLOW_THREADS + if (self->description == Py_None && numcols > 0) { Py_SETREF(self->description, PyTuple_New(numcols)); if (!self->description) { @@ -990,7 +1056,6 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation goto reset_failure; } } - Py_XDECREF(parameters); } if (!multiple) { @@ -1024,12 +1089,13 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation } } self->rowcount = -1L; - return NULL; + return -1; } + if (self->statement && !sqlite3_stmt_busy(self->statement->st)) { Py_CLEAR(self->statement); } - return Py_NewRef((PyObject *)self); + return 0; reset_failure: /* suite to execute when stmt_reset() failed and no exception is set */ @@ -1043,7 +1109,7 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation self->rowcount = -1L; Py_CLEAR(self->statement); cursor_cannot_reset_stmt_error(self, 0); - return NULL; + return -1; } /*[clinic input] @@ -1061,7 +1127,8 @@ pysqlite_cursor_execute_impl(pysqlite_Cursor *self, PyObject *sql, PyObject *parameters) /*[clinic end generated code: output=d81b4655c7c0bbad input=a8e0200a11627f94]*/ { - return _pysqlite_query_execute(self, 0, sql, parameters); + int rc = _pysqlite_query_execute(self, 0, sql, parameters); + return rc < 0 ? NULL : Py_NewRef(self); } /*[clinic input] @@ -1079,7 +1146,8 @@ pysqlite_cursor_executemany_impl(pysqlite_Cursor *self, PyObject *sql, PyObject *seq_of_parameters) /*[clinic end generated code: output=2c65a3c4733fb5d8 input=0d0a52e5eb7ccd35]*/ { - return _pysqlite_query_execute(self, 1, sql, seq_of_parameters); + int rc = _pysqlite_query_execute(self, 1, sql, seq_of_parameters); + return rc < 0 ? NULL : Py_NewRef(self); } /*[clinic input]