Skip to content

fix(registry): don't re-register default bindings on a provided injector - #247

Open
jcdelepine wants to merge 1 commit into
horde:FRAMEWORK_6_0from
jcdelepine:fix/registry-keep-bootstrap-injector
Open

jcdelepine wants to merge 1 commit into
horde:FRAMEWORK_6_0from
jcdelepine:fix/registry-keep-bootstrap-injector

Conversation

@jcdelepine

@jcdelepine jcdelepine commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

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, as discussed in fix(registry): don't clobber $GLOBALS['injector'] on modern requests #227.

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.

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
jcdelepine force-pushed the fix/registry-keep-bootstrap-injector branch from fc7eb85 to 1790449 Compare October 23, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant