Fix multiple ref leaks when raising TypeError

tiran · encukou · commit a72aaace0c4b · 2017-11-27T15:48:31.000+01:00
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 <cheimes@redhat.com>
diff --git 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 <johnb@netscape.com> */
-        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
@@ -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
@@ -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
@@ -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);
