Skip to content

Remove roadrunner specific session handling #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 7, 2021
Merged

Remove roadrunner specific session handling #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2ab507a
Fix removing of session cookie after logout
alexander-schranz Jul 7, 2021
996a54d
Remove roadrunner specific session handling
alexander-schranz Jul 8, 2021
73e4063
Simplify also the Runner
alexander-schranz Jul 8, 2021
60d4d71
Remove session reset
alexander-schranz Sep 6, 2021
964e132
Fix php-cs
alexander-schranz Sep 6, 2021
372a812
Increased required versions
alexander-schranz Sep 6, 2021
2df51f3
Fix tests
alexander-schranz Sep 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
"spiral/roadrunner": "^2.0",
"symfony/console": "^5.2 || ^6.0",
"symfony/dependency-injection": "^5.2 || ^6.0",
"symfony/http-foundation": "^5.2 || ^6.0",
"symfony/http-kernel": "^5.2 || ^6.0",
"symfony/http-foundation": "^5.3 || ^6.0",
"symfony/http-kernel": "^5.4 || ^6.0",
"symfony/psr-http-message-bridge": "^2.1",
"symfony/runtime": "^5.3 || ^6.0"
},
Expand Down
4 changes: 2 additions & 2 deletions src/bref/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
"symfony/runtime": "^5.3 || ^6.0"
},
"require-dev": {
"symfony/http-foundation": "^4.4 || ^5.2 || ^6.0",
"symfony/http-kernel": "^4.4 || ^5.2 || ^6.0",
"symfony/http-foundation": "^5.3 || ^6.0",
"symfony/http-kernel": "^5.4 || ^6.0",
"symfony/phpunit-bridge": "^5.3"
},
"autoload": {
Expand Down
2 changes: 1 addition & 1 deletion src/roadrunner-symfony-nyholm/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"nyholm/psr7": "^1.4",
"spiral/roadrunner": "^2.0",
"symfony/dependency-injection": "^5.3 || ^6.0",
"symfony/http-kernel": "^5.3 || ^6.0",
"symfony/http-kernel": "^5.4 || ^6.0",
"symfony/psr-http-message-bridge": "^2.1",
"symfony/runtime": "^5.3 || ^6.0"
},
Expand Down
79 changes: 2 additions & 77 deletions src/roadrunner-symfony-nyholm/src/Runner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,7 @@
use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory;
use Symfony\Bridge\PsrHttpMessage\HttpFoundationFactoryInterface;
use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpFoundation\Request as SymfonyRequest;
use Symfony\Component\HttpFoundation\Response as SymfonyResponse;
use Symfony\Component\HttpKernel\HttpCache\HttpCache;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelInterface;
use Symfony\Component\HttpKernel\TerminableInterface;
use Symfony\Component\Runtime\RunnerInterface;

Expand All @@ -27,33 +22,12 @@ class Runner implements RunnerInterface
private $httpMessageFactory;
private $psrFactory;

/**
* @var array<string, mixed>
*/
private $sessionOptions;

/**
* @param HttpKernelInterface|KernelInterface $kernel
*/
public function __construct($kernel, ?HttpFoundationFactoryInterface $httpFoundationFactory = null, ?HttpMessageFactoryInterface $httpMessageFactory = null)
public function __construct(HttpKernelInterface $kernel, ?HttpFoundationFactoryInterface $httpFoundationFactory = null, ?HttpMessageFactoryInterface $httpMessageFactory = null)
{
$this->kernel = $kernel;
$this->psrFactory = new Psr7\Factory\Psr17Factory();
$this->httpFoundationFactory = $httpFoundationFactory ?? new HttpFoundationFactory();
$this->httpMessageFactory = $httpMessageFactory ?? new PsrHttpFactory($this->psrFactory, $this->psrFactory, $this->psrFactory, $this->psrFactory);

if ($kernel instanceof HttpCache) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn’t this need anymore?

Copy link
Member Author

@alexander-schranz alexander-schranz Sep 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ($kernel instanceof HttpCache) { was only needed to get the parameters from the wrapped kernel. As we are not longer need to handle the sessions here we could remove this. The support for HttpCache is still here as HttpCache implements HttpKernelInterface and will handle the requests as expected.

Did update the tests which create the Runner instances.

$kernel = $kernel->getKernel();
}

if (!$kernel instanceof KernelInterface) {
throw new \InvalidArgumentException(sprintf('Expected argument of type "%s" or "%s", "%s" given.', KernelInterface::class, HttpCache::class, get_class($kernel)));
}

$kernel->boot();
$container = $kernel->getContainer();
$this->sessionOptions = $container->getParameter('session.storage.options');
$kernel->shutdown();
}

public function run(): int
Expand All @@ -64,66 +38,17 @@ public function run(): int
while ($request = $worker->waitRequest()) {
try {
$sfRequest = $this->httpFoundationFactory->createRequest($request);
$sfResponse = $this->handle($sfRequest);
$sfResponse = $this->kernel->handle($sfRequest);
$worker->respond($this->httpMessageFactory->createResponse($sfResponse));

if ($this->kernel instanceof TerminableInterface) {
$this->kernel->terminate($sfRequest, $sfResponse);
}
} catch (\Throwable $e) {
$worker->getWorker()->error((string) $e);
} finally {
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()
}
}

return 0;
}

