Merge lp:~exarkun/pyopenssl/leaky-nids into lp:~exarkun/pyopenssl/trunk

[Jean-Paul Calderone]

[Jean-Paul Calderone (exarkun)]

Fixes X509Name's getattr to flush any OpenSSL errors it induces. This prevents them from popping up at some random later time when someone checks the error queue.

Also fixes the double-evaluation issue in the flush_error_queue helper which was triggering some gc assertion failures (when using debug build of Python).

[rick_dean (rick-fdd)]

This fixes bug 314814. You're awesome!

We should add a comment near the PyDECREF() of
flush_error_queue() so the bug doesn't return --
not so much for pyOpenSSL, but for other projects
who might copy it.

Latex doc?

[rick_dean (rick-fdd)]

Yes! (See comment above.)

review: Approve

[lp:~exarkun/pyopenssl/leaky-nids]

119. By Jean-Paul Calderone on 2009-07-16

A comment about this local, as recommended by rick

=== modified file 'src/crypto/crypto.c'
--- src/crypto/crypto.c	2009-07-08 16:48:33 +0000
+++ src/crypto/crypto.c	2009-07-16 20:25:19 +0000
@@ -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 }
 };
 

=== modified file 'src/crypto/x509name.c'
--- src/crypto/x509name.c	2009-07-08 16:48:33 +0000
+++ src/crypto/x509name.c	2009-07-16 20:25:19 +0000
@@ -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);
     }
 

=== modified file 'src/util.c'
--- src/util.c	2009-07-08 16:48:33 +0000
+++ src/util.c	2009-07-16 20:25:19 +0000
@@ -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,7 @@
  * Returns:   None
  */
 void
-flush_error_queue(void)
-{
-    Py_DECREF(error_queue_to_list());
+flush_error_queue(void) {
+    PyObject *list = error_queue_to_list();
+    Py_DECREF(list);
 }
-

=== modified file 'test/util.py'
--- test/util.py	2009-07-05 17:05:45 +0000
+++ test/util.py	2009-07-16 20:25:19 +0000
@@ -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):
         """