8000 [HttpKernel] Fix session handling: decouple "save" from setting respo… · symfony/symfony@0cf2c68 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0cf2c68

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpKernel] Fix session handling: decouple "save" from setting response "private"
1 parent f95ac4f commit 0cf2c68

File tree

7 files changed

+140
-18
lines changed
  • SessionListener.php
  • Tests/EventListener

    • 7 files changed

    +140
    -18
    lines changed

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

    Lines changed: 13 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -29,6 +29,7 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
    2929
    private $flashName;
    3030
    private $attributeName;
    3131
    private $data = array();
    32+
    private $wasStarted;
    3233

    3334
    /**
    3435
    * @param SessionStorageInterface $storage A SessionStorageInterface instance
    @@ -140,6 +141,16 @@ public function count()
    140141
    return count($this->getAttributeBag()->all());
    141142
    }
    142143

    144+
    /**
    145+
    * @return bool
    146+
    *
    147+
    * @internal
    148+
    */
    149+
    public function wasStarted()
    150+
    {
    151+
    return $this->wasStarted;
    152+
    }
    153+
    143154
    /**
    144155
    * @return bool
    145156
    *
    @@ -227,7 +238,7 @@ public function getMetadataBag()
    227238
    */
    228239
    public function registerBag(SessionBagInterface $bag)
    229240
    {
    230-
    $this->storage->registerBag(new SessionBagProxy($bag, $this->data));
    241+
    $this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->wasStarted));
    231242
    }
    232243

    233244
    /**
    @@ -257,6 +268,6 @@ public function getFlashBag()
    257268
    */
    258269
    private function getAttributeBag()
    259270
    {
    260-
    return $this->storage->getBag($this->attributeName)->getBag();
    271+
    return $this->getBag($this->attributeName);
    261272
    }
    262273
    }

    ‎src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php

    Lines changed: 4 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -20,11 +20,13 @@ final class SessionBagProxy implements SessionBagInterface
    2020
    {
    2121
    private $bag;
    2222
    private $data;
    23+
    private $wasStarted;
    2324

    24-
    public function __construct(SessionBagInterface $bag, array &$data)
    25+
    public function __construct(SessionBagInterface $bag, array &$data, &$wasStarted)
    2526
    {
    2627
    $this->bag = $bag;
    2728
    $this->data = &$data;
    29+
    $this->wasStarted = &$wasStarted;
    2830
    }
    2931

    3032
    /**
    @@ -56,6 +58,7 @@ public function getName()
    5658
    */
    5759
    public function initialize(array &$array)
    5860
    {
    61+
    $this->wasStarted = true;
    5962
    $this->data[$this->bag->getStorageKey()] = &$array;
    6063

    6164
    $this->bag->initialize($array);

    ‎src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php

    Lines changed: 10 additions & 6 deletions
    Original file line numberDiff line numberDiff line change
    @@ -58,13 +58,17 @@ public function onKernelResponse(FilterResponseEvent $event)
    5858
    return;
    5959
    }
    6060

    61-
    $session = $event->getRequest()->getSession();
    62-
    if ($session && $session->isStarted()) {
    61+
    if (!$session = $event->getRequest()->getSession()) {
    62+
    return;
    63+
    }
    64+
    65+
    if ($wasStarted = $session->isStarted()) {
    6366
    $session->save();
    64-
    if (!$session instanceof Session || !\method_exists($session, 'isEmpty') || !$session->isEmpty()) {
    65-
    $params = session_get_cookie_params();
    66-
    $event->getResponse()->headers->setCookie(new Cookie($session->getName(), $session->getId(), 0 === $params['lifetime'] ? 0 : time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly']));
    67-
    }
    67+
    }
    68+
    69+
    if ($session instanceof Session && \method_exists($session, 'isEmpty') ? !$session->isEmpty() : $wasStarted) {
    70+
    $params = session_get_cookie_params();
    71+
    $event->getResponse()->headers->setCookie(new Cookie($session->getName(), $session->getId(), 0 === $params['lifetime'] ? 0 : time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly']));
    6872
    }
    6973
    }
    7074

    ‎src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php

    Lines changed: 0 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -53,10 +53,6 @@ public function onKernelResponse(FilterResponseEvent $event)
    5353
    $session = $event->getRequest()->getSession();
    5454
    if ($session && $session->isStarted()) {
    5555
    $session->save();
    56-
    $event->getResponse()
    57-
    ->setPrivate()
    58-
    ->setMaxAge(0)
    59-
    ->headers->addCacheControlDirective('must-revalidate');
    6056
    }
    6157
    }
    6258

    ‎src/Symfony/Component/HttpKernel/EventListener/SessionListener.php

    Lines changed: 29 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -12,6 +12,9 @@
    1212
    namespace Symfony\Component\HttpKernel\EventListener;
    1313

    1414
    use Psr\Container\ContainerInterface;
    15+
    use Symfony\Component\HttpFoundation\Session\Session;
    16+
    use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
    17+
    use Symfony\Component\HttpKernel\KernelEvents;
    1518

    1619
    /**
    1720
    * Sets the session in the request.
    @@ -29,6 +32,24 @@ public function __construct(ContainerInterface $container)
    2932
    $this->container = $container;
    3033
    }
    3134

    35+
    public function onKernelResponse(FilterResponseEvent $event)
    36+
    {
    37+
    if (!$event->isMasterRequest()) {
    38+
    return;
    39+
    }
    40+
    41+
    if (!$session = $event->getRequest()->getSession()) {
    42+
    return;
    43+
    }
    44+
    45+
    if ($session->isStarted() || ($session instanceof Session && ((\method_exists($session, 'wasStarted') && $session->wasStarted()) || (\method_exists($session, 'isEmpty') && !$session->isEmpty())))) {
    46+
    $event->getResponse()
    47+
    ->setPrivate()
    48+
    ->setMaxAge(0)
    49+
    ->headers->addCacheControlDirective('must-revalidate');
    50+
    }
    51+
    }
    52+
    3253
    protected function getSession()
    3354
    {
    3455
    if (!$this->container->has('session')) {
    @@ -37,4 +58,12 @@ protected function getSession()
    3758

    3859
    return $this->container->get('session');
    3960
    }
    61+
    62+
    public static function getSubscribedEvents()
    63+
    {
    64+
    return parent::getSubscribedEvents() + array(
    65+
    // low priority to come after regular response listeners
    66+
    KernelEvents::RESPONSE => array(array('onKernelResponse', -1000)),
    67+
    );
    68+
    }
    4069
    }

    ‎src/Symfony/Component/HttpKernel/Tests/EventListener/SaveSessionListenerTest.php

    Lines changed: 1 addition & 5 deletions
    Original file line numberDiff line numberDiff line change
    @@ -32,7 +32,7 @@ public function testOnlyTriggeredOnMasterRequest()
    3232
    $listener->onKernelResponse($event);
    3333
    }
    3434

    35-
    public function testSessionSavedAndResponsePrivate()
    35+
    public function testSessionSaved()
    3636
    {
    3737
    $listener = new SaveSessionListener();
    3838
    $kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();
    @@ -45,9 +45,5 @@ public function testSessionSavedAndResponsePrivate()
    4545
    $request->setSession($session);
    4646
    $response = new Response();
    4747
    $listener->onKernelResponse(new FilterResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response));
    48-
    49-
    $this->assertTrue($response->headers->hasCacheControlDirective('private'));
    50-
    $this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
    51-
    $this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
    5248
    }
    5349
    }

    ‎src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php

    Lines changed: 83 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -0,0 +1,83 @@
    1+
    <?php
    2+
    3+
    /*
    4+
    * This file is part of the Symfony package.
    5+
    *
    6+
    * (c) Fabien Potencier <fabien@symfony.com>
    7+
    *
    8+
    * For the full copyright and license information, please view the LICENSE
    9+
    * file that was distributed with this source code.
    10+
    */
    11+
    12+
    namespace Symfony\Component\HttpKernel\Tests\EventListener;
    13+
    14+
    use PHPUnit\Framework\TestCase;
    15+
    use Symfony\Component\DependencyInjection\Container;
    16+
    use Symfony\Component\HttpFoundation\Request;
    17+
    use Symfony\Component\HttpFoundation\Response;
    18+
    use Symfony\Component\HttpFoundation\Session\Session;
    19+
    use Symfony\Component\HttpKernel\Event\GetResponseEvent;
    20+
    use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
    21+
    use Symfony\Component\HttpKernel\EventListener\AbstractSessionListener;
    22+
    use Symfony\Component\HttpKernel\EventListener\SessionListener;
    23+
    use Symfony\Component\HttpKernel\HttpKernelInterface;
    24+
    25+
    class SessionListenerTest extends TestCase
    26+
    {
    27+
    public function testOnlyTriggeredOnMasterRequest()
    28+
    {
    29+
    $listener = $this->getMockForAbstractClass(AbstractSessionListener::class);
    30+
    $event = $this->getMockBuilder(GetResponseEvent::class)->disableOriginalConstructor()->getMock();
    31+
    $event->expects($this->once())->method('isMasterRequest')->willReturn(false);
    32+
    $event->expects($this->never())->method('getRequest');
    33+
    34+
    // sub request
    35+
    $listener->onKernelRequest($event);
    36+
    }
    37+
    38+
    public function testSessionIsSet()
    39+
    {
    40+
    $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
    41+
    42+
    $container = new Container();
    43+
    $container->set('session', $session);
    44+
    45+
    $request = new Request();
    46+
    $listener = new SessionListener($container);
    47+
    48+
    $event = $this->getMockBuilder(GetResponseEvent::class)->disableOriginalConstructor()->getMock();
    49+
    $event->expects($this->once())->method('isMasterRequest')->willReturn(true);
    50+
    $event->expects($this->once())->method('getRequest')->willReturn($request);
    51+
    52+
    $listener->onKernelRequest($event);
    53+
    54+
    $this->assertTrue($request->hasSession());
    55+
    $this->assertSame($session, $request->getSession());
    56+
    }
    57+
    58+
    public function testResponseIsPrivate()
    59+
    {
    60+
    $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
    61+
    $session->expects($this->once())->method('isStarted')->willReturn(false);
    62+
    if (method_exists($session, 'wasStarted')) {
    63+
    $session->expects($this->once())->method('wasStarted')->willReturn(true);
    64+
    } else {
    65+
    $session->expects($this->once())->method('isEmpty')->willReturn(false);
    66+
    }
    67+
    68+
    $container = new Container();
    69+
    $container->set('session', $session);
    70+
    71+
    $listener = new SessionListener($container);
    72+
    $kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();
    73+
    74+
    $request = new Request();
    75+
    $response = new Response();
    76+
    $listener->onKernelRequest(new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST));
    77+
    $listener->onKernelResponse(new FilterResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response));
    78+
    79+
    $this->assertTrue($response->headers->hasCacheControlDirective('private'));
    80+
    $this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
    81+
    $this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
    82+
    }
    83+
    }

    0 commit comments

    Comments
     (0)
    0