Empty instead of unset the $_SESSION variable

[SwenVanZanten]

We shouldn't destroy a global variable cause other scripts trust them to be there.
For instance the save method of the Symfony NativeSessionStorage trusts the variable to be there.

Assigning an empty array to the variable would do the trick.

[jorijn]

I think PHP has a method for this: session_unset, see: https://www.php.net/manual/en/function.session-unset.php

They even left a note about how it's really wrong to unset the $_SESSION variable entirely.

[SwenVanZanten]

Good one, I checked this function out, yet it didn't work for me.
However I called the function after the session_destroy function was called.
It seems that doesn't work.

I'll adjust the pully

[pitbulk]

What about:

if (OneLogin_Saml2_Utils::isSessionStarted()) {
    session_unset();
    session_destroy();
} else {
    session_unset();
}

[SwenVanZanten]

No problem, however I suggest to put the unset before the destroy without the else:

session_unset();

if (OneLogin_Saml2_Utils::isSessionStarted()) {
    session_destroy();
}

[pitbulk]

Yeah, modify the PR and I will merge, thanks!

[SwenVanZanten]

Yes done! 💪

[pitbulk]

I had to revert it, the test failed.

1. OneLogin_Saml2_AuthTest::testProcessSLOResponseValidDeletingSession
   Failed asserting that true is false.
   /home/travis/build/onelogin/php-saml/tests/src/OneLogin/Saml2/AuthTest.php:560

[SwenVanZanten]

Whoops 😬 I’ll check it out tomorrow Verstuurd vanaf mijn iPhone

…

Op 5 dec. 2019 om 19:00 heeft Sixto Martin ***@***.***> het volgende geschreven:  I had to revert it, the test failed. OneLogin_Saml2_AuthTest::testProcessSLOResponseValidDeletingSession Failed asserting that true is false. /home/travis/build/onelogin/php-saml/tests/src/OneLogin/Saml2/AuthTest.php:560 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.