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

Warning: ini_set(): A session is active. You cannot change the session module's ini settings at this time #3898

Closed
shefik opened this Issue Apr 14, 2018 · 14 comments

Comments

Projects
None yet
3 participants
@shefik
Contributor

shefik commented Apr 14, 2018

Q A
Zikula Version 2.0.7
PHP Version 7.2.1

Expected behavior

Error message should not display.

Actual behavior

Error:

Warning: ini_set(): A session is active. You cannot change the session module's ini settings at this time (500 Internal Server Error)

Steps to reproduce

Try to login. The message appears immediately after the form is submitted.

@shefik

This comment has been minimized.

Contributor

shefik commented Jun 2, 2018

Issue persists with Zikula 2.0.7.

@Guite

This comment has been minimized.

Member

Guite commented Jun 20, 2018

I've just looked into what happens here.

First, the problem is caused by that ini_set() is used to manipulate a session related PHP option after a session already has been started.

The concrete place where ini_set() is being executed is in Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage#regenerate($destroy = false, $lifetime = null) which calls: ini_set('session.cookie_lifetime', $lifetime);

Basically when a login is executed our access controller calls an access helper at https://github.com/zikula/core/blob/2.0/src/system/UsersModule/Controller/AccessController.php#L129 which then calls the session migrate method in https://github.com/zikula/core/blob/2.0/src/system/UsersModule/Helper/AccessHelper.php#L112 - this migrate call calls the NativeSessionStorage#regenerate() method shown above.

So, we have two options to get rid of this problem:

  1. Easy but dirty
    We could simply replace
$this->session->migrate(true, $lifetime);

by

$reportingLevel = error_reporting(E_ALL & ~E_WARNING);
$this->session->migrate(true, $lifetime);
error_reporting($reportingLevel);

to suppress the warning.

  1. Clean but not that easy
    For a better solution we would need to avoid starting the session at all before the migrate call is done.
    Of course we could easily move https://github.com/zikula/core/blob/2.0/src/system/UsersModule/Controller/AccessController.php#L128 one line further below. But how to handle things like https://github.com/zikula/core/blob/2.0/src/system/UsersModule/Controller/AccessController.php#L48 where the session is accessed already much earlier in the controller code?

@craigh

@craigh

This comment has been minimized.

Member

craigh commented Jun 21, 2018

this happens on a clean install with no additional modules, etc?

@shefik

This comment has been minimized.

Contributor

shefik commented Jun 21, 2018

I’ve only tried it on upgrade, and it happens there.

@Guite

This comment has been minimized.

Member

Guite commented Jun 21, 2018

this happens on a clean install with no additional modules, etc?

Yes.

@Guite

This comment has been minimized.

Member

Guite commented Jun 21, 2018

Just tried again to verify no additional bundles are involved:

  1. did a fresh install using PHP 7.2 (I currently have 7.2.5-0ubuntu0.18.04.1 here)
  2. set debug to true in app/config/custom_parameters.yml
  3. logout
  4. login

Important: it is not reproducible if debug is not set to true.

@craigh

This comment has been minimized.

Member

craigh commented Jun 21, 2018

don't remember this happening in earlier releases. What changed?

@Guite

This comment has been minimized.

Member

Guite commented Jun 21, 2018

What changed?

I think this happens only in PHP 7.2. Never experienced it with 7.0 or 7.1.

@Guite

This comment has been minimized.

Member

Guite commented Jul 5, 2018

I added PHP 7.2 to Travis builds and this fails for 2.0 and master (for the 1.5 branch PHP 7 is allowed to fail, thus the build keep green).

Raising severity to blocker.

@Guite Guite added the Blocker label Jul 5, 2018

@Guite Guite added this to the 2.0.8 milestone Jul 5, 2018

@craigh

This comment has been minimized.

Member

craigh commented Jul 5, 2018

Admittedly I do not understand the problem. But it seems like this would also be a problem for symfony and therefor something they would have solved?

@Guite

This comment has been minimized.

Member

Guite commented Jul 5, 2018

Going to investigate this further asap.

@Guite Guite modified the milestones: 2.0.8, 1.5.8 Jul 6, 2018

@Guite Guite removed the Feedback required label Jul 6, 2018

@Guite Guite closed this in 226f895 Jul 6, 2018

Guite added a commit that referenced this issue Jul 6, 2018

@Guite

This comment has been minimized.

Member

Guite commented Jul 6, 2018

I decided to suppress this warning centrally in our session storage subclass. Not the cleanest solution, but first it does not have any side-effects and second it also works if any other session access is done from the outside (e.g. by an event subscriber or a listener of a module).

@Guite

This comment has been minimized.

Member

Guite commented Jul 6, 2018

While everything works now on my test sites, the problem still appears in Travis. Reopening...

@Guite Guite reopened this Jul 6, 2018

Guite added a commit that referenced this issue Jul 6, 2018

@Guite Guite referenced this issue Jul 6, 2018

Merged

Travis update #3914

@Guite

This comment has been minimized.

Member

Guite commented Jul 6, 2018

Fixed by #3914

@Guite Guite closed this Jul 6, 2018

Guite added a commit that referenced this issue Jul 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment