Repository navigation
fix(registry): don't re-register default bindings on a provided injector - #247
Open
jcdelepine wants to merge 1 commit into
Open
jcdelepine wants to merge 1 commit into
jcdelepine wants to merge 1 commit into
Conversation
Horde_Registry::__construct() unconditionally runs DefaultInjectorBindings::register() on the injector it ends up with. That is harmless when the registry creates the injector itself, but on the modern stack an injector is already provided and may already hold resolved instances. Registering the defaults again replaces their binders, so the next lookup builds a fresh object. Observed on a route with a minimal middleware stack (/auth/login: EstablishHordeSession, AuthHordeSession, no HordeCore). AuthHordeSession needs a Horde_Registry, which the container auto-wires. The constructor then creates a Horde_Session shim and calls setup(), which resolves SessionLifecycle through $GLOBALS['injector']. That produced a second SessionLifecycle (setupApplied=false) while the native session was already active, so PHP emitted: session_set_cookie_params(): Session cookie parameters cannot be changed when a session is active and the same warning for session_cache_limiter(), session_name() and session_set_save_handler(). Two changes: - Register the default bindings only on an injector the registry created itself. A provided injector was bound by whoever provided it; RampageBootstrap registers them before any middleware runs. - Bind Horde_Registry to a small factory that seeds $GLOBALS['injector'] with the container's injector when it is unset, so a registry built from DI shares it instead of minting a disconnected one. This keeps RampageBootstrap free of globals, in line with the goal stated in horde#227. The legacy path is unchanged: with $GLOBALS['injector'] unset the registry still creates and registers its own injector.
jcdelepine
force-pushed
the
fix/registry-keep-bootstrap-injector
branch
from
October 23, 2026 08:04
fc7eb85 to
1790449
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Horde_Registry::__construct() unconditionally runs DefaultInjectorBindings::register() on the injector it ends up with. That is harmless when the registry creates the injector itself, but on the modern stack an injector is already provided and may already hold resolved instances. Registering the defaults again replaces their binders, so the next lookup builds a fresh object.
Observed on a route with a minimal middleware stack (/auth/login: EstablishHordeSession, AuthHordeSession, no HordeCore). AuthHordeSession needs a Horde_Registry, which the container auto-wires. The constructor then creates a Horde_Session shim and calls setup(), which resolves SessionLifecycle through $GLOBALS['injector']. That produced a second SessionLifecycle (setupApplied=false) while the native session was already active, so PHP emitted:
session_set_cookie_params(): Session cookie parameters cannot be
changed when a session is active
and the same warning for session_cache_limiter(), session_name() and session_set_save_handler().
Two changes:
The legacy path is unchanged: with $GLOBALS['injector'] unset the registry still creates and registers its own injector.
Impact: after an OIDC back-channel logout, the idle browser's next request
ended with the four session_* warnings, the user was logged out with an error
and could not get back in without deleting the session cookies. With this
change the back-channel logout completes cleanly.
Testing: manual, on a Horde 6 dev instance with OIDC login through CAS. And login/logout on a horde6 dev instance with ldap auth.
The shadowing explanation is inferred from the traces (two SessionLifecycle
instances in one request, obj=241 from EstablishHordeSession and obj=501 from
Horde_Registry->__construct) and from the symptoms disappearing. Corrections
welcome. Happy to use a lazy proxy instead of the factory if you prefer.