bcit-ci / CodeIgniter

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
18.27k stars 7.6k forks source link

BUG: session_write_close(): Failed to write session data using user defined save handler. Localised with suggested fix #6276

Open gidzr opened 7 months ago

gidzr commented 7 months ago

Hi

ISSUE

I've been getting this ongoing warning / error: "session_write_close(): Failed to write session data using user defined save handler".

This has been intermittent but ongoing, ie. every 5 reloads or so.

TESTING

After checking session_write_close, output buffering, all instances of session_start()/etc I only STUMBLED onto the source of this issue, after updating my error handling function AND turning output buffering off in php.ini to get specific visibility over previously suppressed warnings, which exposed a range of issues in "system\libraries\Session\Session.php".

The issues listed below are mostly related to "ini_set()" with one each for session save handler and session_start after headers sent. NOTE: I am aware that these issues are triggered by the output buffering being off, BUT there is some relationship between these items in the session handler, the errors received when the buffer is off, and the save_handler issues.

That is, after commenting each of these out and turning the output buffering back on (4096), the "session_write_close(): Failed to write session" cleared up.

My concern is that these lines are supposed to be doing something useful and commenting out lines is not a solution, just a method to troubleshoot.

QUESTIONS

  1. Are there any known issues from changes relating to session in php 8.0 / 8.1+ ?
  2. Are there any known latent issues or conflicts for the custom session handling function of CI 3?
  3. As this is an intermittent issue, I suspect it's a race issue which would be impossible for me to properly localise and resolve. Do you have any suggestion for a fix that is more consistent with underlying causes like changes to php in v8?

Potential Fixes ? (big question mark)

Adding a test to the ini_set lines.. something like..

!session_id() ? ini_set('session.name', $params['cookie_name']) : 0;

would this fix be consistent with the code? is ini_set only supposed to be set prior to initiation of session and therefore a session test works well?

or alternately..

ini_get('session.name') ? ini_set('session.name', $params['cookie_name']) : 0;

do I need to ensure a session name has been set irrespective of session state?

or is it both?

!session_id() && ini_get('session.name') ? ini_set('session.name', $params['cookie_name']) : 0;

Please let me know what approach would be the healthiest least downstream unintended consequence fix.

Thanks

SUMMARY OF FINDINGS

  1. these lines were in the following functions: _configure(), __construct(), _configure_sid_length()..
  2. in __construct() there's a test to only run lines if is_php('5.4').. I am currently using php 8.1.17, and wondering if perhaps this test is limited to php 5 to 7, but v8 or v8.1 causes issues?

ERRORS AND CODE

__construct() :: CommonHelper Error Logger (set_error_handler section) ::--> [2] session_set_save_handler(): Session save handler cannot be changed after headers have already been sent on line 110 in file ..\Session.php

      session_set_save_handler($wrapper, TRUE);  //where $wrapper = new CI_SessionWrapper($class);

:: CommonHelper Error Logger (set_error_handler section) ::--> [2] session_start(): Session cannot be started after headers have already been sent on line 138 in file ..\Session.php

  session_start();

_configure() :: CommonHelper Error Logger (set_error_handler section) ::--> [2] ini_set(): Session ini settings cannot be changed after headers have already been sent on line 304 in file ..\Session.php

           ini_set('session.name', $params['cookie_name']);

:: CommonHelper Error Logger (set_error_handler section) ::--> [2] session_set_cookie_params(): Session cookie parameters cannot be changed after headers have already been sent on line 335 in file ..\Session.php

            'samesite' => $params['cookie_samesite']

:: CommonHelper Error Logger (set_error_handler section) ::--> [2] ini_set(): Session ini settings cannot be changed after headers have already been sent on line 356 in file ..\Session.php

          ini_set('session.gc_maxlifetime', $expiration);

:: CommonHelper Error Logger (set_error_handler section) ::--> [2] ini_set(): Session ini settings cannot be changed after headers have already been sent on line 366 in file ..\Session.php :: CommonHelper Error Logger (set_error_handler section) ::--> [2] ini_set(): Session ini settings cannot be changed after headers have already been sent on line 367 in file ..\Session.php :: CommonHelper Error Logger (set_error_handler section) ::--> [2] ini_set(): Session ini settings cannot be changed after headers have already been sent on line 368 in file ..\Session.php :: CommonHelper Error Logger (set_error_handler section) ::--> [2] ini_set(): Session ini settings cannot be changed after headers have already been sent on line 369 in file ..\Session.php // Security is king

  ini_set('session.use_trans_sid', 0);
  ini_set('session.use_strict_mode', 1);
  ini_set('session.use_cookies', 1);
  ini_set('session.use_only_cookies', 1);

:: CommonHelper Error Logger (set_error_handler section) ::--> [2] ini_set(): Session ini settings cannot be changed after headers have already been sent on line 427 in file ..\Session.php

         ini_set('session.sid_length', $sid_length);
gidzr commented 7 months ago

