Skip to content

Commit ad64124

Browse files
fabpotfabpot
committed
feature #41390 [HttpKernel] Add session cookie handling in cli context (alexander-schranz, Nyholm)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [HttpKernel] Add session cookie handling in cli context | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #42890 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Currently the session cookie is never set on the `request` object. In a normal webserver setup this is not needed as a session is in php-fpm or apache fastcgi written by php itself when the response is outputted in: https://github.com/symfony/symfony/blob/744b901750c1cc09e85bcb2ebdf0505449a1158f/src/Symfony/Component/HttpFoundation/Response.php#L393 When use a runner like `Roadrunner` the `session` cookie is never set on the `Request` object, as this kind of appserver never really outputs something and is running in the `cli` context. Actually there is no way from outside to know if `$session->save()` was called or not because when the code in `Roadrunner` executes the session is actually written and so closed here: https://github.com/symfony/symfony/blob/744b901750c1cc09e85bcb2ebdf0505449a1158f/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php#L279. The best way so to fix this issue is that in CLI context symfony writes the session cookie on the `Response` object when the session is really saved. This fixes issues which we have in the current roadrunner implementation of php runtime: - [x] https://github.com/php-runtime/roadrunner-symfony-nyholm/blob/58665237ef2bff08a1ebf269e64abbf8cd09fbd7/src/Runner.php#L89 - [x] https://github.com/php-runtime/roadrunner-symfony-nyholm/blob/58665237ef2bff08a1ebf269e64abbf8cd09fbd7/src/Runner.php#L81 Beside Roadrunner there is also Swoole implementation which would require the same here: php-runtime/runtime#60 With this changes the Runners can be simplified a lot: https://github.com/php-runtime/runtime/pull/62/files# ## TODO Reset session variables after successfully response. The following code currently lives also in the runtime implementation but is requires also by all runners and so we should find a place where we put it: ``` if (PHP_SESSION_ACTIVE === session_status()) { session_abort(); } // reset all session variables to initialize state $_SESSION = []; session_id(''); // in this case session_start() will generate us a new session_id() ``` EDIT: Added this now to the `AbstractSessionListener` service via `ResetInterface`. Commits ------- b3e4f66 [HttpKernel] Add session cookie handling in cli context
2 parents 47385e5 + b3e4f66 commit ad64124

File tree

10 files changed

+179
-13
lines changed

10 files changed

+179
-13
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ public function load(array $configs, ContainerBuilder $container)
327327
$this->sessionConfigEnabled = true;
328328
$this->registerSessionConfiguration($config['session'], $container, $loader);
329329
if (!empty($config['test'])) {
330-
$container->getDefinition('test.session.listener')->setArgument(1, '%session.storage.options%');
330+
$container->getDefinition('test.session.listener')->setArgument(2, '%session.storage.options%');
331331
}
332332
}
333333

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

+2
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,10 @@
153153
'session_collector' => service('data_collector.request.session_collector')->ignoreOnInvalid(),
154154
]),
155155
param('kernel.debug'),
156+
param('session.storage.options'),
156157
])
157158
->tag('kernel.event_subscriber')
159+
->tag('kernel.reset', ['method' => 'reset'])
158160

159161
// for BC
160162
->alias('session.storage.filesystem', 'session.storage.mock_file')

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/test.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
use Symfony\Component\BrowserKit\CookieJar;
1717
use Symfony\Component\BrowserKit\History;
1818
use Symfony\Component\DependencyInjection\ServiceLocator;
19-
use Symfony\Component\HttpKernel\EventListener\TestSessionListener;
19+
use Symfony\Component\HttpKernel\EventListener\SessionListener;
2020

