Skip to content

Commit

Permalink
[HttpKernel] fix session tracking in surrogate master requests
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Jun 1, 2018
1 parent 143bdfc commit a15c14e
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 13 deletions.
19 changes: 14 additions & 5 deletions src/Symfony/Component/HttpFoundation/Session/Session.php
Expand Up @@ -29,7 +29,7 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
private $flashName;
private $attributeName;
private $data = array();
private $hasBeenStarted;
private $usageIndex = 0;

/**
* @param SessionStorageInterface $storage A SessionStorageInterface instance
Expand All @@ -54,6 +54,8 @@ public function __construct(SessionStorageInterface $storage = null, AttributeBa
*/
public function start()
{
++$this->usageIndex;

return $this->storage->start();
}

Expand Down Expand Up @@ -142,13 +144,13 @@ public function count()
}

/**
* @return bool
* @return int
*
* @internal
*/
public function hasBeenStarted()
public function getUsageIndex()
{
return $this->hasBeenStarted;
return $this->usageIndex;
}

/**
Expand All @@ -158,6 +160,7 @@ public function hasBeenStarted()
*/
public function isEmpty()
{
++$this->usageIndex;
foreach ($this->data as &$data) {
if (!empty($data)) {
return false;
Expand All @@ -182,6 +185,8 @@ public function invalidate($lifetime = null)
*/
public function migrate($destroy = false, $lifetime = null)
{
++$this->usageIndex;

return $this->storage->regenerate($destroy, $lifetime);
}

Expand All @@ -190,6 +195,8 @@ public function migrate($destroy = false, $lifetime = null)
*/
public function save()
{
++$this->usageIndex;

$this->storage->save();
}

Expand Down Expand Up @@ -230,6 +237,8 @@ public function setName($name)
*/
public function getMetadataBag()
{
++$this->usageIndex;

return $this->storage->getMetadataBag();
}

Expand All @@ -238,7 +247,7 @@ public function getMetadataBag()
*/
public function registerBag(SessionBagInterface $bag)
{
$this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->hasBeenStarted));
$this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->usageIndex));
}

/**
Expand Down
14 changes: 10 additions & 4 deletions src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php
Expand Up @@ -20,20 +20,22 @@ final class SessionBagProxy implements SessionBagInterface
{
private $bag;
private $data;
private $hasBeenStarted;
private $usageIndex;

public function __construct(SessionBagInterface $bag, array &$data, &$hasBeenStarted)
public function __construct(SessionBagInterface $bag, array &$data, &$usageIndex)
{
$this->bag = $bag;
$this->data = &$data;
$this->hasBeenStarted = &$hasBeenStarted;
$this->usageIndex = &$usageIndex;
}

/**
* @return SessionBagInterface
*/
public function getBag()
{
++$this->usageIndex;

return $this->bag;
}

Expand All @@ -42,6 +44,8 @@ public function getBag()
*/
public function isEmpty()
{
++$this->usageIndex;

return empty($this->data[$this->bag->getStorageKey()]);
}

Expand All @@ -58,7 +62,7 @@ public function getName()
*/
public function initialize(array &$array)
{
$this->hasBeenStarted = true;
++$this->usageIndex;
$this->data[$this->bag->getStorageKey()] = &$array;

$this->bag->initialize($array);
Expand All @@ -77,6 +81,8 @@ public function getStorageKey()
*/
public function clear()
{
++$this->usageIndex;

return $this->bag->clear();
}
}
Expand Up @@ -25,6 +25,8 @@
*/
abstract class AbstractSessionListener implements EventSubscriberInterface
{
private $sessionUsageStack = array();

public function onKernelRequest(GetResponseEvent $event)
{
if (!$event->isMasterRequest()) {
Expand All @@ -33,6 +35,7 @@ public function onKernelRequest(GetResponseEvent $event)

$request = $event->getRequest();
$session = $this->getSession();
$this->sessionUsageStack[] = $session instanceof Session ? $session->getUsageIndex() : null;
if (null === $session || $request->hasSession()) {
return;
}
Expand All @@ -50,20 +53,29 @@ public function onKernelResponse(FilterResponseEvent $event)
return;
}

if ($session->isStarted() || ($session instanceof Session && $session->hasBeenStarted())) {
if ($session instanceof Session ? $session->getUsageIndex() !== end($this->sessionUsageStack) : $session->isStarted()) {
$event->getResponse()
->setPrivate()
->setMaxAge(0)
->headers->addCacheControlDirective('must-revalidate');
}
}

/**
* @internal
*/
public function onFinishRequest()
{
array_pop($this->sessionUsageStack);
}

public static function getSubscribedEvents()
{
return array(
KernelEvents::REQUEST => array('onKernelRequest', 128),
// low priority to come after regular response listeners, same as SaveSessionListener
KernelEvents::RESPONSE => array('onKernelResponse', -1000),
KernelEvents::FINISH_REQUEST => array('onFinishRequest'),
);
}

Expand Down
Expand Up @@ -58,8 +58,7 @@ public function testSessionIsSet()
public function testResponseIsPrivate()
{
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
$session->expects($this->once())->method('isStarted')->willReturn(false);
$session->expects($this->once())->method('hasBeenStarted')->willReturn(true);
$session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));

$container = new Container();
$container->set('session', $session);
Expand All @@ -76,4 +75,38 @@ public function testResponseIsPrivate()
$this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
$this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
}

public function testSurrogateMasterRequestIsPublic()
{
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
$session->expects($this->exactly(4))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1, 1, 1));

$container = new Container();
$container->set('session', $session);

$listener = new SessionListener($container);
$kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();

$request = new Request();
$response = new Response();
$response->setCache(array('public' => true, 'max_age' => '30'));
$listener->onKernelRequest(new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST));
$this->assertTrue($request->hasSession());

$subRequest = clone $request;
$this->assertSame($request->getSession(), $subRequest->getSession());
$listener->onKernelRequest(new GetResponseEvent($kernel, $subRequest, HttpKernelInterface::MASTER_REQUEST));
$listener->onKernelResponse(new FilterResponseEvent($kernel, $subRequest, HttpKernelInterface::MASTER_REQUEST, $response));
$listener->onFinishRequest();

$this->assertFalse($response->headers->hasCacheControlDirective('private'));
$this->assertFalse($response->headers->hasCacheControlDirective('must-revalidate'));
$this->assertSame('30', $response->headers->getCacheControlDirective('max-age'));

$listener->onKernelResponse(new FilterResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response));

$this->assertTrue($response->headers->hasCacheControlDirective('private'));
$this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
$this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/composer.json
Expand Up @@ -18,7 +18,7 @@
"require": {
"php": "^5.5.9|>=7.0.8",
"symfony/event-dispatcher": "~2.8|~3.0|~4.0",
"symfony/http-foundation": "^3.4.4|^4.0.4",
"symfony/http-foundation": "~3.4.12|~4.0.12|^4.1.1",
"symfony/debug": "~2.8|~3.0|~4.0",
"symfony/polyfill-ctype": "~1.8",
"psr/log": "~1.0"
Expand Down

0 comments on commit a15c14e

Please sign in to comment.