-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation] fix accessing session bags #32137
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
Conversation
xabbuh
commented
Jun 22, 2019
Q | A |
---|---|
Branch? | 3.4 |
Bug fix? | yes |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #30682 |
License | MIT |
Doc PR |
return $this->storage->getBag($name)->getBag(); | ||
$bag = $this->storage->getBag($name); | ||
|
||
return method_exists($bag, 'getBag') ? $bag->getBag() : $bag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not test against SessionBagProxy
to make the intention clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, otherwise i'd prefer to deprecate the situation (calling getBag on a non-proxy bag).
But i tend to believe one relies on a bug in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how many people out there created their own SessionBagInterface
implementations and also implemented this method this method to work around this issue. IMO we should play safe here to not risk any existing application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me.
Thank you @xabbuh. |
This PR was merged into the 3.4 branch. Discussion ---------- [HttpFoundation] fix accessing session bags | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #30682 | License | MIT | Doc PR | Commits ------- 7a4570d fix accessing session bags