feature #36129 [HttpFoundation][HttpKernel][Security] Improve Unexpec…

…tedSessionUsageException backtrace (mtarld)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[HttpFoundation][HttpKernel][Security] Improve UnexpectedSessionUsageException backtrace

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       |
| License       | MIT
| Doc PR        |

Improve `UnexceptedSessionUsageException` backtrace so that it leads to the place in the userland  where it was told to use session.

Commits
-------

1e1d332 Improve UnexcpectedSessionUsageException backtrace

src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml

@@ -13,6 +13,12 @@
 
         <service id="session" class="Symfony\Component\HttpFoundation\Session\Session" public="true">
             <argument type="service" id="session.storage" />
+            <argument>null</argument> <!-- AttributeBagInterface -->
+            <argument>null</argument> <!-- FlashBagInterface -->
+            <argument type="collection">
+                <argument type="service" id="session_listener" />
+                <argument>onSessionUsage</argument>
+            </argument>
         </service>
 
         <service id="Symfony\Component\HttpFoundation\Session\SessionInterface" alias="session" />

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
 -----

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));
     }
 
     /**

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);

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'));
     }

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
 -----

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 [

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();
+    }
 }

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;
             }
         }
 

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());