From a15c14ebc6e83cc6e5053ff36154bce7ffdbb094 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 1 Jun 2018 14:37:16 +0200 Subject: [PATCH] [HttpKernel] fix session tracking in surrogate master requests --- .../HttpFoundation/Session/Session.php | 19 +++++++--- .../Session/SessionBagProxy.php | 14 +++++-- .../EventListener/AbstractSessionListener.php | 14 ++++++- .../EventListener/SessionListenerTest.php | 37 ++++++++++++++++++- .../Component/HttpKernel/composer.json | 2 +- 5 files changed, 73 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Session/Session.php b/src/Symfony/Component/HttpFoundation/Session/Session.php index a46cffbb8dbd6..f0379c1697b82 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Session.php +++ b/src/Symfony/Component/HttpFoundation/Session/Session.php @@ -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 @@ -54,6 +54,8 @@ public function __construct(SessionStorageInterface $storage = null, AttributeBa */ public function start() { + ++$this->usageIndex; + return $this->storage->start(); } @@ -142,13 +144,13 @@ public function count() } /** - * @return bool + * @return int * * @internal */ - public function hasBeenStarted() + public function getUsageIndex() { - return $this->hasBeenStarted; + return $this->usageIndex; } /** @@ -158,6 +160,7 @@ public function hasBeenStarted() */ public function isEmpty() { + ++$this->usageIndex; foreach ($this->data as &$data) { if (!empty($data)) { return false; @@ -182,6 +185,8 @@ public function invalidate($lifetime = null) */ public function migrate($destroy = false, $lifetime = null) { + ++$this->usageIndex; + return $this->storage->regenerate($destroy, $lifetime); } @@ -190,6 +195,8 @@ public function migrate($destroy = false, $lifetime = null) */ public function save() { + ++$this->usageIndex; + $this->storage->save(); } @@ -230,6 +237,8 @@ public function setName($name) */ public function getMetadataBag() { + ++$this->usageIndex; + return $this->storage->getMetadataBag(); } @@ -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)); } /** diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php index 307836d5f9461..88005ee092e2a 100644 --- a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php @@ -20,13 +20,13 @@ 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; } /** @@ -34,6 +34,8 @@ public function __construct(SessionBagInterface $bag, array &$data, &$hasBeenSta */ public function getBag() { + ++$this->usageIndex; + return $this->bag; } @@ -42,6 +44,8 @@ public function getBag() */ public function isEmpty() { + ++$this->usageIndex; + return empty($this->data[$this->bag->getStorageKey()]); } @@ -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); @@ -77,6 +81,8 @@ public function getStorageKey() */ public function clear() { + ++$this->usageIndex; + return $this->bag->clear(); } } diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php index dff29ee80b418..f871152be9302 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php @@ -25,6 +25,8 @@ */ abstract class AbstractSessionListener implements EventSubscriberInterface { + private $sessionUsageStack = array(); + public function onKernelRequest(GetResponseEvent $event) { if (!$event->isMasterRequest()) { @@ -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; } @@ -50,7 +53,7 @@ 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) @@ -58,12 +61,21 @@ public function onKernelResponse(FilterResponseEvent $event) } } + /** + * @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'), ); } diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php index 34598363c8914..a2d026b88467b 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php @@ -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); @@ -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')); + } } diff --git a/src/Symfony/Component/HttpKernel/composer.json b/src/Symfony/Component/HttpKernel/composer.json index 17c0544c62560..585ab5b37dbaa 100644 --- a/src/Symfony/Component/HttpKernel/composer.json +++ b/src/Symfony/Component/HttpKernel/composer.json @@ -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"