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

Session warning on PHP >=7.2 with debug loglevel #1168

Closed
toth-dev opened this issue Jul 18, 2019 · 1 comment
Closed

Session warning on PHP >=7.2 with debug loglevel #1168

toth-dev opened this issue Jul 18, 2019 · 1 comment

Comments

@toth-dev
Copy link

toth-dev commented Jul 18, 2019

Describe the bug
If you set 'logging.level' => SimpleSAML\Logger::DEBUG, PHP generates warnings at the bottom of the page/XML, like this:

Warning: session_name(): Cannot change session name when headers already sent in /var/www/simplesamlphp/lib/SimpleSAML/SessionHandlerPHP.php on line 72

To Reproduce
Steps to reproduce the behavior:

  1. Run PHP >= 7.2
  2. Set loglevel to debug
  3. Enable SAML2 IDP in your config
  4. Navigate to /simplesaml/saml2/idp/metadata.php?output=xhtml

From my comment on #1092 , see the Dockerfile:

I've created a docker image for reproduction: https://github.com/totpet/docker-test-saml-idp

docker run --rm -p 9080:80 tothp/test-saml-idp:7.3
curl -v http://127.0.0.1:9080/simplesaml/saml2/idp/metadata.php?output=xhtml

Expected behavior
There should be no warning.

Screenshots or logs
(I removed [Thu Jul 18 13:10:09.863068 2019] [php7:notice] [pid 17] [client 172.17.0.2:49774] simplesamlphp from the begining of each line)

ERR [TRa2da8fcc] Error creating session: Headers already sent.
DEBUG [TRa2da8fcc] Localization: load domain 'messages' at '/var/www/simplesamlphp/locales'
DEBUG [TRa2da8fcc] Trying langpath for 'en' as '/var/www/simplesamlphp/locales/en/LC_MESSAGES/'
ERR [TRa2da8fcc] SimpleSAML\\Error\\Exception: Error 2 - session_name(): Cannot change session name when headers already sent at /var/www/simplesamlphp/lib/SimpleSAML/SessionHandlerPHP.php:72
ERR [TRa2da8fcc] Backtrace:
ERR [TRa2da8fcc] 8 /var/www/simplesamlphp/www/_include.php:48 (SimpleSAML_error_handler)
ERR [TRa2da8fcc] 7 [builtin] (session_name)
ERR [TRa2da8fcc] 6 /var/www/simplesamlphp/lib/SimpleSAML/SessionHandlerPHP.php:72 (SimpleSAML\\SessionHandlerPHP::__construct)
ERR [TRa2da8fcc] 5 /var/www/simplesamlphp/lib/SimpleSAML/SessionHandler.php:134 (SimpleSAML\\SessionHandler::createSessionHandler)
ERR [TRa2da8fcc] 4 /var/www/simplesamlphp/lib/SimpleSAML/SessionHandler.php:41 (SimpleSAML\\SessionHandler::getSessionHandler)
ERR [TRa2da8fcc] 3 /var/www/simplesamlphp/lib/SimpleSAML/Session.php:336 (SimpleSAML\\Session::getSession)
ERR [TRa2da8fcc] 2 /var/www/simplesamlphp/lib/SimpleSAML/Session.php:268 (SimpleSAML\\Session::getSessionFromRequest)
ERR [TRa2da8fcc] 1 /var/www/simplesamlphp/lib/SimpleSAML/Logger.php:268 (SimpleSAML\\Logger::flush)
ERR [TRa2da8fcc] 0 [builtin] (N/A)

Additional context

There are previous closed issues with similar reports, like #1092 , #1006 .

It seems that the flush function in lib/SimpleSAML/Logger.php tries to create a session, and on pages that don't use sessions this causes the sending of headers after output is already sent.

I'm not if sure this is the root cause, there could be a problem with the log handler or somewhere else in the session handling.

A possible solution I found (suggested by @dbollaer) is checking for a session before, in the defer funcion (which causes the call of flush later):

diff --git a/lib/SimpleSAML/Logger.php b/lib/SimpleSAML/Logger.php
index c18276ec..410de5be 100644
--- a/lib/SimpleSAML/Logger.php
+++ b/lib/SimpleSAML/Logger.php
@@ -335,6 +335,7 @@ class Logger
      */
     private static function defer($level, $message, $stats)
     {
+        // so that the headers are not sent by flush
+        $session = \SimpleSAML\Session::getSessionFromRequest();
         // save the message for later
         self::$earlyLog[] = ['level' => $level, 'string' => $message, 'statsLog' => $stats];

I'm not well versed in SimpleSAML (or even PHP), but this fixed the warning for me.

toth-dev added a commit to toth-dev/simplesamlphp that referenced this issue Jul 31, 2019
toth-dev added a commit to toth-dev/simplesamlphp that referenced this issue Sep 2, 2019
Fixes simplesamlphp#1168

Co-authored-by: Jaime Pérez Crespo <jaime.perez@uninett.no>
@jaimeperez
Copy link
Member

This should be resolved now.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants