Removed laminas-eventmanager dependency, refactored validators by Jurj-Bogdan · Pull Request #168 · laminas/laminas-session

Jurj-Bogdan · 2025-11-22T15:13:09Z

Q	A
Documentation	no
BC Break	yes
New Feature	no
RFC	yes

Description

Based on #167.

I agree with @gsteel 's suggestion of dropping the "event" flow for session validation, and since the dependency was not used in other places, I have removed laminas/laminas-eventmanager. To go in a bit more detail:

the dependency was dropped.
the ValidatorChain class that was using it has been removed.
- as it was already agreed in previous PRs that going forward validators should only be added via config and only "automatically" called after session start, the whole storing of the configured validators under the "_VALID" key as previously done in ValidatorChain has also been completely removed.
updated the shipped Laminas\Session\ValidatorInterface and all validators using it to suit the new validation flow.

While mentioned in the initial issue, I haven't touched the Csrf validator and its dependency laminas/laminas-validator here, as that's probably better suited for a new PR - right now this validator already cannot be used in the same way as the other validators implementing the own ValidatorInterface ( neither in v3 nor v2 ).

arhimede · 2025-11-25T10:45:40Z

@laminas/tsc-reviewers
Let's take a look at this one in order to have a release of version 3.0 until next Monday at TSC meeting

froschdesign · 2025-11-25T11:12:54Z

While mentioned in the initial issue, I haven't touched the Csrf validator and its dependency laminas/laminas-validator here, as that's probably better suited for a new PR

If this component defines its own interface for validators, then this should also be used by validators in this component. This not only reduces dependencies, but also confusion.

gsteel · 2025-11-25T11:47:30Z

If this component defines its own interface for validators, then this should also be used by validators in this component. This not only reduces dependencies, but also confusion.

The problem here is naming.

It's entirely aceptable to have 2 different types of validator:

One for validating user input (via laminas-validator with the CSRF validator)
One for validating sessions - as described here.

Renaming the session validator interface from Session\ValidatorInterface to SessionValidator\SessionValidatorInterface would reduce the potential for confusion at the expense of another BC break.

I'd support a rename of the interface/namespace as I find it unlikely that many users implement custom validators outside of those already shipped, and, the BC breaks are already very significant in this portion of the library.

The fact that the CSRF validator is in the same namespace is really unfortunate. Extracting it to a separate library (and removing the laminas-validator dependency) would clean things up nicely, but it's even more maintenance.

Jurj-Bogdan · 2025-11-25T11:54:51Z

The fact that the CSRF validator is in the same namespace is really unfortunate. Extracting it to a separate library (and removing the laminas-validator dependency) would clean things up nicely, but it's even more maintenance.

@gsteel Should I maybe remove this feature altogether then? I'm not sure that form CSRF validation should be in the laminas-session library use case, especially not when it requires the laminas-validator dependency without much other use.

src/Validator/HttpUserAgent.php

src/Validator/Id.php

src/SessionManager.php

test/Validator/HttpUserAgentTest.php

test/Validator/IdTest.php

gsteel · 2025-11-25T12:36:52Z

@gsteel Should I maybe remove this feature altogether then? I'm not sure that form CSRF validation should be in the laminas-session library use case, especially not when it requires the laminas-validator dependency without much other use.

I agree, but I'd gather some more opinions first.

Who's going to make sure that there is a CSRF implementation available to users of Laminas Session for example. Is it even necessary based on the current fate of MVC? Mezzio already has an equivalent that doesn't use laminas-validator.

cc @laminas/technical-steering-committee

froschdesign · 2025-11-25T12:54:54Z

Is it even necessary based on the current fate of MVC?

I think the main use is in the CSRF element of laminas-form.

gsteel · 2025-11-25T14:01:17Z

Is it even necessary based on the current fate of MVC?

I think the main use is in the CSRF element of laminas-form.

Ideally, laminas-form would not have hard or soft deps on laminas-session.

Maybe laminas-csrf is necessary, so it can have the dependency on both form and session.

On the other hand, why not just use mezzio-csrf 🤷‍♂️

gsteel · 2025-11-25T14:07:33Z

@Jurj-Bogdan Perhaps it would be a good idea to bring this up as a topic for the next TSC meet.

1 - Is the CSRF validator and Form element relevant any more? Can they both be deprecated and removed?
2 - If not, should we be making a new library specifically for laminas-csrf to decouple session from validator and form from session

For the purposes of this patch, why don't we:

Keep the existing naming (we can rename in a later patch depending on the outcome of the TSC meet)
Retain the CSRF validator and leave it unchanged here (Awaiting TSC meet)

froschdesign · 2025-11-25T14:27:36Z

1 - Is the CSRF validator and Form element relevant any more? Can they both be deprecated and removed?

The current usage of CSRF token generation and validation via laminas-form is easy:

$form->add([
    'type' => Laminas\Form\Element\Csrf::class,
    'name' => 'example',
]);

Nothing else needs to be done. So this is very user-friendly.
I am in favour of decoupling, but user-friendliness must not suffer as a result. We already have this problem with mezzio-flash.

