From 3b3566bb736e2e191986c1d8c9812b63da840766 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Tue, 15 Mar 2022 09:32:42 +0100 Subject: [PATCH 01/11] revised code --- Classes/Annotations/ReadOnly.php | 2 +- Classes/Aspects/ReadOnlyAspect.php | 30 ++++--- Classes/Aspects/ReadOnlyFilter.php | 91 +++++++++++++------- Classes/Http/SessionLockRequestComponent.php | 40 ++++----- Classes/Package.php | 31 ------- Configuration/Development/Settings.yaml | 2 +- Configuration/Objects.yaml | 16 ++-- Configuration/Settings.yaml | 18 ++-- Configuration/Testing/Settings.yaml | 3 + composer.json | 38 ++++---- 10 files changed, 135 insertions(+), 136 deletions(-) delete mode 100644 Classes/Package.php create mode 100644 Configuration/Testing/Settings.yaml diff --git a/Classes/Annotations/ReadOnly.php b/Classes/Annotations/ReadOnly.php index ce2e966..cac72be 100644 --- a/Classes/Annotations/ReadOnly.php +++ b/Classes/Annotations/ReadOnly.php @@ -4,7 +4,7 @@ namespace DigiComp\FlowSessionLock\Annotations; /** * @Annotation - * @Target("METHOD") + * @Target({"METHOD"}) */ final class ReadOnly { diff --git a/Classes/Aspects/ReadOnlyAspect.php b/Classes/Aspects/ReadOnlyAspect.php index f45ab95..355ddf4 100644 --- a/Classes/Aspects/ReadOnlyAspect.php +++ b/Classes/Aspects/ReadOnlyAspect.php @@ -36,37 +36,43 @@ class ReadOnlyAspect /** * @Flow\Around("methodAnnotatedWith(DigiComp\FlowSessionLock\Annotations\ReadOnly) || filter(DigiComp\FlowSessionLock\Aspects\ReadOnlyFilter)") * @param JoinPointInterface $joinPoint - * - * @return void + * @return mixed */ public function demoteLockToReadOnly(JoinPointInterface $joinPoint) { - $handler = $this->bootstrap->getActiveRequestHandler(); - if (! $handler instanceof HttpRequestHandlerInterface) { - $this->logger->debug(\get_class($handler)); + $activeRequestHandler = $this->bootstrap->getActiveRequestHandler(); + if (!$activeRequestHandler instanceof HttpRequestHandlerInterface) { + $this->logger->debug('SessionLock: ' . \get_class($activeRequestHandler)); + return $joinPoint->getAdviceChain()->proceed($joinPoint); } - $componentContext = $handler->getComponentContext(); - /** @var Lock $lock */ - $lock = $componentContext->getParameter(SessionLockRequestComponent::class, 'sessionLock'); + $this->readOnly = true; - if ($lock) { - $this->logger->debug('SessionLock: Release, as this is marked read only'); + + /** @var Lock|null $lock */ + $lock = $activeRequestHandler->getComponentContext()->getParameter( + SessionLockRequestComponent::class, + SessionLockRequestComponent::PARAMETER_NAME + ); + if ($lock !== null) { + $this->logger->debug('SessionLock: Release, as this is marked read only.'); $lock->release(); } + return $joinPoint->getAdviceChain()->proceed($joinPoint); } /** * @Flow\Around("method(Neos\Flow\Session\Session->shutdownObject())") - * * @param JoinPointInterface $joinPoint + * @return mixed|void */ public function doNotSaveSession(JoinPointInterface $joinPoint) { if ($this->readOnly) { return; } - $joinPoint->getAdviceChain()->proceed($joinPoint); + + return $joinPoint->getAdviceChain()->proceed($joinPoint); } } diff --git a/Classes/Aspects/ReadOnlyFilter.php b/Classes/Aspects/ReadOnlyFilter.php index f7d7d04..4925d4c 100644 --- a/Classes/Aspects/ReadOnlyFilter.php +++ b/Classes/Aspects/ReadOnlyFilter.php @@ -4,9 +4,12 @@ namespace DigiComp\FlowSessionLock\Aspects; use Neos\Flow\Annotations as Flow; use Neos\Flow\Aop\Builder\ClassNameIndex; +use Neos\Flow\Aop\Exception as NeosFlowAopException; +use Neos\Flow\Aop\Exception\InvalidPointcutExpressionException; use Neos\Flow\Aop\Pointcut\PointcutFilterComposite; use Neos\Flow\Aop\Pointcut\PointcutFilterInterface; use Neos\Flow\Configuration\ConfigurationManager; +use Neos\Flow\Configuration\Exception\InvalidConfigurationTypeException; use Neos\Flow\Security\Authorization\Privilege\Method\MethodTargetExpressionParser; /** @@ -15,20 +18,32 @@ use Neos\Flow\Security\Authorization\Privilege\Method\MethodTargetExpressionPars */ class ReadOnlyFilter implements PointcutFilterInterface { + /** + * @var ConfigurationManager + */ protected ConfigurationManager $configurationManager; + /** + * @var MethodTargetExpressionParser + */ protected MethodTargetExpressionParser $methodTargetExpressionParser; /** * @var PointcutFilterComposite[] */ - protected ?array $filters = null; + protected ?array $pointcutFilterComposites = null; + /** + * @param ConfigurationManager $configurationManager + */ public function injectConfigurationManager(ConfigurationManager $configurationManager): void { $this->configurationManager = $configurationManager; } + /** + * @param MethodTargetExpressionParser $methodTargetExpressionParser + */ public function injectMethodTargetExpressionParser(MethodTargetExpressionParser $methodTargetExpressionParser): void { $this->methodTargetExpressionParser = $methodTargetExpressionParser; @@ -36,30 +51,28 @@ class ReadOnlyFilter implements PointcutFilterInterface /** * @inheritDoc + * @throws InvalidConfigurationTypeException + * @throws InvalidPointcutExpressionException + * @throws NeosFlowAopException */ public function matches($className, $methodName, $methodDeclaringClassName, $pointcutQueryIdentifier): bool { - if ($this->filters === null) { - $this->buildPointcutFilters(); - } + $this->buildPointcutFilters(); - $matchingFilters = \array_filter( - $this->filters, - function (PointcutFilterInterface $filter) use ( - $className, - $methodName, - $methodDeclaringClassName, - $pointcutQueryIdentifier - ): bool { - return $filter->matches($className, $methodName, $methodDeclaringClassName, $pointcutQueryIdentifier); + foreach ($this->pointcutFilterComposites as $pointcutFilterComposite) { + if ( + $pointcutFilterComposite->matches( + $className, + $methodName, + $methodDeclaringClassName, + $pointcutQueryIdentifier + ) + ) { + return true; } - ); - - if ($matchingFilters === []) { - return false; } - return true; + return false; } /** @@ -80,33 +93,47 @@ class ReadOnlyFilter implements PointcutFilterInterface /** * @inheritDoc + * @throws InvalidConfigurationTypeException + * @throws InvalidPointcutExpressionException + * @throws NeosFlowAopException */ public function reduceTargetClassNames(ClassNameIndex $classNameIndex): ClassNameIndex { - if ($this->filters === null) { - $this->buildPointcutFilters(); - } + $this->buildPointcutFilters(); $result = new ClassNameIndex(); - foreach ($this->filters as $filter) { - $result->applyUnion($filter->reduceTargetClassNames($classNameIndex)); + + foreach ($this->pointcutFilterComposites as $pointcutFilterComposite) { + $result->applyUnion($pointcutFilterComposite->reduceTargetClassNames($classNameIndex)); } + return $result; } + /** + * @throws InvalidConfigurationTypeException + * @throws InvalidPointcutExpressionException + * @throws NeosFlowAopException + */ protected function buildPointcutFilters(): void { - $this->filters = []; - $readOnlyExpressions = $this->configurationManager->getConfiguration( - ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, - 'DigiComp.FlowSessionLock.readOnlyExpressions' - ) ?? []; - foreach ($readOnlyExpressions as $key => $pointcut) { - if ($pointcut === null) { + if ($this->pointcutFilterComposites !== null) { + return; + } + + $this->pointcutFilterComposites = []; + foreach ( + $this->configurationManager->getConfiguration( + ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'DigiComp.FlowSessionLock.readOnlyExpressions' + ) as $key => $pointcutExpression + ) { + if ($pointcutExpression === null) { continue; } - $this->filters[] = $this->methodTargetExpressionParser->parse( - $pointcut, + + $this->pointcutFilterComposites[] = $this->methodTargetExpressionParser->parse( + $pointcutExpression, 'Settings.yaml at "DigiComp.FlowSessionLock.readOnlyExpressions", key: "' . $key . '"' ); } diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 964e9e2..9677b46 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -5,25 +5,19 @@ namespace DigiComp\FlowSessionLock\Http; use Neos\Flow\Annotations as Flow; use Neos\Flow\Http\Component\ComponentContext; use Neos\Flow\Http\Component\ComponentInterface; -use Neos\Flow\Utility\Environment; -use Neos\Utility\Files; use Psr\Log\LoggerInterface; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\LockFactory; class SessionLockRequestComponent implements ComponentInterface { - /** - * @Flow\InjectConfiguration(package="Neos.Flow", path="session") - * @var array - */ - protected $sessionSettings; + public const PARAMETER_NAME = 'sessionLock'; /** * @Flow\Inject(lazy=false) * @var LoggerInterface */ - protected $logger; + protected LoggerInterface $logger; /** * @Flow\Inject(name="DigiComp.FlowSessionLock:LockFactory") @@ -31,40 +25,42 @@ class SessionLockRequestComponent implements ComponentInterface */ protected $lockFactory; + /** + * @Flow\InjectConfiguration(package="Neos.Flow", path="session") + * @var array + */ + protected array $sessionSettings; + + /** + * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="timeToLive") + * @var float + */ + protected float $timeToLive; + /** * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="autoRelease") * @var bool */ protected bool $autoRelease; - /** - * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="timeToLive") - * @var int - */ - protected int $timeToLive; - /** * @inheritDoc */ public function handle(ComponentContext $componentContext) { $sessionCookieName = $this->sessionSettings['name']; - $request = $componentContext->getHttpRequest(); - $cookies = $request->getCookieParams(); + $cookies = $componentContext->getHttpRequest()->getCookieParams(); if (!isset($cookies[$sessionCookieName])) { return; } - $sessionIdentifier = $cookies[$sessionCookieName]; - - $key = new Key( - 'session-' . $sessionIdentifier - ); //TODO: sessionIdentifier might be wrong, probably it should probably be storage identifier + // TODO: sessionIdentifier might be wrong, probably it should probably be storage identifier + $key = new Key('session-' . $cookies[$sessionCookieName]); $lock = $this->lockFactory->createLockFromKey($key, $this->timeToLive, $this->autoRelease); - $componentContext->setParameter(SessionLockRequestComponent::class, 'sessionLock', $lock); + $componentContext->setParameter(SessionLockRequestComponent::class, static::PARAMETER_NAME, $lock); $this->logger->debug('SessionLock: Get ' . $key); $lock->acquire(true); diff --git a/Classes/Package.php b/Classes/Package.php deleted file mode 100644 index 66e97be..0000000 --- a/Classes/Package.php +++ /dev/null @@ -1,31 +0,0 @@ -getSignalSlotDispatcher(); - - $dispatcher->connect( - ConfigurationManager::class, - 'configurationManagerReady', - function (ConfigurationManager $configurationManager) { - $lockStoreDir = $configurationManager->getConfiguration( - ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, - 'DigiComp.FlowSessionLock.lockStoreDir' - ); - if (is_string($lockStoreDir)) { - Files::createDirectoryRecursively($lockStoreDir); - } - } - ); - } -} diff --git a/Configuration/Development/Settings.yaml b/Configuration/Development/Settings.yaml index 0760c4d..23b7e23 100644 --- a/Configuration/Development/Settings.yaml +++ b/Configuration/Development/Settings.yaml @@ -1,3 +1,3 @@ DigiComp: FlowSessionLock: - lockStoreDir: '%FLOW_PATH_DATA%Temporary/Development/SessionLocks' + lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Development/SessionLocks/" diff --git a/Configuration/Objects.yaml b/Configuration/Objects.yaml index 6523a12..95f5480 100644 --- a/Configuration/Objects.yaml +++ b/Configuration/Objects.yaml @@ -1,12 +1,10 @@ DigiComp.FlowSessionLock:LockFactory: - className: 'Symfony\Component\Lock\LockFactory' + className: "Symfony\\Component\\Lock\\LockFactory" arguments: 1: - object: 'DigiComp.FlowSessionLock:LockStore' - -DigiComp.FlowSessionLock:LockStore: - className: 'Symfony\Component\Lock\Store\FlockStore' - scope: 'singleton' - arguments: - 1: - setting: 'DigiComp.FlowSessionLock.lockStoreDir' + object: + factoryObjectName: "Symfony\\Component\\Lock\\Store\\StoreFactory" + factoryMethodName: "createStore" + arguments: + 1: + setting: "DigiComp.FlowSessionLock.lockStoreConnection" diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index e49711d..f5b77b2 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -1,3 +1,10 @@ +DigiComp: + FlowSessionLock: + lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Production/SessionLocks/" + timeToLive: 300.0 + autoRelease: true + readOnlyExpressions: {} + Neos: Flow: http: @@ -5,12 +12,5 @@ Neos: preprocess: chain: lockSession: - position: 'before getSessionCookieFromRequest' - component: 'DigiComp\FlowSessionLock\Http\SessionLockRequestComponent' - -DigiComp: - FlowSessionLock: - lockStoreDir: '%FLOW_PATH_DATA%Temporary/Production/SessionLocks' - readOnlyExpressions: [] - autoRelease: true - timeToLive: 300 + position: "before getSessionCookieFromRequest" + component: "DigiComp\\FlowSessionLock\\Http\\SessionLockRequestComponent" diff --git a/Configuration/Testing/Settings.yaml b/Configuration/Testing/Settings.yaml new file mode 100644 index 0000000..07b1b05 --- /dev/null +++ b/Configuration/Testing/Settings.yaml @@ -0,0 +1,3 @@ +DigiComp: + FlowSessionLock: + lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Testing/SessionLocks/" diff --git a/composer.json b/composer.json index 811e3c2..e774de9 100644 --- a/composer.json +++ b/composer.json @@ -1,25 +1,11 @@ { "name": "digicomp/flowsessionlock", + "description": "Session locking for Neos Flow - it secures the session becoming corrupted by concurrent access to the same session by different requests", "type": "neos-package", - "description": "Sesion locking for Neos Flow - it secures the session becoming corrupted by concurrent access to the same session by different requests", - "keywords": [ - "flow", - "neos" - ], - "authors": [ - { - "name": "Ferdinand Kuhl", - "email": "f.kuhl@digital-competence.de", - "homepage": "http://www.digital-competence.de", - "role": "Developer" - } - ], - "license": "MIT", - "homepage": "https://github.com/digicomp/DigiComp.FlowSessionLock", "require": { - "neos/flow": "^6.2", - "php": "^7.4", - "symfony/lock": "^5.2" + "neos/flow": "^6.2.0", + "php": ">=7.4", + "symfony/lock": "^5.2.0" }, "autoload": { "psr-4": { @@ -33,5 +19,19 @@ "branch-alias": { "dev-develop": "1.0.x-dev" } - } + }, + "authors": [ + { + "name": "Ferdinand Kuhl", + "email": "f.kuhl@digital-competence.de", + "homepage": "https://www.digital-competence.de", + "role": "Developer" + } + ], + "license": "MIT", + "homepage": "https://github.com/digital-competence/FlowSessionLock", + "keywords": [ + "Neos", + "Flow" + ] } From 7420419663d1729212b43a21a24f95a0288cfe86 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Mon, 4 Apr 2022 22:41:28 +0200 Subject: [PATCH 02/11] change dependency to neos/flow --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index e774de9..98ad265 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "description": "Session locking for Neos Flow - it secures the session becoming corrupted by concurrent access to the same session by different requests", "type": "neos-package", "require": { - "neos/flow": "^6.2.0", + "neos/flow": "^6.3.0", "php": ">=7.4", "symfony/lock": "^5.2.0" }, From 737047d856f58f8613831de3fe5cd2566c5592d7 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Tue, 5 Apr 2022 12:31:33 +0200 Subject: [PATCH 03/11] do load lazy if possible --- Classes/Http/SessionLockRequestComponent.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 9677b46..5b59d42 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -14,10 +14,10 @@ class SessionLockRequestComponent implements ComponentInterface public const PARAMETER_NAME = 'sessionLock'; /** - * @Flow\Inject(lazy=false) + * @Flow\Inject * @var LoggerInterface */ - protected LoggerInterface $logger; + protected $logger; /** * @Flow\Inject(name="DigiComp.FlowSessionLock:LockFactory") From e7ca78f855a4ac70eb5b2d43615181ab34557109 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Tue, 5 Apr 2022 15:19:26 +0200 Subject: [PATCH 04/11] code style --- Classes/Http/SessionLockRequestComponent.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 5b59d42..691c95b 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -46,7 +46,7 @@ class SessionLockRequestComponent implements ComponentInterface /** * @inheritDoc */ - public function handle(ComponentContext $componentContext) + public function handle(ComponentContext $componentContext): void { $sessionCookieName = $this->sessionSettings['name']; From f784fb3d3f3a97fdc45748367a85d1091860936d Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Wed, 20 Apr 2022 17:36:30 +0200 Subject: [PATCH 05/11] revised code --- README.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 781bc71..0717b21 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,16 @@ DigiComp.FlowSessionLock ------------------------ -By default the session established by Flow is not "protected" in any way. -This package restricts every request to load the session only, if there are no other requests having it in access currently. -It allows to set custom pointcut which will set the session in "ReadOnly" mode, which allows concurrent requests to read, but disallows the current request to write the session. +By default, the session established by Flow is not "protected" in any way. This package restricts every request to load +the session only, if there are no other requests having it in access currently. It allows to set custom pointcut which +will set the session in "ReadOnly" mode, which allows concurrent requests to read, but disallows the current request to +write the session. -If you want to allow concurrent access somewhere, you can add your trigger pointcut in Settings.yaml like such: - - DigiComp: - FlowSessionLock: - readOnlyExpressions: - 'AcmeLock': 'method(Acme/SuperPackage/Controller/ConcurrentController->concurrentAction())' +If you want to allow concurrent access somewhere, you can add your trigger pointcut in `Settings.yaml` like such: + +```yaml +DigiComp: + FlowSessionLock: + readOnlyExpressions: + MyLock: "method(My\\Package\\Controller\\MyController->myAction())" +``` From f93dfd0df6fcb72be3b5b95402772cf55a318320 Mon Sep 17 00:00:00 2001 From: Robin Krahnen Date: Mon, 2 May 2022 09:56:10 +0200 Subject: [PATCH 06/11] add "declare(strict_types=1);" --- Classes/Annotations/ReadOnly.php | 2 ++ Classes/Aspects/ReadOnlyAspect.php | 2 ++ Classes/Aspects/ReadOnlyFilter.php | 2 ++ Classes/Http/SessionLockRequestComponent.php | 2 ++ 4 files changed, 8 insertions(+) diff --git a/Classes/Annotations/ReadOnly.php b/Classes/Annotations/ReadOnly.php index cac72be..24cc7db 100644 --- a/Classes/Annotations/ReadOnly.php +++ b/Classes/Annotations/ReadOnly.php @@ -1,5 +1,7 @@ Date: Wed, 4 May 2022 19:01:15 +0200 Subject: [PATCH 07/11] optimized versions in composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 98ad265..567cb4a 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ "package-key": "DigiComp.FlowSessionLock" }, "branch-alias": { - "dev-develop": "1.0.x-dev" + "dev-develop": "2.0.x-dev" } }, "authors": [ From ec13bc49608debf5497784fe761c75cf99d46de4 Mon Sep 17 00:00:00 2001 From: Ferdinand Kuhl Date: Mon, 16 May 2022 11:28:39 +0200 Subject: [PATCH 08/11] Do not wait until Lock acquiry but a configurable amount of time (30s per default) --- Classes/Http/SessionLockRequestComponent.php | 22 +++++++++++++++++--- Configuration/Settings.yaml | 1 + 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index aa0b45e..97a6da4 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -8,6 +8,7 @@ use Neos\Flow\Annotations as Flow; use Neos\Flow\Http\Component\ComponentContext; use Neos\Flow\Http\Component\ComponentInterface; use Psr\Log\LoggerInterface; +use Symfony\Component\Lock\Exception\LockAcquiringException; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\LockFactory; @@ -45,6 +46,12 @@ class SessionLockRequestComponent implements ComponentInterface */ protected bool $autoRelease; + /** + * @Flow\InjectConfiguration(package="DigiComp.FlowSessionLock", path="secondsToWait") + * @var int + */ + protected int $secondsToWait; + /** * @inheritDoc */ @@ -64,8 +71,17 @@ class SessionLockRequestComponent implements ComponentInterface $componentContext->setParameter(SessionLockRequestComponent::class, static::PARAMETER_NAME, $lock); - $this->logger->debug('SessionLock: Get ' . $key); - $lock->acquire(true); - $this->logger->debug('SessionLock: Acquired ' . $key); + $this->logger->debug('SessionLock: Try to get "' . $key . '"'); + $timedOut = \time() + $this->secondsToWait; + while (!$lock->acquire() || $timedOut <= \time()) { + \usleep(100000); + } + if (!$lock->isAcquired()) { + throw new LockAcquiringException( + 'Could not acquire the lock for "' . $key . '" in ' . $this->secondsToWait . ' seconds.', + 1652687960 + ); + } + $this->logger->debug('SessionLock: Acquired "' . $key . '"'); } } diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index f5b77b2..f283d84 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -3,6 +3,7 @@ DigiComp: lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Production/SessionLocks/" timeToLive: 300.0 autoRelease: true + secondsToWait: 30 readOnlyExpressions: {} Neos: From 1452d19892fc53d9b29990c3eed13a3f03de799a Mon Sep 17 00:00:00 2001 From: Ferdinand Kuhl Date: Mon, 16 May 2022 12:04:25 +0200 Subject: [PATCH 09/11] Safe one unneccessary check against redis lock --- Classes/Http/SessionLockRequestComponent.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 97a6da4..1a0b546 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -73,14 +73,14 @@ class SessionLockRequestComponent implements ComponentInterface $this->logger->debug('SessionLock: Try to get "' . $key . '"'); $timedOut = \time() + $this->secondsToWait; - while (!$lock->acquire() || $timedOut <= \time()) { + while (!$lock->acquire()) { \usleep(100000); - } - if (!$lock->isAcquired()) { - throw new LockAcquiringException( - 'Could not acquire the lock for "' . $key . '" in ' . $this->secondsToWait . ' seconds.', - 1652687960 - ); + if (\time() >= $timedOut) { + throw new LockAcquiringException( + 'Could not acquire the lock for "' . $key . '" in ' . $this->secondsToWait . ' seconds.', + 1652687960 + ); + } } $this->logger->debug('SessionLock: Acquired "' . $key . '"'); } From 24283b795f4123445759b22d456e2815c2b67285 Mon Sep 17 00:00:00 2001 From: Ferdinand Kuhl Date: Mon, 16 May 2022 12:08:39 +0200 Subject: [PATCH 10/11] Check first, then wait --- Classes/Http/SessionLockRequestComponent.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Http/SessionLockRequestComponent.php b/Classes/Http/SessionLockRequestComponent.php index 1a0b546..43109ad 100644 --- a/Classes/Http/SessionLockRequestComponent.php +++ b/Classes/Http/SessionLockRequestComponent.php @@ -74,13 +74,13 @@ class SessionLockRequestComponent implements ComponentInterface $this->logger->debug('SessionLock: Try to get "' . $key . '"'); $timedOut = \time() + $this->secondsToWait; while (!$lock->acquire()) { - \usleep(100000); if (\time() >= $timedOut) { throw new LockAcquiringException( 'Could not acquire the lock for "' . $key . '" in ' . $this->secondsToWait . ' seconds.', 1652687960 ); } + \usleep(100000); } $this->logger->debug('SessionLock: Acquired "' . $key . '"'); } From b9ed514ef2b52fc52b97a3422b5916c488032f3b Mon Sep 17 00:00:00 2001 From: Ferdinand Kuhl Date: Tue, 17 May 2022 09:54:09 +0200 Subject: [PATCH 11/11] Adding a functional test which covers all functions --- Configuration/Testing/Settings.yaml | 2 + .../Fixtures/Controller/ExampleController.php | 43 ++++++++ .../SessionLockRequestComponentTest.php | 100 ++++++++++++++++++ composer.json | 3 + 4 files changed, 148 insertions(+) create mode 100644 Tests/Functional/Fixtures/Controller/ExampleController.php create mode 100644 Tests/Functional/SessionLockRequestComponentTest.php diff --git a/Configuration/Testing/Settings.yaml b/Configuration/Testing/Settings.yaml index 07b1b05..31d150d 100644 --- a/Configuration/Testing/Settings.yaml +++ b/Configuration/Testing/Settings.yaml @@ -1,3 +1,5 @@ DigiComp: FlowSessionLock: lockStoreConnection: "flock://%FLOW_PATH_DATA%Temporary/Testing/SessionLocks/" + readOnlyExpressions: + TestUnprotected: "method(DigiComp\\FlowSessionLock\\Tests\\Functional\\Fixtures\\Controller\\ExampleController->unprotectedByConfigurationAction())" diff --git a/Tests/Functional/Fixtures/Controller/ExampleController.php b/Tests/Functional/Fixtures/Controller/ExampleController.php new file mode 100644 index 0000000..a5ecab9 --- /dev/null +++ b/Tests/Functional/Fixtures/Controller/ExampleController.php @@ -0,0 +1,43 @@ +serverRequestFactory = $this->objectManager->get(ServerRequestFactoryInterface::class); + $route = new Route(); + $route->setName('Functional Test - SessionRequestComponent::Restricted'); + $route->setUriPattern('test/sessionlock/{@action}'); + $route->setDefaults([ + '@package' => 'DigiComp.FlowSessionLock', + '@subpackage' => 'Tests\Functional\Fixtures', + '@controller' => 'Example', + '@action' => 'protected', + '@format' => 'html', + ]); + $route->setAppendExceedingArguments(true); + $this->router->addRoute($route); + } + + public function expectedDuration(): array + { + $parallelChecker = function ($allRequests, $oneRequest) { + self::assertGreaterThan(ExampleController::CONTROLLER_TIME, $oneRequest * 1000); + self::assertLessThan(ExampleController::CONTROLLER_TIME * 4, $allRequests * 1000); + }; + return [ + [ + 'http://localhost/test/sessionlock/protected', + function ($allRequests, $oneRequest) { + self::assertGreaterThan(ExampleController::CONTROLLER_TIME, $oneRequest * 1000); + self::assertGreaterThan(ExampleController::CONTROLLER_TIME * 4, $allRequests * 1000); + } + ], + [ + 'http://localhost/test/sessionlock/unprotectedbyannotation', + $parallelChecker + ], + [ + 'http://localhost/test/sessionlock/unprotectedbyconfiguration', + $parallelChecker + ] + ]; + } + + /** + * @dataProvider expectedDuration + * @test + */ + public function itDoesNotAllowToEnterMoreThanOneWithTheSameSession(string $url, \Closure $checker): void + { + $request = $this->serverRequestFactory + ->createServerRequest('GET', new Uri($url)); + $start = microtime(true); + $response = $this->browser->sendRequest($request); + $neededForOne = microtime(true) - $start; + + $sessionCookies = array_map(static function ($cookie) { + return Cookie::createFromRawSetCookieHeader($cookie); + }, $response->getHeader('Set-Cookie')); + self::assertNotEmpty($sessionCookies); + + $cookies = array_reduce($sessionCookies, static function ($out, $cookie) { + $out[$cookie->getName()] = $cookie->getValue(); + return $out; + }, []); + $nextRequest = $this->serverRequestFactory + ->createServerRequest('GET', new Uri($url)) + ->withCookieParams($cookies); + $childs = []; + $start = microtime(true); + for ($i = 0; $i < 4; $i++) { + $child = \pcntl_fork(); + if ($child === 0) { + $this->browser->sendRequest($nextRequest); + exit(); + } + $childs[] = $child; + } + foreach ($childs as $child) { + \pcntl_waitpid($child, $status); + } + $neededForAll = microtime(true) - $start; + + $checker($neededForAll, $neededForOne); + } +} diff --git a/composer.json b/composer.json index 567cb4a..e707dfc 100644 --- a/composer.json +++ b/composer.json @@ -7,6 +7,9 @@ "php": ">=7.4", "symfony/lock": "^5.2.0" }, + "require-dev": { + "ext-pcntl": "*" + }, "autoload": { "psr-4": { "DigiComp\\FlowSessionLock\\": "Classes/"