Codeigniter: "Cannot call session save handler in a recursive manner"

[wrabit]

I am getting a timeout in Session_files_driver.phpwith these in my logs:

Severity: Error --> Maximum execution time of 30 seconds exceeded /system/libraries/Session/drivers/Session_files_driver.php 212
Severity: Warning --> Unknown: Cannot call session save handler in a recursive manner Unknown 0
Severity: Warning --> Unknown: Failed to write session data using user defined save handler. (session.save_path: /tmp/php) Unknown 0

What I am doing:

• A view contains a form that posts data to one controller
• The same view performs 2 asynchronous ajax calls to separate and different controllers when dom ready.
• No problem so far
• Data is posted from the form to controller
• This controller, when detects the post, sets some flashdata and redirects back to same method
• When hitting the view again from the redirect, only one ajax call is fired, the other hangs and hits the errors above.

Additional info:

• Removing one ajax call, the operation is fine and it doesn't matter which one is removed.
• Removing the set flash data, things are fine
• Removing the redirect, things are fine

I have separated out the barebones setup of this in a new 3.19 and zipped up here to see, just hit the form submit on the form field on the welcome view.

https://www.dropbox.com/s/2mbkdp1cdhftafj/ci-issue.zip?dl=0

[wrabit]

3.18 doesn't have the issue.

[it-can]

I have the same problem

https://github.com/bcit-ci/CodeIgniter/issues/5530#issuecomment-404860026

[Xcreen]

If you comment/remove the function validateId($id) in your session-driver, it should working again.
Due the changelog it was added because of:
https://github.com/bcit-ci/CodeIgniter/commit/b3f7aae1079e8e484437bc67f4c126f34e7903d8

Fixed a possible session fixation vulnerability where the Session Library enabled session.use_strict_mode but it didn’t actually do anything.

I didnt debugged it deeper, but i think the validate-function dont work correctly, when it get called async.

Edit: Rename the function validateId($id) -> validateSessionId($id) in Session_files_driver.php (Line: 405) and also change the function call in Session_driver.php (Line: 124) and it also works. There maybe them to be a error on calling the correct function.

[it-can]

