[HttpFoundation] Do not return an empty session id if the session was closed #9625
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
fabpot
added a commit
that referenced
this pull request
Nov 28, 2013
…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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A regression from the bug fix #9246 was introduced due to an incomplete fix ;
As the
startedflag on the NativeSessionStorage was nottrueanymore when saving the session, the session id was always empty when saving it, and thus when sending thePHPSESSIDcookie.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
startedis 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.