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

Commit 266e156

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

File tree

7 files changed

+144
-16
lines changed

7 files changed

+144
-16
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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ public function onKernelResponse(FilterResponseEvent $event)
6161
$session = $event->getRequest()->getSession();
6262
if ($session && $session->isStarted()) {
6363
$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-
}
64+
}
65+
if ($session && ($session instanceof Session && \method_exists($session, 'isEmpty') ? !$session->isEmpty() : $session->isStarted())) {
66+
$params = session_get_cookie_params();
67+
$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']));
6868
}
6969
}
7070

‎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: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
namespace Symfony\Component\HttpKernel\EventListener;
1313

1414
use Psr\Container\ContainerInterface;
15+
use Symfony\Component\HttpFoundation\Session\Session;
16+
use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;
17+
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
18+
use Symfony\Component\HttpKernel\KernelEvents;
1519

1620
/**
1721
* Sets the session in the request.
@@ -29,6 +33,33 @@ public function __construct(ContainerInterface $container)
2933
$this->container = $container;
3034
}
3135

36+
public function onKernelResponse(FilterResponseEvent $event)
37+
{
38+
if (!$event->isMasterRequest()) {
39+
return;
40+
}
41+
42+
if (!$session = $event->getRequest()->getSession()) {
43+
return;
44+
}
45+
46+
$sessionWasStarted = $session->isStarted();
47+
if ($sessionWasStarted || !$session instanceof Session) {
48+
// no-op
49+
} elseif (\method_exists($session, 'wasStarted') && $session->wasStarted()) {
50+
$sessionWasStarted = true;
51+
} elseif (\method_exists($session, 'isEmpty') && !$session->isEmpty()) {
52+
$sessionWasStarted = true;
53+
}
54+
55+
if ($sessionWasStarted) {
56+
$event->getResponse()
57+
->setPrivate()
58+
->setMaxAge(0)
59+
->headers->addCacheControlDirective('must-revalidate');
60+
}
61+
}
62+
3263
protected function getSession()
3364
{
3465
if (!$this->container->has('session')) {
@@ -37,4 +68,12 @@ protected function getSession()
3768

3869
return $this->container->get('session');
3970
}
71+
72+
public static function getSubscribedEvents()
73+
{
74+
return parent::getSubscribedEvents() + array(
75+
// low priority to come after regular response listeners
76+
KernelEvents::RESPONSE => array(array('onKernelResponse', -1000)),
77+
);
78+
}
4079
}

‎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