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

Better detection of a succesfull start of a session #15114

Merged
merged 2 commits into from Nov 8, 2019

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Nov 5, 2019

Differentiate between errors and warnings in session error handler and only assume session was not started if there was an error triggered. It should not throw an exception if there was only a notice or a warning.

Needed for WordPress. Eg there a caching plugin was trying to delete some cache after we executed a DB query, and did cache deletion on the file system triggered a warning. Then Zend_Session assumed the session was not started correctly even though it was and the warning was not related to session_start at all

image

Differentiate between errors and warnings in session error handler and only assume session was not started if there was an error triggered. It should not throw an exception if there was only a notice or a warning.
@tsteur tsteur added the Needs Review PRs that need a code review label Nov 5, 2019
@tsteur tsteur added this to the 3.13.0 milestone Nov 5, 2019
@@ -479,14 +479,14 @@ public static function start($options = false)
restore_error_handler();
}

if (!$startedCleanly || Zend_Session_Exception::$sessionStartError != null) {
if (!$startedCleanly || !empty(Zend_Session_Exception::$sessionStartError)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

ideally we would even only go into that logic if !$startedCleanly and not look at Zend_Session_Exception::$sessionStartError at all but I reckon we could still change that later.

if (!isset(self::$sessionStartError)) {
self::$sessionStartError = '';
$message = $errfile . '(Line:' . $errline . '): Error #' . $errno . ' ' . $errstr;
if (E_ERROR === $errno || E_CORE_ERROR === $errno || E_COMPILE_ERROR === $errno) {
Copy link
Member Author

Choose a reason for hiding this comment

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

only trigger a session start exception when one of these 3 errors occur. Those 3 should be enough and no need to handle recoverable error etc.

@diosmosis diosmosis merged commit 19c40ee into 3.x-dev Nov 8, 2019
@diosmosis diosmosis deleted the sessionstartdetect branch November 8, 2019 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants