Prepare AES Crypto provider to use with update 2.0 and PHP 8.4#15
Prepare AES Crypto provider to use with update 2.0 and PHP 8.4#15
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the library for version 2.0, primarily by making optional parameters nullable in crypto providers and adjusting AES encryption calls to drop additional authenticated data.
- Changed
getFingerPrintand AES constructor signatures to accept nullable inputs. - Removed passing of the
$aadparameter fromopenssl_encrypt/openssl_decrypt. - Bumped overall API for v2.0 compatibility.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/RSACryptoServiceProvider.php | Made publicKey nullable in getFingerPrint method signature. |
| src/AESCryptoServiceProvider.php | Updated constructor to accept nullable $cipher and removed AAD usage in encrypt/decrypt calls. |
Comments suppressed due to low confidence (2)
src/AESCryptoServiceProvider.php:118
- Dropping the
$aadparameter fromopenssl_encryptwill break decryption for any data originally encrypted with additional authenticated data. Either fully remove AAD support and update documentation or reintroduce passing$this->aadhere.
$this->tag
src/AESCryptoServiceProvider.php:153
- Commenting out
$this->aadin the decrypt call may cause verification failures when decrypting data that used AAD. Consider removing the AAD field entirely or ensuring it’s correctly passed toopenssl_decrypt.
//$this->aad
fix paths in phpstan configuration
There was a problem hiding this comment.
Pull Request Overview
This PR updates the project to version 2.0 by refining type hints, enhancing documentation, and updating dependencies and CI workflows.
- Updated RSACryptoServiceProvider to generate a fingerprint with a nullable public key and improved documentation.
- Modified AESCryptoServiceProvider by updating the constructor signature and removing the AAD parameter from encryption/decryption routines.
- Upgraded PHPStan and Composer-run-action dependencies and updated configuration files accordingly.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/RSACryptoServiceProvider.php | Updated documentation and method signature for public key fingerprinting. |
| src/AESCryptoServiceProvider.php | Updated constructor type and removed AAD parameter from OpenSSL calls. |
| phpstan.neon | Updated path configuration and removed an obsolete setting. |
| composer.json | Upgraded PHPStan dependency from v0.12.99 to v2.1.17. |
| .github/workflows/php.yml | Upgraded composer-run-action versions for PHPUnit, CS, and PHPStan checks. |
Comments suppressed due to low confidence (2)
src/RSACryptoServiceProvider.php:120
- Consider using strict equality (===) when checking for null to ensure type-safe comparisons.
if ($publicKey == null) {
src/AESCryptoServiceProvider.php:116
- Verify that the removal of the AAD parameter from the OpenSSL calls in the encrypt method is intentional, ensuring the associated data is no longer required.
$this->tag
There was a problem hiding this comment.
Pull Request Overview
This PR updates the AES and RSA crypto providers for version 2.0 compatibility with PHP 8.4. Key changes include improved docblocks and type declarations in RSACryptoServiceProvider, removal of the AAD parameter from encryption and decryption in AESCryptoServiceProvider, updated PHPStan version and configuration, and upgrading the composer-run-action version in the workflows.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/RSACryptoServiceProvider.php | Updated docblock and method signature to support nullable public key. |
| src/AESCryptoServiceProvider.php | Removed the AAD property and its usage from encryption and decryption methods. |
| phpstan.neon | Modified the source path and maintained PHPStan level settings. |
| composer.json | Upgraded the PHPStan dependency version from "^0.12.99" to "^2.1.17". |
| .github/workflows/php.yml | Updated composer-run-action versions to v8.4 for PHPUnit, PHPC_CS, and PHPStan jobs. |
Comments suppressed due to low confidence (1)
src/RSACryptoServiceProvider.php:118
- [nitpick] Consider renaming getFingerPrint() to getFingerprint() for improved clarity in method naming.
public function getFingerPrint(?string $publicKey = null): string
| $this->iv, | ||
| $this->tag, | ||
| $this->aad | ||
| $this->tag |
There was a problem hiding this comment.
Ensure the removal of the AAD parameter from the openssl_encrypt call is intentional; update documentation and test cases to reflect that no additional authenticated data is used.
|
For now skipping the stan issue. This will be fixed prior release 2.0 |
Update to version 2.0