[Session] fixed wrong started state #9246
Conversation
|
ping @Drak |
|
Looks ok. |
This PR was submitted for the master branch but it was merged into the 2.2 branch instead (closes #9246). Discussion ---------- [Session] fixed wrong started state Currently the session stays in the started mode after the call of ```save()``` which will close the session. We found the issue in our custom SAPI which keeps the symfony stack initialized over multiple requests. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- d641cd6 [Session] fixed wrong started states
|
This fixed #7936 |
|
Hi, It seems this may introduce a regression. I'm currently on 2.2.4 a(as I proceeded step by step through each versions), then the login seems to works well. But when I'm on the 2.2.10, I can't login, due to these messages : Indeed, it seems the I just commented the line But I don't think this should be the proper solution (it seems way too much tinkered)... Maybe I'm doing something wrong (like a wrong config, ... which is then not mentionned in the upgrade or changelogs), or this is indeed a regression. If you want, I may create another to discuss this point ? |
|
I have the same issue as Taluu. With same code and reverting back to v2.3.6 i don't have the issue |
This regression was introduced by symfony#9246 ; if this PR was to set the session to "not started" was it was saved (and thus closed), for an unknown reason, the PHPSESSID cookie was never created for some apps. But if we modify the isStarted method, and take into account the $this->closed state, then it should enact the sought behaviour without introducing this (possible) regression
This introduced a regression from symfony#9246, with an incomplete fix ; As the `started` flag on the NativeSessionStorage was not `true` anymore when saving the session, the session id was always empty when saving it, and thus when sending the `PHPSESSID` cookie
This introduced a regression from symfony#9246, with an incomplete fix ; As the `started` flag on the NativeSessionStorage was not `true` anymore when saving the session, the session id was always empty when saving it, and thus when sending the `PHPSESSID` cookie
…ession was closed (Taluu) This PR was merged into the 2.2 branch. Discussion ---------- [HttpFoundation] Do not return an empty session id if the session was closed | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #7936 | License | MIT | Doc PR | ~ A regression from the bug fix #9246 was introduced due to an incomplete fix ; As the `started` flag on the NativeSessionStorage was not `true` anymore when saving the session, the session id was always empty when saving it, and thus when sending the `PHPSESSID` cookie. The previous PR I made (#9530) was another approach to solve this regression, but this one should keep the fix made by @tecbot on #9246, and finish it with another verification. I may miss another place where `started` is used, but I don't really see which other conditions depending on this property should be altered... **Note : this is #9611 rebased on 2.2** This should be mergeable. Commits ------- 5b9a727 When getting the session's id, check if the session is not closed
Currently the session stays in the started mode after the call of
save()which will close the session. We found the issue in our custom SAPI which keeps the symfony stack initialized over multiple requests.