8000 feature #36129 [HttpFoundation][HttpKernel][Security] Improve Unexpec… · symfony/symfony@2130465 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2130465

Browse files
nicolas-grekasnicolas-grekas
committed
feature #36129 [HttpFoundation][HttpKernel][Security] Improve UnexpectedSessionUsageException 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
2 parents 0c74ff4 + 1e1d332 commit 2130465

File tree

10 files changed

+142
-4
lines changed

10 files changed

+142
-4
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313

1414
<service id="session" class="Symfony\Component\HttpFoundation\Session\Session" public="true">
1515
<argument type="service" id="session.storage" />
16+
<argument>null</argument> <!-- AttributeBagInterface -->
17+
<argument>null</argument> <!-- FlashBagInterface -->
18+
<argument type="collection">
19+
<argument type="service" id="session_listener" />
20+
<argument>onSessionUsage</argument>
21+
</argument>
1622
</service>
1723

1824
<service id="Symfony\Component\HttpFoundation\Session\SessionInterface" alias="session" />

‎src/Symfony/Component/HttpFoundation/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ CHANGELOG
1414
according to [RFC 8674](https://tools.ietf.org/html/rfc8674)
1515
* made the Mime component an optional dependency
1616
* added `MarshallingSessionHandler`, `IdentityMarshaller`
17+
* made `Session` accept a callback to report when the session is being used
1718

1819
5.0.0
1920
-----

‎src/Symfony/Component/HttpFoundation/Session/Session.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
3535
private $attributeName;
3636
private $data = [];
3737
private $usageIndex = 0;
38+
private $usageReporter;
3839

39-
public function __construct(SessionStorageInterface $storage = null, AttributeBagInterface $attributes = null, FlashBagInterface $flashes = null)
40+
public function __construct(SessionStorageInterface $storage = null, AttributeBagInterface $attributes = null, FlashBagInterface $flashes = null, callable $usageReporter = null)
4041
{
4142
$this->storage = $storage ?: new NativeSessionStorage();
43+
$this->usageReporter = $usageReporter;
4244

4345
$attributes = $attributes ?: new AttributeBag();
4446
$this->attributeName = $attributes->getName();
@@ -153,6 +155,9 @@ public function isEmpty(): bool
153155
{
154156
if ($this->isStarted()) {
155157
++$this->usageIndex;
158+
if ($this->usageReporter && 0 <= $this->usageIndex) {
159+
($this->usageReporter)();
160+
}
156161
}
157162
foreach ($this->data as &$data) {
158163
if (!empty($data)) {
@@ -229,6 +234,9 @@ public function setName(string $name)
229234
public function getMetadataBag()
230235
{
231236
++$this->usageIndex;
237+
if ($this->usageReporter && 0 <= $this->usageIndex) {
238+
($this->usageReporter)();
239+
}
232240

233241
return $this->storage->getMetadataBag();
234242
}
@@ -238,7 +246,7 @@ public function getMetadataBag()
238246
*/
239247
public function registerBag(SessionBagInterface $bag)
240248
{
241-
$this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->usageIndex));
249+
$this->storage->registerBag(new SessionBagProxy($bag, $this->data, $this->usageIndex, $this->usageReporter));
242250
}
243251

244252
/**

‎src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,22 @@ final class SessionBagProxy implements SessionBagInterface
2121
private $bag;
2222
private $data;
2323
private $usageIndex;
24+
private $usageReporter;
2425

25-
public function __construct(SessionBagInterface $bag, array &$data, ?int &$< 10000 /span>usageIndex)
26+
public function __construct(SessionBagInterface $bag, array &$data, ?int &$usageIndex, ?callable $usageReporter)
2627
{
2728
$this->bag = $bag;
2829
$this->data = &$data;
2930
$this->usageIndex = &$usageIndex;
31+
$this->usageReporter = $usageReporter;
3032
}
3133

3234
public function getBag(): SessionBagInterface
3335
{
3436
++$this->usageIndex;
37+
if ($this->usageReporter && 0 <= $this->usageIndex) {
38+
($this->usageReporter)();
39+
}
3540

3641
return $this->bag;
3742
}
@@ -42,6 +47,9 @@ public function isEmpty(): bool
4247
return true;
4348
}
4449
++$this->usageIndex;
50+
if ($this->usageReporter && 0 <= $this->usageIndex) {
51+
($this->usageReporter)();
52+
}
4553

4654
return empty($this->data[$this->bag->getStorageKey()]);
4755
}
@@ -60,6 +68,10 @@ public function getName(): string
6068
public function initialize(array &$array): void
6169
{
6270
++$this->usageIndex;
71+
if ($this->usageReporter && 0 <= $this->usageIndex) {
72+
($this->usageReporter)();
73+
}
74+
6375
$this->data[$this->bag->getStorageKey()] = &$array;
6476

6577
$this->bag->initialize($array);

‎src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ public function testGetBagWithBagNotImplementingGetBag()
281281
$bag->setName('foo');
282282

283283
$storage = new MockArraySessionStorage();
284-
$storage->registerBag(new SessionBagProxy($bag, $data, $usageIndex));
284+
$storage->registerBag(new SessionBagProxy($bag, $data, $usageIndex, null));
285285

286286
$this->assertSame($bag, (new Session($storage))->getBag('foo'));
287287
}

‎src/Symfony/Component/HttpKernel/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ CHANGELOG
66

77
* allowed using public aliases to reference controllers
88
* added session usage reporting when the `_stateless` attribute of the request is set to `true`
9+
* added `AbstractSessionListener::onSessionUsage()` to report when the session is used while a request is stateless
910

1011
5.0.0
1112
-----

‎src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,37 @@ public function onFinishRequest(FinishRequestEvent $event)
146146
}
147147
}
148148

149+
public function onSessionUsage(): void
150+
{
151+
if (!$this->debug) {
152+
return;
153+
}
154+
155+
if (!$requestStack = $this->container && $this->container->has('request_stack') ? $this->container->get('request_stack') : null) {
156+
return;
157+
}
158+
159+
$stateless = false;
160+
$clonedRequestStack = clone $requestStack;
161+
while (null !== ($request = $clonedRequestStack->pop()) && !$stateless) {
162+
$stateless = $request->attributes->get('_stateless');
163+
}
164+
165+
if (!$stateless) {
166+
return;
167+
}
168+
169+
if (!$session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : $requestStack->getCurrentRequest()->getSession()) {
170+
return;
171+
}
172+
173+
if ($session->isStarted()) {
174+
$session->save();
175+
}
176+
177+
throw new UnexpectedSessionUsageException('Session was used while the request was declared stateless.');
178+
}
179+
149180
public static function getSubscribedEvents(): array
150181
{
151182
return [

‎src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,4 +244,63 @@ public function testSessionIsSavedWhenUnexpectedSessionExceptionThrown()
244244
$this->expectException(UnexpectedSessionUsageException::class);
245245
$listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response));
246246
}
247+
248+
public function testSessionUsageCallbackWhenDebugAndStateless()
249+
{
250+
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
251+
$session->method('isStarted')->willReturn(true);
252+
$session->expects($this->exactly(1))->method('save');
253+
254+
$requestStack = new RequestStack();
255+
256+
$request = new Request();
257+
$request->attributes->set('_stateless', true);
258+
259+
$requestStack->push(new Request());
260+
$requestStack->push($request);
261+
$requestStack->push(new Request());
262+
263+
$container = new Container();
264+
$container->set('initialized_session', $session);
265+
$container->set('request_stack', $requestStack);
266+
267+
$this->expectException(UnexpectedSessionUsageException::class);
268+
(new SessionListener($container, true))->onSessionUsage();
269+
}
270+
271+
public function testSessionUsageCallbackWhenNoDebug()
272+
{
273+
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
274+
$session->method('isStarted')->willReturn(true);
275+
$session->expects($this->exactly(0))->method('save');
276+
277+
$request = new Request();
278+
$request->attributes->set('_stateless', true);
279+
280+
$requestStack = $this->getMockBuilder(RequestStack::class)->getMock();
281+
$requestStack->expects($this->never())->method('getMasterRequest')->willReturn($request);
282+
283+
$container = new Container();
284+
$container->set('initialized_session', $session);
285+
$container->set('request_stack', $requestStack);
286+
287+
(new SessionListener($container))->onSessionUsage();
288+
}
289+
290+
public function testSessionUsageCallbackWhenNoStateless()
291+
{
292+
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
293+
$session->method('isStarted')->willReturn(true);
294+
$session->expects($this->never())->method('save');
295+
296+
$requestStack = new RequestStack();
297+
$requestStack->push(new Request());
298+
$requestStack->push(new Request());
299+
300+
$container = new Container();
301+
$container->set('initialized_session', $session);
302+
$container->set('request_stack', $requestStack);
303+
304+
(new SessionListener($container, true))->onSessionUsage();
305+
}
247306
}

‎src/Symfony/Component/Security/Http/Firewall/ContextListener.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,14 @@ public function authenticate(RequestEvent $event)
9696

9797
if (null !== $session) {
9898
$usageIndexValue = $session instanceof Session ? $usageIndexReference = &$session->getUsageIndex() : 0;
99+
$usageIndexReference = PHP_INT_MIN;
99100
$sessionId = $request->cookies->get($session->getName());
100101
$token = $session->get($this->sessionKey);
101102

102103
if ($this->sessionTrackerEnabler && \in_array($sessionId, [true, $session->getId()], true)) {
103104
$usageIndexReference = $usageIndexValue;
105+
} else {
106+
$usageIndexReference = $usageIndexReference - PHP_INT_MIN + $usageIndexValue;
104107
}
105108
}
106109

‎src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,23 @@ public function testWithPreviousNotStartedSession()
361361
$this->assertSame($usageIndex, $session->getUsageIndex());
362362
}
363363

364+
public function testSessionIsNotReported()
365+
{
366+
$usageReporter = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock();
367+
$usageReporter->expects($this->never())->method('__invoke');
368+
369+
$session = new Session(new MockArraySessionStorage(), null, null, $usageReporter);
370+
371+
$request = new Request();
372+
$request->setSession($session);
373+
$request->cookies->set('MOCKSESSID', true);
374+
375+
$tokenStorage = new TokenStorage();
376+
377+
$listener = new ContextListener($tokenStorage, [], 'context_key', null, null, null, [$tokenStorage, 'getToken']);
378+
$listener(new RequestEvent($this->getMockBuilder(HttpKernelInterface::class)->getMock(), $request, HttpKernelInterface::MASTER_REQUEST));
379+
}
380+
364381
protected function runSessionOnKernelResponse($newToken, $original = null)
365382
{
366383
$session = new Session(new MockArraySessionStorage());

0 commit comments

Comments
 (0)
0