Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpFoundation] Fix regression of delegation of Session::save() to Storage::Save()#31215
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from1 commit
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -219,6 +219,12 @@ public function regenerate($destroy = false, $lifetime = null) | ||
| */ | ||
| public function save() | ||
| { | ||
| // In PHP <7.2 session_write_close() does not error if the session is | ||
| ||
| // not started. | ||
| if (!$this->started) { | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I thought about this approach as well. I hesitated to go this way because of the phpdoc on the interface: Not sure if this would not also be a BC break as someone is relying on this? 😕 Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Actually it seems this see#6362 Edit: but anyway this means if a storage is used that throws an exception on save without being started then this would fail now if the kernel is reset on a non-started (but instantiated) session. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Can we catch that exception in Session::save() and convert it to a warning or deprecation error saying that the storage needs to handle an attempt to save a reset session on its own? The behavior that you describe is what existed prior to this BC break in 3.4.24 for custom storage, prior to that they would have had to handle a reset session on their own anyway. | ||
| return; | ||
| } | ||
| $session = $_SESSION; | ||
| foreach ($this->bags as $bag) { | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -261,13 +261,4 @@ public function testIsEmpty() | ||
| $this->assertTrue($this->session->isEmpty()); | ||
| } | ||
| ||
| } | ||