Code review comment for lp:~rick-fdd/pyopenssl/pkcs12_mod_and_export2

Excellent points! Thanks for the lesson. I pushed all five
fixes to lp:~rick-fdd/pyopenssl/pkcs12_mod_and_export2

--
Rick

On Fri, Jul 24, 2009 at 04:16:38AM -0000, Jean-Paul Calderone wrote:
> I wrote a nicer review, but firefox crashed and ate it. This one's a bit terser, sorry.
>
> 1. crypto_PKCS12_set_ca_certificates should convert cacerts to a tuple first, then iterate over that tuple and check the types of the elements. A broken or weird __iter__ could sneak a non-x509 past the checks as the code is now.
> 2. The return type of crypto_PKCS12_get_friendlyname shouldn't really be crypto_PKeyObj, should it?
> 3. crypto_PKCS12_set_friendlyname should use PyString_CheckExact, to reject str subclasses.
> 4. It's hard to tell for sure, but I *think* PKCS12_create doesn't take over the cacerts stack passed to it. So I think crypto_PKCS12_export is currently leaking a STACK_OF(X509).
> 5. In crypto_PKCS12_New, cacerts is leaked in all the error cases.
>
> --
> https://code.launchpad.net/~rick-fdd/pyopenssl/pkcs12_mod_and_export2/+merge/8962
> You are the owner of lp:~rick-fdd/pyopenssl/pkcs12_mod_and_export2.