pyOpenSSL

Merge lp://staging/~shanemhansen/pyopenssl/dump_publickey into lp://staging/~exarkun/pyopenssl/trunk

dump_publickey
Merge into trunk

Proposed by Shane Hansen on 2012-09-19

Status:

Needs review

Proposed branch:

lp://staging/~shanemhansen/pyopenssl/dump_publickey

Merge into:

lp://staging/~exarkun/pyopenssl/trunk

Diff against target:

180 lines (+119/-1)

3 files modified

OpenSSL/crypto/crypto.c (+56/-0)
OpenSSL/test/test_crypto.py (+54/-1)
doc/api/crypto.rst (+9/-0)

To merge this branch:

bzr merge lp://staging/~shanemhansen/pyopenssl/dump_publickey

Undecided

Confirmed

Link a bug report

Reviewer	Review Type	Date Requested	Status
Jean-Paul Calderone		2012-09-19	Needs Fixing on 2012-10-19
Review via email: mp+125370@code.staging.launchpad.net

Description of the change

This change implements the crypto.dump_publickey API. There are tests for ASN1 and PEM formatting.
There seem to be a number of api's for dumping public keys but I was able to match the PEM and ASN1 output with the openssl pkey tool which uses the PEM_write_bio_PUBKEY and i2d_PUBKEY_bio apis.

This is my first attempt at submitting something like this, so let me know on #pyopenssl if there's anything I can add. Thanks!

Revision history for this message

Jean-Paul Calderone (exarkun) wrote on 2012-10-19:

Thanks. Some observations:

  1. The switch in crypto_dump_publickey has a default case that raises an exception with a message talking about FILETYPE_TEXT being a valid input, but there is no FILETYPE_TEXT case in the switch statement.
  2. There's a `print` statement left in test_dump_publickey_valid_filetype
  3. I'd like to see test_dump_publickey_valid_filetype split into two tests, one for FILETYPE_ASN1 and one for FILETYPE_PEM.
  4. There's a bit of trailing whitespace after test_dump_publickey_valid_filetype which should be deleted.
  5. All APIs should be documented in doc/api/ as well.

Otherwise this looks good. Thanks!

review: Needs Fixing

Revision history for this message

Shane Hansen (shanemhansen) wrote on 2012-10-21:

Thanks for the feedback, I've made the requested changes and repushed.
Let me know if there are any steps I've missed.

On Sat, Oct 20, 2012 at 6:25 PM, Jean-Paul Calderone <
<email address hidden>> wrote:

> The proposal to merge lp:~shanemhansen/pyopenssl/dump_publickey into
> lp:pyopenssl has been updated.
>
> Status: Needs review => Work in progress
>
> For more details, see:
>
> https://code.launchpad.net/~shanemhansen/pyopenssl/dump_publickey/+merge/125370
> --
>
> https://code.launchpad.net/~shanemhansen/pyopenssl/dump_publickey/+merge/125370
> You are the owner of lp:~shanemhansen/pyopenssl/dump_publickey.
>

Unmerged revisions

170. By Shane Hansen on 2012-10-21: Remove spurious print statement, incorrect exception statement,
and split up tests for dump_publickey.
169. By Space Monkey on 2012-09-19: Implement crypto.dump_publickey API (with tests)