f3-factory / fatfree-core

Fat-Free Framework core library
GNU General Public License v3.0
207 stars 86 forks source link

onsuspect(): "session_commit(): Cannot call session save handler in a recursive manner" #219

Open module0x90 opened 7 years ago

module0x90 commented 7 years ago

Hi,

my webapp is using DB sessions and I was recently playing with onsuspect() handler. My code is:

        \Registry::set('session', new \DB\SQL\Session(
                $f3->get('DB'),
                'session',
                FALSE,
                function($session) {
                        $fw=\Base::instance();
                        $fw->clear('SESSION');                        << recursion error
                        $fw->reroute('@login');
                }
        )); // needed for CSRF

My webapp requires the user to login and the logged in user is stored in SESSION.user. After I have logged in, I get the session_id and change the stored IP in the session table. Then I click on some other action (while still logged in) in my web app and it crashes with

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

I am on PHP 7.1.5. This seems to be hitting bug #73461 (https://bugs.php.net/bug.php?id=73461) and a patch would be https://github.com/php/php-src/pull/2196. In my case - though untested - I could get away with unsetting SESSION.user instead of destroying the session.

Stacktrace:

13  Whoops\Exception\ErrorException 
    …/vendor/bcosca/fatfree-core/base.php442
12  session_destroy
    …/vendor/bcosca/fatfree-core/base.php442
11  Base clear
    …/web/index.php193
10  CCWA\{closure}
    …/vendor/bcosca/fatfree-core/base.php1787
9   Base call
    …/vendor/bcosca/fatfree-core/db/sql/session.php72
8   DB\SQL\Session read
    [internal]0
7   session_start
    …/vendor/bcosca/fatfree-core/base.php259
6   Base ref
    …/vendor/bcosca/fatfree-core/base.php307
5   Base exists
    …/app/lib/CCWA/View/Main.php16
4   CCWA\View\Main __construct
    …/app/lib/CCWA/Controller/Base.php25
3   CCWA\Controller\Base beforeroute
    …/vendor/bcosca/fatfree-core/base.php1784
2   Base call
    …/vendor/bcosca/fatfree-core/base.php1609
1   Base run
    …/web/index.php199
0   CCWA\boot
    …/web/index.php203

There was also a discussion on https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!topic/f3-framework/P-1q-9H9fWw with ved. This bug/feature in PHP 7.1.X is hitting F3 in different ways.

Cheers

Thomas

ikkez commented 4 years ago

The best i came up with yet was this:

\Registry::set('session', $sess=new \DB\SQL\Session(
    $f3->get('DB'),
    'session',
    false,
    function($session,$id) {
        $session->destroy($id);
        $session->close();

        $fw=\Base::instance();
        $fw->clear('COOKIE.'.session_name());
        $fw->error('403');
    }
)); 

Though this will destroy the session, it is actually not finally closed yet within the ONSUSPECT handler, as this is called while session_start is called and we cannot call session_destroy before session_start has finished. This means that rerouting from that point is difficult, because the frameworks unload handler will check of active sessions and commits the session, we're just about to close at this point. That's the point were the recursion happens. The only way to get around this (even in php 7.3+) is to throw an error, as the onload handler will skip saving the session in that case. Ideas welcome.

riccardo2k13 commented 2 years ago

I had the same issue. I did it with this compromise.

You can print an html with javascript to do a redirect frontend.

new DB\SQL\Session( $fw->DB, 'sessions', TRUE, function($session, $id) { $session->destroy($id); $session->close(); echo 'Session expired. If not redirected <a href="/login">click to login</a>'; echo '<script>window.location.replace("http://example.com/login");</script>'; die(); } );