gsteel · 2025-11-25T15:14:21Z

Nothing else needs to be done. So this is very user-friendly.

Well, yes and no. Currently form is dependent on validator@v2 which ships the (deprecated) CSRF validator.

When validator is upgraded to v3 in form, the future of the CSRF element will be scrutinised because it automatically attaches the CSRF validator via $element->getInputSpecification() and it will need to depend on session to maintain the functionality of the csrf element.

For people using mezzio with form, all of this is irrelevant - they won't be using the csrf element, nor laminas-session, they'll be using mezzio-session and mezzio-csrf, so we'd be asking mezzio users to install laminas-session for functionality they don't want or need.

Should we be dogmatic about soft dependencies? I prefer to be, mainly because it places clear constraints on installable versions - with soft deps, we could end up with session:2 being installed with form:4 (This wouldn't happen because of validator:2 constraints, but, the point remains)

froschdesign · 2025-11-25T15:48:52Z

@gsteel
The technical details and dependencies are all clear, and I also do not want to have the laminas-session component in a Mezzio application. I am only concerned with user-friendliness! And mezzio-csrf simply is not user-friendly. Also mezzio-flash.
We can throw away the old elements, but we need replacements that are cleanly implemented and easy to use. So I do not want to stick to the current structures, just keep the user in mind.

Jurj-Bogdan · 2025-11-30T10:32:55Z

While a decision is taken for the Csrf validator, i've gone through some of the requested changes. A couple of things:

removed all unnecessary try-catch blocks and left phpunit to fail on the thrown exception where needed
- the exception for this are the php <8.4 and >= 8.4 tests in IdTest, where the datasets specify if the validator should fail or not
added a new factory for the RemoteAddr validator so that it gets its options array passed, but I don't seem to find a required structure for its config - for now i've set it to config.session_manager.options based on test sequences:

session_manager.validators
session_manager.options

This might be better of reorganised, such as session_manager.validators.{validator_name}.options maybe?

gsteel · 2025-12-05T15:53:56Z

Note https://github.com/laminas/technical-steering-committee/blob/main/meetings/minutes/2025-12-01-TSC-Minutes.md#discussion-around-v3-of-laminas-session

In a separate patch, we should delete anything to do with laminas-form or laminas-validator 👍

arhimede · 2025-12-05T16:01:49Z

Note https://github.com/laminas/technical-steering-committee/blob/main/meetings/minutes/2025-12-01-TSC-Minutes.md#discussion-around-v3-of-laminas-session

In a separate patch, we should delete anything to do with laminas-form or laminas-validator 👍

can you open an issue ? to remember later

gsteel

Sorry @Jurj-Bogdan! More nit-picks :)

src/Service/RemoteAddressFactory.php

src/SessionManager.php

test/Service/SessionManagerFactoryTest.php

…rom events Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

…s-strings Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

gsteel

Just a few tweaks to

Reduce baseline additions
Make factories internal to reduce the BC surface

Otherwise 👍

src/Service/RemoteAddressFactory.php

src/Service/SessionManagerFactory.php

test/SessionManagerTest.php

test/Validator/IdTest.php

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

src/Service/RemoteAddressFactory.php

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

gsteel

Nearly there!!!

src/Service/RemoteAddressFactory.php

src/Service/SessionManagerFactory.php

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

gsteel

Phew! Thanks @Jurj-Bogdan LGTM!

@froschdesign Anything to add?

Ocramius

From my PoV, this is a good/stateless design: I provided a lot of feedback on things to improve though, sorry :D

Ocramius · 2026-01-23T15:07:56Z

docs/book/v3/manager.md

+            'classes' => [
+                Session\Validator\RemoteAddr::class,
+                Session\Validator\HttpUserAgent::class,
+            ],
+            'options' => [
+                'remote_addr' => [
+                    'use_proxy' => false,
+                    'trusted_proxies' => [],
+                ],
+            ]


This type is in the docs, but I don't directly see it reflected in any @psalm-type or array shape in this patch?

Ocramius · 2026-01-23T15:10:09Z

src/Service/SessionManagerFactory.php

-     * last, however, does not, due to the differences in implementations, and
-     * the fact that save handlers will often be written in userland. As such
+     * The first two have corresponding factories inside this namespace.
+     * "SaveHandlerInterface", however, does not, due to the differences in implementations,


Instead of a quoted string, use {@see ...}?

Ocramius · 2026-01-23T15:13:02Z

src/Service/SessionManagerFactory.php

+                /** @var list<class-string<ValidatorInterface>> $validators */
+                $validators = $validatorsConfig['classes'];


This type is the one I flagged in the docs: kinda missing anywhere?

Ocramius · 2026-01-23T15:32:34Z

src/Service/SessionManagerFactory.php

+        /** @psalm-var list<class-string<ValidatorInterface>> $uniqueValidators */
+        $uniqueValidators   = array_unique($validators);


Do we even care about uniqueness? What's the disadvantage if more than one is instantiated on accident?

