-
Notifications
You must be signed in to change notification settings - Fork 0
Fix crypto IV handling, SQLite method calls, and exception namespacing #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…od calls, and add imagick extension Co-authored-by: Darknetzz <42413477+Darknetzz@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request addresses code review feedback by fixing critical bugs in cryptography implementation, correcting method resolution in SQLite operations, and ensuring proper exception handling in namespaced context with strict types.
Changes:
- Fixed crypto IV handling by adding
OPENSSL_RAW_DATAflag to prevent mixing base64-encoded ciphertext with raw IV bytes - Corrected unqualified method calls in SQLite.php by adding
$this->prefix - Fixed exception handling to use fully qualified
\Exceptionclass for namespace correctness withstrict_types=1 - Improved code clarity by renaming variables and removing unreachable code
- Added imagick extension to CI workflow for future test coverage
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| PHPUtils/Crypto.php | Adds OPENSSL_RAW_DATA flag to encryption/decryption to properly handle raw bytes instead of base64, fixing data corruption bug |
| PHPUtils/SQLite.php | Fixes unqualified method calls with $this-> prefix, qualifies Exception class with backslash for strict types, removes unreachable code after return statement |
| PHPUtils/Times.php | Renames variables from $now/$then to $currentTime/$inputTime for better semantic clarity |
| .github/workflows/php.yml | Adds imagick extension to CI workflow to support Images class functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -63,13 +63,13 @@ public function decryptwithpw(string $str, string $password, string $method = 'a | |||
| if ($iv !== '') { | |||
| $ivRaw = hex2bin($iv); | |||
| $ciphertext = base64_decode($str, true) ?: $str; | |||
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback logic base64_decode($str, true) ?: $str creates API ambiguity when an external IV is provided. The function accepts both base64-encoded and raw ciphertext, but this dual behavior is not documented in the docstring. Consider either: (1) requiring a specific format and documenting it clearly, or (2) adding a parameter to explicitly specify the input format. The current implementation may lead to silent errors if a user mistakenly passes the wrong format.
Addresses code review feedback on cryptography implementation, SQLite method resolution, and exception handling in namespaced context.
Crypto.php
OPENSSL_RAW_DATAflag to encryption/decryption operations. Previous implementation concatenated base64-encoded ciphertext with raw IV bytes, producing invalid data. Now encrypts to raw bytes, concatenates with raw IV, then base64-encodes the combined output.SQLite.php
sqlite_function()(lines 428-439)sqlite_table_exists()→$this->sqlite_table_exists()catch (Exception $e)→catch (\Exception $e)for namespace correctness withstrict_types=1Times.php
$now/$thento$currentTime/$inputTimefor semantic clarityCI
imagickextension to workflow for Images class test coverage💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.