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

patgrudniewski · 2019-03-25T11:46:04Z

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).

The text was updated successfully, but these errors were encountered:

ro0NL · 2019-03-25T12:04:41Z

i think we should test for SessionBagProxy first 👍

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

Simperfit · 2019-06-21T21:01:56Z

@ro0NL what do you mean ?

ro0NL · 2019-06-22T04:00:09Z

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:

symfony/src/Symfony/Component/HttpFoundation/Session/Session.php

Lines 246 to 249 in 3bbf9da

    
           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 · 2019-06-22T06:13:51Z

Ok I understand now ! I think that’s a good idea ! I think the only way to be sure is by making a PR and see of people agree with you ;). ( I do) Le sam. 22 juin 2019 à 06:00, Roland Franssen <notifications@github.com> a écrit :

…

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: https://github.com/symfony/symfony/blob/3bbf9da47bdf79d175f84a9a9c73a5d7d4f66beb/src/Symfony/Component/HttpFoundation/Session/Session.php#L246-L249 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; — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#30682?email_source=notifications&email_token=AA2KV4QAXZB7GUUPXBP7RDLP3WPV3A5CNFSM4HA2YE5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYJ7SXA#issuecomment-504625500>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA2KV4XK5R34EUHZZGCBNNDP3WPV3ANCNFSM4HA2YE5A> .

xabbuh · 2019-06-22T20:14:12Z

I agree with the reasoning here, see #32137

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

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

xabbuh added Bug HttpFoundation labels Mar 25, 2019

carsonbot added the Status: Needs Review label Mar 25, 2019

xabbuh mentioned this issue Jun 22, 2019

[HttpFoundation] fix accessing session bags #32137

Merged

fabpot closed this as completed Jun 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

patgrudniewski commented Mar 25, 2019

ro0NL commented Mar 25, 2019

Simperfit commented Jun 21, 2019

ro0NL commented Jun 22, 2019

Simperfit commented Jun 22, 2019 via email

xabbuh commented Jun 22, 2019

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

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

Comments

patgrudniewski commented Mar 25, 2019

ro0NL commented Mar 25, 2019

Simperfit commented Jun 21, 2019

ro0NL commented Jun 22, 2019

Simperfit commented Jun 22, 2019 via email

xabbuh commented Jun 22, 2019