Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
module0x90 opened this issue Jun 9, 2017 · 2 comments

Comments

@module0x90
Copy link

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 php/php-src#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
Copy link
Member

ikkez commented Dec 30, 2019

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
Copy link

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(); } );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants