Skip to content

Commit f37af14

Browse files
committed
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
1 parent f5b3eef commit f37af14

4 files changed

Lines changed: 75 additions & 10 deletions

File tree

Lib/test/test_sqlite3/test_factory.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,35 @@ def test_delete_connection_text_factory(self):
156156
with self.assertRaises(AttributeError):
157157
del self.con.text_factory
158158

159+
def test_uninitialized_connection_factories(self):
160+
# gh-152817: skipping __init__() should still result in initialized factories (None not Null)
161+
con = sqlite.Connection.__new__(sqlite.Connection)
162+
self.assertIsNone(con.row_factory)
163+
self.assertIs(con.text_factory, str)
164+
165+
def test_uninitialized_cursor_row_factory(self):
166+
# gh-152817: skipping __init__() should still result in initialized factories (None not Null)
167+
# __init__ must not crash.
168+
cur = sqlite.Cursor.__new__(sqlite.Cursor)
169+
self.assertIsNone(cur.row_factory)
170+
171+
def test_subclass_skipping_super_init(self):
172+
# gh-152817: forgetting to call super().__init__() shouldn't leave a NULL {row,text}_factory
173+
class Connection(sqlite.Connection):
174+
def __init__(self, *args, **kwargs):
175+
pass
176+
177+
class Cursor(sqlite.Cursor):
178+
def __init__(self, *args, **kwargs):
179+
pass
180+
181+
con = Connection(":memory:")
182+
self.assertIsNone(con.row_factory)
183+
self.assertIs(con.text_factory, str)
184+
185+
cur = Cursor(self.con)
186+
self.assertIsNone(cur.row_factory)
187+
159188
def test_sqlite_row_index_unicode(self):
160189
row = self.con.execute("select 1 as \xff").fetchone()
161190
self.assertEqual(row["\xff"], 1)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:mod:`sqlite3`: Prevent ``Cursor`` or ``Connection`` objects from having
2+
uninitialized ``row_factory`` or ``text_factory`` attributes before
3+
``__init__`` is called.

Modules/_sqlite/connection.c

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
300300
self->thread_ident = PyThread_get_thread_ident();
301301
self->statement_cache = statement_cache;
302302
self->blobs = blobs;
303-
self->row_factory = Py_NewRef(Py_None);
304-
self->text_factory = Py_NewRef(&PyUnicode_Type);
303+
// re-initialize the factory members here, as tp_clear() is called above in some cases
304+
Py_XSETREF(self->row_factory, Py_NewRef(Py_None));
305+
Py_XSETREF(self->text_factory, Py_NewRef((PyObject *)&PyUnicode_Type));
305306
self->trace_ctx = NULL;
306307
self->progress_ctx = NULL;
307308
self->authorizer_ctx = NULL;
@@ -339,6 +340,19 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
339340
return -1;
340341
}
341342

