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

Commit 9dffde8

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

File tree

8 files changed

+95
-14
lines changed

8 files changed

+95
-14
lines changed

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

Lines changed: 16 additions & 1 deletion
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 $hasStarted = false;
3233

3334
/**
3435
* @param SessionStorageInterface $storage A SessionStorageInterface instance
@@ -53,6 +54,8 @@ public function __construct(SessionStorageInterface $storage = null, AttributeBa
5354
*/
5455
public function start()
5556
{
57+
$this->hasStarted = true;
58+
5659
return $this->storage->start();
5760
}
5861

@@ -140,6 +143,16 @@ public function count()
140143
return count($this->getAttributeBag()->all());
141144
}
142145

146+
/**
147+
* @return bool
148+
*
149+
* @internal
150+
*/
151+
public function hasStarted()
152+
{
153+
return $this->hasStarted;
154+
}
155+
143156
/**
144157 8000
* @return bool
145158
*
@@ -235,6 +248,8 @@ public function registerBag(SessionBagInterface $bag)
235248
*/
236249
public function getBag($name)
237250
{
251+
$this->hasStarted = true;
252+
238253
return $this->storage->getBag($name)->getBag();
239254
}
240255

@@ -257,6 +272,6 @@ public function getFlashBag()
257272
*/
258273
private function getAttributeBag()
259274
{
260-
return $this->storage->getBag($this->attributeName)->getBag();
275+
return $this->getBag($this->attributeName);
261276
}
262277
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
namespace Symfony\Component\HttpKernel\EventListener;
1313

14+
use Symfony\Component\HttpFoundation\Session\Session;
1415
use Symfony\Component\HttpFoundation\Session\SessionInterface;
16+
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
1517
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
1618
use Symfony\Component\HttpKernel\KernelEvents;
1719
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -38,10 +40,27 @@ public function onKernelRequest(GetResponseEvent $event)
3840
$request->setSession($session);
3941
}
4042

43+
public function onKernelResponse(FilterResponseEvent $event)
44+
{
45+
if (!$event->isMasterRequest()) {
46+
return;
47+
}
48+
49+
$session = $event->getRequest()->getSession();
50+
if ($session instanceof Session && $session->hasStarted()) {
51+
$event->getResponse()
52+
->setPrivate()
53+
->setMaxAge(0)
54+
->headers->addCacheControlDirective('must-revalidate');
55+
}
56+
}
57+
4158
public static function getSubscribedEvents()
4259
{
4360
return array(
4461
KernelEvents::REQUEST => array('onKernelRequest', 128),
62+
// low priority to come after regular response listeners
63+
KernelEvents::RESPONSE => array(array('onKernelResponse', -128)),
4564
);
46 F438 65
}
4766

‎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 ? $session->hasStarted() && !$session->isEmpty() : $sessions->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/Tests/EventListener/AbstractSessionListenerTest.php

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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\HttpFoundation\Request;
16+
use Symfony\Component\HttpFoundation\Response;
17+
use Symfony\Component\HttpFoundation\Session\Session;
18+
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
19+
use Symfony\Component\HttpKernel\EventListener\AbstractSessionListener;
20+
use Symfony\Component\HttpKernel\HttpKernelInterface;
21+
22+
class AbstractSessionListenerTest extends TestCase
23+
{
24+
public function testOnlyTriggeredOnMasterRequest()
25+
{
26+
$listener = $this->getMockForAbstractClass(AbstractSessionListener::class);
27+
$event = $this->getMockBuilder(FilterResponseEvent::class)->disableOriginalConstructor()->getMock();
28+
$event->expects($this->once())->method('isMasterRequest')->willReturn(false);
29+
$event->< EED3 span class=pl-en>expects($this->never())->method('getRequest');
30+
31+
// sub request
32+
$listener->onKernelResponse($event);
33+
}
34+
35+
public function testResponseIsPrivate()
36+
{
37+
$listener = $this->getMockForAbstractClass(AbstractSessionListener::class);
38+
$kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();
39+
40+
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
41+
$session->expects($this->once())->method('hasStarted')->willReturn(true);
42+
43+
$request = new Request();
44+
$request->setSession($session);
45+
$response = new Response();
46+
$listener->onKernelResponse(new FilterResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response));
47+
48+
$this->assertTrue($response->headers->hasCacheControlDirective('private'));
49+
$this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
50+
$this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
51+
}
52+
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -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/TestSessionListenerTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ private function sessionMustBeSaved()
131131

132132
private function sessionHasBeenStarted()
133133
{
134+
$this->session->expects($this->any())
135+
->method('hasStarted')
136+
->will($this->returnValue(true));
134137
$this->session->expects($this->once())
135138
->method('isStarted')
136139
->will($this->returnValue(true));

‎src/Symfony/Component/HttpKernel/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"require": {
1919
"php": "^5.5.9|>=7.0.8",
2020
"symfony/event-dispatcher": "~2.8|~3.0|~4.0",
21-
"symfony/http-foundation": "^3.3.11|~4.0",
21+
"symfony/http-foundation": "^3.4.4|^4.0.4",
2222
"symfony/debug": "~2.8|~3.0|~4.0",
2323
"psr/log": "~1.0"
2424
},

0 commit comments

Comments
 (0)
0