From 0c9e887152fdb54a252ade07a3189e18c31cc150 Mon Sep 17 00:00:00 2001 From: bota Date: Thu, 15 May 2025 16:06:29 +0300 Subject: [PATCH 1/7] modernized http-user-agent-validator Signed-off-by: bota --- src/Validator/EnvironmentValueObject.php | 27 ++++++++++++++++++++++++ src/Validator/HttpUserAgent.php | 21 ++++++------------ test/Validator/HttpUserAgentTest.php | 20 ++++++++++++------ 3 files changed, 46 insertions(+), 22 deletions(-) create mode 100644 src/Validator/EnvironmentValueObject.php diff --git a/src/Validator/EnvironmentValueObject.php b/src/Validator/EnvironmentValueObject.php new file mode 100644 index 00000000..7e1645fc --- /dev/null +++ b/src/Validator/EnvironmentValueObject.php @@ -0,0 +1,27 @@ +httpUserAgent; + } + + public function setHttpUserAgent(?string $httpUserAgent): void + { + $this->httpUserAgent = $httpUserAgent; + } +} diff --git a/src/Validator/HttpUserAgent.php b/src/Validator/HttpUserAgent.php index 1aede073..3d3b2aa3 100644 --- a/src/Validator/HttpUserAgent.php +++ b/src/Validator/HttpUserAgent.php @@ -5,27 +5,18 @@ namespace Laminas\Session\Validator; /** - * @implements ValidatorInterface + * @implements ValidatorInterface */ -class HttpUserAgent implements ValidatorInterface +final class HttpUserAgent implements ValidatorInterface { - /** - * Internal data - * - * @var string - */ - protected $data; - /** * Constructor * get the current user agent and store it in the session as 'valid data' - * - * @param string|null $data */ - public function __construct($data = null) + public function __construct(protected EnvironmentValueObject $env, protected ?string $data = null) { if ($data === null || $data === '') { - $data = $_SERVER['HTTP_USER_AGENT'] ?? null; + $data = $env->getHttpUserAgent() ?? null; } $this->data = $data; } @@ -36,7 +27,7 @@ public function __construct($data = null) */ public function isValid(): bool { - $userAgent = $_SERVER['HTTP_USER_AGENT'] ?? null; + $userAgent = $this->env->getHttpUserAgent() ?? null; return $userAgent === $this->getData(); } @@ -44,7 +35,7 @@ public function isValid(): bool /** * Retrieve token for validating call */ - public function getData(): mixed + public function getData(): ?string { return $this->data; } diff --git a/test/Validator/HttpUserAgentTest.php b/test/Validator/HttpUserAgentTest.php index b2a9ac7b..26923276 100644 --- a/test/Validator/HttpUserAgentTest.php +++ b/test/Validator/HttpUserAgentTest.php @@ -4,16 +4,23 @@ namespace LaminasTest\Session\Validator; +use Laminas\Session\Validator\EnvironmentValueObject; use Laminas\Session\Validator\HttpUserAgent; use PHPUnit\Framework\TestCase; class HttpUserAgentTest extends TestCase { - public function testIsValid(): void + protected EnvironmentValueObject $environment; + + public function setUp(): void { - $_SERVER['HTTP_USER_AGENT'] = 'Test-User-Agent'; + $this->environment = EnvironmentValueObject::fromGlobals(); + } - $validator = new HttpUserAgent(); + public function testIsValid(): void + { + $this->environment->setHttpUserAgent('Test-User-Agent'); + $validator = new HttpUserAgent($this->environment); self::assertNotNull($validator->getData()); self::assertTrue($validator->isValid()); @@ -22,9 +29,8 @@ public function testIsValid(): void public function testIsValidWhenNoUserAgentIsSet(): void { // technically not needed in CLI - unset($_SERVER['HTTP_USER_AGENT']); - - $validator = new HttpUserAgent(); + $this->environment->setHttpUserAgent(null); + $validator = new HttpUserAgent($this->environment); self::assertNull($validator->getData()); self::assertTrue($validator->isValid()); @@ -32,7 +38,7 @@ public function testIsValidWhenNoUserAgentIsSet(): void public function testGetNameReturnsClassName(): void { - $validator = new HttpUserAgent(); + $validator = new HttpUserAgent($this->environment); self::assertSame(HttpUserAgent::class, $validator->getName()); } From 7f2a2a184fbc2955920a4e428152051d743f96f2 Mon Sep 17 00:00:00 2001 From: bota Date: Thu, 15 May 2025 16:17:21 +0300 Subject: [PATCH 2/7] psalm baseline Signed-off-by: bota --- psalm-baseline.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 3f44c1c2..54f7c30d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -445,11 +445,6 @@ - - - - - From 09a2ad1e4e5d819e315865d276bddd74596dfe32 Mon Sep 17 00:00:00 2001 From: bota Date: Mon, 19 May 2025 16:55:21 +0300 Subject: [PATCH 3/7] implementig Environment object Signed-off-by: bota --- psalm-baseline.xml | 1 - src/Validator/Environment.php | 23 +++++++++++++++++++ src/Validator/EnvironmentValueObject.php | 27 ---------------------- src/Validator/HttpUserAgent.php | 21 ++--------------- src/Validator/Id.php | 10 -------- src/Validator/ValidatorInterface.php | 9 -------- src/ValidatorChain.php | 3 +-- test/TestAsset/TestFailingValidator.php | 5 +--- test/Validator/HttpUserAgentTest.php | 29 ++++++++++++------------ test/Validator/IdTest.php | 11 +++++---- test/Validator/StaticValidatorStub.php | 5 +--- 11 files changed, 49 insertions(+), 95 deletions(-) create mode 100644 src/Validator/Environment.php delete mode 100644 src/Validator/EnvironmentValueObject.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 54f7c30d..10e29d66 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -450,7 +450,6 @@ - diff --git a/src/Validator/Environment.php b/src/Validator/Environment.php new file mode 100644 index 00000000..3751664d --- /dev/null +++ b/src/Validator/Environment.php @@ -0,0 +1,23 @@ +httpUserAgent; - } - - public function setHttpUserAgent(?string $httpUserAgent): void - { - $this->httpUserAgent = $httpUserAgent; - } -} diff --git a/src/Validator/HttpUserAgent.php b/src/Validator/HttpUserAgent.php index 3d3b2aa3..1f421aca 100644 --- a/src/Validator/HttpUserAgent.php +++ b/src/Validator/HttpUserAgent.php @@ -4,21 +4,14 @@ namespace Laminas\Session\Validator; -/** - * @implements ValidatorInterface - */ final class HttpUserAgent implements ValidatorInterface { /** * Constructor * get the current user agent and store it in the session as 'valid data' */ - public function __construct(protected EnvironmentValueObject $env, protected ?string $data = null) + public function __construct(private readonly Environment $initial, private readonly Environment $current) { - if ($data === null || $data === '') { - $data = $env->getHttpUserAgent() ?? null; - } - $this->data = $data; } /** @@ -27,17 +20,7 @@ public function __construct(protected EnvironmentValueObject $env, protected ?st */ public function isValid(): bool { - $userAgent = $this->env->getHttpUserAgent() ?? null; - - return $userAgent === $this->getData(); - } - - /** - * Retrieve token for validating call - */ - public function getData(): ?string - { - return $this->data; + return $this->initial->userAgent === $this->current->userAgent; } /** diff --git a/src/Validator/Id.php b/src/Validator/Id.php index 92b917a9..2835a92b 100644 --- a/src/Validator/Id.php +++ b/src/Validator/Id.php @@ -15,8 +15,6 @@ /** * session_id validator - * - * @implements ValidatorInterface */ final class Id implements ValidatorInterface { @@ -67,14 +65,6 @@ public function isValid(): bool return (bool) preg_match($pattern, $id); } - /** - * Retrieve token for validating call (session_id) - */ - public function getData(): ?string - { - return $this->id; - } - /** * Return validator name */ diff --git a/src/Validator/ValidatorInterface.php b/src/Validator/ValidatorInterface.php index 3c5863d9..e6331811 100644 --- a/src/Validator/ValidatorInterface.php +++ b/src/Validator/ValidatorInterface.php @@ -6,8 +6,6 @@ /** * Session validator interface - * - * @template T */ interface ValidatorInterface { @@ -18,13 +16,6 @@ interface ValidatorInterface */ public function isValid(): bool; - /** - * Get data from validator to be used for validation comparisons - * - * @return T - */ - public function getData(): mixed; - /** * Get validator name for use with storing validators between requests */ diff --git a/src/ValidatorChain.php b/src/ValidatorChain.php index 8c00e3c9..81f51c93 100644 --- a/src/ValidatorChain.php +++ b/src/ValidatorChain.php @@ -69,9 +69,8 @@ private function attachValidator($event, $callback, $priority) array_unshift($callback, $test); } if ($context instanceof ValidatorInterface) { - $data = $context->getData(); $name = $context->getName(); - $this->getStorage()->setMetadata('_VALID', [$name => $data]); + $this->getStorage()->setMetadata('_VALID', [$name => '']); } return parent::attach($event, $callback, $priority); diff --git a/test/TestAsset/TestFailingValidator.php b/test/TestAsset/TestFailingValidator.php index 60122ecd..c5dde5ab 100644 --- a/test/TestAsset/TestFailingValidator.php +++ b/test/TestAsset/TestFailingValidator.php @@ -6,12 +6,9 @@ use Laminas\Session\Validator\ValidatorInterface; -/** - * @implements ValidatorInterface - */ final class TestFailingValidator implements ValidatorInterface { - public function getData(): mixed + public function getData(): bool { return false; } diff --git a/test/Validator/HttpUserAgentTest.php b/test/Validator/HttpUserAgentTest.php index 26923276..478a18a3 100644 --- a/test/Validator/HttpUserAgentTest.php +++ b/test/Validator/HttpUserAgentTest.php @@ -4,41 +4,40 @@ namespace LaminasTest\Session\Validator; -use Laminas\Session\Validator\EnvironmentValueObject; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\HttpUserAgent; use PHPUnit\Framework\TestCase; class HttpUserAgentTest extends TestCase { - protected EnvironmentValueObject $environment; - - public function setUp(): void - { - $this->environment = EnvironmentValueObject::fromGlobals(); - } - public function testIsValid(): void { - $this->environment->setHttpUserAgent('Test-User-Agent'); - $validator = new HttpUserAgent($this->environment); + $initialServer = ['HTTP_USER_AGENT' => 'Test-User-Agent']; + $initialEnvironment = Environment::fromGlobals($initialServer); + $currentEnvironment = new Environment('Test-User-Agent'); + + $validator = new HttpUserAgent($initialEnvironment, $currentEnvironment); - self::assertNotNull($validator->getData()); self::assertTrue($validator->isValid()); } public function testIsValidWhenNoUserAgentIsSet(): void { // technically not needed in CLI - $this->environment->setHttpUserAgent(null); - $validator = new HttpUserAgent($this->environment); + $initialServer = []; + $initialEnvironment = Environment::fromGlobals($initialServer); + $currentEnvironment = new Environment(null); + $validator = new HttpUserAgent($initialEnvironment, $currentEnvironment); - self::assertNull($validator->getData()); self::assertTrue($validator->isValid()); } public function testGetNameReturnsClassName(): void { - $validator = new HttpUserAgent($this->environment); + $initialServer = []; + $initialEnvironment = Environment::fromGlobals($initialServer); + $currentEnvironment = new Environment(null); + $validator = new HttpUserAgent($initialEnvironment, $currentEnvironment); self::assertSame(HttpUserAgent::class, $validator->getName()); } diff --git a/test/Validator/IdTest.php b/test/Validator/IdTest.php index 9cf85721..d4c9f5c7 100644 --- a/test/Validator/IdTest.php +++ b/test/Validator/IdTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RunInSeparateProcess; use PHPUnit\Framework\TestCase; +use ReflectionProperty; use function ini_set; use function session_id; @@ -45,9 +46,10 @@ public function testIsValidPhp71(int $bitsPerCharacter, string $id, bool $isVali public function testConstructorSetId(): void { - $id = new Id('1234'); + $id = new Id('1234'); + $idReflection = new ReflectionProperty($id, 'id'); - self::assertSame('1234', $id->getData()); + self::assertSame('1234', $idReflection->getValue($id)); } /** @@ -58,9 +60,10 @@ public function testInitializedWithSessionIdWhenIdIsNotPassed(): void session_start(); $sessionId = session_id(); - $id = new Id(); + $id = new Id(); + $idReflection = new ReflectionProperty($id, 'id'); - self::assertSame($sessionId, $id->getData()); + self::assertSame($sessionId, $idReflection->getValue($id)); } public function testValidatorName(): void diff --git a/test/Validator/StaticValidatorStub.php b/test/Validator/StaticValidatorStub.php index 37209c58..80dee057 100644 --- a/test/Validator/StaticValidatorStub.php +++ b/test/Validator/StaticValidatorStub.php @@ -6,9 +6,6 @@ use Laminas\Session\Validator\ValidatorInterface; -/** - * @implements ValidatorInterface - */ class StaticValidatorStub implements ValidatorInterface { public static int $isValidCallCount = 0; @@ -19,7 +16,7 @@ public function isValid(): bool return $this->getData(); } - public function getData(): mixed + public function getData(): bool { return false; } From 83d8c69aac2804a9c00fc4ec7d887890e37dff29 Mon Sep 17 00:00:00 2001 From: bota Date: Mon, 2 Jun 2025 11:50:57 +0300 Subject: [PATCH 4/7] replaced superglobals usage with Environment object Signed-off-by: bota --- psalm-baseline.xml | 3 ++ src/Validator/Environment.php | 21 ++++++++++-- src/Validator/HttpUserAgent.php | 5 +-- src/Validator/Id.php | 16 ++++----- src/Validator/RemoteAddr.php | 44 +++++++++++-------------- src/ValidatorChain.php | 3 +- test/SessionManagerTest.php | 2 +- test/TestAsset/TestFailingValidator.php | 8 ++--- test/Validator/HttpUserAgentTest.php | 19 +++-------- test/Validator/IdTest.php | 4 +-- test/Validator/RemoteAddrTest.php | 43 ++++++++++++------------ test/Validator/StaticValidatorStub.php | 6 +--- test/Validator/ValidatorChainTest.php | 4 +-- 13 files changed, 86 insertions(+), 92 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 10e29d66..d2d6419e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -457,6 +457,9 @@ + + data]]> + diff --git a/src/Validator/Environment.php b/src/Validator/Environment.php index 3751664d..d7b0ba21 100644 --- a/src/Validator/Environment.php +++ b/src/Validator/Environment.php @@ -8,8 +8,10 @@ final class Environment { - public function __construct(public readonly ?string $userAgent) - { + public function __construct( + public readonly ?string $userAgent = null, + public readonly ?string $remoteAddr = null + ) { } public static function fromGlobals(array $server): self @@ -18,6 +20,19 @@ public static function fromGlobals(array $server): self ? $server['HTTP_USER_AGENT'] : null; - return new self($userAgent); + $remoteAddr = isset($server['REMOTE_ADDR']) && is_string($server['REMOTE_ADDR']) + ? $server['REMOTE_ADDR'] + : null; + + return new self($userAgent, $remoteAddr); + } + + public static function getServerOption(string $name, ?array $superglobal = null): mixed + { + if ($superglobal === null) { + $superglobal = $_SERVER; + } + + return $superglobal[$name] ?? null; } } diff --git a/src/Validator/HttpUserAgent.php b/src/Validator/HttpUserAgent.php index 1f421aca..bb7dc5db 100644 --- a/src/Validator/HttpUserAgent.php +++ b/src/Validator/HttpUserAgent.php @@ -10,7 +10,7 @@ final class HttpUserAgent implements ValidatorInterface * Constructor * get the current user agent and store it in the session as 'valid data' */ - public function __construct(private readonly Environment $initial, private readonly Environment $current) + public function __construct(public readonly ?string $data = null) { } @@ -20,7 +20,8 @@ public function __construct(private readonly Environment $initial, private reado */ public function isValid(): bool { - return $this->initial->userAgent === $this->current->userAgent; + $env = Environment::fromGlobals($_SERVER); + return $env->userAgent === $this->data; } /** diff --git a/src/Validator/Id.php b/src/Validator/Id.php index 2835a92b..82385d92 100644 --- a/src/Validator/Id.php +++ b/src/Validator/Id.php @@ -18,19 +18,21 @@ */ final class Id implements ValidatorInterface { + public readonly string $data; + /** * Constructor * * Allows passing the current session_id; if none provided, uses the PHP * session_id() function to retrieve it. */ - public function __construct(protected ?string $id = null) + public function __construct(?string $data = null) { - if ($id === null || $id === '') { - $id = session_id(); + if ($data === null || $data === '') { + $data = session_id(); } - $this->id = $id; + $this->data = $data; } /** @@ -40,11 +42,7 @@ public function __construct(protected ?string $id = null) */ public function isValid(): bool { - $id = $this->id; - - if ($id === null) { - return false; - } + $id = $this->data; if (PHP_VERSION_ID >= 80400) { trigger_error('session.sid_bits_per_character is deprecated starting with PHP 8.4', E_USER_DEPRECATED); diff --git a/src/Validator/RemoteAddr.php b/src/Validator/RemoteAddr.php index 0fb35583..1955ccc8 100644 --- a/src/Validator/RemoteAddr.php +++ b/src/Validator/RemoteAddr.php @@ -23,14 +23,13 @@ * trusted_proxies?: array, * proxy_header?: non-empty-string, * } - * @implements SessionValidator */ final class RemoteAddr implements SessionValidator { /** * Internal data. */ - private ?string $data; + public readonly ?string $data; /** * Whether to use proxy addresses or not. @@ -58,7 +57,7 @@ final class RemoteAddr implements SessionValidator * * @param OptionsArgument $options */ - public function __construct(?string $data = null, array $options = []) + public function __construct(private readonly ?string $initial = null, array $options = []) { $proxyHeader = $options['proxy_header'] ?? 'X_FORWARDED_FOR'; @@ -66,11 +65,11 @@ public function __construct(?string $data = null, array $options = []) $this->trustedProxies = $options['trusted_proxies'] ?? []; $this->proxyHeader = self::normalizeProxyHeader($proxyHeader); - if ($data === null || $data === '') { - $data = $this->getIpAddress(); + if ($initial === null || $initial === '') { + $initial = $this->getIpAddress(); } - $this->data = $data; + $this->data = $initial; } /** @@ -79,7 +78,7 @@ public function __construct(?string $data = null, array $options = []) */ public function isValid(): bool { - return $this->getIpAddress() === $this->getData(); + return (Environment::fromGlobals($_SERVER))->remoteAddr === $this->data; } /** @@ -93,20 +92,20 @@ public function getUseProxy(): bool /** * Returns client IP address. */ - private function getIpAddress(): string + private function getIpAddress(): ?string { $ip = $this->getIpAddressFromProxy(); - if (false !== $ip) { + if ($ip !== false) { return $ip; } // direct IP address - if (isset($_SERVER['REMOTE_ADDR'])) { - return $_SERVER['REMOTE_ADDR']; + if ($this->initial !== null) { + return $this->initial; } - return ''; + return null; } /** @@ -116,22 +115,25 @@ private function getIpAddress(): string */ private function getIpAddressFromProxy(): string|false { + $environment = Environment::fromGlobals($_SERVER); + if ( ! $this->useProxy - || (isset($_SERVER['REMOTE_ADDR']) && ! in_array($_SERVER['REMOTE_ADDR'], $this->trustedProxies)) + || ($environment->remoteAddr !== null && ! in_array($environment->remoteAddr, $this->trustedProxies)) ) { return false; } - $header = $this->proxyHeader; + $header = $this->proxyHeader; + $proxyHeader = Environment::getServerOption($header); - if (! isset($_SERVER[$header]) || '' === $_SERVER[$header]) { + if ($proxyHeader === null || $proxyHeader === '') { return false; } // Extract IPs - assert(is_string($_SERVER[$header])); - $ips = explode(',', $_SERVER[$header]); + assert(is_string($proxyHeader)); + $ips = explode(',', $proxyHeader); // trim, so we can compare against trusted proxies properly $ips = array_map('trim', $ips); // remove trusted proxy IPs @@ -165,14 +167,6 @@ protected static function normalizeProxyHeader(string $header): string return $header; } - /** - * Retrieve token for validating call - */ - public function getData(): ?string - { - return $this->data; - } - /** * Return validator name */ diff --git a/src/ValidatorChain.php b/src/ValidatorChain.php index 81f51c93..1c323400 100644 --- a/src/ValidatorChain.php +++ b/src/ValidatorChain.php @@ -70,9 +70,8 @@ private function attachValidator($event, $callback, $priority) } if ($context instanceof ValidatorInterface) { $name = $context->getName(); - $this->getStorage()->setMetadata('_VALID', [$name => '']); + $this->getStorage()->setMetadata('_VALID', [$name => $context->data]); } - return parent::attach($event, $callback, $priority); } } diff --git a/test/SessionManagerTest.php b/test/SessionManagerTest.php index d788cbd6..1f79b0d0 100644 --- a/test/SessionManagerTest.php +++ b/test/SessionManagerTest.php @@ -806,7 +806,7 @@ public function testRemoteAddressValidationWillSucceedWithValidPreSetData(): voi $_SESSION = [ '__Laminas' => [ '_VALID' => [ - RemoteAddr::class => '', + RemoteAddr::class => null, ], ], ]; diff --git a/test/TestAsset/TestFailingValidator.php b/test/TestAsset/TestFailingValidator.php index c5dde5ab..b66ffdd8 100644 --- a/test/TestAsset/TestFailingValidator.php +++ b/test/TestAsset/TestFailingValidator.php @@ -8,11 +8,7 @@ final class TestFailingValidator implements ValidatorInterface { - public function getData(): bool - { - return false; - } - + public ?string $data = null; public function getName(): string { return self::class; @@ -20,6 +16,6 @@ public function getName(): string public function isValid(): bool { - return $this->getData(); + return false; } } diff --git a/test/Validator/HttpUserAgentTest.php b/test/Validator/HttpUserAgentTest.php index 478a18a3..9a8affd0 100644 --- a/test/Validator/HttpUserAgentTest.php +++ b/test/Validator/HttpUserAgentTest.php @@ -4,7 +4,6 @@ namespace LaminasTest\Session\Validator; -use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\HttpUserAgent; use PHPUnit\Framework\TestCase; @@ -12,11 +11,8 @@ class HttpUserAgentTest extends TestCase { public function testIsValid(): void { - $initialServer = ['HTTP_USER_AGENT' => 'Test-User-Agent']; - $initialEnvironment = Environment::fromGlobals($initialServer); - $currentEnvironment = new Environment('Test-User-Agent'); - - $validator = new HttpUserAgent($initialEnvironment, $currentEnvironment); + $_SERVER['HTTP_USER_AGENT'] = 'Test-User-Agent'; + $validator = new HttpUserAgent('Test-User-Agent'); self::assertTrue($validator->isValid()); } @@ -24,20 +20,15 @@ public function testIsValid(): void public function testIsValidWhenNoUserAgentIsSet(): void { // technically not needed in CLI - $initialServer = []; - $initialEnvironment = Environment::fromGlobals($initialServer); - $currentEnvironment = new Environment(null); - $validator = new HttpUserAgent($initialEnvironment, $currentEnvironment); + unset($_SERVER['HTTP_USER_AGENT']); + $validator = new HttpUserAgent(); self::assertTrue($validator->isValid()); } public function testGetNameReturnsClassName(): void { - $initialServer = []; - $initialEnvironment = Environment::fromGlobals($initialServer); - $currentEnvironment = new Environment(null); - $validator = new HttpUserAgent($initialEnvironment, $currentEnvironment); + $validator = new HttpUserAgent(null); self::assertSame(HttpUserAgent::class, $validator->getName()); } diff --git a/test/Validator/IdTest.php b/test/Validator/IdTest.php index d4c9f5c7..d2a9599b 100644 --- a/test/Validator/IdTest.php +++ b/test/Validator/IdTest.php @@ -47,7 +47,7 @@ public function testIsValidPhp71(int $bitsPerCharacter, string $id, bool $isVali public function testConstructorSetId(): void { $id = new Id('1234'); - $idReflection = new ReflectionProperty($id, 'id'); + $idReflection = new ReflectionProperty($id, 'data'); self::assertSame('1234', $idReflection->getValue($id)); } @@ -61,7 +61,7 @@ public function testInitializedWithSessionIdWhenIdIsNotPassed(): void $sessionId = session_id(); $id = new Id(); - $idReflection = new ReflectionProperty($id, 'id'); + $idReflection = new ReflectionProperty($id, 'data'); self::assertSame($sessionId, $idReflection->getValue($id)); } diff --git a/test/Validator/RemoteAddrTest.php b/test/Validator/RemoteAddrTest.php index c2ef3388..3401ca96 100644 --- a/test/Validator/RemoteAddrTest.php +++ b/test/Validator/RemoteAddrTest.php @@ -18,7 +18,7 @@ class RemoteAddrTest extends TestCase protected function setUp(): void { - $this->defaultRemoteAddr = new RemoteAddr(); + $this->defaultRemoteAddr = new RemoteAddr(initial: '0.1.2.3'); } protected function backup(): void @@ -38,8 +38,8 @@ protected function restore(): void public function testGetData(): void { - $validator = new RemoteAddr('0.1.2.3'); - self::assertEquals('0.1.2.3', $validator->getData()); + $validator = new RemoteAddr(initial: '0.1.2.3'); + self::assertEquals('0.1.2.3', $validator->data); } public function testDefaultUseProxy(): void @@ -51,8 +51,8 @@ public function testRemoteAddrWithoutProxy(): void { $this->backup(); $_SERVER['REMOTE_ADDR'] = '0.1.2.3'; - $validator = new RemoteAddr(); - self::assertEquals('0.1.2.3', $validator->getData()); + $validator = new RemoteAddr(initial: $_SERVER['REMOTE_ADDR']); + self::assertEquals('0.1.2.3', $validator->data); $this->restore(); } @@ -71,8 +71,8 @@ public function testIgnoreProxyByDefault(): void $this->backup(); $_SERVER['REMOTE_ADDR'] = '0.1.2.3'; $_SERVER['HTTP_CLIENT_IP'] = '1.1.2.3'; - $validator = new RemoteAddr(); - self::assertEquals('0.1.2.3', $validator->getData()); + $validator = new RemoteAddr(initial: $_SERVER['REMOTE_ADDR']); + self::assertEquals('0.1.2.3', $validator->data); $this->restore(); } @@ -85,8 +85,9 @@ public function testHttpXForwardedFor(): void 'use_proxy' => true, 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr(null, $options); - self::assertEquals('1.1.2.3', $validator->getData()); + + $validator = new RemoteAddr(options: $options); + self::assertEquals('1.1.2.3', $validator->data); $this->restore(); } @@ -102,8 +103,8 @@ public function testHttpClientIp(): void 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr(null, $options); - self::assertEquals('2.1.2.3', $validator->getData()); + $validator = new RemoteAddr(options: $options); + self::assertEquals('2.1.2.3', $validator->data); $this->restore(); } @@ -118,8 +119,8 @@ public function testUsesRightMostAddressWhenMultipleHttpXForwardedForAddressesPr 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr(null, $options); - self::assertEquals('1.1.2.3', $validator->getData()); + $validator = new RemoteAddr(options: $options); + self::assertEquals('1.1.2.3', $validator->data); $this->restore(); } @@ -134,8 +135,8 @@ public function testShouldNotUseClientIpHeaderToTestProxyCapabilitiesByDefault() 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr(null, $options); - self::assertEquals('0.1.2.3', $validator->getData()); + $validator = new RemoteAddr($_SERVER['REMOTE_ADDR'], $options); + self::assertEquals('0.1.2.3', $validator->data); $this->restore(); } @@ -150,8 +151,8 @@ public function testWillOmitTrustedProxyIpsFromXForwardedForMatching(): void 'trusted_proxies' => ['1.1.2.3'], ]; - $validator = new RemoteAddr(null, $options); - self::assertEquals('2.1.2.3', $validator->getData()); + $validator = new RemoteAddr(options: $options); + self::assertEquals('2.1.2.3', $validator->data); $this->restore(); } @@ -167,8 +168,8 @@ public function testCanSpecifyWhichHeaderToUseStatically(): void 'proxy_header' => 'Client-Ip', ]; - $validator = new RemoteAddr(null, $options); - self::assertEquals('0.1.2.3', $validator->getData()); + $validator = new RemoteAddr($_SERVER['REMOTE_ADDR'], $options); + self::assertEquals('0.1.2.3', $validator->data); $this->restore(); } @@ -186,8 +187,8 @@ public function testUnknownServerHeader(): void 'proxy_header' => 'Unknown-Header', ]; - $validator = new RemoteAddr(null, $options); - self::assertEmpty($validator->getData()); + $validator = new RemoteAddr(options: $options); + self::assertEmpty($validator->data); $this->restore(); } } diff --git a/test/Validator/StaticValidatorStub.php b/test/Validator/StaticValidatorStub.php index 80dee057..ed45896a 100644 --- a/test/Validator/StaticValidatorStub.php +++ b/test/Validator/StaticValidatorStub.php @@ -9,15 +9,11 @@ class StaticValidatorStub implements ValidatorInterface { public static int $isValidCallCount = 0; + public ?string $data = null; public function isValid(): bool { self::$isValidCallCount++; - return $this->getData(); - } - - public function getData(): bool - { return false; } diff --git a/test/Validator/ValidatorChainTest.php b/test/Validator/ValidatorChainTest.php index e74efbf1..b1d3c02d 100644 --- a/test/Validator/ValidatorChainTest.php +++ b/test/Validator/ValidatorChainTest.php @@ -33,14 +33,14 @@ public function testAttachValidator(): void $validatorMetadata = $this->validatorChain->getStorage()->getMetadata('_VALID'); self::assertIsArray($validatorMetadata); self::assertArrayHasKey($validator->getName(), $validatorMetadata); - self::assertSame($validatorMetadata[$validator->getName()], $validator->getData()); + self::assertSame($validatorMetadata[$validator->getName()], $validator->data); } public function testExistingValidatorsAreAttached(): void { $validator = new StaticValidatorStub(); $storage = new ArrayStorage(); - $storage->setMetadata('_VALID', [$validator::class => $validator->getData()]); + $storage->setMetadata('_VALID', [$validator::class => $validator->data]); $this->validatorChain = new ValidatorChain($storage); From 0cff574d347bb644e64b946914dde13ab251d8b8 Mon Sep 17 00:00:00 2001 From: bota Date: Fri, 6 Jun 2025 20:43:59 +0300 Subject: [PATCH 5/7] updated validator constructors and environmnet object Signed-off-by: bota --- psalm-baseline.xml | 26 +++--- src/SessionManager.php | 47 ++++++++--- src/Validator/Environment.php | 16 +++- src/Validator/HttpUserAgent.php | 5 +- src/Validator/Id.php | 16 ++-- src/Validator/RemoteAddr.php | 77 ++++-------------- src/Validator/ValidatorInterface.php | 2 + src/ValidatorChain.php | 18 ++++- test/Service/SessionManagerFactoryTest.php | 8 +- test/SessionManagerTest.php | 46 ++++++++--- test/TestAsset/TestFailingValidator.php | 7 +- test/Validator/HttpUserAgentTest.php | 10 ++- test/Validator/IdTest.php | 20 ++--- test/Validator/RemoteAddrTest.php | 93 ++++++++++++---------- test/Validator/StaticValidatorStub.php | 7 +- test/Validator/ValidatorChainTest.php | 13 +-- 16 files changed, 230 insertions(+), 181 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index d2d6419e..f577f6d3 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -241,16 +241,9 @@ - - - - - - - @@ -445,20 +438,21 @@ + + + + + - - - - - data]]> + current]]> @@ -876,6 +870,9 @@ + + + @@ -949,4 +946,9 @@ + + + + + diff --git a/src/SessionManager.php b/src/SessionManager.php index 9af5606b..78bb9a1a 100644 --- a/src/SessionManager.php +++ b/src/SessionManager.php @@ -6,15 +6,20 @@ use Laminas\EventManager\Event; use Laminas\EventManager\EventManagerInterface; +use Laminas\Session\Validator\Environment; +use Laminas\Session\Validator\ValidatorInterface; use Traversable; use function array_key_exists; use function array_merge; +use function assert; use function headers_sent; use function is_array; +use function is_string; use function iterator_to_array; use function preg_match; use function register_shutdown_function; +use function serialize; use function session_destroy; use function session_id; use function session_name; @@ -24,6 +29,7 @@ use function session_status; use function session_write_close; use function setcookie; +use function unserialize; use const PHP_SESSION_ACTIVE; @@ -51,7 +57,7 @@ class SessionManager extends AbstractManager /** @var array Default validators */ protected $defaultValidators = [ - Validator\Id::class, + Validator\Id::class => null, ]; /** @var string value returned by session_name() */ @@ -160,16 +166,38 @@ public function start($preserveStorage = false) */ protected function initializeValidatorChain() { - $validatorChain = $this->getValidatorChain(); - $validatorValues = $this->getStorage()->getMetadata('_VALID'); - - foreach ($this->validators as $validator) { - // Ignore validators which are already present in Storage - if (is_array($validatorValues) && array_key_exists($validator, $validatorValues)) { + /** @var array $storage */ + $storage = $this->getStorage()->getMetadata(); + + /** + * @var string|null $data + * @var class-string $validatorName + */ + foreach ($this->validators as $validatorName => $data) { + $validatorValues = $this->getStorage()->getMetadata('_VALID'); + if (is_array($validatorValues) && array_key_exists($validatorName, $validatorValues)) { continue; } - $validator = new $validator(null); + if (isset($storage['environment'])) { + assert(is_string($storage['environment'])); + /** @var Environment $initialEnvironment */ + $initialEnvironment = unserialize($storage['environment']); + } else { + $initialEnvironment = Environment::fromGlobals($_SERVER); + $this->getStorage()->setMetadata('environment', serialize($initialEnvironment)); + } + + if ($data !== null) { + /** @var Environment $currentEnvironment */ + $currentEnvironment = unserialize($data); + } else { + $currentEnvironment = Environment::fromGlobals($_SERVER); + } + + $validatorChain = $this->getValidatorChain(); + + $validator = new $validatorName($initialEnvironment, $currentEnvironment); $validatorChain->attach('session.validate', [$validator, 'isValid']); } } @@ -399,8 +427,7 @@ public function getValidatorChain() public function isValid() { $validator = $this->getValidatorChain(); - - $event = new Event(); + $event = new Event(); $event->setName('session.validate'); $event->setTarget($this); $event->setParams($this); diff --git a/src/Validator/Environment.php b/src/Validator/Environment.php index d7b0ba21..fe533f55 100644 --- a/src/Validator/Environment.php +++ b/src/Validator/Environment.php @@ -5,16 +5,20 @@ namespace Laminas\Session\Validator; use function is_string; +use function session_id; +/** @psalm-import-type OptionsArgument from RemoteAddr */ final class Environment { public function __construct( public readonly ?string $userAgent = null, - public readonly ?string $remoteAddr = null + public readonly ?string $remoteAddr = null, + public readonly ?string $sessionId = null ) { } - public static function fromGlobals(array $server): self + /** @param OptionsArgument $options */ + public static function fromGlobals(array $server, array $options = []): self { $userAgent = isset($server['HTTP_USER_AGENT']) && is_string($server['HTTP_USER_AGENT']) ? $server['HTTP_USER_AGENT'] @@ -24,7 +28,13 @@ public static function fromGlobals(array $server): self ? $server['REMOTE_ADDR'] : null; - return new self($userAgent, $remoteAddr); + if ($remoteAddr === null || (isset($options['use_proxy']) && $options['use_proxy'])) { + $remoteAddr = RemoteAddr::getIpAddress($options, $remoteAddr); + } + + $sessionId = session_id(); + + return new self($userAgent, $remoteAddr, $sessionId); } public static function getServerOption(string $name, ?array $superglobal = null): mixed diff --git a/src/Validator/HttpUserAgent.php b/src/Validator/HttpUserAgent.php index bb7dc5db..14722369 100644 --- a/src/Validator/HttpUserAgent.php +++ b/src/Validator/HttpUserAgent.php @@ -10,7 +10,7 @@ final class HttpUserAgent implements ValidatorInterface * Constructor * get the current user agent and store it in the session as 'valid data' */ - public function __construct(public readonly ?string $data = null) + public function __construct(public readonly Environment $initial, public readonly Environment $current) { } @@ -20,8 +20,7 @@ public function __construct(public readonly ?string $data = null) */ public function isValid(): bool { - $env = Environment::fromGlobals($_SERVER); - return $env->userAgent === $this->data; + return $this->initial->userAgent === $this->current->userAgent; } /** diff --git a/src/Validator/Id.php b/src/Validator/Id.php index 82385d92..f32b37ca 100644 --- a/src/Validator/Id.php +++ b/src/Validator/Id.php @@ -7,7 +7,6 @@ use function ini_get; use function is_numeric; use function preg_match; -use function session_id; use function trigger_error; use const E_USER_DEPRECATED; @@ -18,21 +17,14 @@ */ final class Id implements ValidatorInterface { - public readonly string $data; - /** * Constructor * * Allows passing the current session_id; if none provided, uses the PHP * session_id() function to retrieve it. */ - public function __construct(?string $data = null) + public function __construct(public readonly Environment $initial, public readonly Environment $current) { - if ($data === null || $data === '') { - $data = session_id(); - } - - $this->data = $data; } /** @@ -42,7 +34,9 @@ public function __construct(?string $data = null) */ public function isValid(): bool { - $id = $this->data; + if ($this->initial->sessionId === null) { + return false; + } if (PHP_VERSION_ID >= 80400) { trigger_error('session.sid_bits_per_character is deprecated starting with PHP 8.4', E_USER_DEPRECATED); @@ -60,7 +54,7 @@ public function isValid(): bool default => '#^[0-9a-v]*$#', }; - return (bool) preg_match($pattern, $id); + return (bool) preg_match($pattern, $this->initial->sessionId); } /** diff --git a/src/Validator/RemoteAddr.php b/src/Validator/RemoteAddr.php index 1955ccc8..2ee1de11 100644 --- a/src/Validator/RemoteAddr.php +++ b/src/Validator/RemoteAddr.php @@ -26,50 +26,12 @@ */ final class RemoteAddr implements SessionValidator { - /** - * Internal data. - */ - public readonly ?string $data; - - /** - * Whether to use proxy addresses or not. - * - * As default this setting is disabled - IP address is mostly needed to increase - * security. HTTP_* are not reliable since can easily be spoofed. It can be enabled - * just for more flexibility, but if user uses proxy to connect to trusted services - * it's his/her own risk, only reliable field for IP address is $_SERVER['REMOTE_ADDR']. - */ - private bool $useProxy; - - /** - * List of trusted proxy IP addresses - */ - private array $trustedProxies; - - /** - * HTTP header to introspect for proxies - */ - private string $proxyHeader; - /** * Constructor * get the current user IP and store it in the session as 'valid data' - * - * @param OptionsArgument $options */ - public function __construct(private readonly ?string $initial = null, array $options = []) + public function __construct(public readonly Environment $initial, public readonly Environment $current) { - $proxyHeader = $options['proxy_header'] ?? 'X_FORWARDED_FOR'; - - $this->useProxy = isset($options['use_proxy']) && $options['use_proxy']; - $this->trustedProxies = $options['trusted_proxies'] ?? []; - $this->proxyHeader = self::normalizeProxyHeader($proxyHeader); - - if ($initial === null || $initial === '') { - $initial = $this->getIpAddress(); - } - - $this->data = $initial; } /** @@ -78,31 +40,24 @@ public function __construct(private readonly ?string $initial = null, array $opt */ public function isValid(): bool { - return (Environment::fromGlobals($_SERVER))->remoteAddr === $this->data; - } - - /** - * Checks proxy handling setting. - */ - public function getUseProxy(): bool - { - return $this->useProxy; + return $this->initial->remoteAddr === $this->current->remoteAddr; } /** * Returns client IP address. + * + * @param OptionsArgument $options */ - private function getIpAddress(): ?string + public static function getIpAddress(array $options = [], ?string $remoteAddr = null): ?string { - $ip = $this->getIpAddressFromProxy(); + $ip = self::getIpAddressFromProxy($options, $remoteAddr); if ($ip !== false) { return $ip; } - // direct IP address - if ($this->initial !== null) { - return $this->initial; + if ($remoteAddr !== null) { + return $remoteAddr; } return null; @@ -112,20 +67,22 @@ private function getIpAddress(): ?string * Attempt to get the IP address for a proxied client * * @see http://tools.ietf.org/html/draft-ietf-appsawg-http-forwarded-10#section-5.2 + * + * @param OptionsArgument $options */ - private function getIpAddressFromProxy(): string|false + private static function getIpAddressFromProxy(array $options = [], ?string $remoteAddr = null): string|false { - $environment = Environment::fromGlobals($_SERVER); + $normalizedProxyHeader = self::normalizeProxyHeader($options['proxy_header'] ?? 'X_FORWARDED_FOR'); + $trustedProxies = $options['trusted_proxies'] ?? []; if ( - ! $this->useProxy - || ($environment->remoteAddr !== null && ! in_array($environment->remoteAddr, $this->trustedProxies)) + ! (isset($options['use_proxy']) && $options['use_proxy']) + || ($remoteAddr !== null && ! in_array($remoteAddr, $trustedProxies)) ) { return false; } - $header = $this->proxyHeader; - $proxyHeader = Environment::getServerOption($header); + $proxyHeader = Environment::getServerOption($normalizedProxyHeader); if ($proxyHeader === null || $proxyHeader === '') { return false; @@ -137,7 +94,7 @@ private function getIpAddressFromProxy(): string|false // trim, so we can compare against trusted proxies properly $ips = array_map('trim', $ips); // remove trusted proxy IPs - $ips = array_diff($ips, $this->trustedProxies); + $ips = array_diff($ips, $trustedProxies); // Any left? if (empty($ips)) { return false; diff --git a/src/Validator/ValidatorInterface.php b/src/Validator/ValidatorInterface.php index e6331811..cfec6cd6 100644 --- a/src/Validator/ValidatorInterface.php +++ b/src/Validator/ValidatorInterface.php @@ -9,6 +9,8 @@ */ interface ValidatorInterface { + public function __construct(Environment $initial, Environment $current); + /** * This method will be called at the beginning of * every session to determine if the current environment matches diff --git a/src/ValidatorChain.php b/src/ValidatorChain.php index 1c323400..3352e23a 100644 --- a/src/ValidatorChain.php +++ b/src/ValidatorChain.php @@ -6,21 +6,33 @@ use Laminas\EventManager\EventManager; use Laminas\Session\Storage\StorageInterface; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\ValidatorInterface; use function array_shift; use function array_unshift; use function is_array; +use function serialize; +use function unserialize; class ValidatorChain extends EventManager { public function __construct(protected StorageInterface $storage) { parent::__construct(); - $validators = $storage->getMetadata('_VALID'); + $validators = $storage->getMetadata('_VALID'); + $environment = (string) $storage->getMetadata('environment'); if ($validators) { + /** + * @var class-string $validator + */ foreach ($validators as $validator => $data) { - $this->attachValidator('session.validate', [new $validator($data), 'isValid'], 1); + $currentEnvironment = $data instanceof Environment ? $data : Environment::fromGlobals($_SERVER); + $this->attachValidator( + 'session.validate', + [new $validator(unserialize($environment), $currentEnvironment), 'isValid'], + 1 + ); } } } @@ -70,7 +82,7 @@ private function attachValidator($event, $callback, $priority) } if ($context instanceof ValidatorInterface) { $name = $context->getName(); - $this->getStorage()->setMetadata('_VALID', [$name => $context->data]); + $this->getStorage()->setMetadata('_VALID', [$name => serialize($context->current)]); } return parent::attach($event, $callback, $priority); } diff --git a/test/Service/SessionManagerFactoryTest.php b/test/Service/SessionManagerFactoryTest.php index a8df7447..d22ae707 100644 --- a/test/Service/SessionManagerFactoryTest.php +++ b/test/Service/SessionManagerFactoryTest.php @@ -17,6 +17,7 @@ use Laminas\Session\Storage\ArrayStorage; use Laminas\Session\Storage\StorageInterface; use Laminas\Session\Validator; +use Laminas\Session\Validator\Environment; use LaminasTest\Session\ReflectionPropertyTrait; use LaminasTest\Session\TestAsset\TestManager; use LaminasTest\Session\TestAsset\TestSaveHandler; @@ -107,7 +108,7 @@ public function testFactoryWillAddValidatorViaConfiguration(): void $config = [ 'session_manager' => [ 'validators' => [ - Validator\RemoteAddr::class, + Validator\RemoteAddr::class => null, ], ], ]; @@ -184,7 +185,7 @@ public function testFactoryDoesNotAttachValidatorTwoTimes(): void $storage->setMetadata( '_VALID', [ - Validator\RemoteAddr::class => '1.2.3.4', + Validator\RemoteAddr::class => new Environment(remoteAddr: '1.2.3.4'), ] ); $this->services->setService(StorageInterface::class, $storage); @@ -193,7 +194,7 @@ public function testFactoryDoesNotAttachValidatorTwoTimes(): void [ 'session_manager' => [ 'validators' => [ - Validator\RemoteAddr::class, + Validator\RemoteAddr::class => null, ], ], ] @@ -212,7 +213,6 @@ public function testFactoryDoesNotAttachValidatorTwoTimes(): void $chain = $manager->getValidatorChain(); self::assertInstanceOf(EventManager::class, $chain); - $listeners = iterator_to_array($this->getListenersForEvent('session.validate', $chain)); self::assertCount(2, $listeners); diff --git a/test/SessionManagerTest.php b/test/SessionManagerTest.php index 1f79b0d0..ebf1abea 100644 --- a/test/SessionManagerTest.php +++ b/test/SessionManagerTest.php @@ -14,6 +14,7 @@ use Laminas\Session\Storage\ArrayStorage; use Laminas\Session\Storage\SessionArrayStorage; use Laminas\Session\Storage\SessionStorage; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\Id; use Laminas\Session\Validator\RemoteAddr; use LaminasTest\Session\TestAsset\Php81CompatibleStorageInterface; @@ -38,6 +39,7 @@ use function set_error_handler; use function stristr; use function uniqid; +use function unserialize; use function var_export; use function xdebug_get_headers; @@ -134,13 +136,13 @@ public function testCanPassValidatorsToConstructor(): void public function testAttachDefaultValidatorsByDefault(): void { $manager = new SessionManager(); - $this->assertAttributeEquals([Id::class], 'validators', $manager); + $this->assertAttributeEquals([Id::class => null], 'validators', $manager); } public function testCanMergeValidatorsWithDefault(): void { $defaultValidators = [ - Id::class, + Id::class => null, ]; $validators = [ 'foo', @@ -712,7 +714,13 @@ public function testStartingSessionThatFailsAValidatorShouldRaiseException(): vo { $this->manager = new SessionManager(); $chain = $this->manager->getValidatorChain(); - $chain->attach('session.validate', [new TestAsset\TestFailingValidator(), 'isValid']); + $chain->attach('session.validate', [ + new TestAsset\TestFailingValidator( + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER) + ), + 'isValid', + ]); $this->expectException(RuntimeException::class); $this->expectExceptionMessage('failed'); $this->manager->start(); @@ -773,17 +781,25 @@ public function testProducedSessionManagerWillNotReplaceSessionSuperGlobalValues #[IgnoreDeprecations] public function testValidatorChainSessionMetadataIsPreserved(): void { + $current = Environment::fromGlobals($_SERVER); + $this->manager = new SessionManager(); $this->manager->getValidatorChain() - ->attach('session.validate', [new RemoteAddr(), 'isValid']); + ->attach('session.validate', [ + new RemoteAddr( + Environment::fromGlobals($_SERVER), + $current + ), + 'isValid', + ]); self::assertFalse($this->manager->sessionExists()); $this->manager->start(); - self::assertIsArray($_SESSION['__Laminas']['_VALID']); self::assertArrayHasKey(RemoteAddr::class, $_SESSION['__Laminas']['_VALID']); - self::assertEquals('', $_SESSION['__Laminas']['_VALID'][RemoteAddr::class]); + self::assertIsString($_SESSION['__Laminas']['_VALID'][RemoteAddr::class]); + self::assertEquals($current, unserialize($_SESSION['__Laminas']['_VALID'][RemoteAddr::class])); } #[RunInSeparateProcess] @@ -791,7 +807,13 @@ public function testRemoteAddressValidationWillFailOnInvalidAddress(): void { $this->manager = new SessionManager(); $this->manager->getValidatorChain() - ->attach('session.validate', [new RemoteAddr('123.123.123.123'), 'isValid']); + ->attach('session.validate', [ + new RemoteAddr( + Environment::fromGlobals($_SERVER), + new Environment(remoteAddr: '123.123.123.123') + ), + 'isValid', + ]); $this->expectException(RuntimeException::class); $this->expectExceptionMessage('Session validation failed'); @@ -823,7 +845,7 @@ public function testRemoteAddressValidationWillFailWithInvalidPreSetData(): void $_SESSION = [ '__Laminas' => [ '_VALID' => [ - RemoteAddr::class => '123.123.123.123', + RemoteAddr::class => new Environment(remoteAddr: '123.123.123.123'), ], ], ]; @@ -839,7 +861,13 @@ public function testIdValidationWillFailOnInvalidData(): void { $this->manager = new SessionManager(); $this->manager->getValidatorChain() - ->attach('session.validate', [new Id('invalid-value'), 'isValid']); + ->attach('session.validate', [ + new Id( + new Environment(sessionId: 'invalid_value'), + Environment::fromGlobals($_SERVER) + ), + 'isValid', + ]); $this->expectException(RuntimeException::class); $this->expectExceptionMessage('Session validation failed'); diff --git a/test/TestAsset/TestFailingValidator.php b/test/TestAsset/TestFailingValidator.php index b66ffdd8..9c86f735 100644 --- a/test/TestAsset/TestFailingValidator.php +++ b/test/TestAsset/TestFailingValidator.php @@ -4,11 +4,16 @@ namespace LaminasTest\Session\TestAsset; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\ValidatorInterface; final class TestFailingValidator implements ValidatorInterface { - public ?string $data = null; + public function __construct(Environment $initial, Environment $current) + { + } + + public ?Environment $current = null; public function getName(): string { return self::class; diff --git a/test/Validator/HttpUserAgentTest.php b/test/Validator/HttpUserAgentTest.php index 9a8affd0..f09b628d 100644 --- a/test/Validator/HttpUserAgentTest.php +++ b/test/Validator/HttpUserAgentTest.php @@ -4,6 +4,7 @@ namespace LaminasTest\Session\Validator; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\HttpUserAgent; use PHPUnit\Framework\TestCase; @@ -12,7 +13,10 @@ class HttpUserAgentTest extends TestCase public function testIsValid(): void { $_SERVER['HTTP_USER_AGENT'] = 'Test-User-Agent'; - $validator = new HttpUserAgent('Test-User-Agent'); + $validator = new HttpUserAgent( + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER) + ); self::assertTrue($validator->isValid()); } @@ -21,14 +25,14 @@ public function testIsValidWhenNoUserAgentIsSet(): void { // technically not needed in CLI unset($_SERVER['HTTP_USER_AGENT']); - $validator = new HttpUserAgent(); + $validator = new HttpUserAgent(new Environment(userAgent: null), Environment::fromGlobals($_SERVER)); self::assertTrue($validator->isValid()); } public function testGetNameReturnsClassName(): void { - $validator = new HttpUserAgent(null); + $validator = new HttpUserAgent(new Environment(userAgent: null), Environment::fromGlobals($_SERVER)); self::assertSame(HttpUserAgent::class, $validator->getName()); } diff --git a/test/Validator/IdTest.php b/test/Validator/IdTest.php index d2a9599b..fe6300c6 100644 --- a/test/Validator/IdTest.php +++ b/test/Validator/IdTest.php @@ -5,12 +5,12 @@ namespace LaminasTest\Session\Validator; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\Id; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RunInSeparateProcess; use PHPUnit\Framework\TestCase; -use ReflectionProperty; use function ini_set; use function session_id; @@ -40,18 +40,10 @@ public function testIsValidPhp71(int $bitsPerCharacter, string $id, bool $isVali { ini_set('session.sid_bits_per_character', $bitsPerCharacter); - $validator = new Id($id); + $validator = new Id(new Environment(sessionId: $id), Environment::fromGlobals($_SERVER)); self::assertSame($isValid, $validator->isValid()); } - public function testConstructorSetId(): void - { - $id = new Id('1234'); - $idReflection = new ReflectionProperty($id, 'data'); - - self::assertSame('1234', $idReflection->getValue($id)); - } - /** * @runInSeparateProcess */ @@ -59,16 +51,14 @@ public function testInitializedWithSessionIdWhenIdIsNotPassed(): void { session_start(); $sessionId = session_id(); + $id = new Id(Environment::fromGlobals($_SERVER), Environment::fromGlobals($_SERVER)); - $id = new Id(); - $idReflection = new ReflectionProperty($id, 'data'); - - self::assertSame($sessionId, $idReflection->getValue($id)); + self::assertSame($sessionId, $id->initial->sessionId); } public function testValidatorName(): void { - $id = new Id(); + $id = new Id(Environment::fromGlobals($_SERVER), Environment::fromGlobals($_SERVER)); self::assertSame(Id::class, $id->getName()); } diff --git a/test/Validator/RemoteAddrTest.php b/test/Validator/RemoteAddrTest.php index 3401ca96..2c08da2d 100644 --- a/test/Validator/RemoteAddrTest.php +++ b/test/Validator/RemoteAddrTest.php @@ -4,6 +4,7 @@ namespace LaminasTest\Session\Validator; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\RemoteAddr; use PHPUnit\Framework\TestCase; @@ -12,14 +13,7 @@ */ class RemoteAddrTest extends TestCase { - protected array $backup; - - protected RemoteAddr $defaultRemoteAddr; - - protected function setUp(): void - { - $this->defaultRemoteAddr = new RemoteAddr(initial: '0.1.2.3'); - } + protected array $backup = []; protected function backup(): void { @@ -36,23 +30,13 @@ protected function restore(): void $_SERVER = $this->backup; } - public function testGetData(): void - { - $validator = new RemoteAddr(initial: '0.1.2.3'); - self::assertEquals('0.1.2.3', $validator->data); - } - - public function testDefaultUseProxy(): void - { - self::assertFalse($this->defaultRemoteAddr->getUseProxy()); - } - public function testRemoteAddrWithoutProxy(): void { $this->backup(); $_SERVER['REMOTE_ADDR'] = '0.1.2.3'; - $validator = new RemoteAddr(initial: $_SERVER['REMOTE_ADDR']); - self::assertEquals('0.1.2.3', $validator->data); + $validator = + new RemoteAddr(Environment::fromGlobals($_SERVER), Environment::fromGlobals($_SERVER)); + self::assertEquals('0.1.2.3', $validator->current->remoteAddr); $this->restore(); } @@ -60,8 +44,8 @@ public function testIsValid(): void { $this->backup(); $_SERVER['REMOTE_ADDR'] = '0.1.2.3'; - $validator = new RemoteAddr(); - $_SERVER['REMOTE_ADDR'] = '1.1.2.3'; + $validator = + new RemoteAddr(new Environment(remoteAddr: '0.1.1.3'), Environment::fromGlobals($_SERVER)); self::assertFalse($validator->isValid()); $this->restore(); } @@ -71,8 +55,11 @@ public function testIgnoreProxyByDefault(): void $this->backup(); $_SERVER['REMOTE_ADDR'] = '0.1.2.3'; $_SERVER['HTTP_CLIENT_IP'] = '1.1.2.3'; - $validator = new RemoteAddr(initial: $_SERVER['REMOTE_ADDR']); - self::assertEquals('0.1.2.3', $validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER) + ); + self::assertEquals('0.1.2.3', $validator->current->remoteAddr); $this->restore(); } @@ -86,8 +73,11 @@ public function testHttpXForwardedFor(): void 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr(options: $options); - self::assertEquals('1.1.2.3', $validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER, $options), + Environment::fromGlobals($_SERVER, $options) + ); + self::assertEquals('1.1.2.3', $validator->current->remoteAddr); $this->restore(); } @@ -103,8 +93,11 @@ public function testHttpClientIp(): void 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr(options: $options); - self::assertEquals('2.1.2.3', $validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER, $options), + Environment::fromGlobals($_SERVER, $options) + ); + self::assertEquals('2.1.2.3', $validator->current->remoteAddr); $this->restore(); } @@ -119,8 +112,12 @@ public function testUsesRightMostAddressWhenMultipleHttpXForwardedForAddressesPr 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr(options: $options); - self::assertEquals('1.1.2.3', $validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER, $options), + Environment::fromGlobals($_SERVER, $options) + ); + + self::assertEquals('1.1.2.3', $validator->current->remoteAddr); $this->restore(); } @@ -135,8 +132,11 @@ public function testShouldNotUseClientIpHeaderToTestProxyCapabilitiesByDefault() 'trusted_proxies' => ['0.1.2.3'], ]; - $validator = new RemoteAddr($_SERVER['REMOTE_ADDR'], $options); - self::assertEquals('0.1.2.3', $validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER, $options), + Environment::fromGlobals($_SERVER, $options) + ); + self::assertEquals('0.1.2.3', $validator->current->remoteAddr); $this->restore(); } @@ -151,8 +151,11 @@ public function testWillOmitTrustedProxyIpsFromXForwardedForMatching(): void 'trusted_proxies' => ['1.1.2.3'], ]; - $validator = new RemoteAddr(options: $options); - self::assertEquals('2.1.2.3', $validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER, $options), + Environment::fromGlobals($_SERVER, $options) + ); + self::assertEquals('2.1.2.3', $validator->current->remoteAddr); $this->restore(); } @@ -168,14 +171,21 @@ public function testCanSpecifyWhichHeaderToUseStatically(): void 'proxy_header' => 'Client-Ip', ]; - $validator = new RemoteAddr($_SERVER['REMOTE_ADDR'], $options); - self::assertEquals('0.1.2.3', $validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER, $options), + Environment::fromGlobals($_SERVER, $options) + ); + self::assertEquals('0.1.2.3', $validator->current->remoteAddr); $this->restore(); } public function testGetName(): void { - self::assertEquals(RemoteAddr::class, $this->defaultRemoteAddr->getName()); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER) + ); + self::assertEquals(RemoteAddr::class, $validator->getName()); } public function testUnknownServerHeader(): void @@ -187,8 +197,11 @@ public function testUnknownServerHeader(): void 'proxy_header' => 'Unknown-Header', ]; - $validator = new RemoteAddr(options: $options); - self::assertEmpty($validator->data); + $validator = new RemoteAddr( + Environment::fromGlobals($_SERVER, $options), + Environment::fromGlobals($_SERVER, $options) + ); + self::assertEmpty($validator->current->remoteAddr); $this->restore(); } } diff --git a/test/Validator/StaticValidatorStub.php b/test/Validator/StaticValidatorStub.php index ed45896a..b895d552 100644 --- a/test/Validator/StaticValidatorStub.php +++ b/test/Validator/StaticValidatorStub.php @@ -4,12 +4,17 @@ namespace LaminasTest\Session\Validator; +use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\ValidatorInterface; class StaticValidatorStub implements ValidatorInterface { + public function __construct(Environment $initial, Environment $current) + { + } + public static int $isValidCallCount = 0; - public ?string $data = null; + public ?Environment $current = null; public function isValid(): bool { diff --git a/test/Validator/ValidatorChainTest.php b/test/Validator/ValidatorChainTest.php index b1d3c02d..bed9d412 100644 --- a/test/Validator/ValidatorChainTest.php +++ b/test/Validator/ValidatorChainTest.php @@ -5,10 +5,13 @@ namespace LaminasTest\Session\Validator; use Laminas\Session\Storage\ArrayStorage; +use Laminas\Session\Validator\Environment; use Laminas\Session\ValidatorChain; use LaminasTest\Session\TestAsset\TestFailingValidator; use PHPUnit\Framework\TestCase; +use function serialize; + class ValidatorChainTest extends TestCase { private ValidatorChain $validatorChain; @@ -26,22 +29,20 @@ public function testGetStorage(): void public function testAttachValidator(): void { - $validator = new TestFailingValidator(); + $validator = new TestFailingValidator(Environment::fromGlobals($_SERVER), Environment::fromGlobals($_SERVER)); $this->validatorChain->attach('test', [$validator, 'isValid']); $validatorMetadata = $this->validatorChain->getStorage()->getMetadata('_VALID'); self::assertIsArray($validatorMetadata); - self::assertArrayHasKey($validator->getName(), $validatorMetadata); - self::assertSame($validatorMetadata[$validator->getName()], $validator->data); } public function testExistingValidatorsAreAttached(): void { - $validator = new StaticValidatorStub(); + $validator = new StaticValidatorStub(Environment::fromGlobals($_SERVER), Environment::fromGlobals($_SERVER)); $storage = new ArrayStorage(); - $storage->setMetadata('_VALID', [$validator::class => $validator->data]); - + $storage->setMetadata('_VALID', [$validator::class => null]); + $storage->setMetadata('environment', serialize(Environment::fromGlobals($_SERVER))); $this->validatorChain = new ValidatorChain($storage); $this->validatorChain->trigger('session.validate'); From 4fa838cc15de0780fc5a99fd9fe89eee9007bdf8 Mon Sep 17 00:00:00 2001 From: bota Date: Wed, 18 Jun 2025 15:30:56 +0300 Subject: [PATCH 6/7] allow adding validators only from config Signed-off-by: bota --- psalm-baseline.xml | 33 +--------- src/SessionManager.php | 18 +++--- src/Validator/Environment.php | 22 ++----- src/Validator/HttpUserAgent.php | 7 ++- src/Validator/Id.php | 11 ++-- src/Validator/RemoteAddr.php | 60 ++++++++---------- src/Validator/ValidatorInterface.php | 5 +- src/ValidatorChain.php | 20 +----- test/Service/SessionManagerFactoryTest.php | 8 +-- test/SessionManagerTest.php | 55 ++++++---------- test/TestAsset/TestFailingValidator.php | 2 +- .../TestSaveHandlerWithValidator.php | 63 ------------------- test/Validator/IdTest.php | 2 +- test/Validator/RemoteAddrTest.php | 43 +++++++------ test/Validator/StaticValidatorStub.php | 29 --------- test/Validator/ValidatorChainTest.php | 14 ----- 16 files changed, 109 insertions(+), 283 deletions(-) delete mode 100644 test/TestAsset/TestSaveHandlerWithValidator.php delete mode 100644 test/Validator/StaticValidatorStub.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index f577f6d3..9fe0b675 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -438,22 +438,10 @@ - - - - - - - - - - - current]]> - @@ -806,6 +794,7 @@ + @@ -921,21 +910,6 @@ - - - - - - - - - - - - - - - @@ -946,9 +920,4 @@ - - - - - diff --git a/src/SessionManager.php b/src/SessionManager.php index 78bb9a1a..1fbfe5c8 100644 --- a/src/SessionManager.php +++ b/src/SessionManager.php @@ -57,7 +57,7 @@ class SessionManager extends AbstractManager /** @var array Default validators */ protected $defaultValidators = [ - Validator\Id::class => null, + Validator\Id::class, ]; /** @var string value returned by session_name() */ @@ -66,6 +66,8 @@ class SessionManager extends AbstractManager /** @var EventManagerInterface Validation chain to determine if session is valid */ protected $validatorChain; + protected array $options = []; + /** * Constructor * @@ -83,6 +85,8 @@ public function __construct( $validators = array_merge($this->defaultValidators, $validators); } + $this->options = $options; + parent::__construct($config, $storage, $saveHandler, $validators); register_shutdown_function([$this, 'writeClose']); } @@ -170,10 +174,9 @@ protected function initializeValidatorChain() $storage = $this->getStorage()->getMetadata(); /** - * @var string|null $data * @var class-string $validatorName */ - foreach ($this->validators as $validatorName => $data) { + foreach ($this->validators as $validatorName) { $validatorValues = $this->getStorage()->getMetadata('_VALID'); if (is_array($validatorValues) && array_key_exists($validatorName, $validatorValues)) { continue; @@ -188,16 +191,11 @@ protected function initializeValidatorChain() $this->getStorage()->setMetadata('environment', serialize($initialEnvironment)); } - if ($data !== null) { - /** @var Environment $currentEnvironment */ - $currentEnvironment = unserialize($data); - } else { - $currentEnvironment = Environment::fromGlobals($_SERVER); - } + $currentEnvironment = Environment::fromGlobals($_SERVER); $validatorChain = $this->getValidatorChain(); + $validator = new $validatorName($initialEnvironment, $currentEnvironment, $this->options); - $validator = new $validatorName($initialEnvironment, $currentEnvironment); $validatorChain->attach('session.validate', [$validator, 'isValid']); } } diff --git a/src/Validator/Environment.php b/src/Validator/Environment.php index fe533f55..a1d15552 100644 --- a/src/Validator/Environment.php +++ b/src/Validator/Environment.php @@ -7,18 +7,17 @@ use function is_string; use function session_id; -/** @psalm-import-type OptionsArgument from RemoteAddr */ final class Environment { public function __construct( public readonly ?string $userAgent = null, public readonly ?string $remoteAddr = null, + public readonly ?string $forwardedFor = null, public readonly ?string $sessionId = null ) { } - /** @param OptionsArgument $options */ - public static function fromGlobals(array $server, array $options = []): self + public static function fromGlobals(array $server): self { $userAgent = isset($server['HTTP_USER_AGENT']) && is_string($server['HTTP_USER_AGENT']) ? $server['HTTP_USER_AGENT'] @@ -28,21 +27,12 @@ public static function fromGlobals(array $server, array $options = []): self ? $server['REMOTE_ADDR'] : null; - if ($remoteAddr === null || (isset($options['use_proxy']) && $options['use_proxy'])) { - $remoteAddr = RemoteAddr::getIpAddress($options, $remoteAddr); - } + $forwardedFor = isset($server['HTTP_X_FORWARDED_FOR']) && is_string($server['HTTP_X_FORWARDED_FOR']) + ? $server['HTTP_X_FORWARDED_FOR'] + : null; $sessionId = session_id(); - return new self($userAgent, $remoteAddr, $sessionId); - } - - public static function getServerOption(string $name, ?array $superglobal = null): mixed - { - if ($superglobal === null) { - $superglobal = $_SERVER; - } - - return $superglobal[$name] ?? null; + return new self($userAgent, $remoteAddr, $forwardedFor, $sessionId); } } diff --git a/src/Validator/HttpUserAgent.php b/src/Validator/HttpUserAgent.php index 14722369..4ea063a9 100644 --- a/src/Validator/HttpUserAgent.php +++ b/src/Validator/HttpUserAgent.php @@ -10,8 +10,11 @@ final class HttpUserAgent implements ValidatorInterface * Constructor * get the current user agent and store it in the session as 'valid data' */ - public function __construct(public readonly Environment $initial, public readonly Environment $current) - { + public function __construct( + public readonly Environment $initial, + public readonly Environment $current, + array $option = [] + ) { } /** diff --git a/src/Validator/Id.php b/src/Validator/Id.php index f32b37ca..4232cb6a 100644 --- a/src/Validator/Id.php +++ b/src/Validator/Id.php @@ -23,8 +23,11 @@ final class Id implements ValidatorInterface * Allows passing the current session_id; if none provided, uses the PHP * session_id() function to retrieve it. */ - public function __construct(public readonly Environment $initial, public readonly Environment $current) - { + public function __construct( + public readonly Environment $initial, + public readonly Environment $current, + array $options = [] + ) { } /** @@ -34,7 +37,7 @@ public function __construct(public readonly Environment $initial, public readonl */ public function isValid(): bool { - if ($this->initial->sessionId === null) { + if ($this->current->sessionId === null) { return false; } @@ -54,7 +57,7 @@ public function isValid(): bool default => '#^[0-9a-v]*$#', }; - return (bool) preg_match($pattern, $this->initial->sessionId); + return (bool) preg_match($pattern, $this->current->sessionId); } /** diff --git a/src/Validator/RemoteAddr.php b/src/Validator/RemoteAddr.php index 2ee1de11..999acb6b 100644 --- a/src/Validator/RemoteAddr.php +++ b/src/Validator/RemoteAddr.php @@ -9,13 +9,8 @@ use function array_diff; use function array_map; use function array_pop; -use function assert; use function explode; use function in_array; -use function is_string; -use function str_replace; -use function strpos; -use function strtoupper; /** * @psalm-type OptionsArgument = array{ @@ -26,12 +21,27 @@ */ final class RemoteAddr implements SessionValidator { + public ?string $initialData = null; + public ?string $currentData = null; + /** * Constructor * get the current user IP and store it in the session as 'valid data' + * + * @param OptionsArgument $options */ - public function __construct(public readonly Environment $initial, public readonly Environment $current) - { + public function __construct( + public readonly Environment $initial, + public readonly Environment $current, + array $options = [] + ) { + if (isset($options['use_proxy']) && $options['use_proxy'] === true) { + $this->initialData = $this->getIpAddress($this->initial, $options); + $this->currentData = $this->getIpAddress($this->current, $options); + } else { + $this->initialData = $this->initial->remoteAddr; + $this->currentData = $this->current->remoteAddr; + } } /** @@ -40,7 +50,7 @@ public function __construct(public readonly Environment $initial, public readonl */ public function isValid(): bool { - return $this->initial->remoteAddr === $this->current->remoteAddr; + return $this->initialData === $this->currentData; } /** @@ -48,16 +58,16 @@ public function isValid(): bool * * @param OptionsArgument $options */ - public static function getIpAddress(array $options = [], ?string $remoteAddr = null): ?string + public static function getIpAddress(Environment $initial, array $options = []): ?string { - $ip = self::getIpAddressFromProxy($options, $remoteAddr); + $ip = self::getIpAddressFromProxy($initial, $options); if ($ip !== false) { return $ip; } - if ($remoteAddr !== null) { - return $remoteAddr; + if ($initial->remoteAddr !== null) { + return $initial->remoteAddr; } return null; @@ -70,26 +80,24 @@ public static function getIpAddress(array $options = [], ?string $remoteAddr = n * * @param OptionsArgument $options */ - private static function getIpAddressFromProxy(array $options = [], ?string $remoteAddr = null): string|false + private static function getIpAddressFromProxy(Environment $initial, array $options = []): string|false { - $normalizedProxyHeader = self::normalizeProxyHeader($options['proxy_header'] ?? 'X_FORWARDED_FOR'); - $trustedProxies = $options['trusted_proxies'] ?? []; + $trustedProxies = $options['trusted_proxies'] ?? []; if ( ! (isset($options['use_proxy']) && $options['use_proxy']) - || ($remoteAddr !== null && ! in_array($remoteAddr, $trustedProxies)) + || ($initial->remoteAddr !== null && ! in_array($initial->remoteAddr, $trustedProxies)) ) { return false; } - $proxyHeader = Environment::getServerOption($normalizedProxyHeader); + $proxyHeader = $initial->forwardedFor; if ($proxyHeader === null || $proxyHeader === '') { return false; } // Extract IPs - assert(is_string($proxyHeader)); $ips = explode(',', $proxyHeader); // trim, so we can compare against trusted proxies properly $ips = array_map('trim', $ips); @@ -108,22 +116,6 @@ private static function getIpAddressFromProxy(array $options = [], ?string $remo return array_pop($ips); } - /** - * Normalize a header string - * - * Normalizes a header string to a format that is compatible with - * $_SERVER - */ - protected static function normalizeProxyHeader(string $header): string - { - $header = strtoupper($header); - $header = str_replace('-', '_', $header); - if (0 !== strpos($header, 'HTTP_')) { - $header = 'HTTP_' . $header; - } - return $header; - } - /** * Return validator name */ diff --git a/src/Validator/ValidatorInterface.php b/src/Validator/ValidatorInterface.php index cfec6cd6..cf08e264 100644 --- a/src/Validator/ValidatorInterface.php +++ b/src/Validator/ValidatorInterface.php @@ -6,10 +6,13 @@ /** * Session validator interface + * + * @psalm-import-type OptionsArgument from RemoteAddr */ interface ValidatorInterface { - public function __construct(Environment $initial, Environment $current); + /** @param OptionsArgument $options */ + public function __construct(Environment $initial, Environment $current, array $options = []); /** * This method will be called at the beginning of diff --git a/src/ValidatorChain.php b/src/ValidatorChain.php index 3352e23a..03a41d69 100644 --- a/src/ValidatorChain.php +++ b/src/ValidatorChain.php @@ -6,35 +6,17 @@ use Laminas\EventManager\EventManager; use Laminas\Session\Storage\StorageInterface; -use Laminas\Session\Validator\Environment; use Laminas\Session\Validator\ValidatorInterface; use function array_shift; use function array_unshift; use function is_array; -use function serialize; -use function unserialize; class ValidatorChain extends EventManager { public function __construct(protected StorageInterface $storage) { parent::__construct(); - $validators = $storage->getMetadata('_VALID'); - $environment = (string) $storage->getMetadata('environment'); - if ($validators) { - /** - * @var class-string $validator - */ - foreach ($validators as $validator => $data) { - $currentEnvironment = $data instanceof Environment ? $data : Environment::fromGlobals($_SERVER); - $this->attachValidator( - 'session.validate', - [new $validator(unserialize($environment), $currentEnvironment), 'isValid'], - 1 - ); - } - } } /** @@ -82,7 +64,7 @@ private function attachValidator($event, $callback, $priority) } if ($context instanceof ValidatorInterface) { $name = $context->getName(); - $this->getStorage()->setMetadata('_VALID', [$name => serialize($context->current)]); + $this->getStorage()->setMetadata('_VALID', [$name]); } return parent::attach($event, $callback, $priority); } diff --git a/test/Service/SessionManagerFactoryTest.php b/test/Service/SessionManagerFactoryTest.php index d22ae707..affc5bd1 100644 --- a/test/Service/SessionManagerFactoryTest.php +++ b/test/Service/SessionManagerFactoryTest.php @@ -17,7 +17,6 @@ use Laminas\Session\Storage\ArrayStorage; use Laminas\Session\Storage\StorageInterface; use Laminas\Session\Validator; -use Laminas\Session\Validator\Environment; use LaminasTest\Session\ReflectionPropertyTrait; use LaminasTest\Session\TestAsset\TestManager; use LaminasTest\Session\TestAsset\TestSaveHandler; @@ -108,7 +107,7 @@ public function testFactoryWillAddValidatorViaConfiguration(): void $config = [ 'session_manager' => [ 'validators' => [ - Validator\RemoteAddr::class => null, + Validator\RemoteAddr::class, ], ], ]; @@ -185,16 +184,17 @@ public function testFactoryDoesNotAttachValidatorTwoTimes(): void $storage->setMetadata( '_VALID', [ - Validator\RemoteAddr::class => new Environment(remoteAddr: '1.2.3.4'), + Validator\RemoteAddr::class, ] ); + $this->services->setService(StorageInterface::class, $storage); $this->services->setService( 'config', [ 'session_manager' => [ 'validators' => [ - Validator\RemoteAddr::class => null, + Validator\RemoteAddr::class, ], ], ] diff --git a/test/SessionManagerTest.php b/test/SessionManagerTest.php index ebf1abea..3c43fc78 100644 --- a/test/SessionManagerTest.php +++ b/test/SessionManagerTest.php @@ -18,12 +18,14 @@ use Laminas\Session\Validator\Id; use Laminas\Session\Validator\RemoteAddr; use LaminasTest\Session\TestAsset\Php81CompatibleStorageInterface; +use LaminasTest\Session\TestAsset\TestFailingValidator; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RunInSeparateProcess; use PHPUnit\Framework\TestCase; use Traversable; use function array_merge; +use function assert; use function extension_loaded; use function headers_sent; use function ini_get; @@ -136,13 +138,13 @@ public function testCanPassValidatorsToConstructor(): void public function testAttachDefaultValidatorsByDefault(): void { $manager = new SessionManager(); - $this->assertAttributeEquals([Id::class => null], 'validators', $manager); + $this->assertAttributeEquals([Id::class], 'validators', $manager); } public function testCanMergeValidatorsWithDefault(): void { $defaultValidators = [ - Id::class => null, + Id::class, ]; $validators = [ 'foo', @@ -715,7 +717,7 @@ public function testStartingSessionThatFailsAValidatorShouldRaiseException(): vo $this->manager = new SessionManager(); $chain = $this->manager->getValidatorChain(); $chain->attach('session.validate', [ - new TestAsset\TestFailingValidator( + new TestFailingValidator( Environment::fromGlobals($_SERVER), Environment::fromGlobals($_SERVER) ), @@ -729,8 +731,7 @@ public function testStartingSessionThatFailsAValidatorShouldRaiseException(): vo #[RunInSeparateProcess] public function testResumeSessionThatFailsAValidatorShouldRaiseException(): void { - $this->manager = new SessionManager(); - $this->manager->setSaveHandler(new TestAsset\TestSaveHandlerWithValidator()); + $this->manager = new SessionManager(validators: [TestFailingValidator::class]); $this->expectException(RuntimeException::class); $this->expectExceptionMessage('failed'); $this->manager->start(); @@ -781,25 +782,26 @@ public function testProducedSessionManagerWillNotReplaceSessionSuperGlobalValues #[IgnoreDeprecations] public function testValidatorChainSessionMetadataIsPreserved(): void { - $current = Environment::fromGlobals($_SERVER); - $this->manager = new SessionManager(); + self::assertFalse($this->manager->sessionExists()); + $this->manager->start(); + $environment = unserialize((string) $this->manager->getStorage()->getMetadata('environment')); + assert($environment instanceof Environment); $this->manager->getValidatorChain() ->attach('session.validate', [ new RemoteAddr( - Environment::fromGlobals($_SERVER), - $current + $environment, + Environment::fromGlobals($_SERVER) ), 'isValid', ]); - self::assertFalse($this->manager->sessionExists()); - - $this->manager->start(); self::assertIsArray($_SESSION['__Laminas']['_VALID']); - self::assertArrayHasKey(RemoteAddr::class, $_SESSION['__Laminas']['_VALID']); - self::assertIsString($_SESSION['__Laminas']['_VALID'][RemoteAddr::class]); - self::assertEquals($current, unserialize($_SESSION['__Laminas']['_VALID'][RemoteAddr::class])); + self::assertIsString($_SESSION['__Laminas']['_VALID'][0]); + self::assertEquals( + Environment::fromGlobals($_SERVER), + unserialize((string) $_SESSION['__Laminas']['environment']) + ); } #[RunInSeparateProcess] @@ -828,7 +830,7 @@ public function testRemoteAddressValidationWillSucceedWithValidPreSetData(): voi $_SESSION = [ '__Laminas' => [ '_VALID' => [ - RemoteAddr::class => null, + RemoteAddr::class, ], ], ]; @@ -838,23 +840,6 @@ public function testRemoteAddressValidationWillSucceedWithValidPreSetData(): voi self::assertTrue($this->manager->isValid()); } - #[RunInSeparateProcess] - public function testRemoteAddressValidationWillFailWithInvalidPreSetData(): void - { - $this->manager = new SessionManager(); - $_SESSION = [ - '__Laminas' => [ - '_VALID' => [ - RemoteAddr::class => new Environment(remoteAddr: '123.123.123.123'), - ], - ], - ]; - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('Session validation failed'); - $this->manager->start(); - } - #[RunInSeparateProcess] #[IgnoreDeprecations] public function testIdValidationWillFailOnInvalidData(): void @@ -863,8 +848,8 @@ public function testIdValidationWillFailOnInvalidData(): void $this->manager->getValidatorChain() ->attach('session.validate', [ new Id( - new Environment(sessionId: 'invalid_value'), - Environment::fromGlobals($_SERVER) + Environment::fromGlobals($_SERVER), + new Environment(sessionId: 'invalid_value') ), 'isValid', ]); diff --git a/test/TestAsset/TestFailingValidator.php b/test/TestAsset/TestFailingValidator.php index 9c86f735..9a81ec6b 100644 --- a/test/TestAsset/TestFailingValidator.php +++ b/test/TestAsset/TestFailingValidator.php @@ -9,7 +9,7 @@ final class TestFailingValidator implements ValidatorInterface { - public function __construct(Environment $initial, Environment $current) + public function __construct(Environment $initial, Environment $current, array $options = []) { } diff --git a/test/TestAsset/TestSaveHandlerWithValidator.php b/test/TestAsset/TestSaveHandlerWithValidator.php deleted file mode 100644 index 880b33d3..00000000 --- a/test/TestAsset/TestSaveHandlerWithValidator.php +++ /dev/null @@ -1,63 +0,0 @@ -isValid()); } diff --git a/test/Validator/RemoteAddrTest.php b/test/Validator/RemoteAddrTest.php index 2c08da2d..40afc590 100644 --- a/test/Validator/RemoteAddrTest.php +++ b/test/Validator/RemoteAddrTest.php @@ -74,10 +74,11 @@ public function testHttpXForwardedFor(): void ]; $validator = new RemoteAddr( - Environment::fromGlobals($_SERVER, $options), - Environment::fromGlobals($_SERVER, $options) + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER), + $options ); - self::assertEquals('1.1.2.3', $validator->current->remoteAddr); + self::assertEquals('1.1.2.3', $validator->currentData); $this->restore(); } @@ -94,10 +95,11 @@ public function testHttpClientIp(): void ]; $validator = new RemoteAddr( - Environment::fromGlobals($_SERVER, $options), - Environment::fromGlobals($_SERVER, $options) + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER), + $options ); - self::assertEquals('2.1.2.3', $validator->current->remoteAddr); + self::assertEquals('2.1.2.3', $validator->currentData); $this->restore(); } @@ -113,11 +115,12 @@ public function testUsesRightMostAddressWhenMultipleHttpXForwardedForAddressesPr ]; $validator = new RemoteAddr( - Environment::fromGlobals($_SERVER, $options), - Environment::fromGlobals($_SERVER, $options) + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER), + $options ); - self::assertEquals('1.1.2.3', $validator->current->remoteAddr); + self::assertEquals('1.1.2.3', $validator->currentData); $this->restore(); } @@ -133,8 +136,9 @@ public function testShouldNotUseClientIpHeaderToTestProxyCapabilitiesByDefault() ]; $validator = new RemoteAddr( - Environment::fromGlobals($_SERVER, $options), - Environment::fromGlobals($_SERVER, $options) + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER), + $options ); self::assertEquals('0.1.2.3', $validator->current->remoteAddr); $this->restore(); @@ -152,10 +156,11 @@ public function testWillOmitTrustedProxyIpsFromXForwardedForMatching(): void ]; $validator = new RemoteAddr( - Environment::fromGlobals($_SERVER, $options), - Environment::fromGlobals($_SERVER, $options) + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER), + $options ); - self::assertEquals('2.1.2.3', $validator->current->remoteAddr); + self::assertEquals('2.1.2.3', $validator->currentData); $this->restore(); } @@ -172,8 +177,9 @@ public function testCanSpecifyWhichHeaderToUseStatically(): void ]; $validator = new RemoteAddr( - Environment::fromGlobals($_SERVER, $options), - Environment::fromGlobals($_SERVER, $options) + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER), + $options ); self::assertEquals('0.1.2.3', $validator->current->remoteAddr); $this->restore(); @@ -198,8 +204,9 @@ public function testUnknownServerHeader(): void ]; $validator = new RemoteAddr( - Environment::fromGlobals($_SERVER, $options), - Environment::fromGlobals($_SERVER, $options) + Environment::fromGlobals($_SERVER), + Environment::fromGlobals($_SERVER), + $options ); self::assertEmpty($validator->current->remoteAddr); $this->restore(); diff --git a/test/Validator/StaticValidatorStub.php b/test/Validator/StaticValidatorStub.php deleted file mode 100644 index b895d552..00000000 --- a/test/Validator/StaticValidatorStub.php +++ /dev/null @@ -1,29 +0,0 @@ -validatorChain->getStorage()->getMetadata('_VALID'); self::assertIsArray($validatorMetadata); } - - public function testExistingValidatorsAreAttached(): void - { - $validator = new StaticValidatorStub(Environment::fromGlobals($_SERVER), Environment::fromGlobals($_SERVER)); - $storage = new ArrayStorage(); - $storage->setMetadata('_VALID', [$validator::class => null]); - $storage->setMetadata('environment', serialize(Environment::fromGlobals($_SERVER))); - $this->validatorChain = new ValidatorChain($storage); - - $this->validatorChain->trigger('session.validate'); - self::assertSame(1, $validator::$isValidCallCount); - } } From b16cc9f65fbd216f8d208508d0415e8a46e24704 Mon Sep 17 00:00:00 2001 From: bota Date: Wed, 18 Jun 2025 15:41:35 +0300 Subject: [PATCH 7/7] fixed php 8.4 tests Signed-off-by: bota --- test/SessionManagerTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/test/SessionManagerTest.php b/test/SessionManagerTest.php index 3c43fc78..aad3385f 100644 --- a/test/SessionManagerTest.php +++ b/test/SessionManagerTest.php @@ -729,6 +729,7 @@ public function testStartingSessionThatFailsAValidatorShouldRaiseException(): vo } #[RunInSeparateProcess] + #[IgnoreDeprecations] public function testResumeSessionThatFailsAValidatorShouldRaiseException(): void { $this->manager = new SessionManager(validators: [TestFailingValidator::class]);