Ocramius · 2026-01-23T15:34:24Z

src/Validator/Id.php

+
        if ($sessionId === null) {
-            return false;
+            throw new SessionValidationFailedException('Session id validation failed');


We should explain what failed (missing session ID)

Ocramius · 2026-01-23T15:34:47Z

src/Validator/Id.php

-    {
-        return self::class;
+        if (! (bool) preg_match($pattern, $sessionId)) {
+            throw new SessionValidationFailedException('Session id validation failed');


We should explain what failed (missing pattern mismatch - the ID and pattern should probably be part of the exception)

Ocramius · 2026-01-23T15:35:04Z

src/Validator/HttpUserAgent.php

    {
-        return self::class;
+        if ($initial->getUserAgent() !== $current->getUserAgent()) {
+            throw new SessionValidationFailedException('Http user agent validation failed');


The mismatching values should be part of the exception

Ocramius · 2026-01-23T15:35:32Z

src/Validator/RemoteAddr.php

+        }
+
+        if ($this->initialData !== $this->currentData) {
+            throw new SessionValidationFailedException('Remote address validation failed');


We should add the before/after values to the exception

Ocramius · 2026-01-23T15:36:07Z

src/Validator/ValidatorInterface.php

+    /** @throws SessionValidationFailedException */
+    public function validate(EnvironmentInterface $initial, EnvironmentInterface $current): void;


This is obviously a major breaking change, but a welcome one 👍

gsteel requested changes Nov 25, 2025

View reviewed changes

gsteel requested changes Dec 5, 2025

View reviewed changes

Jurj-Bogdan added 3 commits December 12, 2025 11:03

Removed laminas-eventmanager dependency, refactored validators away f…

e2dd903

…rom events Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

SessionManager to accept ValidatorInterface instances instead of clas…

5569e03

…s-strings Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

requested changes, minor documentation updates

f4acb7f

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

Jurj-Bogdan force-pushed the validator-chain-refactor branch from 4df2c90 to f4acb7f Compare December 12, 2025 13:38

Jurj-Bogdan requested a review from gsteel December 16, 2025 11:55

gsteel requested changes Dec 16, 2025

View reviewed changes

gsteel added this to the 3.0.0 milestone Dec 16, 2025

gsteel added BC Break Enhancement Dependencies Updates and changes to dependencies labels Dec 16, 2025

reduce psalm baseline

f5eb281

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

froschdesign reviewed Jan 9, 2026

View reviewed changes

src/Service/RemoteAddressFactory.php Outdated Show resolved Hide resolved

update validator config structure

3994152

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

gsteel requested changes Jan 23, 2026

View reviewed changes

src/Service/RemoteAddressFactory.php Outdated Show resolved Hide resolved

src/Service/RemoteAddressFactory.php Outdated Show resolved Hide resolved

src/Service/SessionManagerFactory.php Outdated Show resolved Hide resolved

removed unnecessary exception. other requested changes

be14b9d

Signed-off-by: Jurj-Bogdan <jurj@dotkernel.com>

gsteel approved these changes Jan 23, 2026

View reviewed changes

froschdesign approved these changes Jan 23, 2026

View reviewed changes

gsteel self-assigned this Jan 23, 2026

gsteel merged commit abfbac5 into laminas:3.0.x Jan 23, 2026
16 of 17 checks passed

Ocramius reviewed Jan 23, 2026

View reviewed changes

arhimede mentioned this pull request Jan 27, 2026

[RFC]: Remove laminas/laminas-eventmanager dependency #167

Closed

Jurj-Bogdan mentioned this pull request Feb 10, 2026

Remove laminas/service-manager dependency and factory stubs #131

Draft

		/** @var list<class-string<ValidatorInterface>> $validators */
		$validators = $validatorsConfig['classes'];

		/** @psalm-var list<class-string<ValidatorInterface>> $uniqueValidators */
		$uniqueValidators = array_unique($validators);

		/** @throws SessionValidationFailedException */
		public function validate(EnvironmentInterface $initial, EnvironmentInterface $current): void;

Uh oh!

Conversation

Jurj-Bogdan commented Nov 22, 2025

Description

Uh oh!

arhimede commented Nov 25, 2025

Uh oh!

froschdesign commented Nov 25, 2025

Uh oh!

gsteel commented Nov 25, 2025

Uh oh!

Jurj-Bogdan commented Nov 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gsteel commented Nov 25, 2025

Uh oh!

froschdesign commented Nov 25, 2025

Uh oh!

gsteel commented Nov 25, 2025

Uh oh!

gsteel commented Nov 25, 2025

Uh oh!

froschdesign commented Nov 25, 2025

Uh oh!

gsteel commented Nov 25, 2025

Uh oh!

froschdesign commented Nov 25, 2025

Uh oh!

Jurj-Bogdan commented Nov 30, 2025

Uh oh!

gsteel commented Dec 5, 2025

Uh oh!

arhimede commented Dec 5, 2025

Uh oh!

gsteel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gsteel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gsteel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gsteel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Ocramius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment