8000 [HttpKernel] Make session-related services extra-lazy · symfony/symfony@5f53558 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5f53558

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpKernel] Make session-related services extra-lazy
1 parent 6e6ac9e commit 5f53558

File tree

9 files changed

+98
-45
lines changed

9 files changed

+98
-45
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,14 @@
6363
<tag name="container.service_locator" />
6464
<argument type="collection">
6565
<argument key="session" type="service" id="session" on-invalid="ignore" />
66+
<argument key="initialized_session" type="service" id="session" on-invalid="ignore_uninitialized" />
6667
</argument>
6768
</service>
6869
</argument>
6970
</service>
7071

7172
<service id="session.save_listener" class="Symfony\Component\HttpKernel\EventListener\SaveSessionListener">
72-
<tag name="kernel.event_subscriber" />
73+
<deprecated>The "%service_id%" service is deprecated since Symfony 4.1 and will be removed in 5.0. Use the "session_listener" service instead.</deprecated>
7374
</service>
7475

7576
<!-- for BC -->

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,12 @@ public function get($key, $default = null)
720720
*/
721721
public function getSession()
722722
{
723-
return $this->session;
723+
$session = $this->session;
724+
if (!$session instanceof SessionInterface && null !== $session) {
725+
$this->setSession($session = $session());
726+
}
727+
728+
return $session;
724729
}
725730

726731
/**
@@ -732,7 +737,7 @@ public function getSession()
732737
public function hasPreviousSession()
733738
{
734739
// the check for $this->session avoids malicious users trying to fake a session cookie with proper name
735-
return $this->hasSession() && $this->cookies->has($this->session->getName());
740+
return $this->hasSession() && $this->cookies->has($this->getSession()->getName());
736741
}
737742

738743
/**
@@ -759,6 +764,14 @@ public function setSession(SessionInterface $session)
759764
$this->session = $session;
760765
}
761766

767+
/**
768+
* @internal
769+
*/
770+
public function setSessionFactory(callable $factory)
771+
{
772+
$this->session = $factory;
773+
}
774+
762775
/**
763776
* Returns the client IP addresses.
764777
*

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

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpKernel\EventListener;
1313

14+
use Psr\Container\ContainerInterface;
1415
use Symfony\Component\HttpFoundation\Session\Session;
1516
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1617
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
@@ -22,22 +23,31 @@
2223
* Sets the session in the request.
2324
*
2425
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
26+
* @author Tobias Schultze <http://tobion.de>
2527
*/
2628
abstract class AbstractSessionListener implements EventSubscriberInterface
2729
{
30+
pro 67DE tected $container;
31+
32+
public function __construct(ContainerInterface $container = null)
33+
{
34+
$this->container = $container;
35+
}
36+
2837
public function onKernelRequest(GetResponseEvent $event)
2938
{
3039
if (!$event->isMasterRequest()) {
3140
return;
3241
}
3342

3443
$request = $event->getRequest();
35-
$session = $this->getSession();
36-
if (null === $session || $request->hasSession()) {
37-
return;
44+
if ($request->hasSession()) {
45+
// no-op
46+
} elseif (method_exists($request, 'setSessionFactory')) {
47+
$request->setSessionFactory(function () { return $this->getSession(); });
48+
} elseif ($session = $this->getSession()) {
49+
$request->setSession($session);
3850
}
39-
40-
$request->setSession($session);
4151
}
4252

4353
public function onKernelResponse(FilterResponseEvent $event)
@@ -46,7 +56,7 @@ public function onKernelResponse(FilterResponseEvent $event)
4656
return;
4757
}
4858

49-
if (!$session = $event->getRequest()->getSession()) {
59+
if (!$session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : $event->getRequest()->getSession()) {
5060
return;
5161
}
5262

@@ -56,13 +66,42 @@ public function onKernelResponse(FilterResponseEvent $event)
5666
->setMaxAge(0)
5767
->headers->addCacheControlDirective('must-revalidate');
5868
}
69+
70+
if ($session->isStarted()) {
71+
/*
72+
* Saves the session, in case it is still open, before sending the response/headers.
73+
*
74+
* This ensures several things in case the developer did not save the session explicitly:
75+
*
76+
* * If a session save handler without locking is used, it ensures the data is available
77+
* on the next request, e.g. after a redirect. PHPs auto-save at script end via
78+
* session_register_shutdown is executed after fastcgi_finish_request. So in this case
79+
* the data could be missing the next request because it might not be saved the moment
80+
* the new request is processed.
81+
* * A locking save handler (e.g. the native 'files') circumvents concurrency problems like
82+
* the one above. But by saving the session before long-running things in the terminate event,
83+
* we ensure the session is not blocked longer than needed.
84+
* * When regenerating the session ID no locking is involved in PHPs session design. See
85+
* https://bugs.php.net/bug.php?id=61470 for a discussion. So in this case, the session must
86+
* be saved anyway before sending the headers with the new session ID. Otherwise session
87+
* data could get lost again for concurrent requests with the new ID. One result could be
88+
* that you get logged out after just logging in.
89+
*
90+
* This listener should be executed as one of the last listeners, so that previous listeners
91+
* can still operate on the open session. This prevents the overhead of restarting it.
92+
* Listeners after closing the session can still work with the session as usual because
93+
* Symfonys session implementation starts the session on demand. So writing to it after
94+
* it is saved will just restart it.
95+
*/
96+
$session->save();
97+
}
5998
}
6099

61100
public static function getSubscribedEvents()
62101
{
63102
return array(
64103
KernelEvents::REQUEST => array('onKernelRequest', 128),
65-
// low priority to come after regular response listeners, same as SaveSessionListener
104+
// low priority to come after regular response listeners, but higher than StreamedResponseListener
66105
KernelEvents::RESPONSE => array('onKernelResponse', -1000),
67106
);
68107
}

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

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,36 +11,16 @@
1111

1212
namespace Symfony\Component\HttpKernel\EventListener;
1313

14+
@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.1 and will be removed in 5.0. Use AbstractSessionListener instead.', SaveSessionListener::class), E_USER_DEPRECATED);
15+
1416
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1517
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
1618
use Symfony\Component\HttpKernel\KernelEvents;
1719

1820
/**
19-
* Saves the session, in case it is still open, before sending the response/headers.
20-
*
21-
* This ensures several things in case the developer did not save the session explicitly:
22-
*
23-
* * If a session save handler without locking is used, it ensures the data is available
24-
* on the next request, e.g. after a redirect. PHPs auto-save at script end via
25-
* session_register_shutdown is executed after fastcgi_finish_request. So in this case
26-
* the data could be missing the next request because it might not be saved the moment
27-
* the new request is processed.
28-
* * A locking save handler (e.g. the native 'files') circumvents concurrency problems like
29-
* the one above. But by saving the session before long-running things in the terminate event,
30-
* we ensure the session is not blocked longer than needed.
31-
* * When regenerating the session ID no locking is involved in PHPs session design. See
32-
* https://bugs.php.net/bug.php?id=61470 for a discussion. So in this case, the session must
33-
* be saved anyway before sending the headers with the new session ID. Otherwise session
34-
* data could get lost again for concurrent requests with the new ID. One result could be
35-
* that you get logged out after just logging in.
36-
*
37-
* This listener should be executed as one of the last listeners, so that previous listeners
38-
* can still operate on the open session. This prevents the overhead of restarting it.
39-
* Listeners after closing the session can still work with the session as usual because
40-
* Symfonys session implementation starts the session on demand. So writing to it after
41-
* it is saved will just restart it.
42-
*
4321
* @author Tobias Schultze <http://tobion.de>
22+
*
23+
* @deprecated since Symfony 4.1, to be removed in 5.0. Use AbstractSessionListener instead.
4424
*/
4525
class SaveSessionListener implements EventSubscriberInterface
4626
{

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
*/
2323
class SessionListener extends AbstractSessionListener
2424
{
25-
private $container;
26-
2725
public function __construct(ContainerInterface $container)
2826
{
2927
$this->container = $container;

‎src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,12 @@ protected function createSubRequest($uri, Request $request)
124124
$subRequest->headers->set('Surrogate-Capability', $request->headers->get('Surrogate-Capability'));
125125
}
126126

127-
if ($session = $request->getSession()) {
128-
$subRequest->setSession($session);
127+
static $setSession;
128+
129+
if (null === $setSession) {
130+
$setSession = \Closure::bind(function ($subRequest, $request) { $subRequest->session = $request->session; }, null, Request::class);
129131
}
132+
$setSession($subRequest, $request);
130133

131134
return $subRequest;
132135
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
use Symfony\Component\HttpKernel\EventListener\SaveSessionListener;
2020
use Symfony\Component\HttpKernel\HttpKernelInterface;
2121

22+
/**
23+
* @group legacy
24+
*/
2225
class SaveSessionListenerTest extends TestCase
2326
{
2427
public function testOnlyTriggeredOnMasterRequest()

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\DependencyInjection\Container;
16+
use Symfony\Component\DependencyInjection\ServiceLocator;
1617
use Symfony\Component\HttpFoundation\Request;
1718
use Symfony\Component\HttpFoundation\Response;
1819
use Symfony\Component\HttpFoundation\Session\Session;
@@ -58,22 +59,33 @@ public function testSessionIsSet()
5859
public function testResponseIsPrivate()
5960
{
6061
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
61-
$session->expects($this->once())->method('isStarted')->willReturn(false);
62+
$session->expects($this->exactly(2))->method('isStarted')->willReturn(false);
6263
$session->expects($this->once())->method('hasBeenStarted')->willReturn(true);
6364

6465
$container = new Container();
65-
$container->set('session', $session);
66+
$container->set('initialized_session', $session);
6667

6768
$listener = new SessionListener($container);
6869
$kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();
6970

70-
$request = new Request();
7171
$response = new Response();
72-
$listener->onKernelRequest(new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST));
73-
$listener->onKernelResponse(new FilterResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response));
72+
$listener->onKernelResponse(new FilterResponseEvent($kernel, new Request(), HttpKernelInterface::MASTER_REQUEST, $response));
7473

7574
$this->assertTrue($response->headers->hasCacheControlDirective('private'));
7675
$this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
7776
$this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
7877
}
78+
79+
public function testUninitilizedSession()
80+
{
81+
$event = $this->getMockBuilder(FilterResponseEvent::class)->disableOriginalConstructor()->getMock();
82+
$event->expects($this->once())->method('isMasterRequest')->willReturn(true);
83+
84+
$container = new < 3E14 span class=pl-v>ServiceLocator(array(
85+
'initialized_session' => function () {},
86+
));
87+
88+
$listener = new SessionListener($container);
89+
$listener->onKernelResponse($event);
90+
}
7991
}

‎src/Symfony/Component/Security/Http/HttpUtils.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,13 @@ public function createRedirectResponse(Request $request, $path, $status = 302)
7777
public function createRequest(Request $request, $path)
7878
{
7979
$newRequest = Request::create($this->generateUri($request, $path), 'get', array(), $request->cookies->all(), array(), $request->server->all());
80-
if ($request->hasSession()) {
81-
$newRequest->setSession($request->getSession());
80+
81+
static $setSession;
82+
83+
if (null === $setSession) {
84+
$setSession = \Closure::bind(function ($newRequest, $request) { $newRequest->session = $request->session; }, null, Request::class);
8285
}
86+
$setSession($newRequest, $request);
8387

8488
if ($request->attributes->has(Security::AUTHENTICATION_ERROR)) {
8589
$newRequest->attributes->set(Security::AUTHENTICATION_ERROR, $request->attributes->get(Security::AUTHENTICATION_ERROR));

0 commit comments

Comments
 (0)
0