From bc48db242411744716fb148a737f6c45bdb7710d Mon Sep 17 00:00:00 2001 From: Mathias Arlaud Date: Fri, 14 Feb 2020 18:10:13 +0100 Subject: [PATCH] [FrameworkBundle][HttpFoundation] Add `_stateless` --- .../Resources/config/session.xml | 2 + .../FrameworkExtensionTest.php | 4 +- src/Symfony/Component/HttpKernel/CHANGELOG.md | 1 + .../EventListener/AbstractSessionListener.php | 39 +++++++---- .../EventListener/SessionListener.php | 4 +- .../UnexpectedSessionUsageException.php | 19 ++++++ .../EventListener/SessionListenerTest.php | 66 +++++++++++++++++++ 7 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 src/Symfony/Component/HttpKernel/Exception/UnexpectedSessionUsageException.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml index 0cb7b4e200..464d4609ee 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml @@ -66,7 +66,9 @@ + + %kernel.debug% diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 86b3fae486..30ebae8852 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -504,7 +504,7 @@ abstract class FrameworkExtensionTest extends TestCase $this->assertNull($container->getDefinition('session.storage.native')->getArgument(1)); $this->assertNull($container->getDefinition('session.storage.php_bridge')->getArgument(0)); - $expected = ['session', 'initialized_session']; + $expected = ['session', 'initialized_session', 'logger']; $this->assertEquals($expected, array_keys($container->getDefinition('session_listener')->getArgument(0)->getValues())); } @@ -1301,7 +1301,7 @@ abstract class FrameworkExtensionTest extends TestCase { $container = $this->createContainerFromFile('session_cookie_secure_auto'); - $expected = ['session', 'initialized_session', 'session_storage', 'request_stack']; + $expected = ['session', 'initialized_session', 'logger', 'session_storage', 'request_stack']; $this->assertEquals($expected, array_keys($container->getDefinition('session_listener')->getArgument(0)->getValues())); } diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md index 17b7122023..ada9fafe60 100644 --- a/src/Symfony/Component/HttpKernel/CHANGELOG.md +++ b/src/Symfony/Component/HttpKernel/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG ----- * allowed using public aliases to reference controllers + * added session usage reporting when the `_stateless` attribute of the request is set to `true` 5.0.0 ----- diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php index 9c9c422e4e..871ffc61d5 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpKernel\Event\FinishRequestEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\Event\ResponseEvent; +use Symfony\Component\HttpKernel\Exception\UnexpectedSessionUsageException; use Symfony\Component\HttpKernel\KernelEvents; /** @@ -41,10 +42,12 @@ abstract class AbstractSessionListener implements EventSubscriberInterface protected $container; private $sessionUsageStack = []; + private $debug; - public function __construct(ContainerInterface $container = null) + public function __construct(ContainerInterface $container = null, bool $debug = false) { $this->container = $container; + $this->debug = $debug; } public function onKernelRequest(RequestEvent $event) @@ -82,16 +85,6 @@ abstract class AbstractSessionListener implements EventSubscriberInterface return; } - if ($session instanceof Session ? $session->getUsageIndex() !== end($this->sessionUsageStack) : $session->isStarted()) { - if ($autoCacheControl) { - $response - ->setExpires(new \DateTime()) - ->setPrivate() - ->setMaxAge(0) - ->headers->addCacheControlDirective('must-revalidate'); - } - } - if ($session->isStarted()) { /* * Saves the session, in case it is still open, before sending the response/headers. @@ -120,6 +113,30 @@ abstract class AbstractSessionListener implements EventSubscriberInterface */ $session->save(); } + + if ($session instanceof Session ? $session->getUsageIndex() === end($this->sessionUsageStack) : !$session->isStarted()) { + return; + } + + if ($autoCacheControl) { + $response + ->setExpires(new \DateTime()) + ->setPrivate() + ->setMaxAge(0) + ->headers->addCacheControlDirective('must-revalidate'); + } + + if (!$event->getRequest()->attributes->get('_stateless', false)) { + return; + } + + if ($this->debug) { + throw new UnexpectedSessionUsageException('Session was used while the request was declared stateless.'); + } + + if ($this->container->has('logger')) { + $this->container->get('logger')->warning('Session was used while the request was declared stateless.'); + } } public function onFinishRequest(FinishRequestEvent $event) diff --git a/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php index a53ade797c..e982a795b2 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php @@ -28,9 +28,9 @@ use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage; */ class SessionListener extends AbstractSessionListener { - public function __construct(ContainerInterface $container) + public function __construct(ContainerInterface $container, bool $debug = false) { - $this->container = $container; + parent::__construct($container, $debug); } protected function getSession(): ?SessionInterface diff --git a/src/Symfony/Component/HttpKernel/Exception/UnexpectedSessionUsageException.php b/src/Symfony/Component/HttpKernel/Exception/UnexpectedSessionUsageException.php new file mode 100644 index 0000000000..0145b1691c --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Exception/UnexpectedSessionUsageException.php @@ -0,0 +1,19 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\Exception; + +/** + * @author Mathias Arlaud + */ +class UnexpectedSessionUsageException extends \LogicException +{ +} diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php index 8fc9f6bc9c..a155cc93ab 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpKernel\Tests\EventListener; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\HttpFoundation\Request; @@ -24,6 +25,7 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\Event\ResponseEvent; use Symfony\Component\HttpKernel\EventListener\AbstractSessionListener; use Symfony\Component\HttpKernel\EventListener\SessionListener; +use Symfony\Component\HttpKernel\Exception\UnexpectedSessionUsageException; use Symfony\Component\HttpKernel\HttpKernelInterface; class SessionListenerTest extends TestCase @@ -178,4 +180,68 @@ class SessionListenerTest extends TestCase $this->assertTrue($response->headers->has('Expires')); $this->assertLessThanOrEqual((new \DateTime('now', new \DateTimeZone('UTC'))), (new \DateTime($response->headers->get('Expires')))); } + + public function testSessionUsageExceptionIfStatelessAndSessionUsed() + { + $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock(); + $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1)); + + $container = new Container(); + $container->set('initialized_session', $session); + + $listener = new SessionListener($container, true); + $kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock(); + + $request = new Request(); + $request->attributes->set('_stateless', true); + $listener->onKernelRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST)); + + $this->expectException(UnexpectedSessionUsageException::class); + $listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, new Response())); + } + + public function testSessionUsageLogIfStatelessAndSessionUsed() + { + $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock(); + $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1)); + + $logger = $this->getMockBuilder(LoggerInterface::class)->disableOriginalConstructor()->getMock(); + $logger->expects($this->exactly(1))->method('warning'); + + $container = new Container(); + $container->set('initialized_session', $session); + $container->set('logger', $logger); + + $listener = new SessionListener($container, false); + $kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock(); + + $request = new Request(); + $request->attributes->set('_stateless', true); + $listener->onKernelRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST)); + + $listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, new Response())); + } + + public function testSessionIsSavedWhenUnexpectedSessionExceptionThrown() + { + $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock(); + $session->method('isStarted')->willReturn(true); + $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1)); + $session->expects($this->exactly(1))->method('save'); + + $container = new Container(); + $container->set('initialized_session', $session); + + $listener = new SessionListener($container, true); + $kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock(); + + $request = new Request(); + $request->attributes->set('_stateless', true); + + $listener->onKernelRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST)); + + $response = new Response(); + $this->expectException(UnexpectedSessionUsageException::class); + $listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response)); + } }