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

remove id check when checking for started session #98

Open
wants to merge 6 commits into
base: 2.23.x
Choose a base branch
from

Conversation

mimmi20
Copy link

@mimmi20 mimmi20 commented Dec 11, 2024

Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Fixes #97

Description

In Release 2.22.0 the check for the Constant SID was removed. This lead to the error that the Session could not be started when a session ID was set.

@mimmi20
Copy link
Author

mimmi20 commented Dec 11, 2024

@froschdesign @gsteel There are two tests failing now (testSessionExistsReturnsTrueWhenSessionStartedThenWritten and testCallingWriteCloseShouldNotAlterSessionExistsStatus).

I dont unserstand why session_write_close shall not change the Session-exists-Status.

PHP's page states

session_write_close
(PHP 4 >= 4.0.4, PHP 5, PHP 7, PHP 8)

session_write_close — Write session data and end session

src/SessionManager.php Outdated Show resolved Hide resolved
@gsteel
Copy link
Member

gsteel commented Dec 11, 2024

I dont unserstand why session_write_close shall not change the Session-exists-Status.

Closing the session persists the session data for the next request, it doesn't kill the session; There is still an active session once it has been written to storage.

To kill the active session, it must be destroyed with session_destroy() or $manager->destroy()

Signed-off-by: Thomas Müller <[email protected]>
Signed-off-by: Thomas Müller <[email protected]>
Signed-off-by: Thomas Müller <[email protected]>
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests that now emit deprecations on 8.4 can be silenced by annotating the test with #[IgnoreDeprecations]

test/SessionManagerTest.php Show resolved Hide resolved
Signed-off-by: Thomas Müller <[email protected]>
Signed-off-by: Thomas Müller <[email protected]>
@mimmi20 mimmi20 requested a review from gsteel December 11, 2024 19:19
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

Successfully merging this pull request may close these issues.

Can not start Session
2 participants