@Xcreen would this not break other session drivers? and that commit (https://github.com/bcit-ci/CodeIgniter/commit/b3f7aae1079e8e484437bc67f4c126f34e7903d8) would only work on php < 7 right...

[narfbg]

What PHP version is this on? Using which session driver?

[it-can]

files driver
My problem was with php 7.1.20

[narfbg]

@it-can Yes, you said that in #5530 ... I'm asking the OP here.

[wrabit]

Files 7.2.1

[narfbg]

@Xcreen and you?

[dkowalskidev]

My solution probably resolve this problem.
https://github.com/bcit-ci/CodeIgniter/pull/5550

[narfbg]

No, #5550 doesn't resolve anything.

Anyway, I assume @Xcreen has no intention of replying or is just unavailable, but he did stumble upon a clue ...

validateId() is supposed to be auto-called by PHP 7+ only if the handler implements SessionUpdateTimestampHandlerInterface, which it doesn't because that's fucking ridiculous, poorly documented and I'd told the guy who wrote that logic that it's a bad idea, but he listens to nobody but the voices in his head.
The security researchers who reported the issue that warranted b3f7aae1079e8e484437bc67f4c126f34e7903d8 were supposed to verify it, and they didn't catch that either.

Yet, validateId() is somehow being auto-called and triggering those errors. The messages are confusing because that's what tends to happen when the bug is in PHP itself and it somehow doesn't straight-up crash with a segmentation fault.

So, a logical solution should be to rename validateId() to something else and then drop the version checks so that CI always calls it:

diff --git a/system/libraries/Session/Session_driver.php b/system/libraries/Session/Session_driver.php
index 2fe30b8..a26b3a3 100644
--- a/system/libraries/Session/Session_driver.php
+++ b/system/libraries/Session/Session_driver.php
@@ -121,7 +121,7 @@ abstract class CI_Session_driver implements SessionHandlerInterface {
     */
    public function php5_validate_id()
    {
-       if (PHP_VERSION_ID < 70000 && isset($_COOKIE[$this->_config['cookie_name']]) && ! $this->validateId($_COOKIE[$this->_config['cookie_name']]))
+       if (isset($_COOKIE[$this->_config['cookie_name']]) && ! $this->validateSessionId($_COOKIE[$this->_config['cookie_name']]))
        {
            unset($_COOKIE[$this->_config['cookie_name']]);
        }
diff --git a/system/libraries/Session/drivers/Session_database_driver.php b/system/libraries/Session/drivers/Session_database_driver.php
index 074accf..f342690 100644
--- a/system/libraries/Session/drivers/Session_database_driver.php
+++ b/system/libraries/Session/drivers/Session_database_driver.php
@@ -353,7 +353,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
     * @param   string  $id
     * @return  bool
     */
-   public function validateId($id)
+   public function validateSessionId($id)
    {
        // Prevent previous QB calls from messing with our queries
        $this->_db->reset_query();
diff --git a/system/libraries/Session/drivers/Session_files_driver.php b/system/libraries/Session/drivers/Session_files_driver.php
index 654f300..8c7fac0 100644
--- a/system/libraries/Session/drivers/Session_files_driver.php
+++ b/system/libraries/Session/drivers/Session_files_driver.php
@@ -402,7 +402,7 @@ class CI_Session_files_driver extends CI_Session_driver implements SessionHandle
     * @param   string  $id
     * @return  bool
     */
-   public function validateId($id)
+   public function validateSessionId($id)
    {
        return is_file($this->_file_path.$id);
    }
diff --git a/system/libraries/Session/drivers/Session_memcached_driver.php b/system/libraries/Session/drivers/Session_memcached_driver.php
index 9e81168..b464439 100644
--- a/system/libraries/Session/drivers/Session_memcached_driver.php
+++ b/system/libraries/Session/drivers/Session_memcached_driver.php
@@ -303,7 +303,7 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
     * @param   string  $id
     * @return  bool
     */
-   public function validateId($id)
+   public function validateSessionId($id)
    {
        $this->_memcached->get($this->_key_prefix.$id);
        return ($this->_memcached->getResultCode() === Memcached::RES_SUCCESS);
diff --git a/system/libraries/Session/drivers/Session_redis_driver.php b/system/libraries/Session/drivers/Session_redis_driver.php
index d7a5755..f351aff 100644
--- a/system/libraries/Session/drivers/Session_redis_driver.php
+++ b/system/libraries/Session/drivers/Session_redis_driver.php
@@ -323,7 +323,7 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
     * @param   string  $id
     * @return  bool
     */
-   public function validateId($id)
+   public function validateSessionId($id)
    {
        return (bool) $this->_redis->exists($this->_key_prefix.$id);
    }

Can you all confirm that this works for you?

[narfbg]

FYI, regardless of how it looks like, this has nothing to do with AJAX requests, flash data, (a)synchronicity or anything ... those things just happen to be a trigger in your cases (@digitaloutback, @it-can).

[it-can]

Cool I’ll test it tomorrow!

[dkowalskidev]

I checked the solution you proposed. It doesn't work in my case. I have PHP 5.6.34

[it-can]

@narfbg the fix is also not working for me... But when i use this php5_validate_id() function it works (because it will skip on php7, but PHP < 7 will still have a problem I think)

public function php5_validate_id()
    {
        if (PHP_VERSION_ID < 70000 && isset($_COOKIE[$this->_config['cookie_name']]) && ! $this->validateSessionId($_COOKIE[$this->_config['cookie_name']]))
        {
            unset($_COOKIE[$this->_config['cookie_name']]);
        }
    }

Also I found that commenting out $this->php5_validate_id(); in the session_files_driver.php, makes it work again...

--EDIT--

When commenting out return is_file($this->_file_path.$id); with return true; it also works for me... So could is_file() be a problem? file_exists() also seems to work, but this also returns true is it is a dir......

[narfbg]

Ok, now this is getting really bizzare ...

@dkowalskidev Does it only now not work on 5.6 or did you have the original issue on 5.6 as well? Also, are you getting the same error messages as @it-can and @digitaloutback?

@it-can Still the same error messages? And can you check what is_file() returns before the error triggers?

[dkowalskidev]

@narfbg Yes, I have the same message error. My issue was on 5.6. When I use file_exists() instead is_file() it works.

[it-can]

@narfbg same errors still...

[it-can]

Mmm found something else...

https://github.com/bcit-ci/CodeIgniter/blob/3.1-stable/system/libraries/Session/Session.php#L108

If i change this:

if (is_php('5.4'))
{
    session_set_save_handler($class, TRUE);
}
else
{
    session_set_save_handler(
        array($class, 'open'),
        array($class, 'close'),
        array($class, 'read'),
        array($class, 'write'),
        array($class, 'destroy'),
        array($class, 'gc')
    );
    register_shutdown_function('session_write_close');
}

to this:

session_set_save_handler(
    array($class, 'open'),
    array($class, 'close'),
    array($class, 'read'),
    array($class, 'write'),
    array($class, 'destroy'),
    array($class, 'gc')
);
register_shutdown_function('session_write_close');

it works again... weird huh...

[dkowalskidev]

is_php('5.4') is return true.

[it-can]

@dkowalskidev yeah 5.4 and up

Can you test this also?

[dkowalskidev]

@it-can Yes, it still doesn't work.

[it-can]

Mmm weird for me it does

[narfbg]

That change to the session_set_save_handler() call should only "work" without the patch I asked you to try, because it explicitly sets the id validation function to nothing and thus it's not called ... That again doesn't help us with anything.

Still no reply on what the is_file() call returns before the error? Also, what are the permissions on your session directory? Does it help if you do chmod +x on it? If not, does changing the is_file() call to file_exists() && ~ ! is_dir() help?

[narfbg]

On a similar note, does clearstatcache(TRUE) before the is_file() call help?

[it-can]

clearstatcache(TRUE) does not work for me.

file_exists() && ! is_dir() does also not work for me

permission are

drwx-wx-wt  2 root root 4.0K Jul 27 19:16 sessions

a session has this:

-rw------- 1 vagrant vagrant 3.2K Jun 22 17:45 sessionblabla

I cannot seem to get a response of is_file() when it fails...

Fix suggested in https://github.com/bcit-ci/CodeIgniter/pull/5550 seems to work though

[narfbg]

What do you mean you can't get a response from is_file()?

According to the error messages, the crash occurs during read(), so is_file() should've been called already at that point ... Try exit right after you dump it.

[it-can]

@narfbg it seems it never reaches that point when the request / session fails, all other requests it returns true

[it-can]

@narfbg if I add clearstatcache(true, $this->_file_path.$session_id); before line 209 in Session_files_driver.php it works...

[mimoza39]

hello,
i have same issue with php 7.0,
i upgraded to 3.1.9 yesterday
my session duration is 7200sec,
i have a user with an openned page on my website since yesterday, every 7200sec i have the session error :
Maximum execution time of 3600 seconds exceeded drivers/Session_files_driver.php
error are at lines 210 , 212 or 422
i try to uncomment the $this->php5_validate_id(); line and i'll let you know

[it-can]

@mimoza39 also please try my suggestion, to see if it works...

if I add clearstatcache(true, $this->_file_path.$session_id); before line 209 in Session_files_driver.php it works...

[mimoza39]

hello
remove the call to $this->php5_validate_id(); function solved the issue on my side
i have php7 , a production server with a lot of traffic, i can't "test" things sorry
regards

[narfbg]

@it-can Well, this still doesn't make sense for a number of reasons, but I guess we're getting somewhere.

If the is_file() call in validateSessionId() is what messes things up, then that's where we should call clearstatcache() too. What if you do this:

 $result = is_file($this->_file_path.$id);
 clearstatcache(TRUE, $this->_file_path.$id);
 return $result;

And I still do need the value of $result.

[it-can]

@narfbg works for me :-)

$result = true

[dkowalskidev]

@narfbg it works in my case too

[Xcreen]

Sorry for the late reply, i was on larger trip the last month and didnt checked my github.
I used files-driver with php 7.0.4.
As i can you see, you already found and fixed the issue 👍
I did a quick test and it also worked for me.

[webalchemist]

Just to add another data point on this issue.
A few days ago, I noticed that my old session files weren't being deleted. On searching the forum I found a suggestion that the php.ini session settings needed to be changed to the following:
session.gc_probability = 1 (not 0 as it was by default)
session.gc_divisor = 1000
session.gc_maxlifetime = 1440

After making these changes, the old session files were correctly removed, but I started to see the error discussed in this thread appearing in my logs. The error never appeared before I made these changes.
(running CI 3.1.9, PHP 7.2)

I'll try the fix suggested here and let you know if it works for me.

[killradio]

Problem is filesize return cached result (system/libraries/Session/drivers/Session_files_driver.php:210). clearstatcache(); before this line -> solves this issue, thx @it-can.

[webalchemist]

Its also related to the garbage collection. If the session garbage collector is turned off then the error doesn't occur. I guess the garbage collector deletes the session file without clearing the stat cache for that file.

The suggested fix does work. So does turning off the session garbage collection and deleting the old session files via a cronjob (as suggested in the php.net docs)

[killradio]

@webalchemist are you sure about getting error:
Severity: Error --> Maximum execution time of 30 seconds exceeded /system/libraries/Session/drivers/Session_files_driver.php 212, after turning on gc?

Or your error is smth like this:
ERROR - 2018-12-05 12:59:07 --> Severity: Warning --> unlink(/var/www/localhost/tmp/ci_session3fjb46cchbo33ajppihh0m4qtecoi928 ): No such file or directory /var/www/localhost/system/libraries/Session/drivers/Session_files_driver. php 384

First error is 100% endless loop: during multiple script calls session data is changed and is less than it was -> $read >= $length from loop will never occur -> php stuck.

[Mul-tiMedia]

Problem is filesize return cached result (system/libraries/Session/drivers/Session_files_driver.php:210). clearstatcache(); before this line -> solves this issue, thx @it-can.

Thank you @killradio this solved our problems!

[webalchemist]

@killradio It was definitely the first error (max execution time @ Session_files_driver.php 212). Searching for that error is how I found this thread.

As I said, the clearstatcache() patch does resolve the issue, but so (for me) did turning off the PHP session garbage collection.

initially I had session.gc_probability = 0 - no error
I changed it to session.gc_probability = 1 - started to get the error
revert to session.gc_probability = 0 - and cleared old session data via cron - no error