Skip to content

Commit

Permalink
Fix for CONPY-118: Leak when using text protocol
Browse files Browse the repository at this point in the history 
Removed statement allocation from the cursor init function and
allocate it only in executemany and execute if paraemters were
supplied.
  • Loading branch information
@9EOR9
9EOR9 committed Oct 2, 2020
1 parent 08673bb commit bce98d7
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 36 deletions.
5 changes: 4 additions & 1 deletion include/mariadb_python.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ typedef CRITICAL_SECTION pthread_mutex_t;
#include <limits.h>
#endif /* defined(_WIN32) */

#define CHECK_TYPE(obj, type) \
(Py_TYPE((obj)) == type || PyType_IsSubtype(Py_TYPE((obj)), type))

#ifndef MIN
#define MIN(a,b) (a) < (b) ? (a) : (b)
#endif
Expand Down Expand Up @@ -491,7 +494,7 @@ if ((obj)->thread_state)\
}

#define MARIADB_CHECK_STMT(cursor)\
if (!cursor->stmt || !cursor->stmt->mysql || cursor->is_closed)\
if (!cursor->connection->mysql || cursor->is_closed)\
{\
(cursor)->is_closed= 1;\
mariadb_throw_exception(cursor->stmt, Mariadb_ProgrammingError, 1,\
Expand Down
10 changes: 5 additions & 5 deletions mariadb/mariadb_connection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ MrdbConnection_Initialize(MrdbConnection *self,
if (autocommit_obj)
{
if (autocommit_obj != Py_None &&
Py_TYPE(autocommit_obj) != &PyBool_Type)
!CHECK_TYPE(autocommit_obj, &PyBool_Type))
{
PyErr_SetString(PyExc_TypeError, "Argument must be boolean or None");
return -1;
Expand Down Expand Up @@ -448,7 +448,7 @@ MrdbConnection_Initialize(MrdbConnection *self,
}

if (!autocommit_obj ||
(autocommit_obj && Py_TYPE(autocommit_obj) == &PyBool_Type))
(autocommit_obj && CHECK_TYPE(autocommit_obj, &PyBool_Type)))
{
uint8_t autocommit= (autocommit_obj) ?
(uint8_t) PyLong_AsUnsignedLong(autocommit_obj) : 0;
Expand Down Expand Up @@ -1142,7 +1142,7 @@ static int MrdbConnection_setreconnect(MrdbConnection *self,
return 0;
}

if (!args || Py_TYPE(args) != &PyBool_Type) {
if (!args || !CHECK_TYPE(args, &PyBool_Type)) {
PyErr_SetString(PyExc_TypeError, "Argument must be boolean");
return -1;
}
Expand Down Expand Up @@ -1176,7 +1176,7 @@ static int MrdbConnection_setdb(MrdbConnection *self, PyObject *db,

MARIADB_CHECK_CONNECTION(self, -1);

if (!db || Py_TYPE(db) != &PyUnicode_Type)
if (!db || !CHECK_TYPE(db, &PyUnicode_Type))
{
PyErr_SetString(PyExc_TypeError, "Argument must be string");
return -1;
Expand Down Expand Up @@ -1321,7 +1321,7 @@ static int MrdbConnection_setautocommit(MrdbConnection *self, PyObject *arg,

MARIADB_CHECK_CONNECTION(self, -1);

if (!arg || Py_TYPE(arg) != &PyBool_Type)
if (!arg || !CHECK_TYPE(arg, &PyBool_Type))
{
PyErr_SetString(PyExc_TypeError, "Argument must be boolean");
return -1;
Expand Down
68 changes: 38 additions & 30 deletions mariadb/mariadb_cursor.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,30 +280,12 @@ static int MrdbCursor_initialize(MrdbCursor *self, PyObject *args,

self->is_prepared= is_prepared;
self->is_text= 0;

if (!(self->stmt= mysql_stmt_init(self->connection->mysql)))
{
mariadb_throw_exception(self->connection->mysql, NULL, 0, NULL);
return -1;
}
self->stmt= NULL;

self->cursor_type= cursor_type;
self->prefetch_rows= prefetch_rows;
self->row_array_size= 1;

if (self->cursor_type || self->prefetch_rows)
{
if (!(self->stmt = mysql_stmt_init(self->connection->mysql)))
{
mariadb_throw_exception(self->connection->mysql, Mariadb_OperationalError, 0, NULL);
return -1;
}
}
else
return 0;

mysql_stmt_attr_set(self->stmt, STMT_ATTR_CURSOR_TYPE, &self->cursor_type);
mysql_stmt_attr_set(self->stmt, STMT_ATTR_PREFETCH_ROWS, &self->prefetch_rows);
return 0;
}
/* }}} */
Expand Down Expand Up @@ -488,7 +470,7 @@ static void ma_set_result_column_value(MrdbCursor *self, PyObject *row, uint32_t
closes the statement handle of current cursor. After call to
cursor_close the cursor can't be reused anymore
*/
static
static
void ma_cursor_close(MrdbCursor *self)
{
if (!self->is_closed)
Expand All @@ -503,6 +485,10 @@ void ma_cursor_close(MrdbCursor *self)
self->stmt= NULL;
}
MrdbCursor_clear(self, 0);

if (self->is_text && self->stmt)
mysql_stmt_close(self->stmt);

if (self->parser)
{
MrdbParser_end(self->parser);
Expand Down Expand Up @@ -610,13 +596,13 @@ static Py_ssize_t data_count(PyObject *data)
if (!data)
return 0;

if (Py_TYPE(data) == &PyTuple_Type)
if (CHECK_TYPE(data, &PyTuple_Type))
{
return PyTuple_Size(data);
} else if (Py_TYPE(data) == &PyList_Type)
} else if (CHECK_TYPE(data, &PyList_Type))
{
return PyList_Size(data);
} else if (Py_TYPE(data) == &PyDict_Type)
} else if (CHECK_TYPE(data, &PyDict_Type))
{
return PyDict_Size(data);
}
Expand Down Expand Up @@ -704,6 +690,15 @@ PyObject *MrdbCursor_execute(MrdbCursor *self,
{
uint8_t do_prepare= 1;

if (!self->stmt)
{
if (!(self->stmt= mysql_stmt_init(self->connection->mysql)))
{
mariadb_throw_exception(self->connection->mysql, NULL, 0, NULL);
goto error;
}
}

if ((self->is_prepared && self->statement))
do_prepare= 0;

Expand Down Expand Up @@ -736,13 +731,14 @@ PyObject *MrdbCursor_execute(MrdbCursor *self,
{
if (self->parser->paramstyle == PYFORMAT)
{
if (Py_TYPE(Data) != &PyDict_Type)
if (!CHECK_TYPE(Data, &PyDict_Type))
{
PyErr_SetString(PyExc_TypeError, "argument 2 must be dict");
goto error;
}
}
else if (Py_TYPE(Data) != &PyTuple_Type && Py_TYPE(Data) != &PyList_Type)
else if (!CHECK_TYPE(Data, &PyTuple_Type) &&
!CHECK_TYPE(Data, &PyList_Type))
{
PyErr_SetString(PyExc_TypeError, "Argument 2 must be Tuple or List!");
goto error;
Expand All @@ -760,6 +756,8 @@ PyObject *MrdbCursor_execute(MrdbCursor *self,
}
if (do_prepare)
{
mysql_stmt_attr_set(self->stmt, STMT_ATTR_CURSOR_TYPE, &self->cursor_type);
mysql_stmt_attr_set(self->stmt, STMT_ATTR_PREFETCH_ROWS, &self->prefetch_rows);
mysql_stmt_attr_set(self->stmt, STMT_ATTR_PREBIND_PARAMS, &self->param_count);
mysql_stmt_attr_set(self->stmt, STMT_ATTR_CB_USER_DATA, (void *)self);
mysql_stmt_bind_param(self->stmt, self->params);
Expand Down Expand Up @@ -1306,6 +1304,7 @@ MrdbCursor_executemany(MrdbCursor *self,
int rc;
uint8_t do_prepare= 1;
char errmsg[128];

MARIADB_CHECK_STMT(self);

if (PyErr_Occurred())
Expand Down Expand Up @@ -1338,6 +1337,16 @@ MrdbCursor_executemany(MrdbCursor *self,
MrdbParser_end(self->parser);
self->parser= NULL;
}

if (!self->stmt)
{
if (!(self->stmt= mysql_stmt_init(self->connection->mysql)))
{
mariadb_throw_exception(self->connection->mysql, NULL, 0, NULL);
goto error;
}
}

self->is_text= 0;

if (!self->parser)
Expand Down Expand Up @@ -1501,7 +1510,7 @@ MrdbCursor_getbuffered(MrdbCursor *self)
static int
MrdbCursor_setbuffered(MrdbCursor *self, PyObject *arg)
{
if (!arg || Py_TYPE(arg) != &PyBool_Type)
if (!arg || !CHECK_TYPE(arg, &PyBool_Type))
{
PyErr_SetString(PyExc_TypeError, "Argument must be boolean");
return -1;
Expand Down Expand Up @@ -1550,7 +1559,7 @@ MrdbCursor_iternext(PyObject *self)
static PyObject
*MrdbCursor_closed(MrdbCursor *self)
{
if (self->is_closed || !self->stmt || self->stmt->mysql == NULL)
if (self->is_closed || self->connection->mysql == NULL)
Py_RETURN_TRUE;
Py_RETURN_FALSE;
}
Expand Down Expand Up @@ -1585,12 +1594,11 @@ MrdbCursor_callproc(MrdbCursor *self, PyObject *args)

MARIADB_CHECK_STMT(((MrdbCursor *)self));

if (!PyArg_ParseTuple(args, "s#|O!", &sp, &sp_len,
&PyTuple_Type, &data))
if (!PyArg_ParseTuple(args, "s#|O", &sp, &sp_len, &data))
return NULL;

if (data)
param_count= (uint32_t)PyTuple_Size(data);
param_count= (uint32_t)data_count(data);

stmt_len= sp_len + 5 + 3 + param_count * 2 + 1;
if (!(stmt= (char *)PyMem_RawCalloc(1, stmt_len)))
Expand Down

0 comments on commit bce98d7

Please sign in to comment.