Use correct Secure Cell API in PHPThemis #601
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's not enough to name the function "with_passphrase" and accept "passphrase" arguments. You also need to call Themis Core API that uses passphrases. PHP 5 calls the correct API but PHP 7 fell prey to a copy-paste mistake.
|
This is also the reason why I don’t like to have two APIs which accept identical arguments, do similar but different things with only tiny difference in naming. |
|
next time better to start to implement new features in php7 and then copy-paste them to php5, because second is a candidate to drop ) |
Make sure this isssue does not happen again. Say, when we port PHPThemis to PHP 7.4 using FFI module.
vixentael
approved these changes
Mar 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey, #594, it's not enough to name the function
with_passphraseand acceptpassphrasearguments. You also need to call Themis Core API that uses passphrases.PHP 5 calls the correct API but PHP 7 fell prey to a copy-paste mistake.
Moral of the story: copy-paste is bad 🤦♂
(Caught by integration tests with Python in #596, they’re good. I wonder if we need PHP 5 ⟺ PHP 7 integration tests now. Since our current tests use whatever PHP is installed in the system.)
Checklist