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

Fix "Session object destruction failed" - update #86

Merged
merged 7 commits into from Jun 21, 2016
Merged

Conversation

@luhla
Copy link
Contributor

luhla commented Mar 7, 2016

No description provided.

@luhla

This comment has been minimized.

Copy link
Contributor Author

luhla commented Mar 7, 2016

Sorry, have to study how run tests before pull request...

@hrach

This comment has been minimized.

Copy link
Contributor

hrach commented Mar 7, 2016

And try to fix the current one, not to open another :)

@@ -217,9 +217,9 @@ public function regenerateId()
throw new Nette\InvalidStateException('Cannot regenerate session ID after HTTP headers have been sent' . ($file ? " (output started at $file:$line)." : '.'));
}
if (session_id() !== '') {
session_write_close();

This comment has been minimized.

Copy link
@enumag

enumag Mar 21, 2016

Contributor

@luhla Maybe the session_write_close(); should be before the if. That should fix the failing test as well.

This comment has been minimized.

Copy link
@luhla

luhla Mar 22, 2016

Author Contributor

I did try it before, but result was also failed checks. Solution is session_write_close(); before session_regenerate_id(TRUE); in my case but I wouldnt do annoing tests here.

@dg dg force-pushed the nette:master branch from 51568f8 to 2381182 Apr 11, 2016
@dg dg force-pushed the nette:master branch from 19c6fa9 to 28af524 Apr 21, 2016
@dg dg force-pushed the nette:master branch from 6286f91 to d523f35 May 9, 2016
@dg dg force-pushed the nette:master branch from 67a7020 to 3fd485a Jun 17, 2016
@dg

This comment has been minimized.

Copy link
Member

dg commented Jun 17, 2016

Can you explain what is it good for?

@luhla

This comment has been minimized.

Copy link
Contributor Author

luhla commented Jun 20, 2016

Getting "session_regenerate_id(): Session object destruction failed" when calling before session_write_close();
But i can`t call session_write_close(); before session_id().

@dg

This comment has been minimized.

Copy link
Member

dg commented Jun 20, 2016

Is it needed to call session_write_close() when session_id() === '' ?

Or now we can use http://php.net/manual/en/function.session-status.php

@luhla

This comment has been minimized.

Copy link
Contributor Author

luhla commented Jun 20, 2016

Getting error when using:

if (session_id() !== '') {
        session_regenerate_id(TRUE);
}

Everithing fine when using:

if (session_status() === PHP_SESSION_ACTIVE ) {
        session_regenerate_id(TRUE);
}
luhla added 2 commits Jun 20, 2016
Change sessions state detection
readded session_write_close(); deleted by mistake
@dg

This comment has been minimized.

Copy link
Member

dg commented Jun 20, 2016

IMHO session_write_close(); should be in condition too.

luhla added 2 commits Jun 20, 2016
session_write_close moved to condition
session_write_close readded after condition
@dg

This comment has been minimized.

Copy link
Member

dg commented Jun 20, 2016

Did you change the order of session_write_close and session_regenerate_id intentionally?

@luhla

This comment has been minimized.

Copy link
Contributor Author

luhla commented Jun 20, 2016

Make no sense for me, where should be session_write_close() "in condition too" ?
After session_regenerate_id(TRUE) it is unnecessary because there is another session_write_close() after condition. But it should be always before $backup = $_SESSION;
Giving session_write_close() before session_regenerate_id(TRUE) is not obviously good idea.

@dg

This comment has been minimized.

Copy link
Member

dg commented Jun 20, 2016

After session_regenerate_id(TRUE) it is unnecessary because there is another session_write_close() after condition

Why must be another after condition? Why call it when session is not active?

@luhla

This comment has been minimized.

Copy link
Contributor Author

luhla commented Jun 20, 2016

Well, thats true, it might not, when seesion status is not active, nothing will be writen to session. I will fix it.

@dg dg merged commit 80efe13 into nette:master Jun 21, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage remained the same at 80.451%
Details
@dg

This comment has been minimized.

Copy link
Member

dg commented Jun 21, 2016

Thanks!

@luhla

This comment has been minimized.

Copy link
Contributor Author

luhla commented Jun 21, 2016

Thanks for nothing, you did the solution. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.