From a72aaace0c4bd1085d23bb740fbc1183c7a16aba Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Mon, 27 Nov 2017 12:14:04 +0100 Subject: [PATCH] Fix multiple ref leaks when raising TypeError Multiple functions had a reference leak while raising a TypeError with Py_BuildValue(). The CAPI function Py_BuildValue() returns a new reference. PyErr_SetObject() does NOT consume the reference. Instead it increments the reference count of the object again. A new function LDAPerror_TypeError() handles raising TypeError with custom message and object. https://github.com/python-ldap/python-ldap/pull/31/files Closes: #24 Signed-off-by: Christian Heimes --- Modules/LDAPObject.c | 31 +++++++++++-------------------- Modules/errors.c | 24 ++++++++++++++++++++---- Modules/errors.h | 1 + Modules/ldapcontrol.c | 12 ++++-------- 4 files changed, 36 insertions(+), 32 deletions(-) diff --git a/Modules/LDAPObject.c b/Modules/LDAPObject.c index ce0ff52..3e08473 100644 --- a/Modules/LDAPObject.c +++ b/Modules/LDAPObject.c @@ -113,9 +113,7 @@ Tuple_to_LDAPMod( PyObject* tup, int no_op ) Py_ssize_t i, len, nstrs; if (!PyTuple_Check(tup)) { - PyErr_SetObject(PyExc_TypeError, Py_BuildValue("sO", - "expected a tuple", tup)); - return NULL; + return LDAPerror_TypeError("expected a tuple", tup); } if (no_op) { @@ -167,9 +165,7 @@ Tuple_to_LDAPMod( PyObject* tup, int no_op ) if (item == NULL) goto error; if (!PyBytes_Check(item)) { - PyErr_SetObject( PyExc_TypeError, Py_BuildValue( "sO", - "expected a byte string in the list", item)); - Py_DECREF(item); + LDAPerror_TypeError("expected a byte string in the list", item); goto error; } lm->mod_bvalues[i]->bv_len = PyBytes_Size(item); @@ -214,17 +210,13 @@ List_to_LDAPMods( PyObject *list, int no_op ) { PyObject *item; if (!PySequence_Check(list)) { - PyErr_SetObject( PyExc_TypeError, Py_BuildValue("sO", - "expected list of tuples", list )); - return NULL; + return LDAPerror_TypeError("expected list of tuples", list); } len = PySequence_Length(list); if (len < 0) { - PyErr_SetObject( PyExc_TypeError, Py_BuildValue("sO", - "expected list of tuples", list )); - return NULL; + return LDAPerror_TypeError("expected list of tuples", list); } lms = PyMem_NEW(LDAPMod *, len + 1); @@ -274,8 +266,8 @@ attrs_from_List( PyObject *attrlist, char***attrsp, PyObject** seq) { } else if (PyUnicode_Check(attrlist)) { #endif /* caught by John Benninghoff */ - PyErr_SetObject( PyExc_TypeError, Py_BuildValue("sO", - "expected *list* of strings, not a string", attrlist )); + LDAPerror_TypeError( + "expected *list* of strings, not a string", attrlist); goto error; } else { *seq = PySequence_Fast(attrlist, "expected list of strings or None"); @@ -296,16 +288,15 @@ attrs_from_List( PyObject *attrlist, char***attrsp, PyObject** seq) { #if PY_MAJOR_VERSION == 2 /* Encoded by Python to UTF-8 */ if (!PyBytes_Check(item)) { -#else - if (!PyUnicode_Check(item)) { -#endif - PyErr_SetObject(PyExc_TypeError, Py_BuildValue("sO", - "expected string in list", item)); + LDAPerror_TypeError("expected bytes in list", item); goto error; } -#if PY_MAJOR_VERSION == 2 attrs[i] = PyBytes_AsString(item); #else + if (!PyUnicode_Check(item)) { + LDAPerror_TypeError("expected string in list", item); + goto error; + } attrs[i] = PyUnicode_AsUTF8(item); #endif } diff --git a/Modules/errors.c b/Modules/errors.c index e5cb0ee..3c4da78 100644 --- a/Modules/errors.c +++ b/Modules/errors.c @@ -36,11 +36,15 @@ static PyObject* errobjects[ LDAP_ERROR_MAX-LDAP_ERROR_MIN+1 ]; PyObject* LDAPerr(int errnum) { - if (errnum >= LDAP_ERROR_MIN && errnum <= LDAP_ERROR_MAX) + if (errnum >= LDAP_ERROR_MIN && errnum <= LDAP_ERROR_MAX) { PyErr_SetNone(errobjects[errnum+LDAP_ERROR_OFFSET]); - else - PyErr_SetObject(LDAPexception_class, - Py_BuildValue("{s:i}", "errnum", errnum)); + } else { + PyObject *args = Py_BuildValue("{s:i}", "errnum", errnum); + if (args == NULL) + return NULL; + PyErr_SetObject(LDAPexception_class, args); + Py_DECREF(args); + } return NULL; } @@ -123,6 +127,18 @@ LDAPerror( LDAP *l, char *msg ) } } +/* Raise TypeError with custom message and object */ +PyObject* +LDAPerror_TypeError(const char *msg, PyObject *obj) { + PyObject *args = Py_BuildValue("sO", msg, obj); + if (args == NULL) { + return NULL; + } + PyErr_SetObject(PyExc_TypeError, args); + Py_DECREF(args); + return NULL; +} + /* initialisation */ diff --git a/Modules/errors.h b/Modules/errors.h index 32aebff..5bd3576 100644 --- a/Modules/errors.h +++ b/Modules/errors.h @@ -9,6 +9,7 @@ extern PyObject* LDAPexception_class; extern PyObject* LDAPerror( LDAP*, char*msg ); +extern PyObject* LDAPerror_TypeError(const char *, PyObject *); extern void LDAPinit_errors( PyObject* ); PyObject* LDAPerr(int errnum); diff --git a/Modules/ldapcontrol.c b/Modules/ldapcontrol.c index 6ff27c5..7e31f72 100644 --- a/Modules/ldapcontrol.c +++ b/Modules/ldapcontrol.c @@ -72,9 +72,7 @@ Tuple_to_LDAPControl( PyObject* tup ) Py_ssize_t len; if (!PyTuple_Check(tup)) { - PyErr_SetObject(PyExc_TypeError, Py_BuildValue("sO", - "expected a tuple", tup)); - return NULL; + return LDAPerror_TypeError("expected a tuple", tup); } if (!PyArg_ParseTuple( tup, "sbO", &oid, &iscritical, &bytes )) @@ -107,8 +105,7 @@ Tuple_to_LDAPControl( PyObject* tup ) berbytes.bv_val = PyBytes_AsString(bytes); } else { - PyErr_SetObject(PyExc_TypeError, Py_BuildValue("sO", - "expected bytes", bytes)); + LDAPerror_TypeError("expected bytes", bytes); LDAPControl_DEL(lc); return NULL; } @@ -130,9 +127,8 @@ LDAPControls_from_object(PyObject* list, LDAPControl ***controls_ret) PyObject* item; if (!PySequence_Check(list)) { - PyErr_SetObject(PyExc_TypeError, Py_BuildValue("sO", - "expected a list", list)); - return 0; + LDAPerror_TypeError("expected a list", list); + return 0; } len = PySequence_Length(list);