From 1e1d332c7cc251337984703a9a6c9a6fb365a52f Mon Sep 17 00:00:00 2001 From: Mathias Arlaud Date: Tue, 25 Feb 2020 16:40:39 +0100 Subject: [PATCH] Improve UnexcpectedSessionUsageException backtrace --- .../Resources/config/session.xml | 6 ++ .../Component/HttpFoundation/CHANGELOG.md | 1 + .../HttpFoundation/Session/Session.php | 12 +++- .../Session/SessionBagProxy.php | 14 ++++- .../Tests/Session/SessionTest.php | 2 +- src/Symfony/Component/HttpKernel/CHANGELOG.md | 1 + .../EventListener/AbstractSessionListener.php | 31 ++++++++++ .../EventListener/SessionListenerTest.php | 59 +++++++++++++++++++ .../Http/Firewall/ContextListener.php | 3 + .../Tests/Firewall/ContextListenerTest.php | 17 ++++++ 10 files changed, 142 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml index e63967ea3591..c2f621fe511d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml @@ -13,6 +13,12 @@ + null + null + + + onSessionUsage + diff --git a/src/Symfony/Component/HttpFoundation/CHANGELOG.md b/src/Symfony/Component/HttpFoundation/CHANGELOG.md index 56b76d84b8c6..4355b5af9a50 100644 --- a/src/Symfony/Component/HttpFoundation/CHANGELOG.md +++ b/src/Symfony/Component/HttpFoundation/CHANGELOG.md @@ -14,6 +14,7 @@ CHANGELOG according to [RFC 8674](https://tools.ietf.org/html/rfc8674) * made the Mime component an optional dependency * added `MarshallingSessionHandler`, `IdentityMarshaller` + * made `Session` accept a callback to report when the session is being used 5.0.0 ----- diff --git a/src/Symfony/Component/HttpFoundation/Session/Session.php b/src/Symfony/Component/HttpFoundation/Session/Session.php index 8b02d2d0d995..4a8af4d50e70 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Session.php +++ b/src/Symfony/Component/HttpFoundation/Session/Session.php @@ -35,10 +35,12 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable private $attributeName; private $data = []; private $usageIndex = 0; + private $usageReporter; - public function __construct(SessionStorageInterface $storage = null, AttributeBagInterface $attributes = null, FlashBagInterface $flashes = null) + public function __construct(SessionStorageInterface $storage = null, AttributeBagInterface $attributes = null, FlashBagInterface $flashes = null, callable $usageReporter = null) { $this->storage = $storage ?: new NativeSessionStorage(); + $this->usageReporter = $usageReporter; $attributes = $attributes ?: new AttributeBag(); $this->attributeName = $attributes->getName(); @@ -153,6 +155,9 @@ public function isEmpty(): bool { if ($this->isStarted()) { ++$this->usageIndex; + if ($this->usageReporter && 0 <= $this->usageIndex) { + ($this->usageReporter)(); + } } foreach ($this->data as &$data) { if (!empty($data)) { @@ -229,6 +234,9 @@ public function setName(string $name) public function getMetadataBag() { ++$this->usageIndex; + if ($this->usageReporter && 0 <= $this->usageIndex) { + ($this->usageReporter)(); + } return $this->storage->getMetadataBag(); } @@ -238,7 +246,7 @@ public function getMetadataBag() */ public function registerBag(SessionBagInterface $bag) { - $this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->usageIndex)); + $this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->usageIndex, $this->usageReporter)); } /** diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php index 0ae8231ef8fb..90aa010c9072 100644 --- a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php @@ -21,17 +21,22 @@ final class SessionBagProxy implements SessionBagInterface private $bag; private $data; private $usageIndex; + private $usageReporter; - public function __construct(SessionBagInterface $bag, array &$data, ?int &$usageIndex) + public function __construct(SessionBagInterface $bag, array &$data, ?int &$usageIndex, ?callable $usageReporter) { $this->bag = $bag; $this->data = &$data; $this->usageIndex = &$usageIndex; + $this->usageReporter = $usageReporter; } public function getBag(): SessionBagInterface { ++$this->usageIndex; + if ($this->usageReporter && 0 <= $this->usageIndex) { + ($this->usageReporter)(); + } return $this->bag; } @@ -42,6 +47,9 @@ public function isEmpty(): bool return true; } ++$this->usageIndex; + if ($this->usageReporter && 0 <= $this->usageIndex) { + ($this->usageReporter)(); + } return empty($this->data[$this->bag->getStorageKey()]); } @@ -60,6 +68,10 @@ public function getName(): string public function initialize(array &$array): void { ++$this->usageIndex; + if ($this->usageReporter && 0 <= $this->usageIndex) { + ($this->usageReporter)(); + } + $this->data[$this->bag->getStorageKey()] = &$array; $this->bag->initialize($array); diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php index e216bfc8c2ee..caa5d4f9a885 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php @@ -281,7 +281,7 @@ public function testGetBagWithBagNotImplementingGetBag() $bag->setName('foo'); $storage = new MockArraySessionStorage(); - $storage->registerBag(new SessionBagProxy($bag, $data, $usageIndex)); + $storage->registerBag(new SessionBagProxy($bag, $data, $usageIndex, null)); $this->assertSame($bag, (new Session($storage))->getBag('foo')); } diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md index ada9fafe6010..d24bf8bfff5e 100644 --- a/src/Symfony/Component/HttpKernel/CHANGELOG.md +++ b/src/Symfony/Component/HttpKernel/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * allowed using public aliases to reference controllers * added session usage reporting when the `_stateless` attribute of the request is set to `true` + * added `AbstractSessionListener::onSessionUsage()` to report when the session is used while a request is stateless 5.0.0 ----- diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php index 871ffc61d561..1fe3264f7d30 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php @@ -146,6 +146,37 @@ public function onFinishRequest(FinishRequestEvent $event) } } + public function onSessionUsage(): void + { + if (!$this->debug) { + return; + } + + if (!$requestStack = $this->container && $this->container->has('request_stack') ? $this->container->get('request_stack') : null) { + return; + } + + $stateless = false; + $clonedRequestStack = clone $requestStack; + while (null !== ($request = $clonedRequestStack->pop()) && !$stateless) { + $stateless = $request->attributes->get('_stateless'); + } + + if (!$stateless) { + return; + } + + if (!$session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : $requestStack->getCurrentRequest()->getSession()) { + return; + } + + if ($session->isStarted()) { + $session->save(); + } + + throw new UnexpectedSessionUsageException('Session was used while the request was declared stateless.'); + } + public static function getSubscribedEvents(): array { return [ diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php index a155cc93ab71..8df2ce51698e 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php @@ -244,4 +244,63 @@ public function testSessionIsSavedWhenUnexpectedSessionExceptionThrown() $this->expectException(UnexpectedSessionUsageException::class); $listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response)); } + + public function testSessionUsageCallbackWhenDebugAndStateless() + { + $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock(); + $session->method('isStarted')->willReturn(true); + $session->expects($this->exactly(1))->method('save'); + + $requestStack = new RequestStack(); + + $request = new Request(); + $request->attributes->set('_stateless', true); + + $requestStack->push(new Request()); + $requestStack->push($request); + $requestStack->push(new Request()); + + $container = new Container(); + $container->set('initialized_session', $session); + $container->set('request_stack', $requestStack); + + $this->expectException(UnexpectedSessionUsageException::class); + (new SessionListener($container, true))->onSessionUsage(); + } + + public function testSessionUsageCallbackWhenNoDebug() + { + $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock(); + $session->method('isStarted')->willReturn(true); + $session->expects($this->exactly(0))->method('save'); + + $request = new Request(); + $request->attributes->set('_stateless', true); + + $requestStack = $this->getMockBuilder(RequestStack::class)->getMock(); + $requestStack->expects($this->never())->method('getMasterRequest')->willReturn($request); + + $container = new Container(); + $container->set('initialized_session', $session); + $container->set('request_stack', $requestStack); + + (new SessionListener($container))->onSessionUsage(); + } + + public function testSessionUsageCallbackWhenNoStateless() + { + $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock(); + $session->method('isStarted')->willReturn(true); + $session->expects($this->never())->method('save'); + + $requestStack = new RequestStack(); + $requestStack->push(new Request()); + $requestStack->push(new Request()); + + $container = new Container(); + $container->set('initialized_session', $session); + $container->set('request_stack', $requestStack); + + (new SessionListener($container, true))->onSessionUsage(); + } } diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index 8dc90fd5d7ab..6f427533f24b 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -96,11 +96,14 @@ public function authenticate(RequestEvent $event) if (null !== $session) { $usageIndexValue = $session instanceof Session ? $usageIndexReference = &$session->getUsageIndex() : 0; + $usageIndexReference = PHP_INT_MIN; $sessionId = $request->cookies->get($session->getName()); $token = $session->get($this->sessionKey); if ($this->sessionTrackerEnabler && \in_array($sessionId, [true, $session->getId()], true)) { $usageIndexReference = $usageIndexValue; + } else { + $usageIndexReference = $usageIndexReference - PHP_INT_MIN + $usageIndexValue; } } diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php index 8cf3eeb6b647..f1a21b02f07a 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php @@ -361,6 +361,23 @@ public function testWithPreviousNotStartedSession() $this->assertSame($usageIndex, $session->getUsageIndex()); } + public function testSessionIsNotReported() + { + $usageReporter = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + $usageReporter->expects($this->never())->method('__invoke'); + + $session = new Session(new MockArraySessionStorage(), null, null, $usageReporter); + + $request = new Request(); + $request->setSession($session); + $request->cookies->set('MOCKSESSID', true); + + $tokenStorage = new TokenStorage(); + + $listener = new ContextListener($tokenStorage, [], 'context_key', null, null, null, [$tokenStorage, 'getToken']); + $listener(new RequestEvent($this->getMockBuilder(HttpKernelInterface::class)->getMock(), $request, HttpKernelInterface::MASTER_REQUEST)); + } + protected function runSessionOnKernelResponse($newToken, $original = null) { $session = new Session(new MockArraySessionStorage());