I'm not sure if I'm the right person for reviewing this, but
here it goes:
1. X509Obj_Sequence_2_Stack:
- `stack` should be freed in the error branch of the for loop.
- `sk_X509_push` can fail in low memory conditions. Its return
value should be checked and appropriate action taken.
2. crypto_pkcs7_sign:
- What happens if the PKey key contains only a public key or
no key at all? It would be nice if this case was tested.
3. X509Stack_2_Sequence:
- Both `X509_dup` and `crypto_X509_New` can fail in low memory
conditions. An appropriate action should be taken in that case.
4. crypto_PKCS7_get0_signers:
- `signer_certs` don't get freed if the list was created without
errors. This looks wrong to me.
- Why are `signer_certs` freed with `sk_X509_pop_free` instead
of the more conventional `sk_X509_free`?
- I don't like the name of this method. Is there a good reason
for 0 in the name, apart from compatibility with the original
OpenSSL name?
5. Documentation:
- Docstrings should describe the required parameters and their
types, preferably in epydoc format.
- Latex documentation needs to be written.
6. Tests:
- TestCase methods should have docstrings explaining what they
are testing.
- It would be good if there were some tests against "known good"
serialized data, either stored as a string in the module or
generated by spawning openssl.
I'm not qualified to say anything about the API itself, since I
find the win32 API intriguing :)
Hello,
I'm not sure if I'm the right person for reviewing this, but
here it goes:
1. X509Obj_ Sequence_ 2_Stack:
- `stack` should be freed in the error branch of the for loop.
- `sk_X509_push` can fail in low memory conditions. Its return
value should be checked and appropriate action taken.
2. crypto_pkcs7_sign:
- What happens if the PKey key contains only a public key or
no key at all? It would be nice if this case was tested.
3. X509Stack_ 2_Sequence:
- Both `X509_dup` and `crypto_X509_New` can fail in low memory
conditions. An appropriate action should be taken in that case.
4. crypto_ PKCS7_get0_ signers:
- `signer_certs` don't get freed if the list was created without
errors. This looks wrong to me.
- Why are `signer_certs` freed with `sk_X509_pop_free` instead
of the more conventional `sk_X509_free`?
- I don't like the name of this method. Is there a good reason
for 0 in the name, apart from compatibility with the original
OpenSSL name?
5. Documentation:
- Docstrings should describe the required parameters and their
types, preferably in epydoc format.
- Latex documentation needs to be written.
6. Tests:
- TestCase methods should have docstrings explaining what they
are testing.
- It would be good if there were some tests against "known good"
serialized data, either stored as a string in the module or
generated by spawning openssl.
I'm not qualified to say anything about the API itself, since I
find the win32 API intriguing :)
Thanks for working on this!
Regards,
Ziga