2121
return static function (ContainerConfigurator $container) {
2222
$container->parameters()->set('test.client.parameters', []);
@@ -35,11 +35,13 @@
3535
->set('test.client.history', History::class)->share(false)
3636
->set('test.client.cookiejar', CookieJar::class)->share(false)
3737

38-
->set('test.session.listener', TestSessionListener::class)
38+
->set('test.session.listener', SessionListener::class)
3939
->args([
4040
service_locator([
4141
'session' => service('.session.do-not-use')->ignoreOnInvalid(),
4242
]),
43+
param('kernel.debug'),
44+
param('session.storage.options'),
4345
])
4446
->tag('kernel.event_subscriber')
4547

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ class Request
186186
protected $format;
187187

188188
/**
189-
* @var SessionInterface|callable
189+
* @var SessionInterface|callable(): SessionInterface
190190
*/
191191
protected $session;
192192

@@ -775,6 +775,8 @@ public function setSession(SessionInterface $session)
775775

776776
/**
777777
* @internal
778+
*
779+
* @param callable(): SessionInterface $factory
778780
*/
779781
public function setSessionFactory(callable $factory)
780782
{

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

+89-3
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,16 @@
1313

1414
use Psr\Container\ContainerInterface;
1515
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
16+
use Symfony\Component\HttpFoundation\Cookie;
1617
use Symfony\Component\HttpFoundation\Session\Session;
1718
use Symfony\Component\HttpFoundation\Session\SessionInterface;
19+
use Symfony\Component\HttpFoundation\Session\SessionUtils;
1820
use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
1921
use Symfony\Component\HttpKernel\Event\RequestEvent;
2022
use Symfony\Component\HttpKernel\Event\ResponseEvent;
2123
use Symfony\Component\HttpKernel\Exception\UnexpectedSessionUsageException;
2224
use Symfony\Component\HttpKernel\KernelEvents;
25+
use Symfony\Contracts\Service\ResetInterface;
2326

2427
/**
2528
* Sets the session onto the request on the "kernel.request" event and saves
@@ -36,18 +39,24 @@
3639
*
3740
* @internal
3841
*/
39-
abstract class AbstractSessionListener implements EventSubscriberInterface
42+
abstract class AbstractSessionListener implements EventSubscriberInterface, ResetInterface
4043
{
4144
public const NO_AUTO_CACHE_CONTROL_HEADER = 'Symfony-Session-NoAutoCacheControl';
4245

4346
protected $container;
4447
private $sessionUsageStack = [];
4548
private $debug;
4649

47-
public function __construct(ContainerInterface $container = null, bool $debug = false)
50+
/**
51+
* @var array<string, mixed>
52+
*/
53+
private $sessionOptions;
54+
55+
public function __construct(ContainerInterface $container = null, bool $debug = false, array $sessionOptions = [])
4856
{
4957
$this->container = $container;
5058
$this->debug = $debug;
59+
$this->sessionOptions = $sessionOptions;
5160
}
5261

5362
public function onKernelRequest(RequestEvent $event)
@@ -60,7 +69,22 @@ public function onKernelRequest(RequestEvent $event)
6069
if (!$request->hasSession()) {
6170
// This variable prevents calling `$this->getSession()` twice in case the Request (and the below factory) is cloned
6271
$sess = null;
63-
$request->setSessionFactory(function () use (&$sess) { return $sess ?? $sess = $this->getSession(); });
72+
$request->setSessionFactory(function () use (&$sess, $request) {
73+
if (!$sess) {
74+
$sess = $this->getSession();
75+
}
76+
77+
/*
78+
* For supporting sessions in php runtime with runners like roadrunner or swoole the session
79+
* cookie need read from the cookie bag and set on the session storage.
80+
*/
81+
if ($sess && !$sess->isStarted()) {
82+
$sessionId = $request->cookies->get($sess->getName(), '');
83+
$sess->setId($sessionId);
84+
}
85+
86+
return $sess;
87+
});
6488
}
6589

6690
$session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : null;
@@ -109,6 +133,54 @@ public function onKernelResponse(ResponseEvent $event)
109133
* it is saved will just restart it.
110134
*/
111135
$session->save();
136+
137+
/*
138+
* For supporting sessions in php runtime with runners like roadrunner or swoole the session
139+
* cookie need to be written on the response object and should not be written by PHP itself.
140+
*/
141+
$sessionName = $session->getName();
142+
$sessionId = $session->getId();
143+
$sessionCookiePath = $this->sessionOptions['cookie_path'] ?? '/';
144+
$sessionCookieDomain = $this->sessionOptions['cookie_domain'] ?? null;
145+
$sessionCookieSecure = $this->sessionOptions['cookie_secure'] ?? false;
146+
$sessionCookieHttpOnly = $this->sessionOptions['cookie_httponly'] ?? true;
147+
$sessionCookieSameSite = $this->sessionOptions['cookie_samesite'] ?? Cookie::SAMESITE_LAX;
148+
149+
SessionUtils::popSessionCookie($sessionName, $sessionCookiePath);
150+
151+
$request = $event->getRequest();
152+
$requestSessionCookieId = $request->cookies->get($sessionName);
153+
154+
if ($requestSessionCookieId && $session->isEmpty()) {
155+
$response->headers->clearCookie(
156+
$sessionName,
157+
$sessionCookiePath,
158+
$sessionCookieDomain,
159+
$sessionCookieSecure,
160+
$sessionCookieHttpOnly,
161+
$sessionCookieSameSite
162+
);
163+
} elseif ($sessionId !== $requestSessionCookieId) {
164+
$expire = 0;
165+
$lifetime = $this->sessionOptions['cookie_lifetime'] ?? null;
166+
if ($lifetime) {
167+
$expire = time() + $lifetime;
168+
}
169+
170+
$response->headers->setCookie(
171+
Cookie::create(
172+
$sessionName,
173+
$sessionId,
174+
$expire,
175+
$sessionCookiePath,
176+
$sessionCookieDomain,
177+
$sessionCookieSecure,
178+
$sessionCookieHttpOnly,
179+
false,
180+
$sessionCookieSameSite
181+
)
182+
);
183+
}
112184
}
113185

114186
if ($session instanceof Session ? $session->getUsageIndex() === end($this->sessionUsageStack) : !$session->isStarted()) {
@@ -188,6 +260,20 @@ public static function getSubscribedEvents(): array
188260
];
189261
}
190262

263+
public function reset(): void
264+
{
265+
if (\PHP_SESSION_ACTIVE === session_status()) {
266+
session_abort();
267+
}
268+
269+
session_unset();
270+
$_SESSION = [];
271+
272+
if (!headers_sent()) { // session id can only be reset when no headers were so we check for headers_sent first
273+
session_id('');
274+
}
275+
}
276+
191277
/**
192278
* Gets the session object.
193279
*

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

+4
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
* @author Fabien Potencier <fabien@symfony.com>
2929
*
3030
* @internal
31+
*
32+
* @deprecated the TestSessionListener use the default SessionListener instead
3133
*/
3234
abstract class AbstractTestSessionListener implements EventSubscriberInterface
3335
{
@@ -37,6 +39,8 @@ abstract class AbstractTestSessionListener implements EventSubscriberInterface
3739
public function __construct(array $sessionOptions = [])
3840
{
3941
$this->sessionOptions = $sessionOptions;
42+
43+
trigger_deprecation('symfony/http-kernel', '5.4', 'The %s is deprecated use the %s instead.', __CLASS__, AbstractSessionListener::class);
4044
}
4145

4246
public function onKernelRequest(RequestEvent $event)

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

-6
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace Symfony\Component\HttpKernel\EventListener;
1313

14-
use Psr\Container\ContainerInterface;
1514
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1615
use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;
1716
use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -29,11 +28,6 @@
2928
*/
3029
class SessionListener extends AbstractSessionListener
3130
{
32-
public function __construct(ContainerInterface $container, bool $debug = false)
33-
{
34-
parent::__construct($container, $debug);
35-
}
36-
3731
public function onKernelRequest(RequestEvent $event)
3832
{
3933
parent::onKernelRequest($event);

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

+2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
* @author Fabien Potencier <fabien@symfony.com>
2121
*
2222
* @final
23+
*
24+
* @deprecated the TestSessionListener use the default SessionListener instead
2325
*/
2426
class TestSessionListener extends AbstractTestSessionListener
2527
{

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

+73
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public function testOnlyTriggeredOnMainRequest()
4747
public function testSessionIsSet()
4848
{
4949
$session = $this->createMock(Session::class);
50+
$session->expects($this->exactly(1))->method('getName')->willReturn('PHPSESSID');
5051

5152
$requestStack = $this->createMock(RequestStack::class);
5253
$requestStack->expects($this->once())->method('getMainRequest')->willReturn(null);
@@ -73,6 +74,7 @@ public function testSessionIsSet()
7374
public function testSessionUsesFactory()
7475
{
7576
$session = $this->createMock(Session::class);
77+
$session->expects($this->exactly(1))->method('getName')->willReturn('PHPSESSID');
7678
$sessionFactory = $this->createMock(SessionFactory::class);
7779
$sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
7880

@@ -142,6 +144,32 @@ public function testResponseIsStillPublicIfSessionStartedAndHeaderPresent()
142144
$this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
143145
}
144146

147+
public function testSessionSaveAndResponseHasSessionCookie()
148+
{
149+
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
150+
$session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
151+
$session->expects($this->exactly(1))->method('getId')->willReturn('123456');
152+
$session->expects($this->exactly(1))->method('getName')->willReturn('PHPSESSID');
153+
$session->expects($this->exactly(1))->method('save');
154+
$session->expects($this->exactly(1))->method('isStarted')->willReturn(true);
155+
156+
$container = new Container();
157+
$container->set('initialized_session', $session);
158+
159+
$listener = new SessionListener($container);
160+
$kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();
161+
162+
$request = new Request();
163+
$listener->onKernelRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST));
164+
165+
$response = new Response();
166+
$listener->onKernelResponse(new ResponseEvent($kernel, new Request(), HttpKernelInterface::MASTER_REQUEST, $response));
167+
168+
$cookies = $response->headers->getCookies();
169+
$this->assertSame('PHPSESSID', $cookies[0]->getName());
170+
$this->assertSame('123456', $cookies[0]->getValue());
171+
}
172+
145173
public function testUninitializedSession()
146174
{
147175
$kernel = $this->createMock(HttpKernelInterface::class);
@@ -166,6 +194,7 @@ public function testUninitializedSession()
166194
public function testSurrogateMainRequestIsPublic()
167195
{
168196
$session = $this->createMock(Session::class);
197+
$session->expects($this->exactly(2))->method('getName')->willReturn('PHPSESSID');
169198
$session->expects($this->exactly(4))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1, 1, 1));
170199

171200
$container = new Container();
@@ -205,6 +234,7 @@ public function testSurrogateMainRequestIsPublic()
205234
public function testGetSessionIsCalledOnce()
206235
{
207236
$session = $this->createMock(Session::class);
237+
$session->expects($this->exactly(2))->method('getName')->willReturn('PHPSESSID');
208238
$sessionStorage = $this->createMock(NativeSessionStorage::class);
209239
$kernel = $this->createMock(KernelInterface::class);
210240

@@ -282,6 +312,7 @@ public function testSessionUsageLogIfStatelessAndSessionUsed()
282312
public function testSessionIsSavedWhenUnexpectedSessionExceptionThrown()
283313
{
284314
$session = $this->createMock(Session::class);
315+
$session->expects($this->exactly(1))->method('getName')->willReturn('PHPSESSID');
285316
$session->method('isStarted')->willReturn(true);
286317
$session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
287318
$session->expects($this->exactly(1))->method('save');
@@ -368,4 +399,46 @@ public function testSessionUsageCallbackWhenNoStateless()
368399

369400
(new SessionListener($container, true))->onSessionUsage();
370401
}
402+
403+
/**
404+
* @runInSeparateProcess
405+
*/
406+
public function testReset()
407+
{
408+
session_start();
409+
$_SESSION['test'] = ['test'];
410+
session_write_close();
411+
412+
$this->assertNotEmpty($_SESSION);
413+
$this->assertNotEmpty(session_id());
414+
415+
$container = new Container();
416+
417+
(new SessionListener($container, true))->reset();
418+
419+
$this->assertEmpty($_SESSION);
420+
$this->assertEmpty(session_id());
421+
$this->assertSame(\PHP_SESSION_NONE, session_status());
422+
}
423+
424+
/**
425+
* @runInSeparateProcess
426+
*/
427+
public function testResetUnclosedSession()
428+
{
429+
session_start();
430+
$_SESSION['test'] = ['test'];
431+
432+
$this->assertNotEmpty($_SESSION);
433+
$this->assertNotEmpty(session_id());
434+
$this->assertSame(\PHP_SESSION_ACTIVE, session_status());
435+
436+
$container = new Container();
437+
438+
(new SessionListener($container, true))->reset();
439+
440+
$this->assertEmpty($_SESSION);
441+
$this->assertEmpty(session_id());
442+
$this->assertSame(\PHP_SESSION_NONE, session_status());
443+
}
371444
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
* Tests SessionListener.
2929
*
3030
* @author Bulat Shakirzyanov <mallluhuct@gmail.com>
31+
* @group legacy
3132
*/
3233
class TestSessionListenerTest extends TestCase
3334
{

0 commit comments

Comments
 (0)