343+
static PyObject *
344+
pysqlite_connection_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
345+
{
346+
pysqlite_Connection *self = (pysqlite_Connection *)type->tp_alloc(type, 0);
347+
if (self == NULL) {
348+
return NULL;
349+
}
350+
// row_factory and text_factory should never be uninitialized, even if tp_init is bypassed.
351+
self->row_factory = Py_NewRef(Py_None);
352+
self->text_factory = Py_NewRef((PyObject *)&PyUnicode_Type);
353+
return (PyObject *)self;
354+
}
355+
342356
/*[clinic input]
343357
# Create a new destination 'connect' for the docstring and methoddef only.
344358
# 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)
549563
return NULL;
550564
}
551565

552-
if (cursor && self->row_factory != Py_None) {
566+
if (cursor && self->row_factory != NULL && !Py_IsNone(self->row_factory)) {
553567
Py_INCREF(self->row_factory);
554568
Py_XSETREF(((pysqlite_Cursor *)cursor)->row_factory, self->row_factory);
555569
}
@@ -561,7 +575,8 @@ static PyObject *
561575
connection_get_row_factory(PyObject *op, void *closure)
562576
{
563577
pysqlite_Connection *self = (pysqlite_Connection *)op;
564-
return Py_NewRef(self->row_factory);
578+
PyObject *row_factory = self->row_factory;
579+
return Py_NewRef(row_factory != NULL ? row_factory : Py_None);
565580
}
566581

567582
static int
@@ -581,7 +596,9 @@ static PyObject *
581596
connection_get_text_factory(PyObject *op, void *closure)
582597
{
583598
pysqlite_Connection *self = (pysqlite_Connection *)op;
584-
return Py_NewRef(self->text_factory);
599+
PyObject *text_factory = self->text_factory;
600+
return Py_NewRef(text_factory != NULL ? text_factory
601+
: (PyObject *)&PyUnicode_Type);
585602
}
586603

587604
static int
@@ -2722,6 +2739,7 @@ static PyType_Slot connection_slots[] = {
27222739
{Py_tp_methods, connection_methods},
27232740
{Py_tp_members, connection_members},
27242741
{Py_tp_getset, connection_getset},
2742+
{Py_tp_new, pysqlite_connection_new},
27252743
{Py_tp_init, pysqlite_connection_init},
27262744
{Py_tp_call, pysqlite_connection_call},
27272745
{Py_tp_traverse, connection_traverse},

Modules/_sqlite/cursor.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,18 @@ pysqlite_cursor_init_impl(pysqlite_Cursor *self,
143143
return 0;
144144
}
145145

146+
static PyObject *
147+
pysqlite_cursor_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
148+
{
149+
pysqlite_Cursor *self = (pysqlite_Cursor *)type->tp_alloc(type, 0);
150+
if (self == NULL) {
151+
return NULL;
152+
}
153+
// row_factory should never be uninitialized, even if tp_init is bypassed.
154+
self->row_factory = Py_NewRef(Py_None);
155+
return (PyObject *)self;
156+
}
157+
146158
static inline int
147159
stmt_reset(pysqlite_Statement *self)
148160
{
@@ -413,7 +425,9 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self)
413425
}
414426

415427
nbytes = sqlite3_column_bytes(self->statement->st, i);
416-
if (self->connection->text_factory == (PyObject*)&PyUnicode_Type) {
428+
PyObject *text_factory = self->connection->text_factory;
429+
if (text_factory == NULL ||
430+
text_factory == (PyObject*)&PyUnicode_Type) {
417431
converted = PyUnicode_FromStringAndSize(text, nbytes);
418432
if (!converted && PyErr_ExceptionMatches(PyExc_UnicodeDecodeError)) {
419433
PyErr_Clear();
@@ -434,12 +448,12 @@ _pysqlite_fetch_one_row(pysqlite_Cursor* self)
434448
Py_DECREF(error_msg);
435449
}
436450
}
437-
} else if (self->connection->text_factory == (PyObject*)&PyBytes_Type) {
451+
} else if (text_factory == (PyObject*)&PyBytes_Type) {
438452
converted = PyBytes_FromStringAndSize(text, nbytes);
439-
} else if (self->connection->text_factory == (PyObject*)&PyByteArray_Type) {
453+
} else if (text_factory == (PyObject*)&PyByteArray_Type) {
440454
converted = PyByteArray_FromStringAndSize(text, nbytes);
441455
} else {
442-
converted = PyObject_CallFunction(self->connection->text_factory, "y#", text, nbytes);
456+
converted = PyObject_CallFunction(text_factory, "y#", text, nbytes);
443457
}
444458
} else {
445459
/* coltype == SQLITE_BLOB */
@@ -1176,7 +1190,7 @@ pysqlite_cursor_iternext(PyObject *op)
11761190
}
11771191
return NULL;
11781192
}
1179-
if (!Py_IsNone(self->row_factory)) {
1193+
if (self->row_factory != NULL && !Py_IsNone(self->row_factory)) {
11801194
PyObject *factory = self->row_factory;
11811195
PyObject *args[] = { op, row, };
11821196
PyObject *new_row = PyObject_Vectorcall(factory, args, 2, NULL);
@@ -1421,6 +1435,7 @@ static PyType_Slot cursor_slots[] = {
14211435
{Py_tp_methods, cursor_methods},
14221436
{Py_tp_members, cursor_members},
14231437
{Py_tp_getset, cursor_getsets},
1438+
{Py_tp_new, pysqlite_cursor_new},
14241439
{Py_tp_init, pysqlite_cursor_init},
14251440
{Py_tp_traverse, cursor_traverse},
14261441
{Py_tp_clear, cursor_clear},

0 commit comments

Comments
 (0)