private function handle(SymfonyRequest $request): SymfonyResponse
{
$sessionName = $this->sessionOptions['name'] ?? \session_name();
/** @var string $requestSessionId */
$requestSessionId = $request->cookies->get($sessionName, '');

// TODO invalid session id should be expired: see F at https://github.com/php-runtime/runtime/issues/46
\session_id($requestSessionId);

$response = $this->kernel->handle($request);

if ($request->hasSession()) {
$sessionId = \session_id();
// we can not use $session->isStarted() here as this state is not longer available at this time
// TODO session cookie should only be set when persisted by symfony: see E at https://github.com/php-runtime/runtime/issues/46
if ($sessionId && $sessionId !== $requestSessionId) {
$expires = 0;
$lifetime = $this->sessionOptions['cookie_lifetime'] ?? null;
if ($lifetime) {
$expires = time() + $lifetime;
}

$response->headers->setCookie(
Cookie::create(
$sessionName,
$sessionId,
$expires,
$this->sessionOptions['cookie_path'] ?? '/',
$this->sessionOptions['cookie_domain'] ?? null,
$this->sessionOptions['cookie_secure'] ?? null,
$this->sessionOptions['cookie_httponly'] ?? true,
false,
$this->sessionOptions['cookie_samesite'] ?? Cookie::SAMESITE_LAX
)
);
}
}

return $response;
}
}
7 changes: 2 additions & 5 deletions src/roadrunner-symfony-nyholm/src/Runtime.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

namespace Runtime\RoadRunnerSymfonyNyholm;

use Symfony\Component\HttpKernel\HttpCache\HttpCache;
use Symfony\Component\HttpKernel\KernelInterface;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Runtime\RunnerInterface;
use Symfony\Component\Runtime\SymfonyRuntime;

Expand All @@ -16,9 +15,7 @@ class Runtime extends SymfonyRuntime
{
public function getRunner(?object $application): RunnerInterface
{
if ($application instanceof KernelInterface
|| ($application instanceof HttpCache && $application->getKernel() instanceof KernelInterface)
) {
if ($application instanceof HttpKernelInterface) {
return new Runner($application);
}

Expand Down
60 changes: 13 additions & 47 deletions src/roadrunner-symfony-nyholm/tests/RuntimeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
namespace Runtime\RoadRunnerSymfonyNyholm;

use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Console\Application;
use Symfony\Component\HttpKernel\HttpCache\HttpCache;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelInterface;
use Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner;
use Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner;

/**
* @author Alexander Schranz <alexander@sulu.io>
Expand All @@ -20,25 +20,6 @@ public function testGetRuntimeHttpKernel(): void
->disableOriginalConstructor()
->getMock();

$kernel->expects($this->once())
->method('boot');

$container = $this->getMockBuilder(ContainerInterface::class)
->disableOriginalConstructor()
->getMock();

$container->expects($this->once())
->method('getParameter')
->with('session.storage.options')
->will($this->returnValue([]));

$kernel->expects($this->once())
->method('getContainer')
->will($this->returnValue($container));

$kernel->expects($this->once())
->method('shutdown');

$runtime = new Runtime();
$runner = $runtime->getRunner($kernel);

Expand All @@ -51,48 +32,33 @@ public function testGetRuntimeHttpCache(): void
->disableOriginalConstructor()
->getMock();

$kernel = $this->getMockBuilder(KernelInterface::class)
->disableOriginalConstructor()
->getMock();

$httpCache->expects($this->atLeastOnce())
->method('getKernel')
->will($this->returnValue($kernel));
$runtime = new Runtime();
$runner = $runtime->getRunner($httpCache);

$kernel->expects($this->once())
->method('boot');
$this->assertInstanceOf(Runner::class, $runner);
}

$container = $this->getMockBuilder(ContainerInterface::class)
public function testGetRuntimeHttpKernelInterface(): void
{
$kernel = $this->getMockBuilder(HttpKernelInterface::class)
->disableOriginalConstructor()
->getMock();

$container->expects($this->once())
->method('getParameter')
->with('session.storage.options')
->will($this->returnValue([]));

$kernel->expects($this->once())
->method('getContainer')
->will($this->returnValue($container));

$kernel->expects($this->once())
->method('shutdown');

$runtime = new Runtime();
$runner = $runtime->getRunner($httpCache);
$runner = $runtime->getRunner($kernel);

$this->assertInstanceOf(Runner::class, $runner);
}

public function testGetRuntimeHttpKernelInterface(): void
public function testGetRuntimeApplication(): void
{
$kernel = $this->getMockBuilder(HttpKernelInterface::class)
$kernel = $this->getMockBuilder(Application::class)
->disableOriginalConstructor()
->getMock();

$runtime = new Runtime();
$runner = $runtime->getRunner($kernel);

$this->assertInstanceOf(HttpKernelRunner::class, $runner);
$this->assertInstanceOf(ConsoleApplicationRunner::class, $runner);
}
}
4 changes: 2 additions & 2 deletions src/swoole/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
"require-dev": {
"illuminate/http": "^8.48",
"swoole/ide-helper": "^4.6",
"symfony/http-foundation": "^4.4 || ^5.2 || ^6.0",
"symfony/http-kernel": "^4.4 || ^5.2 || ^6.0",
"symfony/http-foundation": "^5.3 || ^6.0",
"symfony/http-kernel": "^5.4 || ^6.0",
"symfony/phpunit-bridge": "^5.3"
},
"autoload": {
Expand Down