Skip to content
This repository has been archived by the owner on Jan 18, 2025. It is now read-only.

Casting message to bytes in PyCrypto verifier. #203

Merged
merged 1 commit into from
Jul 13, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jun 24, 2015

Also

  • Removing bare except: statements in both the PyCrypto and
    OpenSSL verifiers (in the verify method).
  • Catching the only exception possible in the OpenSSL verifier
    (it is OpenSSL.crypto.Error).
  • Converting the signature to bytes (if not already) in the
    OpenSSL verifier.
  • Adding a test with both a unicode and bytes signature for
    each verifier.

Fixes #201.

/cc @nathanielmanistaatgoogle

@dhermes
Copy link
Contributor Author

dhermes commented Jun 24, 2015

@nathanielmanistaatgoogle @craigcitro As mentioned above, I removed the bare except: statements the verifiers. It's unclear if they were there intentionally or just done since the exception types were neglected.

@@ -59,7 +59,7 @@ def test_sign_and_verify(self):
self._check_sign_and_verify('privatekey.%s' % self.format)

def test_sign_and_verify_from_converted_pkcs12(self):
"""Tests that following instructions to convert from PKCS12 to PEM works."""

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@craigcitro
Copy link
Contributor

i don't know that there's a reason for the bare except: clauses -- but if there is, we'll probably get a bug report soon enough. 😉

Also
- Removing bare `except:` statements in both the PyCrypto and
  OpenSSL verifiers (in the `verify` method).
- Catching the only exception possible in the OpenSSL verifier
  (it is `OpenSSL.crypto.Error`).
- Converting the signature to bytes (if not already) in the
  OpenSSL verifier.
- Adding a test with both a unicode and bytes signature for
  each verifier.

Fixes googleapis#201.
@dhermes
Copy link
Contributor Author

dhermes commented Jul 2, 2015

@nathanielmanistaatgoogle I have updated the docstrings on the updated verify() methods.

Sorry I let this thing fall away.

Are we good to go here?

@nathanielmanistaatgoogle
Copy link
Contributor

LGTM; I think we're good to go here.

nathanielmanistaatgoogle added a commit that referenced this pull request Jul 13, 2015
Casting message to bytes in PyCrypto verifier.
@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 76b3c40 into googleapis:master Jul 13, 2015
@dhermes
Copy link
Contributor Author

dhermes commented Jul 13, 2015

Thanks!

@dhermes dhermes deleted the fix-201 branch July 13, 2015 20:30
@dhermes dhermes mentioned this pull request Aug 26, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants