session_regenerate_id(true) on PHP 7 caught in infinite loop

[marcoliverteschke]

We recently updated a site to PHP 7.0.8. After the update Zebra_Session caused an internal server error.

We managed to track it down to session_regenerate_id(true) being called by read(). It gets caught in some sort of loop and gets called over and over.

Setting session_regnerate_id(false) instead solves the problem, but of course that means the old session is retained.

It appears the problem occurs on brand-new sessions that don't have any data stored in them. When I switch back from false to true in an already active session, it seems to work fine.

[alisonw]

Thanks for the clue - I'm developing a new system so updated my version of Zebra_session along with moving to PHP 7 and was getting caught in a full memory leak when creating the new session. Switching to false has sorted it, though I needed to add a return of true to the public function regenerate_id() call.

[frumbert]

Wow, I just went to php 7.1 and hit this as well. Thanks for the heads-up!

/edit : session_regenerate_id(false) is still throwing recursion errors for me, e.g.

session_regenerate_id(): Cannot call session save handler in a recursive manner in ...

One of the threads I googled mentioned session_regenerate_id which is php function calls close and open method of user session handler since PHP7. session_regenerate_id in PHP5 doesn't call close and open method. . There was also discussion of bugs in session_regenerate_id during php7 development...

[alisonw]

OK, so it kept erroring when I wasn't looking resulting in my spending a few hours simplifying the code and getting different error messages. And in usual fashion, the error message we've been seeing has very little to do with the error!

The problem is that PHP7 doesn't like the null return in the write function.

As a provisional fix, replace

             if (@$this->_mysql_affected_rows() > 1) return true;
            // if the row was inserted
            // return an empty string
            else return '';

with just 'return true'. Result is no errors! :-)

[viniciushsantana]

Guys, is this wrapper still maintained? If yes, is there any plans for a fix?

Thanks!

[stefangabos]

Yes, it is maintained! I'm yet to move to PHP7, so I was just waiting for confirmations from other users that this solution works and that it doesn't break anything.

[stefangabos]

I've committed the fix provided by @alisonw - thanks a lot!

[viniciushsantana]

Thanks a lot, @stefangabos!

Quick question: those changes are already available for download via Composer?

[stefangabos]

No, not yet. I'm still looking over the other issues and will publish a new release tomorrow.

[alisonw]

Thanks @stefangabos - just to note that it is still occasionally throwing the same error for me but unpredictably (maybe twice in fifty calls or so). As I'm actively working on the code that the session handler is being used for it is quite possible that it's my work rather than Zebra which is the problem.

[stefangabos]

ok, thanks for the heads up. i don't have PHP7 but rely on Zebra_Session on a daily basis on lots of places and in some with really heavy traffic without any issues - but no PHP7. instead, here's something that you may run into and please tell me if maybe it is something that's similar to this issue reported here: since Zebra_Session uses row locking if you have a page that takes some time to complete and you open another tab of the same app, you'll notice that it "hangs" because it will try to wait for the row lock to be released. if this what this is all about then there's actually nothing to be done about it and maybe I should add an option for not using row locking...

[noideaxd]

@stefangabos just letting you know, that this doesn't resolve the errors on PHP 7.1.3. I'm currently in the process of migrating a rather large traffic site from 5.4.8 > 7.1.3 and on the staging site it makes use of Zebra_Session but i'm still getting session_regenerate_id(): Cannot call session save handler in a recursive manner, for every session.

[stefangabos]

@noideaxd it seems that in PHP7 session_regenerate_id() makes a call to the read() method; in this method I had a call to session_regenerate_id() if there was no active session. i committed the update, can you please check if it is working now? thanks!

[noideaxd]

@stefangabos Okay, so I applied the updated and i was getting the following error:

Unknown: Cannot call session save handler in a recursive manner in Unknown on line 0

I continued by adding an option to session_start ();

       session_start([ 'read_and_close'  => true, ]);

For now i'm getting no errors! (Though what i've done probably isn't correct)

[stefangabos]

@noideaxd that actually sounds like a good idea, but it's PHP7 only. I'd like to find something that works on PHP5 also. can you please also try removing line line 638 and undoing your change and see if that works? thanks for your help!

[noideaxd]

@stefangabos gave it a go and ended up with

PHP Warning: Unknown: Cannot call session save handler in a recursive manner in Unknown on line 0

So for now i'll continue working with my changes

[noideaxd]

@stefangabos

I guess you could use something like

     if (PHP_VERSION_ID < 70000) {
         session_start ();
     } else {
        session_start([ 'read_and_close'  => true, ]);  
     }

to work for both 5.x.x and 7.x.x

[stefangabos]

What versions of PHP7 are you guys experiencing problems with? I just installed 7.0.18 and the current commit seems to be working

[noideaxd]

7.1.4 is what i'm using

[littlehawk]

I'm using 7.0.10. The only error detected after updating your class to the latest release (ver. 2.1.5) is in the stop() function when regenerate_id() function is call. Error is:

Catchable fatal error: session_regenerate_id(): Failed to create(read) session ID: user in .../.../Session.class.php on line 564

By the time I commented the line and everything works fine. I hope this can help solve the problem in the meantime I'm trying to fix it ;-)

[stefangabos]

Thanks! I'm using 7.1.4 and had already removed that line from my local copy although, in my case, it was working without errors with that line present.

[littlehawk]

Thanks to you, @stefangabos ! I would like to bring to your attention the changes I made to the stop() function:

public function stop()
{
    $this->gc();
    session_unset();

    // Remove PHPSESSID from browser
    if ( isset( $_COOKIE[session_name()] ) ) {
        setcookie( session_name(), "", time() - 3600, "/" );
    }

    // Clear session from globals
    $_SESSION = array();

    // Clear session from disk
    session_destroy();
}

Ciao! 👍

[stefangabos]

good idea to run the garbage collector on stop, i guess. also, i've seen the COOKIE un-setting in the manual but wasn't sure about it... for how long have you been using it with these changes?

[stefangabos]

Here's my take on the stop method which runs the garbage collector using the environment's settings (usually set to 0.1% chance, as running this more often on something with high traffic would be overkill)

// get garbage collector related settings
$gc_settings = $this->get_settings();

// respecting the probability
if ($gc_settings['session.gc_probability'] / $gc_settings['session.gc_divisor'] > rand(0, 100))

    // run the garbage collector
    $this->gc();

// if a cookie is used to pass the session id
if (ini_get('session.use_cookies')) {

    // get session cookie's properties
    $params = session_get_cookie_params();

    // unset the cookie
    setcookie(session_name(), '', time() - 42000, $params['path'], $params['domain'], $params['secure'], $params['httponly']);

}

// destroy the session
session_unset();
session_destroy();

[stefangabos]

...although I don't think is of any use running the gc manually as PHP already takes care of that

[littlehawk]

More than one year, @stefangabos ;-)