Skip to content

refactor(http): deprecate HeaderSessionIdResolver - #2280

Open
osbre wants to merge 1 commit into
tempestphp:3.xfrom
osbre:deprecate/header-session-id-resolver
Open

refactor(http): deprecate HeaderSessionIdResolver#2280
osbre wants to merge 1 commit into
tempestphp:3.xfrom
osbre:deprecate/header-session-id-resolver

Conversation

@osbre

@osbre osbre commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Deprecates HeaderSessionIdResolver, scheduled for removal in 4.0.

Why

  • Unused: Not wired up by any initializer or selectable via configuration.
  • Never a default: Replaced by CookieSessionIdResolver prior to the first tagged release (v1.0.0-alpha.1).
  • Undocumented: Absent from all in-repo and official documentation.
  • Header name never matches: It looks up an underscore header name (*_session_id), but request headers are normalized to hyphens by our own request layer (Laminas ServerRequestFactory, after the SAPI collapses both forms to HTTP_..._..._), so the lookup can't match a client-supplied identifier. This is the unresolved half of Broken HeaderSessionIdResolver #1892; fix(http): broken import in header session id resolver #1893 fixed the broken import but not the name mismatch.

Given the above, deprecating and later removing it seems cleaner than investing further in fixing it. Happy to adjust if there's an intended use case I've missed.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Benchmark Results

Comparison of deprecate/header-session-id-resolver against 3.x (16a1a2a6db94d90ca803ae4442520b880813d4e5).

Open to see the benchmark results
Benchmark Set Mem. Peak Time Variability
ViewRenderBench(benchPlainHtml) - 22.035mb 0.00% 491.177μs -6.19% ±3.27% +51.93%
DiscoveryScanBench(benchFullDiscoveryScan) - 25.859mb +0.00% 18.721ms +6.09% ±1.93% -2.05%

Generated by phpbench against commit 78e317e

@brendt

brendt commented Sep 7, 2026

Copy link
Copy Markdown
Member

I agree that it might be broken at the moment, but isn't this important for async request? That being said, you could use cookies there as well. I actually don't remember why we ever added support for it. @innocenzi do you perhaps know?

@innocenzi

Copy link
Copy Markdown
Member

Hey @brendt, this existed before I touched the session implementation. Honestly unsure about its use case, but you must have had one if you introduced it. Or maybe it was just an easy way of authenticating prior to having cookie-based auth?

I'm fine with removing it unless we actually have a use case that justify its presence

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.

3 participants