TESTING UPDATE - 100% REPRODUCABLE.

I've managed to replicate the error by doing the following:

  1. Go to ci_sessions table in DB
  2. Delete all session cookies
  3. Reload page.. on the first creation of a new cookie, the error occurs
gidzr commented 7 months ago

RESEARCH OF POSSIBLE CAUSE

per https://stackoverflow.com/questions/34117651/php7-symfony-2-8-failed-to-write-session-data, and https://forum.codeigniter.com/thread-80404.html

Make sure the write method of your session handler returns bool - true on success.

The php session_set_save_handler documentation doesn't mention this. It is however mentioned in the SessionHandlerInterface documentation:

The return value (usually TRUE on success, FALSE on failure). Note this value is returned internally to PHP for processing. PHP7 is more strict with session handling for custom session handlers. Symfony's custom session handler for its write method is, for whatever reason, returning false. Previously this did not trigger an error but now it does. In earlier versions of PHP, returning nothing did not result in an error. Since PHP 7.0, returning nothing results in the error: Warning: session_write_close(): Failed to write session data (user). Please verify that the current setting of session.save_path is correct (/tmp).

https://stackoverflow.com/questions/37707905/php7-symfony-3-1-0-vagrant-failed-to-write-session-data "Any adjusting of ini-Files in /etc/php/7.0/ wasn't neccessary (those files have still default values only)."

gidzr commented 7 months ago

UNRESOLVED

This removed the error - but also stops CI from generating session data.

https://stackoverflow.com/questions/61947392/codeigniter-why-cant-i-access-a-session-set-from-a-controller-in-another-cont

  1. php.ini, change: ; session.auto_start=0 session.auto_start=1

Results: Bug resolved- but no more session data getting stored in ci_sessions table

COMMENTS

  1. Updates in php 7+ cause the the saving of session handler to respond differently to a FALSE. When using php 5.3+, this wasn't a problem as the errors were ignored. This aligns with many similar changes to php which no longer ignore errors.
  2. The underlying root error appears to stem from a session closing or being overwritten, perhaps due to conflicts in sesion_start() calls. For some people it's occurred when calling multiple controllers simultaneously, although that wasn't my case.
  3. One coder mentioned these issues started when changing to php-fhm, so it could be the way the underlying php operates sessions differently with the new service.
  4. Adding session auto start into the php.ini ensure that the session re-starts if it accidentally fails or stops for whatever reason, and the bubbling up of this ensures that the errors in the custom session function do not occur.

HOWEVER, that still means the custom session handler is lacking adequate handling to a FALSE response from the SessionHandlerInterface.

I understand CI3 is retired. It still works well for me, so I am not interested in migrating to CI4, but I will make some customised changes to the session handling class to better deal with FALSE coming from the session handler.

If anyone at CI central has some ideas on how to best do this, please let me know

Thanks

privatecore commented 7 months ago

@gidzr can't reproduce

php 8.1.17 (8.1.2 also) + nginx 1.18.0 + mysql 8.0.34

$config['sess_driver'] = 'database';
$config['sess_cookie_name'] = 'ci_session';
$config['sess_expiration'] = 24 * 3600;
$config['sess_save_path'] = 'ci_sessions';
$config['sess_time_to_update'] = 3600;

show create table ci_sessions:

CREATE TABLE `ci_sessions` (
  `id` varchar(128) NOT NULL,
  `ip_address` varchar(45) NOT NULL,
  `timestamp` int unsigned NOT NULL DEFAULT '0',
  `data` blob NOT NULL,
  KEY `ci_sessions_timestamp` (`timestamp`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4

is_php is CodeIgniter's function, it checks if your php version >= passed parameter string: https://github.com/bcit-ci/CodeIgniter/blob/3.1-stable/system/core/Common.php#L63

check your php + db driver logs (looks like you have issues with inserting values into db - mb deadlock?) https://github.com/bcit-ci/CodeIgniter/blob/3.1-stable/system/libraries/Session/drivers/Session_database_driver.php#L404

bunglegrind commented 7 months ago

@gidzr could you please provide more config data, e.g., php.ini settings, config.php, controller code, environment (webserver os db), etc.?

Are you able to reproduce your issue on a clean system (see https://github.com/bcit-ci/CodeIgniter/discussions/6247#discussioncomment-8617361 )

gidzr commented 6 months ago

Hey @bunglegrind .. I've had to workaround it for now. I'll have to resolve this later though and will get back to you when I can try and reproduce. If I stumble on the issue / fix I'll also post back.

jamieburchell commented 3 months ago

@bunglegrind Is it related to this https://github.com/bcit-ci/CodeIgniter/issues/5703 ?

bunglegrind commented 3 months ago

@bunglegrind Is it related to this #5703 ?

No idea. I was never able to reproduce the issue.

gidzr commented 3 months ago

@bunglegrind and @jamieburchell - thanks for checking in and keeping up with this..

I'll look into the linked issue and retest, but will not have time until year-end. But I'll certainly aim to get back to this.