Conversation
Co-authored-by: magicbug <84308+magicbug@users.noreply.github.com>
Co-authored-by: magicbug <84308+magicbug@users.noreply.github.com>
|
@copilot The LOTW functionality will all work correct ? |
Yes, LOTW functionality will work correctly. The changes only modify CodeIgniter 3 core framework files ( LOTW code (
The session change removes a deprecated |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Pull Request Overview
This PR adds PHP 8.4 compatibility to CodeIgniter 3's Session library by removing the deprecated ini_set('session.sid_length') call, along with comprehensive documentation of all PHP 8.4 compatibility changes applied to Cloudlog.
- Removes
ini_set('session.sid_length')call to avoid PHP 8.4 deprecation warnings - Adds documentation file summarizing all PHP 8.x compatibility changes (Dynamic Properties, E_STRICT, mbstring, etc.)
- Uses calculated
$sid_lengthvalue internally for session ID validation instead of setting it via INI
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
system/libraries/Session/Session.php |
Removes deprecated ini_set('session.sid_length') call; uses $sid_length variable for validation regex only |
PHP_8.4_COMPATIBILITY_SUMMARY.md |
Comprehensive documentation of all PHP 8.x compatibility changes applied to Cloudlog's CI3 core |
Updated the type hint for the $params argument in encrypt and decrypt methods to ?array, allowing null values. This improves compatibility with optional parameters.
This reverts commit da438b5.
Adds backup authentication cookie to address session issues and enhances logging for debugging session and cookie states. Updates encryption methods to handle null/empty data, refines session validation logic, and modernizes cookie parameter usage for PHP compatibility. Also improves error reporting and session cleanup during logout.
Adds session validation and debug logging to User controller's edit method. Updates User_model authorize() to handle missing users with error logging. Refactors Cache library to declare driver properties and ensure driver loading for PHP 8.2+ compatibility.
Inserted calls to user_model->validate_session() in all controller constructors to ensure user sessions are validated before authorization checks. This improves session handling and security across the application.
Replaces manual setcookie calls with CodeIgniter's delete_cookie helper for 'remember_me' and 'cloudlog_auth' cookies during logout. This ensures proper handling of cookie prefixes and improves consistency with framework practices.
Debug log statements were removed from User controller, User_model, and Session library to reduce log verbosity and improve performance. This cleanup helps focus logs on relevant events and avoids cluttering log files with routine session and authentication details.
magicbug
changed the title
| } catch (Exception $e) { | ||
| log_message('error', 'Failed to decrypt backup auth cookie: ' . $e->getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: Authentication Restoration: The Zero Problem
In the backup auth cookie restoration logic, the condition if ($user_id) will fail if the decrypted user_id is 0, since 0 is falsy in PHP. This prevents users with user_id = 0 (if such exists) or similar edge cases from being restored. The check should verify non-null or non-empty explicitly rather than relying on truthiness.
| 'secure' => $this->_config['cookie_secure'], | ||
| 'httponly' => TRUE, | ||
| 'samesite' => '' | ||
| ] |
There was a problem hiding this comment.
Bug: Cookie samesite Ambiguity Leads to Instability
Setting 'samesite' => '' (empty string) in cookie parameters may not behave as expected across different PHP and browser versions. An empty string for samesite is ambiguous; it should be either removed from the array, set to a valid value like 'Lax', or conditionally handled based on PHP version. This could cause session cookies to behave inconsistently.
Added a session validation check in the User controller's index method. If the session is not valid, users are redirected to the login page before authorization is checked.
Introduces user session validation checks to all controller actions before authorization checks. This ensures users are redirected to the login page if their session is invalid, improving security and consistency across the application.
Cloudlog runs on CodeIgniter 3, which requires patches for PHP 8.4 compatibility. This PR implements the necessary CI3 core updates.
Changes
Dynamic Properties (PHP 8.2+)
#[AllowDynamicProperties]toCI_Modelinsystem/core/Model.phpE_STRICT Deprecation (PHP 8.4)
system/core/Exceptions.php:Session ID Length (PHP 8.4)
ini_set('session.sid_length')calls fromsystem/libraries/Session/Session.php$sid_lengthinternally for validation regex onlymbstring.func_overload (PHP 8.0+)
mbstring.func_overloadin PHP 8.0Application Code
No changes needed to
/applicationfolder - already PHP 8.4 compatible.See
PHP_8.4_COMPATIBILITY_SUMMARY.mdfor complete analysis.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Note
Updates CI3 core for PHP 8.4, strengthens session/auth handling, and enforces session validation across controllers with supporting docs.
system/core/Exceptions.php: GuardE_STRICTwith PHP version check.system/libraries/Session/Session.php: Use array-basedsetcookie()/session_set_cookie_params()withSameSite; stop setting deprecatedsession.sid_length; keep internal SID validation.system/libraries/Encryption.php: PHP 8 type-hint tweaks; null-safety for data/strlen.system/libraries/Cache/Cache.php: Declare driver properties and lazy-load inis_supported()to avoid dynamic properties issues.application/models/User_model.php: Restore session from encrypted backup cookie; stricterauthorize()when user missing; improved logging.application/controllers/User.php: Safer remember-me cookie handling, add encrypted backupcloudlog_authcookie; ensure cookie cleanup on logout; minor logging and session commit.validate_session()(redirect touser/loginwhen missing) beforeauthorize(); simplified auth failures to dashboard redirect.PHP_8.4_COMPATIBILITY_SUMMARY.md: Adds summary of applied changes and testing guidance.Written by Cursor Bugbot for commit 445706e. This will update automatically on new commits. Configure here.