From f37af14b0e29efa828a95c414a2e0a1f12ace8b9 Mon Sep 17 00:00:00 2001 From: Steve Stagg Date: Fri, 3 Jul 2026 13:03:26 +0100 Subject: [PATCH] gh-152954 - Add tp_new to sqlite3 Connection and Cursor objects to initialize row_factory and text_factory before python code can access it (causes crash when NULL), also additional defensive checks, because the cost/performance here is tiny and it's safer to check --- Lib/test/test_sqlite3/test_factory.py | 29 +++++++++++++++++++ ...-07-03-13-02-11.gh-issue-152954.1wY65D.rst | 3 ++ Modules/_sqlite/connection.c | 28 ++++++++++++++---- Modules/_sqlite/cursor.c | 25 ++++++++++++---- 4 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-07-03-13-02-11.gh-issue-152954.1wY65D.rst diff --git a/Lib/test/test_sqlite3/test_factory.py b/Lib/test/test_sqlite3/test_factory.py index a9abeab31936880..b386b76cb7dd199 100644 --- a/Lib/test/test_sqlite3/test_factory.py +++ b/Lib/test/test_sqlite3/test_factory.py @@ -156,6 +156,35 @@ def test_delete_connection_text_factory(self): with self.assertRaises(AttributeError): del self.con.text_factory + def test_uninitialized_connection_factories(self): + # gh-152817: skipping __init__() should still result in initialized factories (None not Null) + con = sqlite.Connection.__new__(sqlite.Connection) + self.assertIsNone(con.row_factory) + self.assertIs(con.text_factory, str) + + def test_uninitialized_cursor_row_factory(self): + # gh-152817: skipping __init__() should still result in initialized factories (None not Null) + # __init__ must not crash. + cur = sqlite.Cursor.__new__(sqlite.Cursor) + self.assertIsNone(cur.row_factory) + + def test_subclass_skipping_super_init(self): + # gh-152817: forgetting to call super().__init__() shouldn't leave a NULL {row,text}_factory + class Connection(sqlite.Connection): + def __init__(self, *args, **kwargs): + pass + + class Cursor(sqlite.Cursor): + def __init__(self, *args, **kwargs): + pass + + con = Connection(":memory:") + self.assertIsNone(con.row_factory) + self.assertIs(con.text_factory, str) + + cur = Cursor(self.con) + self.assertIsNone(cur.row_factory) + def test_sqlite_row_index_unicode(self): row = self.con.execute("select 1 as \xff").fetchone() self.assertEqual(row["\xff"], 1) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-03-13-02-11.gh-issue-152954.1wY65D.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-03-13-02-11.gh-issue-152954.1wY65D.rst new file mode 100644 index 000000000000000..450d277f2be58d6 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-03-13-02-11.gh-issue-152954.1wY65D.rst @@ -0,0 +1,3 @@ +:mod:`sqlite3`: Prevent ``Cursor`` or ``Connection`` objects from having +uninitialized ``row_factory`` or ``text_factory`` attributes before +``__init__`` is called. diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 892740b05e55c98..476e88103d203af 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -300,8 +300,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database, self->thread_ident = PyThread_get_thread_ident(); self->statement_cache = statement_cache; self->blobs = blobs; - self->row_factory = Py_NewRef(Py_None); - self->text_factory = Py_NewRef(&PyUnicode_Type); + // re-initialize the factory members here, as tp_clear() is called above in some cases + Py_XSETREF(self->row_factory, Py_NewRef(Py_None)); + Py_XSETREF(self->text_factory, Py_NewRef((PyObject *)&PyUnicode_Type)); self->trace_ctx = NULL; self->progress_ctx = NULL; self->authorizer_ctx = NULL; @@ -339,6 +340,19 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database, return -1; } +static PyObject * +pysqlite_connection_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +{ + pysqlite_Connection *self = (pysqlite_Connection *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; + } + // row_factory and text_factory should never be uninitialized, even if tp_init is bypassed. + self->row_factory = Py_NewRef(Py_None); + self->text_factory = Py_NewRef((PyObject *)&PyUnicode_Type); + return (PyObject *)self; +} + /*[clinic input] # Create a new destination 'connect' for the docstring and methoddef only. # This makes it possible to keep the signatures for Connection.__init__ and @@ -549,7 +563,7 @@ pysqlite_connection_cursor_impl(pysqlite_Connection *self, PyObject *factory) return NULL; } - if (cursor && self->row_factory != Py_None) { + if (cursor && self->row_factory != NULL && !Py_IsNone(self->row_factory)) { Py_INCREF(self->row_factory); Py_XSETREF(((pysqlite_Cursor *)cursor)->row_factory, self->row_factory); } @@ -561,7 +575,8 @@ static PyObject * connection_get_row_factory(PyObject *op, void *closure) { pysqlite_Connection *self = (pysqlite_Connection *)op; - return Py_NewRef(self->row_factory); + PyObject *row_factory = self->row_factory; + return Py_NewRef(row_factory != NULL ? row_factory : Py_None); } static int @@ -581,7 +596,9 @@ static PyObject * connection_get_text_factory(PyObject *op, void *closure) { pysqlite_Connection *self = (pysqlite_Connection *)op; - return Py_NewRef(self->text_factory); + PyObject *text_factory = self->text_factory; + return Py_NewRef(text_factory != NULL ? text_factory + : (PyObject *)&PyUnicode_Type); } static int @@ -2722,6 +2739,7 @@ static PyType_Slot connection_slots[] = { {Py_tp_methods, connection_methods}, {Py_tp_members, connection_members}, {Py_tp_getset, connection_getset}, + {Py_tp_new, pysqlite_connection_new}, {Py_tp_init, pysqlite_connection_init}, {Py_tp_call, pysqlite_connection_call}, {Py_tp_traverse, connection_traverse}, diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 5a61e43617984d9..bcc4f77a4ce0c11 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -143,6 +143,18 @@ pysqlite_cursor_init_impl(pysqlite_Cursor *self, return 0; } +static PyObject * +pysqlite_cursor_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +{ + pysqlite_Cursor *self = (pysqlite_Cursor *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; + } + // row_factory should never be uninitialized, even if tp_init is bypassed. + self->row_factory = Py_NewRef(Py_None); + return (PyObject *)self; +} + static inline int stmt_reset(pysqlite_Statement *self) { @@ -413,7 +425,9 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self) } nbytes = sqlite3_column_bytes(self->statement->st, i); - if (self->connection->text_factory == (PyObject*)&PyUnicode_Type) { + PyObject *text_factory = self->connection->text_factory; + if (text_factory == NULL || + text_factory == (PyObject*)&PyUnicode_Type) { converted = PyUnicode_FromStringAndSize(text, nbytes); if (!converted && PyErr_ExceptionMatches(PyExc_UnicodeDecodeError)) { PyErr_Clear(); @@ -434,12 +448,12 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self) Py_DECREF(error_msg); } } - } else if (self->connection->text_factory == (PyObject*)&PyBytes_Type) { + } else if (text_factory == (PyObject*)&PyBytes_Type) { converted = PyBytes_FromStringAndSize(text, nbytes); - } else if (self->connection->text_factory == (PyObject*)&PyByteArray_Type) { + } else if (text_factory == (PyObject*)&PyByteArray_Type) { converted = PyByteArray_FromStringAndSize(text, nbytes); } else { - converted = PyObject_CallFunction(self->connection->text_factory, "y#", text, nbytes); + converted = PyObject_CallFunction(text_factory, "y#", text, nbytes); } } else { /* coltype == SQLITE_BLOB */ @@ -1176,7 +1190,7 @@ pysqlite_cursor_iternext(PyObject *op) } return NULL; } - if (!Py_IsNone(self->row_factory)) { + if (self->row_factory != NULL && !Py_IsNone(self->row_factory)) { PyObject *factory = self->row_factory; PyObject *args[] = { op, row, }; PyObject *new_row = PyObject_Vectorcall(factory, args, 2, NULL); @@ -1421,6 +1435,7 @@ static PyType_Slot cursor_slots[] = { {Py_tp_methods, cursor_methods}, {Py_tp_members, cursor_members}, {Py_tp_getset, cursor_getsets}, + {Py_tp_new, pysqlite_cursor_new}, {Py_tp_init, pysqlite_cursor_init}, {Py_tp_traverse, cursor_traverse}, {Py_tp_clear, cursor_clear},