From 26fc4e683f96b51237fb206f3afbf93525adc49c Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 27 Jun 2018 13:18:14 +0200 Subject: [PATCH 1/3] [HttpFoundation] fix session tracking counter --- src/Symfony/Component/HttpFoundation/Session/Session.php | 8 +++----- .../Component/HttpFoundation/Session/SessionBagProxy.php | 4 ---- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Session/Session.php b/src/Symfony/Component/HttpFoundation/Session/Session.php index f0379c1697..31da54c73e 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Session.php +++ b/src/Symfony/Component/HttpFoundation/Session/Session.php @@ -160,7 +160,9 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable */ public function isEmpty() { - ++$this->usageIndex; + if ($this->isStarted()) { + ++$this->usageIndex; + } foreach ($this->data as &$data) { if (!empty($data)) { return false; @@ -185,8 +187,6 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable */ public function migrate($destroy = false, $lifetime = null) { - ++$this->usageIndex; - return $this->storage->regenerate($destroy, $lifetime); } @@ -195,8 +195,6 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable */ public function save() { - ++$this->usageIndex; - $this->storage->save(); } diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php index 88005ee092..d6b242d4ee 100644 --- a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php @@ -44,8 +44,6 @@ final class SessionBagProxy implements SessionBagInterface */ public function isEmpty() { - ++$this->usageIndex; - return empty($this->data[$this->bag->getStorageKey()]); } @@ -81,8 +79,6 @@ final class SessionBagProxy implements SessionBagInterface */ public function clear() { - ++$this->usageIndex; - return $this->bag->clear(); } } From 89ed756462fd4660acfd8ad1c2224dffa0fccf3e Mon Sep 17 00:00:00 2001 From: David Maicher Date: Mon, 25 Jun 2018 20:31:52 +0200 Subject: [PATCH 2/3] failing test to reproduce session problem --- .../TestBundle/Controller/SessionController.php | 8 ++++++++ .../TestBundle/Resources/config/routing.yml | 4 ++++ .../Tests/Functional/SessionTest.php | 16 ++++++++++++++++ .../Component/HttpFoundation/Session/Session.php | 2 -- .../HttpFoundation/Session/SessionBagProxy.php | 5 +++++ 5 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SessionController.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SessionController.php index a1d91e7929..0c82d8cc73 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SessionController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SessionController.php @@ -43,6 +43,14 @@ class SessionController implements ContainerAwareInterface return new Response(sprintf('Welcome back %s, nice to meet you.', $name)); } + public function cacheableAction() + { + $response = new Response('all good'); + $response->setSharedMaxAge(100); + + return $response; + } + public function logoutAction(Request $request) { $request->getSession()->invalidate(); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml index 11b85a86d3..923204ab0f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml @@ -2,6 +2,10 @@ session_welcome: path: /session defaults: { _controller: TestBundle:Session:welcome } +session_cacheable: + path: /cacheable + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SessionController::cacheableAction } + session_welcome_name: path: /session/{name} defaults: { _controller: TestBundle:Session:welcome } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php index 166d8a3391..2e1634220c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php @@ -126,6 +126,22 @@ class SessionTest extends WebTestCase $this->assertContains('Welcome back client2, nice to meet you.', $crawler2->text()); } + /** + * @dataProvider getConfigs + */ + public function testCorrectCacheControlHeadersForCacheableAction($config, $insulate) + { + $client = $this->createClient(array('test_case' => 'Session', 'root_config' => $config)); + if ($insulate) { + $client->insulate(); + } + + $client->request('GET', '/cacheable'); + + $response = $client->getResponse(); + $this->assertSame('public, s-maxage=100', $response->headers->get('cache-control')); + } + public function getConfigs() { return array( diff --git a/src/Symfony/Component/HttpFoundation/Session/Session.php b/src/Symfony/Component/HttpFoundation/Session/Session.php index 31da54c73e..c0978d552f 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Session.php +++ b/src/Symfony/Component/HttpFoundation/Session/Session.php @@ -54,8 +54,6 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable */ public function start() { - ++$this->usageIndex; - return $this->storage->start(); } diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php index d6b242d4ee..3504bdfe7b 100644 --- a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php @@ -44,6 +44,11 @@ final class SessionBagProxy implements SessionBagInterface */ public function isEmpty() { + if (!isset($this->data[$this->bag->getStorageKey()])) { + return true; + } + ++$this->usageIndex; + return empty($this->data[$this->bag->getStorageKey()]); } From 5ed40c095be3c9387d70a664ab98c289ba53d25b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 27 Jun 2018 17:17:35 +0200 Subject: [PATCH 3/3] [HttpFoundation] fix registration of session proxies --- .../Session/Storage/NativeSessionStorage.php | 2 -- .../Storage/Proxy/SessionHandlerProxy.php | 18 +++++++++- .../Storage/Proxy/SessionHandlerProxyTest.php | 33 +++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php index 6416a03fdf..41410bd323 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php @@ -411,8 +411,6 @@ class NativeSessionStorage implements SessionStorageInterface } if ($this->saveHandler instanceof SessionHandlerProxy) { - session_set_save_handler($this->saveHandler->getHandler(), false); - } elseif ($this->saveHandler instanceof \SessionHandlerInterface) { session_set_save_handler($this->saveHandler, false); } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php index 53c1209a1c..b11cc397a0 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php @@ -14,7 +14,7 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Proxy; /** * @author Drak */ -class SessionHandlerProxy extends AbstractProxy implements \SessionHandlerInterface +class SessionHandlerProxy extends AbstractProxy implements \SessionHandlerInterface, \SessionUpdateTimestampHandlerInterface { protected $handler; @@ -82,4 +82,20 @@ class SessionHandlerProxy extends AbstractProxy implements \SessionHandlerInterf { return (bool) $this->handler->gc($maxlifetime); } + + /** + * {@inheritdoc} + */ + public function validateId($sessionId) + { + return !$this->handler instanceof \SessionUpdateTimestampHandlerInterface || $this->handler->validateId($sessionId); + } + + /** + * {@inheritdoc} + */ + public function updateTimestamp($sessionId, $data) + { + return $this->handler instanceof \SessionUpdateTimestampHandlerInterface ? $this->handler->updateTimestamp($sessionId, $data) : $this->write($sessionId, $data); + } } diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Proxy/SessionHandlerProxyTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Proxy/SessionHandlerProxyTest.php index 682825356a..0b48250e01 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Proxy/SessionHandlerProxyTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Proxy/SessionHandlerProxyTest.php @@ -121,4 +121,37 @@ class SessionHandlerProxyTest extends TestCase $this->proxy->gc(86400); } + + /** + * @requires PHPUnit 5.1 + */ + public function testValidateId() + { + $mock = $this->getMockBuilder(array('SessionHandlerInterface', 'SessionUpdateTimestampHandlerInterface'))->getMock(); + $mock->expects($this->once()) + ->method('validateId'); + + $proxy = new SessionHandlerProxy($mock); + $proxy->validateId('id'); + + $this->assertTrue($this->proxy->validateId('id')); + } + + /** + * @requires PHPUnit 5.1 + */ + public function testUpdateTimestamp() + { + $mock = $this->getMockBuilder(array('SessionHandlerInterface', 'SessionUpdateTimestampHandlerInterface'))->getMock(); + $mock->expects($this->once()) + ->method('updateTimestamp'); + + $proxy = new SessionHandlerProxy($mock); + $proxy->updateTimestamp('id', 'data'); + + $this->mock->expects($this->once()) + ->method('write'); + + $this->proxy->updateTimestamp('id', 'data'); + } }