Skip to content

[HttpFoundation] PHP7.2 session migration error #28577

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

Closed
Flofr opened this issue Sep 24, 2018 · 12 comments
Closed

[HttpFoundation] PHP7.2 session migration error #28577

Flofr opened this issue Sep 24, 2018 · 12 comments

Comments

@Flofr
Copy link

Flofr commented Sep 24, 2018

Hello,

I upgrade my PHP version from PHP 5.6.36 to PHP 7.2, but I have a problem when I want to use the migrate function of Session component.

My simple line of code :
$this->request->getSession()->migrate(true, $newLifetime);

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

I have already a session opened before by my login page, but I want to change the session life time on login.
My Symfony version 2.8.44
Thanks for your help

@xabbuh
Copy link
Member

xabbuh commented Sep 24, 2018

Can you create a small example application that allows to reproduce your issue?

@Flofr
Copy link
Author

Flofr commented Sep 25, 2018

Thank for your help.
I do this soon as possible

@Flofr
Copy link
Author

Flofr commented Sep 25, 2018

This is the repo with the sample : https://github.com/Flofr/Bug-session-php7.2-sf2.8

You have to go on the login page /login with the login "test" and pwd "test"
The listener AuthenticationHandler change the session lifetime and throw the exception on PHP7.2

@Flofr
Copy link
Author

Flofr commented Oct 1, 2018

Hi,
Any news ? Did you reproduce the bug with my sources ?
I found another ticket in another project (using Symfony) where they talk about the same bug : zikula/core#3898

@Flofr
Copy link
Author

Flofr commented Oct 10, 2018

For helping you, the test in Symfony code is not complete.
This test is only for session migrate without parameter, but the bug is when you migrate with new lifetime.
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php

@xabbuh
Copy link
Member

xabbuh commented Oct 11, 2018

I am not sure what we can do here. Maybe deprecate being able to pass a lifetime here?

@Simperfit
Copy link
Contributor

@xabbuh is this still a bug ?

@xabbuh
Copy link
Member

xabbuh commented Jun 19, 2019

I think so.

@joeyhub
Copy link

joeyhub commented Jun 25, 2019

What's wrong with using session_set_cookie_params? https://www.php.net/manual/en/function.session-set-cookie-params.php

@joeyhub
Copy link

joeyhub commented Jun 25, 2019

Note that the manual might give the wrong impression that it's just a wrapper for ini_set, TIAS.

@joeyhub
Copy link

joeyhub commented Jun 25, 2019

You're probably going to just have to session destroy then recreate or something.

$old = $_SESSION;

if ($destroy) {
 session_destroy();
} else {
 if ($howDoesRegenWork) {
  session_abort();
 } else {
  session_write_close();
 }
}

session_set_cookie_params($timeout);
session_start();
$_SESSION = $old;

Set what that does, otherwise may just have to look at the PHP source. Hopefully nothing unpleasant would be needed such as session_encode/session_decode, etc. Might want some other hacks for other places like session_id to set, etc.

Personally I usually end up just not using the PHP handler for sessions as it has been causing me headaches since 2008. You don't get proper locking, it's stuck as a singleton, etc. Don't know why you would make a framework only to fallback to a flawed by design session API. It's only good for prototyping, mockups and simple use cases.

@Guite
Copy link
Contributor

Guite commented Jan 24, 2020

We had a quite hard fight with this one (zikula/core#3898, zikula/core#3986, zikula/core#4078), but now I think this is solved.

You're probably going to just have to session destroy then recreate or something.

Yes. I finally found a fix that seems to work.

Background of the problem:

First, the NativeSessionStorage::regenerate method returns if there is no active session (see here). Afterwards it tries to set a given custom lifetime using ini_set (see here) which causes the original problem (Warning: ini_set(): A session is active. You cannot change the session module's ini settings at this time).

So what I did is this: zikula/core@e3b9d42...c7e625a It looks a bit confusing because I had to duplicate some code of the class in my child class (due to #35460).

The actual fix (it looks so simple) is replacing:

ini_set('session.cookie_lifetime', $lifetime);

by:

$this->save();
ini_set('session.cookie_lifetime', $lifetime);
$this->start();

nicolas-grekas added a commit that referenced this issue Apr 5, 2020
…e lifetime (Guite)

This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Fixed session migration with custom cookie lifetime

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #28577
| License       | MIT
| Doc PR        |

This PR adds the fix proposed in #28577 (comment)

Commits
-------

3e824de [HttpFoundation] Fixed session migration with custom cookie lifetime
symfony-splitter pushed a commit to symfony/http-foundation that referenced this issue Apr 5, 2020
…e lifetime (Guite)

This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Fixed session migration with custom cookie lifetime

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #28577
| License       | MIT
| Doc PR        |

This PR adds the fix proposed in symfony/symfony#28577 (comment)

Commits
-------

3e824de385 [HttpFoundation] Fixed session migration with custom cookie lifetime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants