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

fixes for PHP 7.2 #140

Merged
merged 3 commits into from
Dec 1, 2018
Merged

fixes for PHP 7.2 #140

merged 3 commits into from
Dec 1, 2018

Conversation

framer99
Copy link
Contributor

This PR includes three PHP7.2 fixes:

PHP7.2 and $_SESSION not getting saved as discussed in #130.

The fix posted for #129

A previously unknown but long time issue exposed by PHP7.2 now issuing a warning stating session was already started when trying to set gc.max_lifetime.

Fix as described by daviddpd in github ona repo issue opennetadmin#129:
  opennetadmin#129

PHP 7.x will no longer convert a variable initialized as null string
to an array.
As of PHP 7.2 , a warning is issued that you cannot set session module
ini settings after the session is active.

    PHP Warning:  ini_set(): A session is active. You cannot change
                  the session module's ini settings at this time in
                  /opt/ona/www/config/config.inc.php on line 232

Apparently it has never worked to set gc.max_lifetime after session start
but no warning was issued so it was a silent failure.

    https://bugs.php.net/bug.php?id=75650
As of PHP 7.1, $_SESSION is no longer intialized if the custom session
read handler returns false or null. With $_SEESION not initialized, the
the custom session write handler is not getting called (just close()).

The way the ONA customer session handlers are written, this new PHP behavior
results new sessions never being written to the database.
@mattpascoe mattpascoe merged commit 5f66419 into opennetadmin:master Dec 1, 2018
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.

2 participants