bcit-ci / CodeIgniter

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

CI 3.0.2 php 7 RC 4 database sessions expire when sessions rotate #4179

Closed blasto333 closed 8 years ago

blasto333 commented 8 years ago

When running php 7 RC 4 I noticed that I kept getting logged out during session rotation.

You can easily reproduce by setting the following in config.php (You can make sess_time_to_update larger if you wish; this is just a quick way to reproduce it)

$config['sess_driver'] = 'database';
$config['sess_cookie_name'] = 'phppos';
$config['sess_expiration'] = 0;
$config['sess_save_path'] = 'sessions';
$config['sess_match_ip'] = FALSE;
$config['sess_time_to_update'] = 1;
$config['sess_regenerate_destroy'] = FALSE;

Login; wait 5 seconds; then refresh and the session is destroyed and a cookie is session id is generated. Then once stuck in the loop you cannot login at all. This happens in Chrome + Safari, Firefox on Mac.

I have also attached mysql general log: https://www.dropbox.com/s/0jsrr85a1aoq6z5/mysql_general_log_ci_3_php_7.txt?dl=0

PHP 7 is from homebrew (RC 4) https://www.dropbox.com/s/3h3p08nuxz51j3u/Screenshot%202015-10-17%2023.02.18.png?dl=0

This does not happen in php 5.x. I am also using mysql 5.6

Seems to be related to:

public function sess_regenerate($destroy = FALSE)
{
    $_SESSION['__ci_last_regenerate'] = time();
    session_regenerate_id($destroy);
}

If I comment that part out; it works. Not sure if it is a php bug or not; but thought I would share.

narfbg commented 8 years ago

Your session isn't being destroyed ... if you look at that database log, there's no DELETE query in it.

blasto333 commented 8 years ago

I didn't meant destroyed from database; but destroyed from an end users perspective of being logged out and the cookie session Id changing

Chris Muench Sent from my iPhone

On Oct 18, 2015, at 1:07 PM, Andrey Andreev notifications@github.com wrote:

Your session isn't being destroyed ... if you look at that database log, there's no DELETE query in it.

— Reply to this email directly or view it on GitHub https://github.com/bcit-ci/CodeIgniter/issues/4179#issuecomment-149030871.

narfbg commented 8 years ago

Well ok, but please start respecting what terms mean from now on ... it's confusing otherwise. Ironically, you'd have to explain now what exactly you mean by "logout"; not from a user's perspective, but how does it relate to sessions - that may mean a lot of things.

Session IDs changing is natural - that's what's supposed to happen after the sess_time_to_update interval.

blasto333 commented 8 years ago

Ok got it. What is happening is I am losing session data when it rotates. Specially person_id value.

Here is exactly what happens if I put the following at the bottom of index.php for testing.

var_dump(session_id()); var_dump($_SESSION);

AFTER Login:

string(40) "eda75a5324406607a9ef9a44cbc3e8dd27e3e42a" array(2) { ["__ci_last_regenerate"]=> int(1445188864) ["person_id"]=> string(1) "1" }

Refresh page when session re-generates and user is logged out because person_id key is gone

string(40) "76be5a2d473f4f4123a1a6f18e65d1b0d0e5b4b8" array(1) { ["__ci_last_regenerate"]=> int(1445188896) }

Notice how person_id goes away.

narfbg commented 8 years ago

I see ...

Judging by your MySQL logs, it appears that PHP7 would always use the same SessionHandler object, while PHP5 supposedly instantiates a separate object when it regenerates the ID. If this guess it correct, then adding $this->_row_exists = FALSE; between lines 161,162 in system/libraries/Session/drivers/Session_database_driver.php should solve the problem.

Does it?

blasto333 commented 8 years ago

This fixes it!! Thank you!

Do you see any other potential issues with sessions for php 7 besides this one? I am trying to make my code ready for php 7 before it is released.

Chris Muench Owner PHP Point Of Sale, LLC http://www.phppointofsale.com

On Oct 18, 2015, at 1:47 PM, Andrey Andreev notifications@github.com wrote:

I see ...

Judging by your MySQL logs, it appears that PHP7 would always use the same SessionHandler object, while PHP5 supposedly instantiates a separate object when it regenerates the ID. If this guess it correct, then adding $this->_row_exists = FALSE; between lines 161,162 in system/libraries/Session/drivers/Session_database_driver.php should solve the problem.

Does it?

— Reply to this email directly or view it on GitHub https://github.com/bcit-ci/CodeIgniter/issues/4179#issuecomment-149034162.

narfbg commented 8 years ago

If I had noticed any breaking changes, I would've already fixed them (and there is actually one such that is already accounted for) ...

But then again, we just got this one, so you never know. Shit happens, I guess, but if more people were trying the PHP7 RCs, we'd be able to fix any potential issues before the big release.

blasto333 commented 8 years ago

Ok I will keep an eye on it and only my development in PHP 7 for now. So far this is all I have found. I am pretty happy with no real problems so far besides this one for such a major upgrade.

Thank you for all your help

Chris Muench Sent from my iPhone

On Oct 18, 2015, at 2:04 PM, Andrey Andreev notifications@github.com wrote:

If I had noticed any breaking changes, I would've already fixed them (and there is actually one such that is already accounted for) ...

But then again, we just got this one, so you never know. Shit happens, I guess, but if more people were trying the PHP7 RCs, we'd be able to fix any potential issues before the big release.

— Reply to this email directly or view it on GitHub https://github.com/bcit-ci/CodeIgniter/issues/4179#issuecomment-149035620.