Various fixes which make pyOpenSSL more likely to work with PyPy

  * Fix a consistent refcounting bug across most modules
  * switch from tp_setattr to tp_setattro
  * Fix a general bug in error handling when setting invalid X509Name attributes

19 files changed, 150 insertions, 18 deletions

diff --git a/OpenSSL/crypto/crl.c b/OpenSSL/crypto/crl.c
index bc76f22..eec5bcb 100644
--- a/OpenSSL/crypto/crl.c
+++ b/OpenSSL/crypto/crl.c
@@ -276,12 +276,15 @@ PyTypeObject crypto_CRL_Type = {
 };
 
 int init_crypto_crl(PyObject *module) {
-       if (PyType_Ready(&crypto_CRL_Type) < 0) {
-                  return 0;
-       }
+    if (PyType_Ready(&crypto_CRL_Type) < 0) {
+        return 0;
+    }
 
-       if (PyModule_AddObject(module, "CRL", (PyObject *)&crypto_CRL_Type) != 0) {
-                  return 0;
-       }
-       return 1;
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_CRL_Type);
+    if (PyModule_AddObject(module, "CRL", (PyObject *)&crypto_CRL_Type) != 0) {
+        return 0;
+    }
+    return 1;
 }
diff --git a/OpenSSL/crypto/crypto.c b/OpenSSL/crypto/crypto.c
index 1e2abc2..8bea656 100644
--- a/OpenSSL/crypto/crypto.c
+++ b/OpenSSL/crypto/crypto.c
@@ -836,13 +836,21 @@ PyOpenSSL_MODINIT(crypto) {
     crypto_API[crypto_PKCS7_New_NUM]     = (void *)crypto_PKCS7_New;
     crypto_API[crypto_NetscapeSPKI_New_NUM]     = (void *)crypto_NetscapeSPKI_New;
     c_api_object = PyCObject_FromVoidPtr((void *)crypto_API, NULL);
-    if (c_api_object != NULL)
+    if (c_api_object != NULL) {
+        /* PyModule_AddObject steals a reference.
+         */
+        Py_INCREF(c_api_object);
         PyModule_AddObject(module, "_C_API", c_api_object);
+    }
 #endif
 
     crypto_Error = PyErr_NewException("OpenSSL.crypto.Error", NULL, NULL);
     if (crypto_Error == NULL)
         goto error;
+
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF(crypto_Error);
     if (PyModule_AddObject(module, "Error", crypto_Error) != 0)
         goto error;
 
diff --git a/OpenSSL/crypto/crypto.h b/OpenSSL/crypto/crypto.h
index 8c803a8..35fbe74 100644
--- a/OpenSSL/crypto/crypto.h
+++ b/OpenSSL/crypto/crypto.h
@@ -110,7 +110,7 @@ extern void **crypto_API;
     PyObject *crypto_dict, *crypto_api_object; \
     crypto_dict = PyModule_GetDict(crypto_module); \
     crypto_api_object = PyDict_GetItemString(crypto_dict, "_C_API"); \
-    if (PyCObject_Check(crypto_api_object)) { \
+    if (crypto_api_object && PyCObject_Check(crypto_api_object)) { \
       crypto_API = (void **)PyCObject_AsVoidPtr(crypto_api_object); \
     } \
   } \
diff --git a/OpenSSL/crypto/netscape_spki.c b/OpenSSL/crypto/netscape_spki.c
index 1792178..9369d50 100644
--- a/OpenSSL/crypto/netscape_spki.c
+++ b/OpenSSL/crypto/netscape_spki.c
@@ -298,10 +298,16 @@ init_crypto_netscape_spki(PyObject *module) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference
+     */
+    Py_INCREF((PyObject *)&crypto_NetscapeSPKI_Type);
     if (PyModule_AddObject(module, "NetscapeSPKI", (PyObject *)&crypto_NetscapeSPKI_Type) != 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference
+     */
+    Py_INCREF((PyObject *)&crypto_NetscapeSPKI_Type);
     if (PyModule_AddObject(module, "NetscapeSPKIType", (PyObject *)&crypto_NetscapeSPKI_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/pkcs12.c b/OpenSSL/crypto/pkcs12.c
index 6f795c5..79047e3 100644
--- a/OpenSSL/crypto/pkcs12.c
+++ b/OpenSSL/crypto/pkcs12.c
@@ -552,10 +552,16 @@ init_crypto_pkcs12(PyObject *module) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_PKCS12_Type);
     if (PyModule_AddObject(module, "PKCS12", (PyObject *)&crypto_PKCS12_Type) != 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_PKCS12_Type);
     if (PyModule_AddObject(module, "PKCS12Type", (PyObject *)&crypto_PKCS12_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/pkcs7.c b/OpenSSL/crypto/pkcs7.c
index a074800..1770f7f 100644
--- a/OpenSSL/crypto/pkcs7.c
+++ b/OpenSSL/crypto/pkcs7.c
@@ -204,6 +204,9 @@ init_crypto_pkcs7(PyObject *module) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_PKCS7_Type);
     if (PyModule_AddObject(module, "PKCS7Type", (PyObject *)&crypto_PKCS7_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/pkey.c b/OpenSSL/crypto/pkey.c
index 938a222..0a13aa3 100644
--- a/OpenSSL/crypto/pkey.c
+++ b/OpenSSL/crypto/pkey.c
@@ -252,10 +252,16 @@ init_crypto_pkey(PyObject *module)
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_PKey_Type);
     if (PyModule_AddObject(module, "PKey", (PyObject *)&crypto_PKey_Type) != 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_PKey_Type);
     if (PyModule_AddObject(module, "PKeyType", (PyObject *)&crypto_PKey_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/revoked.c b/OpenSSL/crypto/revoked.c
index e9b1297..93f9946 100644
--- a/OpenSSL/crypto/revoked.c
+++ b/OpenSSL/crypto/revoked.c
@@ -434,6 +434,9 @@ int init_crypto_revoked(PyObject *module) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_Revoked_Type);
     if (PyModule_AddObject(module, "Revoked", (PyObject *)&crypto_Revoked_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/x509.c b/OpenSSL/crypto/x509.c
index 5d03d2e..a12220b 100644
--- a/OpenSSL/crypto/x509.c
+++ b/OpenSSL/crypto/x509.c
@@ -883,10 +883,14 @@ init_crypto_x509(PyObject *module)
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509_Type);
     if (PyModule_AddObject(module, "X509", (PyObject *)&crypto_X509_Type) != 0) {
         return 0;
     }
 
+    Py_INCREF((PyObject *)&crypto_X509_Type);
     if (PyModule_AddObject(module, "X509Type", (PyObject *)&crypto_X509_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/x509ext.c b/OpenSSL/crypto/x509ext.c
index bdaac7d..adbe084 100644
--- a/OpenSSL/crypto/x509ext.c
+++ b/OpenSSL/crypto/x509ext.c
@@ -316,11 +316,17 @@ init_crypto_x509extension(PyObject *module)
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509Extension_Type);
     if (PyModule_AddObject(module, "X509Extension",
                            (PyObject *)&crypto_X509Extension_Type) != 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509Extension_Type);
     if (PyModule_AddObject(module, "X509ExtensionType",
                            (PyObject *)&crypto_X509Extension_Type) != 0) {
         return 0;
diff --git a/OpenSSL/crypto/x509name.c b/OpenSSL/crypto/x509name.c
index 91f617a..a62c957 100644
--- a/OpenSSL/crypto/x509name.c
+++ b/OpenSSL/crypto/x509name.c
@@ -195,14 +195,30 @@ crypto_X509Name_getattro(crypto_X509NameObj *self, PyObject *nameobj)
  *            value - The value to set
  */
 static int
-crypto_X509Name_setattr(crypto_X509NameObj *self, char *name, PyObject *value)
+crypto_X509Name_setattro(crypto_X509NameObj *self, PyObject *nameobj, PyObject *value)
 {
     int nid;
     int result;
     char *buffer;
+    char *name;
+
+    if (!PyBytes_CheckExact(nameobj) && !PyUnicode_CheckExact(nameobj)) {
+        PyErr_Format(PyExc_TypeError,
+                     "attribute name must be string, not '%.200s'",
+                     Py_TYPE(nameobj)->tp_name);
+        return -1;
+    }
+
+#ifdef PY3
+    name = PyBytes_AsString(PyUnicode_AsASCIIString(nameobj));
+#else
+    name = PyBytes_AsString(nameobj);
+#endif
 
     if ((nid = OBJ_txt2nid(name)) == NID_undef)
     {
+        /* Just like the case in the getattr function */
+        flush_error_queue();
         PyErr_SetString(PyExc_AttributeError, "No such attribute");
         return -1;
     }
@@ -475,7 +491,7 @@ PyTypeObject crypto_X509Name_Type = {
     (destructor)crypto_X509Name_dealloc,
     NULL, /* print */
     NULL, /* getattr */
-    (setattrfunc)crypto_X509Name_setattr,
+    NULL, /* setattr */
     NULL, /* reserved */
     (reprfunc)crypto_X509Name_repr,
     NULL, /* as_number */
@@ -485,7 +501,7 @@ PyTypeObject crypto_X509Name_Type = {
     NULL, /* call */
     NULL, /* str */
     (getattrofunc)crypto_X509Name_getattro, /* getattro */
-    NULL, /* setattro */
+    (setattrofunc)crypto_X509Name_setattro, /* setattro */
     NULL, /* as_buffer */
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */
     crypto_X509Name_doc, /* tp_doc */
@@ -521,10 +537,16 @@ init_crypto_x509name(PyObject *module)
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509Name_Type);
     if (PyModule_AddObject(module, "X509Name", (PyObject *)&crypto_X509Name_Type) != 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509Name_Type);
     if (PyModule_AddObject(module, "X509NameType", (PyObject *)&crypto_X509Name_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/x509req.c b/OpenSSL/crypto/x509req.c
index 4d7467d..a2d1f11 100644
--- a/OpenSSL/crypto/x509req.c
+++ b/OpenSSL/crypto/x509req.c
@@ -413,10 +413,16 @@ init_crypto_x509req(PyObject *module)
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509Req_Type);
     if (PyModule_AddObject(module, "X509Req", (PyObject *)&crypto_X509Req_Type) != 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509Req_Type);
     if (PyModule_AddObject(module, "X509ReqType", (PyObject *)&crypto_X509Req_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/crypto/x509store.c b/OpenSSL/crypto/x509store.c
index c6fa10c..bf22756 100644
--- a/OpenSSL/crypto/x509store.c
+++ b/OpenSSL/crypto/x509store.c
@@ -138,6 +138,9 @@ init_crypto_x509store(PyObject *module)
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&crypto_X509Store_Type);
     if (PyModule_AddObject(module, "X509StoreType", (PyObject *)&crypto_X509Store_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/rand/rand.c b/OpenSSL/rand/rand.c
index bce5e89..8307ac6 100644
--- a/OpenSSL/rand/rand.c
+++ b/OpenSSL/rand/rand.c
@@ -288,6 +288,9 @@ PyOpenSSL_MODINIT(rand) {
         goto error;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF(rand_Error);
     if (PyModule_AddObject(module, "Error", rand_Error) != 0) {
         goto error;
     }
diff --git a/OpenSSL/ssl/connection.c b/OpenSSL/ssl/connection.c
index bef8c2f..5b304b1 100755
--- a/OpenSSL/ssl/connection.c
+++ b/OpenSSL/ssl/connection.c
@@ -1432,10 +1432,16 @@ init_ssl_connection(PyObject *module) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&ssl_Connection_Type);
     if (PyModule_AddObject(module, "Connection", (PyObject *)&ssl_Connection_Type) != 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&ssl_Connection_Type);
     if (PyModule_AddObject(module, "ConnectionType", (PyObject *)&ssl_Connection_Type) != 0) {
         return 0;
     }
diff --git a/OpenSSL/ssl/context.c b/OpenSSL/ssl/context.c
index ea7847f..4edb012 100644
--- a/OpenSSL/ssl/context.c
+++ b/OpenSSL/ssl/context.c
@@ -1310,10 +1310,16 @@ init_ssl_context(PyObject *module) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&ssl_Context_Type);
     if (PyModule_AddObject(module, "Context", (PyObject *)&ssl_Context_Type) < 0) {
         return 0;
     }
 
+    /* PyModule_AddObject steals a reference.
+     */
+    Py_INCREF((PyObject *)&ssl_Context_Type);
     if (PyModule_AddObject(module, "ContextType", (PyObject *)&ssl_Context_Type) < 0) {
         return 0;
     }
diff --git a/OpenSSL/ssl/ssl.c b/OpenSSL/ssl/ssl.c
index 7a827a0..0dd9871 100644
--- a/OpenSSL/ssl/ssl.c
+++ b/OpenSSL/ssl/ssl.c
@@ -139,8 +139,12 @@ PyOpenSSL_MODINIT(SSL) {
     ssl_API[ssl_Context_New_NUM]    = (void *)ssl_Context_New;
     ssl_API[ssl_Connection_New_NUM] = (void *)ssl_Connection_New;
     ssl_api_object = PyCObject_FromVoidPtr((void *)ssl_API, NULL);
-    if (ssl_api_object != NULL)
+    if (ssl_api_object != NULL) {
+        /* PyModule_AddObject steals a reference.
+         */
+        Py_INCREF(ssl_api_object);
         PyModule_AddObject(module, "_C_API", ssl_api_object);
+    }
 #endif
 
     /* Exceptions */
@@ -148,18 +152,24 @@ PyOpenSSL_MODINIT(SSL) {
  * ADD_EXCEPTION(dict,name,base) expands to a correct Exception declaration,
  * inserting OpenSSL.SSL.name into dict, derviving the exception from base.
  */
-#define ADD_EXCEPTION(_name, _base)                                    \
-do {                                                                          \
+#define ADD_EXCEPTION(_name, _base)                                     \
+do {                                                                    \
     ssl_##_name = PyErr_NewException("OpenSSL.SSL."#_name, _base, NULL);\
     if (ssl_##_name == NULL)                                            \
-        goto error;                                                           \
+        goto error;                                                     \
+    /* PyModule_AddObject steals a reference. */                        \
+    Py_INCREF(ssl_##_name);                                             \
     if (PyModule_AddObject(module, #_name, ssl_##_name) != 0)           \
-        goto error;                                                           \
+        goto error;                                                     \
 } while (0)
 
     ssl_Error = PyErr_NewException("OpenSSL.SSL.Error", NULL, NULL);
-    if (ssl_Error == NULL)
+    if (ssl_Error == NULL) {
         goto error;
+    }
+
+    /* PyModule_AddObject steals a reference. */
+    Py_INCREF(ssl_Error);
     if (PyModule_AddObject(module, "Error", ssl_Error) != 0)
         goto error;
 
diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py
index 661ee53..3d17767 100644
--- a/OpenSSL/test/test_crypto.py
+++ b/OpenSSL/test/test_crypto.py
@@ -618,6 +618,33 @@ class X509NameTests(TestCase):
                 name, type(name), X509NameType))
 
 
+    def test_onlyStringAttributes(self):
+        """
+        Attempting to set a non-L{str} attribute name on an L{X509NameType}
+        instance causes L{TypeError} to be raised.
+        """
+        name = self._x509name()
+        # Beyond these cases, you may also think that unicode should be
+        # rejected.  Sorry, you're wrong.  unicode is automatically converted to
+        # str outside of the control of X509Name, so there's no way to reject
+        # it.
+        self.assertRaises(TypeError, setattr, name, None, "hello")
+        self.assertRaises(TypeError, setattr, name, 30, "hello")
+        class evil(str):
+            pass
+        self.assertRaises(TypeError, setattr, name, evil(), "hello")
+
+
+    def test_setInvalidAttribute(self):
+        """
+        Attempting to set any attribute name on an L{X509NameType} instance for
+        which no corresponding NID is defined causes L{AttributeError} to be
+        raised.
+        """
+        name = self._x509name()
+        self.assertRaises(AttributeError, setattr, name, "no such thing", None)
+
+
     def test_attributes(self):
         """
         L{X509NameType} instances have attributes for each standard (?)
diff --git a/OpenSSL/util.h b/OpenSSL/util.h
index 4cef481..e634b01 100644
--- a/OpenSSL/util.h
+++ b/OpenSSL/util.h
@@ -137,4 +137,8 @@ extern PyObject* PyOpenSSL_LongToHex(PyObject *o);
 #define PyOpenSSL_LongToHex(o) PyNumber_ToBase(o, 16)
 #endif
 
+#ifndef Py_TYPE
+#define Py_TYPE(ob)             (((PyObject*)(ob))->ob_type)
+#endif
+
 #endif