Skip to content
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

[HttpFoundation] Session::getBag($name) does not match Storage\SessionStorageInterface::getBag($name) #30682

Closed
patgrudniewski opened this issue Mar 25, 2019 · 5 comments

Comments

@patgrudniewski
Copy link

Symfony version(s) affected: 3.4.0 and higher

Description
There is getBag method at Symfony\Component\HttpFoundation\Session which calls $this->storage->getBag($name)->getBag().

As defined in constructor $this->storage is instance of Symfony\Component\HttpFoundation\Session\Storage\SessionStorageInterface which contains method getBag(string $name). As defined in comment it is expected that SessionStorageInterface::getBag(string $name) should return instance of Symfony\Component\HttpFoundation\Session\SessionBagInterface.

The problem is that SessionBagInterface does not contain getBag method so it's impossible to expect that $this->storage->getBag($name) will return object containing getBag() method.

How to reproduce

use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;

$bag = new AtrributeBag();
$bag->setName('foo');

$storage = new MockArraySessionStorage();
$storage->registerBag($bag);

$session = new Session($storage);
$session->getBag('foo'); // this will effect with "Error: Call to undefined method Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag::getBag()

Possible Solution
I'm not sure, probably removing second getBag call from Session::getBag($name).

@patgrudniewski patgrudniewski changed the title HttpFoundation\Session::getBag($name) does not match HttpFoundation\Storage\SessionStorageInterface::getBag($name) [HttpFoundation] Session::getBag($name) does not match Storage\SessionStorageInterface::getBag($name) Mar 25, 2019
@ro0NL
Copy link
Contributor

ro0NL commented Mar 25, 2019

i think we should test for SessionBagProxy first 👍

probably it's intended to use SessionInterface::registerBag() as main entrypoint (which wraps the bag)

@Simperfit
Copy link
Contributor

@ro0NL what do you mean ?

@ro0NL
Copy link
Contributor

ro0NL commented Jun 22, 2019

There's a difference between registering a bag on the storage vs. on the session.

When registering a bag on the session it get's wrapped:

public function registerBag(SessionBagInterface $bag)
{
$this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->usageIndex));
}

When you register thru MockArraySessionStorage it's not wrapped, however we call getBag($name)->getBag() from the Session which at this points expects a wrapped bag.

So i propose in Session::getBag() something like

return $bag instanceof SessionBagProxy ? $bag->getBag() : $bag;

@Simperfit
Copy link
Contributor

Simperfit commented Jun 22, 2019 via email

@xabbuh
Copy link
Member

xabbuh commented Jun 22, 2019

I agree with the reasoning here, see #32137

@fabpot fabpot closed this as completed Jun 26, 2019
fabpot added a commit that referenced this issue Jun 26, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants