Skip to content

[NFR]: HTTP Request/Response rework #17054

Description

@niden

Result of the #12485, bug reports, inflexibility and personal frustration.

Rewrite Request and Response as facade + typed bags + lazy concerns

Problem

  • Phalcon\Http\Request is a ~2200-line god-class; Phalcon\Http\Response is ~850 lines. Both mix transport, parsing, IP/proxy resolution, content negotiation, body handling, cookies, events, DI wiring, and BC affordances on a single surface.
  • Hard to test, hard to extend, hard to reason about. Logic for headers, query, body, files, auth, accept, and client IP all lives on Request directly.
  • Request re-scans $_SERVER on every header/var lookup; no caching of normalized header keys; $_SERVER mutations leak into request state because nothing is captured.
  • Inconsistent key conventions: HTTP-KEY - HTTP_KEY conversion done lazily and inconsistently between "get one" and "get all".
  • No default-value parameter on any of the readers - callers write the same ?? 'fallback' guard at every call site.
  • Filtering is wired through stringly-typed names (getQuery('id', 'int')) - typos like 'innt' fail silently or at runtime.
  • Phalcon\Http\Response\Cookies couples encrypted/signed cookies + session restore with the response facade.
  • setFileToSend() lives directly on the response - no separation between "what the body is" and "how it gets sent".

Why rewrite vs fix

  • The architecture is the bug. The complaints in [NFR]: Rework the Phalcon HTTP stack #12485 (no state, no caching, inconsistent keys, no defaults, can't mock sub-pieces) aren't surface bugs - they're consequences of a single class doing twelve jobs. Patching them in place means adding more state and more code paths to a class that already can't be held in working memory. The class would grow to ~2500 lines doing the same twelve jobs, only worse-coupled.
  • Encapsulation can't be retrofitted. Fixing "_any code can mutate $SERVER and break the request" requires changing when state is captured (at construction, not on demand). That's a structural change, not a bugfix. You can't add encapsulation as a method.
  • Performance fixes need state. Memoizing Authorization::resolveHeaders(), caching normalized header keys, reading php://input once - every one of these is a "stash a result on the object" change. Adding eight more caches to Request is exactly the bloat the rewrite eliminates. The rewrite gets these for free as a side effect of decomposition.
  • Testing surface is unworkable today. Mocking Request means mocking superglobals or stubbing 2200 lines. The decomposition produces ServerBag, HeaderBag, Upload, Client, Inspector etc. - each independently constructable and testable in a few lines.
  • Each fix doubles the surface. Patching the existing class adds new code paths without retiring old ones (typed readers next to untyped getters, cached lookups next to live ones). A rewrite collapses both into a single tested implementation. Long-term maintenance cost of patching > cost of rewriting.
  • No BC cost. The decisive point. The rewrite ships behind the same public API. The downside that usually kills rewrites (breaking users) isn't present here. The only risk is regressions in the rewrite, which the test suite catches and which is the same risk that patching would carry.
  • Recurring bugs originate in the same coupling. The request object handles auth headers incorrectly #12480 (HTTP auth), [NFR]: Rework the Phalcon HTTP stack #12485 (HTTP rework), and the various trusted-proxy / IP-resolution tickets are all symptoms of one root cause: too much logic in one place. Isolating each concern surfaces the contract explicitly and lets fixes land in focused units instead of touching the god-class.
  • Future-proofing is cheaper from clean code. Property hooks, readonly classes, native enums, middleware, async runtimes — every one of these is easier to adopt against bags + lazy concerns than against a 2200-line monolith.

Implementation

  • Facade + bags + lazy concerns. Request becomes a ~200-line facade holding typed bags (ServerBag, HeaderBag, QueryBag, InputBag, CookieBag, FileBag) and lazily-hydrated specialized objects (Accept, Authorization, Client, Inspector). Same for Response (~250 lines).
  • Raw arrays in bags. Bags use protected array $items with get/has/set/remove/all + typed readers
    getInt/getString/... + ArrayAccess/Countable/IteratorAggregate. Filtering is not a bag concern - domain layer.
  • Phalcon\Http\Contracts\* interfaces (MutableUriStruct, UploadStruct, ResponseStruct, ResponseHeadersCollection, ResponseSenderService, ResponseBodyHandler, ResponseBodySenderService) mirroring upstream interop contracts (star interop)
  • Direct property access. Request exposes bags as public readonly properties - no __get, no per-access method overhead. Lazy concerns sit behind getAccept()/getAuthorization()/getClient()/getInspector() so they don't construct at request boot.
  • Mutability preserved. Both Request and Response stay mutable to match current semantics and middleware needs.
  • Performance. php://input read lazily on first access. Authorization::resolveHeaders() memoized. HeaderBag::newFromServer() walks $_SERVER once. Empty UploadCollection is a shared singleton. BC delegation in the facade is flat.
  • Full BC.

Metadata

Metadata

Assignees

Labels

Projects

Status
Backlog

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions