Fix X509Name.__getattr__ and flush_error_queue
diff --git a/ChangeLog b/ChangeLog
index 8f83fa2..98ae8ff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2009-07-16 Jean-Paul Calderone <exarkun@twistedmatrix.com>
+
+ * test/util.py: Changed the base TestCase's tearDown to assert that
+ no errors were left in the OpenSSL error queue by the test.
+ * src/crypto/crypto.c: Add a private helper in support of the
+ TestCase.tearDown change.
+ * src/crypto/x509name.c: Changed X509Name's getattr implementation
+ to clean up the error queue. Fixes LP#314814.
+ * test/util.c: Changed flush_error_queue to avoid a reference
+ counting bug caused by macro expansion.
+
2009-07-16 Rick Dean <rick@fdd.com>
* src/rand.c: Added OpenSSL.rand.bytes to get random bytes directly.
diff --git a/src/crypto/crypto.c b/src/crypto/crypto.c
index 7a4228d..21501c1 100644
--- a/src/crypto/crypto.c
+++ b/src/crypto/crypto.c
@@ -2,7 +2,7 @@
* crypto.c
*
* Copyright (C) AB Strakt 2001, All rights reserved
- * Copyright (C) Jean-Paul Calderone 2008, All rights reserved
+ * Copyright (C) Jean-Paul Calderone 2008-2009, All rights reserved
*
* Main file of crypto sub module.
* See the file RATIONALE for a short explanation of why this module was written.
@@ -535,6 +535,17 @@
return PyString_FromString(str);
}
+static char crypto_exception_from_error_queue_doc[] = "\n\
+Raise an exception from the current OpenSSL error queue.\n\
+";
+
+static PyObject *
+crypto_exception_from_error_queue(PyObject *spam, PyObject *eggs) {
+ exception_from_error_queue(crypto_Error);
+ return NULL;
+}
+
+
/* Methods in the OpenSSL.crypto module (i.e. none) */
static PyMethodDef crypto_methods[] = {
/* Module functions */
@@ -547,6 +558,7 @@
{ "load_pkcs7_data", (PyCFunction)crypto_load_pkcs7_data, METH_VARARGS, crypto_load_pkcs7_data_doc },
{ "load_pkcs12", (PyCFunction)crypto_load_pkcs12, METH_VARARGS, crypto_load_pkcs12_doc },
{ "X509_verify_cert_error_string", (PyCFunction)crypto_X509_verify_cert_error_string, METH_VARARGS, crypto_X509_verify_cert_error_string_doc },
+ { "_exception_from_error_queue", (PyCFunction)crypto_exception_from_error_queue, METH_NOARGS, crypto_exception_from_error_queue_doc },
{ NULL, NULL }
};
diff --git a/src/crypto/x509name.c b/src/crypto/x509name.c
index 6d08470..39fdcf8 100644
--- a/src/crypto/x509name.c
+++ b/src/crypto/x509name.c
@@ -2,7 +2,7 @@
* x509name.c
*
* Copyright (C) AB Strakt 2001, All rights reserved
- * Copyright (C) Jean-Paul Calderone 2008, All rights reserved
+ * Copyright (C) Jean-Paul Calderone 2008-2009, All rights reserved
*
* X.509 Name handling, mostly thin wrapping.
* See the file RATIONALE for a short explanation of why this module was written.
@@ -153,8 +153,15 @@
int nid, len;
char *utf8string;
- if ((nid = OBJ_txt2nid(name)) == NID_undef)
- {
+ if ((nid = OBJ_txt2nid(name)) == NID_undef) {
+ /*
+ * This is a bit weird. OBJ_txt2nid indicated failure, but it seems
+ * a lower level function, a2d_ASN1_OBJECT, also feels the need to
+ * push something onto the error queue. If we don't clean that up
+ * now, someone else will bump into it later and be quite confused.
+ * See lp#314814.
+ */
+ flush_error_queue();
return Py_FindMethod(crypto_X509Name_methods, (PyObject *)self, name);
}
diff --git a/src/util.c b/src/util.c
index b1faad5..ae6ee5e 100644
--- a/src/util.c
+++ b/src/util.c
@@ -2,6 +2,7 @@
* util.c
*
* Copyright (C) AB Strakt 2001, All rights reserved
+ * Copyright (C) Jean-Paul Calderone 2009, All rights reserved
*
* Utility functions.
* See the file RATIONALE for a short explanation of why this module was written.
@@ -19,15 +20,13 @@
* Returns: A list of errors (new reference)
*/
PyObject *
-error_queue_to_list(void)
-{
+error_queue_to_list(void) {
PyObject *errlist, *tuple;
long err;
errlist = PyList_New(0);
- while ((err = ERR_get_error()) != 0)
- {
+ while ((err = ERR_get_error()) != 0) {
tuple = Py_BuildValue("(sss)", ERR_lib_error_string(err),
ERR_func_error_string(err),
ERR_reason_error_string(err));
@@ -38,8 +37,7 @@
return errlist;
}
-void exception_from_error_queue(PyObject *the_Error)
-{
+void exception_from_error_queue(PyObject *the_Error) {
PyObject *errlist = error_queue_to_list();
PyErr_SetObject(the_Error, errlist);
Py_DECREF(errlist);
@@ -52,8 +50,12 @@
* Returns: None
*/
void
-flush_error_queue(void)
-{
- Py_DECREF(error_queue_to_list());
+flush_error_queue(void) {
+ /*
+ * Make sure to save the errors to a local. Py_DECREF might expand such
+ * that it evaluates its argument more than once, which would lead to
+ * very nasty things if we just invoked it with error_queue_to_list().
+ */
+ PyObject *list = error_queue_to_list();
+ Py_DECREF(list);
}
-
diff --git a/test/util.py b/test/util.py
index fdc9348..d3572db 100644
--- a/test/util.py
+++ b/test/util.py
@@ -12,6 +12,8 @@
from tempfile import mktemp
from unittest import TestCase
+from OpenSSL.crypto import Error, _exception_from_error_queue
+
class TestCase(TestCase):
"""
@@ -30,7 +32,12 @@
shutil.rmtree(temp)
elif os.path.exists(temp):
os.unlink(temp)
-
+ try:
+ _exception_from_error_queue()
+ except Error, e:
+ if e.args != ([],):
+ self.fail("Left over errors in OpenSSL error queue: " + repr(e))
+
def failUnlessIdentical(self, first, second, msg=None):
"""