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

Move $padding argument to RsaOptions #11

Merged
merged 5 commits into from
Jun 21, 2016
Merged

Move $padding argument to RsaOptions #11

merged 5 commits into from
Jun 21, 2016

Conversation

Moln
Copy link

@Moln Moln commented Dec 25, 2015

Can I commit this?

$rsa = Zend\Crypt\PublicKey\Rsa::factory([
    'private_key' => $privateKey,
    'public_key' => $publicKey,
    'openssl_padding' => OPENSSL_PKCS1_PADDING
]);

$rsa->decrypt($rsa->encrypt('123456'));

//================================

$rsa = Zend\Crypt\PublicKey\Rsa::factory([
    'private_key' => $privateKey,
    'public_key' => $publicKey,
]);

$rsa->decrypt($rsa->encrypt('123456')); //Default padding == OPENSSL_PKCS1_OAEP_PADDING

@@ -314,7 +318,11 @@ public function decrypt(
break;
}

return $key->decrypt($data);
if (null === $this->getOptions()->getPadding()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't repeat the call chain to $this->getOptions()->getPadding(), as it is not guaranteed to return the same values over subsequent calls anyway. Assign it to a variable instead.

* Fix `$opensslPadding` annotation.
* Delete unnecessary calls in `RsaTest`.
@Moln
Copy link
Author

Moln commented Dec 28, 2015

Sorry for my carelessness.

@Ocramius Thanks.

@Ocramius
Copy link
Member

@Moln no need to be sorry, that's what the code reviews are for :-)

@weierophinney weierophinney added this to the 3.0.0 milestone Jun 21, 2016
@weierophinney weierophinney self-assigned this Jun 21, 2016
weierophinney added a commit that referenced this pull request Jun 21, 2016
weierophinney added a commit that referenced this pull request Jun 21, 2016
weierophinney added a commit that referenced this pull request Jun 21, 2016
@weierophinney weierophinney merged commit e443c3e into zendframework:master Jun 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants