[FrameworkBundle][HttpFoundation] make session service resettable #30620
Conversation
dmaicher
force-pushed
the
issue-30619
branch
3 times, most recently
from
6e670cd
to
08b1dae
Compare
|
Before merging: the way tests are written is not compatible with phpunit 4.8. |
@nicolas-grekas because of I find quite a lot of usages of that in the 3.4 branch. |
|
Thank you @dmaicher. |
…ettable (dmaicher) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle][HttpFoundation] make session service resettable | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #30619 | License | MIT | Doc PR | - This fixes #30619 by making the session service "resettable" via `ServicesResetter`. Commits ------- e46ef76 [FrameworkBundle][HttpFoundation] make session service resettable
|
This caused a critical issue in Drupal 8. |
|
@Chi-teck sorry to hear that. Please open an issue if you think we should discuss/track this. |
|
The issue in drupal was that we use a custom session storage such that if a user is not logged into the website then we don't start the session UNTIL something is saved to prevent setting cookies unnecessarily. Since our storage save function isn't called anymore until the session is started, we don't save the session at all anymore. Our initial workaround was to adjust our isStarted method to return true after bootstrap, but we are finding that is causing problems in other places that depend on isStarted() to tell us whether the session is actually started or not. Might it make sense to move the responsibility for checking if the session is started from Session::save and into SessionStorageInterface::save? This would avoid the BC issues for custom storage that depended on managing whether they should save or not on their own, but still allow for the bug fix for the bundled storage classes. |
…session service resettable (dmaicher)" (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- Revert "bug #30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)" | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This reverts commit 029fb2e, reversing changes made to 9dad29d. Reverts #30620 Replaces #31215 We don't need to solve this in 3.4 Making the session resettable should be done on master, by implementing `ResetInterface`. On 3.4 apps, one should write a dedicated `SessionResetter` that would implement the reverted logic. Commits ------- 4177331 Revert "bug #30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)"
* 3.4: Revert "bug #30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)" [Workflow] Fixed dumping when many transition with same name exist fix ConsoleFormatter - call to a member function format() on string
* 4.2: Revert "bug #30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)" [Workflow] Fixed dumping when many transition with same name exist relax assertions in tests fix ConsoleFormatter - call to a member function format() on string Made `debug:container` and `debug:autowiring` ignore starting backslash in service id [Validator] Translate messages into Japanese Fix Thai translation in validators.th.xlf [FramworkBundle] mark any env vars found in the ide setting as used
This fixes #30619 by making the session service "resettable" via
ServicesResetter.