From 105b4408b36c9ee178a8237762259d981b4b9e62 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:42:47 +0000 Subject: [PATCH 01/16] docs(session): plan lifecycle and persistence audit Record the settled Session design, upstream research, owner decisions, rejected alternatives, implementation boundaries, and complete regression matrix. Keep the package-specific recovery and anti-overengineering rules beside the implementation plan so long-running work can restore its full context without rereading the framework-wide audit history. --- ...-persistence-and-current-laravel-parity.md | 1197 +++++++++++++++++ 1 file changed, 1197 insertions(+) create mode 100644 docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md diff --git a/docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md b/docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md new file mode 100644 index 000000000..6c795418b --- /dev/null +++ b/docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md @@ -0,0 +1,1197 @@ +# Complete Session Lifecycles, Persistence, and Current Laravel Parity + +## Status + +Pre-implementation audit and second-opinion consensus are complete. This plan +consolidates the settled design, including the owner-approved Hypervel default +Redis session prefix. The owner approved the public/configuration gates and +test return-type scope. Plan review, implementation, validation, fresh +self-review, and post-implementation code review are complete. + +## Scope + +Complete the `session` package audit as one coherent work unit, including the +lowest owning Foundation, Filesystem, Contracts, Support facade, configuration, +metadata, testing, and documentation boundaries required by the verified +findings. + +The final implementation must: + +- isolate mutable `Store` state between Store objects and coroutines; +- marshal decoded JSON error bags before merging storage into live state; +- publish flash aging and JSON persistence state only after storage commits; +- never save a session whose startup did not commit; +- preserve the primary request failure when an after-response save retry also + fails; +- delegate lock cleanup to Cache's established callback boundary; +- make cookie-backed sessions binary-safe and strictly validate their envelope; +- correct file and database handler state and write behavior without adding + retries or new I/O; +- adopt current supported Laravel Session behavior and application defaults; +- declare every supported Session configuration key at its single owner; +- give Redis sessions a dedicated application-scoped key namespace by default; +- remove dead bindings, unsafe unused APIs, stale suppressions, and stale docs; +- make split-package dependencies complete and truthful; and +- preserve Hypervel's cloned cache-store isolation, Redis pooling, guard-scoped + password confirmation, and other intentional Session behavior. + +This is not a redesign around per-request Store clones, a context registry, +serializer service, persistence transaction object, retry loop, or new session +driver abstraction. + +## Post-compaction recovery and anti-overengineering rules + +After compaction, read `AGENTS.md` and this plan in full before resuming. This +plan carries the relevant anti-overengineering rules so the framework-wide +audit plan does not need to be reread during implementation. + +- Require a supported, realistic path and meaningful harm before adding a fix. + Merely conceivable states do not justify machinery. +- Prefer existing Laravel or Hypervel APIs and the lowest owner. Do not + duplicate Cache lock cleanup, Filesystem behavior, config resolution, or + coroutine-context lifecycle in Session. +- Add no registry, WeakMap, counter, mutex, state machine, retry loop, + compatibility decoder, configurable policy, or abstraction unless a verified + requirement below cannot be completed without it. +- Do not add defensive guards where native types already fail correctly. The + three checks in this plan are justified exceptions: invalid serialization + silently selects PHP, a false handler write is an explicit persistence + failure, and a non-Redis session store currently fails with an opaque + undefined-method error. +- Do not make source more complex to satisfy PHPStan. Use truthful types and + local narrowing; retain a scoped ignore only where correct magic proxying + cannot be modeled. +- Do not preserve stale code because it works or because removing it causes + churn. Backward compatibility for flawed Hypervel-only internals is not a + goal. Current Laravel public APIs and conventional extension points remain a + goal unless a documented Swoole requirement demands a difference. +- Do not add enforcement for deliberate framework escape hatches or unsupported + misuse. In particular, user replacement of the reserved `errors` session key + does not warrant a second error-bag type path. +- Hot paths must not gain container resolution, locks, hashing, I/O, retries, + logging, yields, or retained worker state. Any newly discovered meaningful + cost must return to owner review before implementation. +- Avoiding overengineering never permits an incomplete fix. Every verified + failure must be closed at its real owner, with stale comments/tests removed. + +## Fixed architecture and research + +### Runtime ownership + +| Surface | Final owner and lifetime | +|---|---| +| Named Session drivers | Worker-cached by `SessionManager` | +| Active request Store | Fixed `Store::CONTEXT_KEY`, one value per coroutine | +| Store ID, attributes, and started flag | Per Store object and per coroutine | +| Handler request/existence state | Object-ID-derived context keys; Database existence resets at construction and cloning | +| Cache-backed handler | Isolated cloned `Repository` with a deep-cloned store | +| Durable HTTP-test state | Explicit child-to-parent synchronization of the active Store only | +| Session persistence | `Store::save()` publishes live state only after a successful handler write | +| Blocked request lock | Cache `Lock::block(..., $callback)` | +| Redis connection and store prefix | Applied once to the cloned Redis cache store | +| Application serialization default | Framework config (`json`) | +| Direct `new Store(...)` default | Constructor default (`php`), matching Laravel | + +`Repository::__clone()` clones its underlying store. Redis session connection +and prefix changes therefore affect only the handler's clone; they never mutate +the shared Cache manager repository or Redis store. + +### Upstream references + +The implementation reference is the current local Laravel Framework checkout +at `examples/laravel/framework`, commit +`23e9e71f382b91510c70b5b6f9ae0776f1b88e12`. The current application-config +reference is `examples/laravel/laravel`, commit +`2eb457783ee0e1f034612c2fae690924532d4ca4`. + +Historical commits are discovery evidence only: + +| Change | Discovery commit | Complete introduced surface | +|---|---|---| +| Redis session prefix | `3093ff3a61` / Laravel PR #60700 | `SessionManager`, `SessionManagerTest` | +| Collection short-circuit sync | `dd3a9225c1` / Laravel PR #60745 | `Store` only for Hypervel | +| JSON default for new applications | `75cef503c6edc3447dd79053a648ea981857e15b` | application `config/session.php` only | + +Port current source and tests, not historical diff text. Hypervel intentionally +adapts the Redis prefix check to preserve `"0"`, retains its cloned store and +default `session` Redis connection, and ships a dedicated prefix default: + +```php +'prefix' => env('SESSION_PREFIX', app_id() . '_session:'), +``` + +Laravel's current application config deliberately uses the literal JSON +default, not an environment variable: + +```php +'serialization' => 'json', +``` + +Keep it literal. Serialization-strategy drift between deployments can silently +read an existing session as empty; applications needing PHP object sessions +must make that code-owned compatibility decision in configuration. + +## Finding summary + +| ID | Category | Severity | Verified failure or gap | Final boundary | +|---|---|---:|---|---| +| `session-01` | Defect | Major | Fixed Store context keys make two Store objects share ID, attributes, and started state | Precomputed object-specific context keys | +| `session-02` | Defect and upstream defect | Major | Failed or false writes age flash state or replace the live error bag before commit; repeated JSON save crashes | Two local snapshots and post-write publication | +| `session-03` | Defect | Major | Failed startup registers a later empty write against the cookie-derived ID | `Request::hasSession()` commit flag | +| `session-04` | Defect | Major | Manual lock release can replace request failure or run after failed acquire | Cache lock callback form | +| `session-05` | Defect and upstream defect | Major | JSON cookie envelope rejects binary serialized data and accepts invalid decoded shapes | Private PHP-serialized envelope | +| `session-06` | Defect | Minor | File GC passes `string|false` and counts failed deletes | Finder pathname and successful-delete count | +| `session-07` | Defect | Minor | Direct database write caches stale false existence after `read()` updates context | Refresh local existence once | +| `session-08` | Current Laravel parity and configuration improvement | Minor | Redis sessions cannot own a distinct prefix | Truthful RedisStore setup and declared default | +| `session-09` | Current Laravel parity | Improvement | Store collection checks lag current upstream and `hasAny()` scans all keys | Current `doesntContain()` / `contains()` shape | +| `session-10` | Configuration and security defect | Major | Supported keys are undeclared, defaults are duplicated, and invalid serialization silently selects PHP | Canonical config plus constructor validation | +| `session-11` | Dead-code cleanup | Improvement | Redundant `StartSession` singleton, empty provider boot method, and unsupported-driver cache wrapper remain | Delete all three | +| `session-12` | Userland footgun and API cleanup | Minor | Unused Hypervel-only `setConnection()` mutates a worker-cached handler | Remove it; document retained upstream mutator | +| `session-13` | Metadata defect | Minor | Split package omits direct runtime dependencies | Complete manifest and metadata regression | +| `session-14` | Documentation defect and upstream defect | Minor | Custom-driver GC describes seconds as a Unix timestamp | Correct public wording | +| `session-15` | Contract defect | Major | Nullable Store IDs violate Symfony, handler, guard, and Laravel string boundaries | Lazy non-null `getId()` | +| `session-16` | Type-consistency improvement | Improvement | Array/Null handler signatures lag their four typed Hypervel siblings | Complete native interface types | +| `session-17` | Static-analysis maintenance defect | Minor | Nine unmatched ignores can hide future real errors | Delete only stale suppressions | +| `session-18` | Intentional runtime difference | Minor | Array sessions are worker-local and unsuitable for production | Concise task-oriented documentation | +| `session-19` | Defect | Major | A failed after-response save retry escapes the exception renderer and replaces the primary request failure | Contain only the retry failure | +| `session-20` | Defect and upstream defect | Major | Starting a JSON Store can replace an already-live validation error bag with an empty one | Marshal only the decoded storage payload before merging | +| `session-21` | Defect | Major | A reused Database handler object ID can inherit `exists=true`, update zero rows, and report a silently lost write | Initialize object-specific state on construction and cloning | +| `session-22` | Defect | Major | The file handler reports success after false or partial filesystem writes | Require the complete byte count | +| `filesystem-12` | Type-consistency improvement | Improvement | Concrete `Filesystem::delete()` alone omits its contract's native union | Add `array|string` | + +## Owner decisions + +The owner approved these public, configuration, and Improvement-category +decisions: + +- the declared blocking keys and current Laravel JSON application default; +- the non-null `getId()` contract; +- removal of the unused Hypervel-only database-handler mutator; +- conversion of false handler writes into a persistence exception; +- containment of an after-response save retry failure so the primary request + failure remains renderable; +- removal of the dead protected `createCacheBased()` wrapper; +- current Store collection synchronization; +- redundant provider cleanup; +- Array/Null handler native types; +- concrete Filesystem type convergence; +- concise array-driver runtime documentation; +- Database handler context initialization and complete file-write validation; + and +- bounded metadata, documentation, and stale-suppression cleanup found during + code review. + +The owner approved adding `: void` to all 89 existing test methods across the +eight affected Session test files, as well as every new test method. + +## 1. Isolate each Store's coroutine state + +### Store keys and construction + +In `src/session/src/Store.php`, keep the fixed active-request key and replace the +three state keys with prefixes: + +```php +public const CONTEXT_KEY = '__session.store'; +public const STARTED_CONTEXT_KEY_PREFIX = '__session.store.started.'; +public const ATTRIBUTES_CONTEXT_KEY_PREFIX = '__session.store.attributes.'; +public const ID_CONTEXT_KEY_PREFIX = '__session.store.id.'; + +protected readonly string $startedContextKey; +protected readonly string $attributesContextKey; +protected readonly string $idContextKey; +``` + +Validate serialization first, then derive and initialize every key before +calling `setId()`. Constructor initialization is required because PHP may reuse +an object ID after `Manager::forgetDrivers()` frees a Store in the same +coroutine: + +```php +protected const SUPPORTED_SERIALIZATIONS = ['json', 'php']; + +public function __construct( + protected string $name, + protected SessionHandlerInterface $handler, + ?string $id = null, + protected string $serialization = 'php' +) { + if (! in_array($serialization, self::SUPPORTED_SERIALIZATIONS, true)) { + throw new InvalidArgumentException(sprintf( + 'Session serialization [%s] is not supported. Supported: "%s".', + $serialization, + implode('", "', self::SUPPORTED_SERIALIZATIONS), + )); + } + + $suffix = (string) spl_object_id($this); + + $this->startedContextKey = self::STARTED_CONTEXT_KEY_PREFIX . $suffix; + $this->attributesContextKey = self::ATTRIBUTES_CONTEXT_KEY_PREFIX . $suffix; + $this->idContextKey = self::ID_CONTEXT_KEY_PREFIX . $suffix; + + CoroutineContext::set($this->startedContextKey, false); + CoroutineContext::set($this->attributesContextKey, []); + + $this->setId($id); +} +``` + +The constant stays untyped to match every existing constant in `Store.php`. +Tests assert the public exception, not the protected constant. The validation +must precede all context writes so rejected construction leaves no orphan +slots. + +Route all Store state methods through the precomputed properties: + +```php +protected function getAttributes(): array +{ + return CoroutineContext::get($this->attributesContextKey, []); +} + +public function isStarted(): bool +{ + return CoroutineContext::get($this->startedContextKey, false); +} + +public function setId(?string $id): void +{ + CoroutineContext::set( + $this->idContextKey, + $this->isValidId($id) ? $id : $this->generateSessionId() + ); +} +``` + +Do not add clone handling. There is no supported Store clone consumer, and +manager-per-request cloning would bypass the established cached-driver model. + +### Non-null ID contract + +Restore Laravel's truthful string contract in: + +- `Hypervel\Contracts\Session\Session::getId()`; +- `Store::getId()` and `Store::id()`; and +- `Hypervel\Support\Facades\Session` metadata. + +A manager-cached Store may first be used in a coroutine other than its +construction coroutine, so `getId()` lazily creates the missing value: + +```php +public function getId(): string +{ + /** @var string|null $id */ + $id = CoroutineContext::get($this->idContextKey); + + if ($id === null) { + $id = $this->generateSessionId(); + CoroutineContext::set($this->idContextKey, $id); + } + + return $id; +} +``` + +This is one normal context read and an exceptional first-use write. Delete the +now-dead nullable-ID workaround from Foundation's +`Testing\Concerns\InteractsWithSession::startSession()`. + +The lazy ID also means an unsupported `save()` on a manager-cached Store in a +different coroutine, without first assigning or starting its request session, +writes an empty session under a generated ID instead of reaching a nullable-ID +type failure. Add no guard for that stray path. + +### Foundation test-context bridge + +`MakesHttpRequests` must synchronize the active Store's object-specific keys. +Use one testing-local helper shared by snapshot creation and key enumeration: + +```php +protected function sessionStoreContextKeys(SessionStore $session): array +{ + $suffix = (string) spl_object_id($session); + + return [ + SessionStore::STARTED_CONTEXT_KEY_PREFIX . $suffix, + SessionStore::ID_CONTEXT_KEY_PREFIX . $suffix, + SessionStore::ATTRIBUTES_CONTEXT_KEY_PREFIX . $suffix, + ]; +} +``` + +The snapshot maps those keys to `isStarted()`, `getId()`, and `all()`. The full +sync-key list contains `Store::CONTEXT_KEY` plus the dynamic keys derived from +the active Store still present in the child context: + +```php +protected function sessionContextKeys(): array +{ + $keys = [SessionStore::CONTEXT_KEY]; + $session = CoroutineContext::get(SessionStore::CONTEXT_KEY); + + if ($session instanceof SessionStore) { + array_push($keys, ...$this->sessionStoreContextKeys($session)); + } + + return $keys; +} +``` + +The derivation must run eagerly inside the waiter child: that child's copied +context still contains the prior active Store even when the completed request +has no session. `RequestContextSynchronizer` then removes all four absent values +from the parent. Array ordering is not significant because both method +arguments are evaluated before parent synchronization begins. Do not use a +generator, scan the entire context, or retain every discarded Store identity. + +Keep one concise WHY comment at that derivation site: it must read the child's +copied active Store even when the completed request has no session. + +### Regressions + +- Two Store objects in one coroutine retain independent IDs, attributes, and + started flags. +- Destroying/forgetting one Store and constructing another with a reused object + ID starts with empty attributes and `started=false`. +- A Store constructed outside a request coroutine lazily creates a valid ID in + the request coroutine. +- Model object-ID reuse deterministically by seeding all three stale slots in + an anonymous Store subclass immediately before its parent constructor. +- HTTP-test flash/session state still synchronizes to the parent. +- A following request without a session clears the fixed active Store and its + three dynamic slots. + +## 2. Make persistence transactional and serialization explicit + +### Pure storage error-bag marshalling + +Marshal only the decoded storage payload before merging it into live state: + +```php +protected function loadSession(): void +{ + // Marshal the decoded payload before merging: marshalling the merged result + // would iterate an already-live ViewErrorBag and replace it with an empty one. + $this->replaceAttributes($this->marshalErrorBagIn($this->readFromHandler())); +} + +protected function marshalErrorBagIn(array $attributes): array +{ + if ($this->serialization !== 'json' || ! array_key_exists('errors', $attributes)) { + return $attributes; + } + + $errorBag = new ViewErrorBag; + + foreach ($attributes['errors'] as $key => $value) { + $messageBag = new MessageBag($value['messages']); + + $errorBag->put($key, $messageBag->setFormat($value['format'])); + } + + $attributes['errors'] = $errorBag; + + return $attributes; +} +``` + +Use the truthful pure-transformation title, "Marshal the ViewErrorBag in the +given session attributes." Likewise, title +`prepareErrorBagForSerialization()` "Prepare the ViewErrorBag in the given +session attributes for JSON serialization." + +Current Laravel has the same defect: it merges decoded data into live +attributes and then marshals the merged value, so an already-live +`ViewErrorBag` is iterated as if it were a decoded array and replaced with an +empty bag. The pure boundary preserves a live bag when storage has no `errors` +key, while a persisted error bag correctly wins when storage supplies one. +This also removes repeated coroutine-context access from JSON error-bag +startup. + +### Pure flash aging + +Replace the mutating-only flash aging implementation with one pure array +transformation used by both public `ageFlashData()` and `save()`: + +```php +protected function ageFlashDataIn(array $attributes): array +{ + Arr::forget($attributes, Arr::get($attributes, '_flash.old', [])); + Arr::set($attributes, '_flash.old', Arr::get($attributes, '_flash.new', [])); + Arr::set($attributes, '_flash.new', []); + + return $attributes; +} + +public function ageFlashData(): void +{ + $this->setAttributes($this->ageFlashDataIn($this->getAttributes())); +} +``` + +Do not introduce a transaction object or rollback bookkeeping. + +### Two-snapshot save boundary + +`save()` owns: + +1. an aged live snapshot that retains `ViewErrorBag`; +2. a storage-only snapshot where JSON error bags become arrays; +3. serialization and a truthfully checked handler write; and +4. publication of the live snapshot only after the write succeeds. + +```php +public function save(): void +{ + // Publish the aged attributes only after the handler commits, so a failed + // write leaves the live flash data and error bag intact for the retry. + $attributes = $this->ageFlashDataIn($this->getAttributes()); + $attributesForStorage = $this->prepareErrorBagForSerialization($attributes); + + $serialized = $this->serialization === 'json' + ? json_encode($attributesForStorage, JSON_THROW_ON_ERROR) + : serialize($attributesForStorage); + + $written = $this->handler->write( + $this->getId(), + $this->prepareForStorage($serialized) + ); + + if ($written === false) { + throw new RuntimeException('Unable to write the session data.'); + } + + $this->setAttributes($attributes); + CoroutineContext::set($this->startedContextKey, false); +} +``` + +Make `prepareErrorBagForSerialization(array $attributes): array` pure. If the +strategy is not JSON or the top-level `errors` key is absent, return the input. +Otherwise, transform the reserved `ViewErrorBag` in the supplied copy and return +it. Do not add an `instanceof` guard for user misuse of the reserved key. + +`JSON_THROW_ON_ERROR` makes cyclic and unsupported values fail at the real +serialization boundary. A failed encoding, false handler write, or throwing +handler write leaves attributes, flash markers, error bag, and started state +untouched. False is an explicit `SessionHandlerInterface` failure reachable +through `CacheBasedSessionHandler`; converting it to a `RuntimeException` with +the new Session-owned message makes the response truthful and uses the +after-response retry path already used by throwing stores. Cache keeps its +existing `KeyWriteFailed` event; Session adds no event, logger, retry loop, or +exception class. + +`save()` deliberately calls the pure helper rather than the public mutating +`ageFlashData()`. A subclass that customized save-time aging by overriding that +public method must instead override the protected pure helper. Do not add a +compatibility hook that would reintroduce pre-commit mutation. + +### App default and direct-construction default + +Add Laravel's current Session Serialization config section verbatim in style, +with Hypervel naming: + +```php +'serialization' => 'json', +``` + +Keep the constructor defaults on `Store` and `EncryptedStore` as `'php'`. +`SessionManager` always supplies the application config, while direct +construction is a Laravel public/testing surface whose default remains PHP. +Do not remove the constructor default, add padding arguments to callers, or add +`SESSION_SERIALIZATION`. + +The constructor guard deliberately rejects third strategy strings. Such strings +were never a working extension point: the implementation has four exact JSON +checks and otherwise silently chooses PHP. The supported subclass hooks remain +`prepareForStorage()` and `prepareForUnserialize()`. Do not add an enum, +validator service, or serializer registry. + +### Regressions + +- Throwing PHP write leaves live flash data unchanged; retry writes it once + and ages it once. +- Throwing JSON write leaves a live `ViewErrorBag`; retry succeeds. +- False cache-backed write throws the persistence exception, leaves live state + unchanged, and a successful retry persists the flash exactly once. +- Two consecutive successful JSON saves with a live error bag succeed and keep + the live bag. +- Cyclic/unencodable JSON throws `JsonException` before handler write. +- Invalid serialization fails at construction with the rejected and supported + values in the message and leaves no per-Store context slots. +- Framework config defaults to JSON; an explicit application config override to + PHP constructs a PHP Store. +- Direct `new Store(...)` and `new EncryptedStore(...)` retain PHP defaults. +- Starting a JSON Store with a live error bag and no persisted errors retains + the exact live bag and its messages. +- Persisted JSON error arrays override and reconstruct correctly when a live + error bag is already present. +- The existing Foundation JSON-session assertion fixture uses an explicit JSON + Store and reloads the persisted payload into a second JSON Store over the + same handler and the first Store's ID before asserting the reconstructed + error bag. Do not retain its current misleading PHP-default construction and + save-only comment. + +## 3. Correct middleware commit and lock boundaries + +### Failed startup + +In `StartSession::handleStatefulRequest()`, register after-response persistence +only after the request owns a successfully started session: + +```php +} catch (Throwable $throwable) { + if ($request->hasSession()) { + $this->exceptionHandler->afterResponse(function () use ($request): void { + try { + $this->saveSession($request); + } catch (Throwable) { + // The request failure stays primary; a retry failure must not + // replace it or escape the exception renderer. + } + }); + } + + throw $throwable; +} +``` + +`setHypervelSession()` runs only after `start()` succeeds, making +`hasSession()` the existing commit flag. Do not add another boolean. Route, +render, and immediate-save failures still register persistence because startup +already committed. + +The first save failure still propagates into `Kernel::handle()`, which reports +and renders it. Only the after-response retry is contained: it runs from the +exception renderer after the response is built, so allowing a second failure +to escape would discard that response and replace the already-reported primary +exception. Do not report the retry from this catch; reporting may itself throw, +the primary failure was already reported, and cache-backed writes already emit +their existing failure event. This is the same cleanup-failure precedence +pattern used by Cache locks, not a general exception-swallowing policy. + +### Lock ownership + +Replace manual acquisition plus `finally` release with Cache's callback API: + +```php +return $lock->block( + $request->route()->waitsFor() + ?? $this->manager->defaultRouteBlockWaitSeconds(), + fn (): Response => $this->handleStatefulRequest($request, $session, $next), +); +``` + +Retain the truthful `Repository&LockProvider` local narrowing, but remove its +stale `@phpstan-ignore`. Cache already preserves a callback failure if release +also fails and does not release when acquisition times out. + +### Canonical config reads + +Declare these keys in `src/foundation/config/session.php`: + +```php +'block' => (bool) env('SESSION_BLOCK', false), +'block_store' => env('SESSION_BLOCK_STORE'), +'block_lock_seconds' => (int) env('SESSION_BLOCK_LOCK_SECONDS', 10), +'block_wait_seconds' => (int) env('SESSION_BLOCK_WAIT_SECONDS', 10), +``` + +Remove corresponding call-site defaults in `SessionManager` and use typed +getters for non-null values. Keep nullable `driver`, `connection`, `store`, +`block_store`, and `prefix` on nullable reads. + +In `StartSession`: + +- read declared cookie fields directly; +- type cookie `domain` as `?string`; +- use `$config ??= $this->manager->getSessionConfig()` rather than truthiness; +- read declared `lifetime`, `expire_on_close`, and `driver` directly; and +- delete dead `??` fallbacks that mask malformed merged configuration. + +Callback-returned malformed cookie config may fail naturally; do not add a +second validator. + +### Regressions + +- A throwing session `read()` neither registers nor performs an after-response + write. +- Route failure after successful startup remains persistable. +- A persistently failing save is reported and rendered from the first failure; + the after-response retry cannot escape or replace it. +- Callback failure stays primary when lock release also throws. +- Lock acquisition timeout never calls release. +- Cookie config resolves the canonical merged shape and retains null domain. +- Config tests cover declared block defaults and environment conversion. + +## 4. Correct handler boundaries + +### Cookie handler + +Replace the JSON outer envelope with private PHP serialization: + +```php +public function write(string $sessionId, string $data): bool +{ + $this->cookie->queue($sessionId, serialize([ + 'data' => $data, + 'expires' => $this->availableAt($this->minutes * 60), + ]), $this->expireOnClose ? 0 : $this->minutes); + + return true; +} +``` + +Read untrusted cookie data through the immediately checked native boundary: + +```php +$decoded = @unserialize($value, ['allowed_classes' => false]); + +if (! is_array($decoded) + || ! isset($decoded['data'], $decoded['expires']) + || ! is_string($decoded['data']) + || ! is_int($decoded['expires']) + || $this->currentTime() > $decoded['expires']) { + return ''; +} + +return $decoded['data']; +``` + +The suppression is justified only because the native warning is converted +immediately into the documented empty-read result. Symfony raw-URL-encodes +non-raw cookie values, so serialized binary bytes remain header-safe. Do not +base64 the envelope, invent a frame, or retain a compatibility decoder. + +Tests cover binary PHP-serialized payload round-trip, expiry, garbage input, +top-level objects, missing `data`, non-string `data`, and non-integer +`expires`. + +### File handler + +Type the constructor properties: + +```php +protected string $path, +protected int $minutes, +``` + +Use Finder's always-string pathname and count only successful deletion: + +```php +foreach ($files as $file) { + if ($this->files->delete($file->getPathname())) { + ++$deletedSessions; + } +} +``` + +Propagate false and partial writes through the handler's existing bool +contract: + +```php +public function write(string $sessionId, string $data): bool +{ + return $this->files->put($this->path . '/' . $sessionId, $data, true) === strlen($data); +} +``` + +`Filesystem::put()` returns the byte count or false. Exact comparison handles +empty content, false failures, and short writes without another I/O operation. + +Move the GC regression to `ParallelTesting::tempDir()` and guarantee cleanup in +`finally`. Change the existing false/true delete assertion from two deletions +to one. Keep `destroy()` idempotently true. + +### Database handler + +Refresh the local existence value after the cold read: + +```php +$exists = $this->getExists(); + +if (! $exists) { + $this->read($sessionId); + $exists = $this->getExists(); +} +``` + +Reset the handler-specific existence state at both lifecycle boundaries: + +```php +public function __construct(...) +{ + $this->setExists(false); +} + +public function __clone(): void +{ + $this->setExists(false); +} +``` + +Keep `getExists()` and `setExists()` on the existing dynamic object-ID-derived +key. PHP can reuse the handler object ID after `Session::forgetDrivers()` frees +the Store and handler together, and cloning bypasses the constructor. Without +both resets, stale `exists=true` can skip the cold read, update zero rows, and +falsely report success. Do not precompute the key: the saved nanoseconds are +irrelevant beside SQL I/O, while a copied key would break the public, +Laravel-compatible clone path. + +The normal started-session write retains one context lookup. Only direct write +against unknown local state adds a second in-memory lookup, replacing an +avoidable duplicate insert exception and update round trip. + +Add an integration regression that preinserts a row, uses a fresh tracking +subclass, directly calls `write()`, proves `performUpdate()` rather than +`performInsert()` ran, and verifies stored data. Use the existing protected +methods; add no production seam. + +Add a second deterministic integration regression whose anonymous tracking +subclass seeds its own stale existence slot immediately before +`parent::__construct()`. Prove construction resets it and a direct write +inserts and persists rather than updating zero rows. + +Add a clone regression whose anonymous subclass seeds the clone's new-ID slot +before `parent::__clone()`. Prove clone initialization resets it without +changing the source handler's state, then inserts and persists through the +clone. Retain Laravel's existing clone test because it catches key sharing but +not object-ID reuse. + +Do not classify every `performInsert()` `QueryException`, add an upsert, or add +another existence query. The canonical schema's supported race is already +handled, and no broader realistic failure was established. + +Remove unused Hypervel-only `setConnection()`. Add the standard warning to +retained upstream `setContainer()`: + +> Boot or tests only. Mutating the container on a shared handler during request +> handling can expose the wrong request or authentication state to concurrent +> coroutines. + +Keep public `connection()` because pooled access requires a fresh connection. + +### Complete SessionHandlerInterface types + +Bring only the seven missing parameter lists into line with the typed handlers: + +```php +// ArraySessionHandler +public function open(string $savePath, string $sessionName): bool; +public function read(string $sessionId): false|string; +public function write(string $sessionId, string $data): bool; +public function destroy(string $sessionId): bool; +public function gc(int $lifetime): int; + +// NullSessionHandler +public function write(string $sessionId, string $data): bool; +public function destroy(string $sessionId): bool; +``` + +Do not widen `NullSessionHandler::read()` from its correct `string` return. +Update the `FakeNullSessionHandler` test subclass signature. This is +package-local type completion, not a broad Laravel typing sweep. + +## 5. Give Redis sessions an isolated prefix + +Add a Session Redis Prefix config section near the existing Session Cache Store +configuration: + +```php +'prefix' => env('SESSION_PREFIX', app_id() . '_session:'), +``` + +`SESSION_PREFIX` is the RedisStore-level prefix for session keys. The lower +phpredis connection-wide `REDIS_PREFIX` remains independent and still applies. +This dedicated default prevents session records from being mislabeled with +`cache.prefix` and remains safe when the connection-wide prefix is empty. + +Update `SessionManager::createRedisDriver()`: + +```php +$handler = $this->createCacheHandler('redis'); +$store = $handler->getCache()->getStore(); + +if (! $store instanceof RedisStore) { + throw new InvalidArgumentException( + 'The [session.driver] value [redis] requires [session.store] to reference a Redis cache store.' + ); +} + +$store->setConnection( + $this->config->get('session.connection') ?? 'session' +); + +$prefix = $this->config->get('session.prefix'); + +if ($prefix !== null && $prefix !== '') { + $store->setPrefix($prefix); +} +``` + +The exact null/empty check preserves valid `"0"`. An application may explicitly +set null/empty to retain the selected cache store's prefix, matching current +Laravel fallback semantics. The default Hypervel config chooses the dedicated +session prefix. The construction-time store guard makes the local type truthful +and replaces an opaque undefined-method failure. + +Tests prove: + +- the framework default is `app_id() . '_session:'`; +- custom and `"0"` prefixes apply; +- explicit null/empty retains the clone's cache prefix; +- the original repository/store retains its own prefix and connection; +- the default `session` Redis connection remains; +- an incompatible `session.store` fails with the descriptive message. + +No live Redis service is needed because this behavior ends at store +construction and performs no command. + +## 6. Complete parity, cleanup, metadata, and documentation + +### Store collection sync + +Port the current upstream implementation without changing signatures: + +```php +return collect($keys)->doesntContain($missingCallback); +return collect($keys)->doesntContain($nullCallback); +return collect($keys)->contains($presentCallback); +``` + +`exists()` and `has()` are merge-alignment only. +`hasAny()` gains early exit. Add/merge current upstream tests and preserve +Hypervel enum-key coverage. + +### Provider cleanup + +Delete: + +- the redundant `StartSession::class` singleton; +- its now-unused import; +- the empty `boot()` method; and +- dead `SessionManager::createCacheBased()`, whose only Laravel callers are the + three unsupported cache drivers Hypervel intentionally omits. + +Unbound concrete auto-singletoning already gives `StartSession` identical +worker lifetime. Keep canonical `session` and `session.store` bindings and +command registration. Custom session handlers continue to use +`Session::extend()`; do not retain a dead protected wrapper as a second +extension mechanism. Record the omitted drivers and wrapper in the package +README's Laravel-differences section and at the wrapper's upstream insertion +point in `SessionManager`. + +### Stale PHPStan suppressions + +Delete only: + +- all four ignores in `StartSession`; and +- ignores on `AuthenticateSession` lines currently attached to Session + `has()`/`get()`, the array element, auth default-driver argument, and local + `redirectTo()`; and +- the two unmatched adapter-wrapper ignores in `FilesystemManager`. + +Keep the three live `Route` guards themselves because `Request::route()` is +natively `mixed`. Keep the six genuine AuthManager magic-proxy ignores. Do not +refactor guard resolution merely to eliminate valid suppressions. + +Track the remaining framework-wide unmatched inline ignores and global patterns +in `docs/todo.md`; do not turn this package-local cleanup into a broad +suppression sweep. + +### Filesystem concrete type + +Change only the concrete boundary: + +```php +public function delete(array|string $paths): bool +``` + +Remove its now-redundant type-only `@param`. The contract and three sibling +implementations already declare the union. Preserve the existing array and +extra-argument behavior. This adds no runtime work and does not replace the +Session GC fix. + +### Contract import cleanup + +While changing the Session contract's `getId()` return type, import +`Hypervel\Http\Request` and use the short name on `setRequestOnHandler()`. +This is the file's only inline class FQCN and violates the repository import +rule. + +### Split metadata + +Add these proven direct dependencies to `src/session/composer.json`: + +```text +ext-ctype +ext-mbstring +hypervel/container +symfony/console +symfony/finder +symfony/http-foundation +``` + +Use `^8.1` for all three Symfony packages, matching the root and every sibling +split package. + +Add `tests/Session/PackageMetadataTest.php` covering the complete direct +runtime dependency list, not only the new rows: + +```text +ext-ctype +ext-mbstring +ext-session +hypervel/auth +hypervel/cache +hypervel/collections +hypervel/console +hypervel/container +hypervel/context +hypervel/contracts +hypervel/cookie +hypervel/database +hypervel/filesystem +hypervel/http +hypervel/macroable +hypervel/routing +hypervel/support +symfony/console +symfony/finder +symfony/http-foundation +``` + +Keep both `hypervel/session -> hypervel/auth` and +`hypervel/auth -> hypervel/session`. Both are honest: Session imports +auth-owned `PasswordConfirmation` and `AuthenticationException`, while Auth's +SessionGuard consumes Session. Do not relocate those public auth concepts, +create a bridge package, or add shims solely to make the Composer graph acyclic. + +### Public documentation + +Update `src/boost/docs/session.md` in its existing Laravel-style prose: + +- explain JSON as the application default, its scalar/array suitability, and + explicit PHP object-session opt-in/security implication; +- clarify that the array driver is held only in one worker's memory and is not + suitable for production sessions; +- document `SESSION_PREFIX` under Redis prerequisites, including its dedicated + default and distinction from connection selection; +- document `SESSION_BLOCK=true` and the optional block store/lock/wait settings + while retaining route-level `block()` guidance; +- describe custom-driver `$lifetime` as an age in seconds, not a Unix + timestamp; and +- state that custom-handler `write()` returns true on success and false on + failure, with a false result rejecting the request rather than accepting + lost persistence. + +Keep explanations task-oriented. Do not add architecture prose, internal +context-key details, handler commit internals, or an exhaustive config catalog. + +The previously suspected duplicate Markdown fence does not exist and receives +no change. + +### Audit records + +After implementation, validation, self-review, and code-review sign-off: + +- update the Session route and checklist state in + `docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md`; +- update `session-01` through `session-22` and `filesystem-12` in the audit + ledger from the final implementation; and +- record under `session-01` that constructor initialization must cover every + per-object context slot; record under `session-02` that `save()` now uses the + protected pure aging + seam instead of the public mutator, that false writes are persistence + failures, and that the corrected Foundation JSON round trip independently + proves why the live error bag must survive publication. Record under + `session-15` the unsupported never-started save consequence described above. + Record under `session-19` that the first failure remains reported and + rendered while only the after-response retry failure is contained. Record + under `session-20` that JSON error-bag marshalling is a pure decoded-storage + transformation and a shared Laravel defect. Record under `session-21` that + Database handler construction and cloning reset every object-specific state + slot. + Record under `session-22` that the file handler validates the complete byte + count while `destroy()` remains idempotently true. + +Do not mark either audit record complete before the full workflow is complete. + +### Test return-type scope + +Add `: void` to the 89 existing test methods across the eight affected Session +test files and to every new test method. Work one file at a time and do not use +bulk modification tools. + +## File-oriented implementation checklist + +Work one file at a time. This list is a routing checklist, not a second +description of the design above. + +### Source and public metadata + +- [ ] `src/session/src/Store.php` +- [ ] `src/contracts/src/Session/Session.php` +- [ ] `src/support/src/Facades/Session.php` +- [ ] `src/foundation/src/Testing/Concerns/InteractsWithSession.php` +- [ ] `src/foundation/src/Testing/Concerns/MakesHttpRequests.php` +- [ ] `src/session/src/Middleware/StartSession.php` +- [ ] `src/session/src/Middleware/AuthenticateSession.php` +- [ ] `src/session/src/CookieSessionHandler.php` +- [ ] `src/session/src/FileSessionHandler.php` +- [ ] `src/session/src/DatabaseSessionHandler.php` +- [ ] `src/session/src/ArraySessionHandler.php` +- [ ] `src/session/src/NullSessionHandler.php` +- [ ] `src/session/src/SessionManager.php` +- [ ] `src/session/src/SessionServiceProvider.php` +- [ ] `src/session/src/EncryptedStore.php` if constructor-adjacent typing/docs require alignment +- [ ] `src/filesystem/src/Filesystem.php` +- [ ] `src/filesystem/src/FilesystemManager.php` +- [ ] `src/foundation/config/session.php` +- [ ] `src/session/composer.json` +- [ ] `src/boost/docs/session.md` +- [ ] `docs/todo.md` +- [ ] `docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md` +- [ ] `docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md` + +### Tests + +- [ ] `tests/Session/SessionStoreTest.php` +- [ ] `tests/Session/SessionManagerTest.php` +- [ ] `tests/Session/SessionConfigTest.php` +- [ ] `tests/Session/Middleware/StartSessionTest.php` +- [ ] `tests/Session/Middleware/AuthenticateSessionTest.php` only if stale-ignore behavior needs no source-only validation +- [ ] `tests/Session/ArraySessionHandlerTest.php` +- [ ] new `tests/Session/CookieSessionHandlerTest.php` +- [ ] `tests/Session/CookieSessionHandlerCoroutineSafetyTest.php` +- [ ] `tests/Session/FileSessionHandlerTest.php` +- [ ] `tests/Integration/Session/CookieSessionHandlerTest.php` +- [ ] `tests/Integration/Session/DatabaseSessionHandlerTest.php` +- [ ] `tests/Integration/Session/SessionPersistenceTest.php` +- [ ] `tests/Foundation/Testing/RequestContextSynchronizerTest.php` +- [ ] `tests/Foundation/Testing/Concerns/MakesHttpRequestsTest.php` +- [ ] `tests/Filesystem/FilesystemTest.php` +- [ ] new `tests/Session/PackageMetadataTest.php` + +Do not touch a listed file merely because it appears here. If the implemented +boundary requires no code or regression change in that file, leave it clean. + +## Test and validation plan + +### Immediate cadence + +After changing each test file, run that file immediately with +`./vendor/bin/phpunit --no-progress`. Do not postpone failures to the package +gate. + +Focused groups: + +```bash +./vendor/bin/phpunit --no-progress tests/Session +./vendor/bin/phpunit --no-progress tests/Integration/Session +./vendor/bin/phpunit --no-progress tests/Foundation/Testing +./vendor/bin/phpunit --no-progress tests/Filesystem/FilesystemTest.php +``` + +The Database session integration runs through the existing database test +harness and should be exercised against configured local supported services +where available. Redis-prefix tests are construction tests and require no Redis +service. + +### Regression matrix + +| Area | Required proof | +|---|---| +| Store identity | Two Stores and reused object IDs cannot share state | +| Coroutine reuse | Manager-cached Store gets an ID and clean state in a new coroutine | +| Foundation bridge | Active state copies; no-session request clears prior dynamic state | +| JSON startup | Live error bag survives absent storage; persisted errors win when present | +| Save failure | Throwing and false writes leave flash/error/started state unchanged; retry succeeds | +| JSON success | Consecutive saves retain live error bag | +| Serialization validation | Invalid value fails before context writes; config/direct defaults differ intentionally | +| Startup and retry failure | No retry before `Request::hasSession()`; retry failure cannot escape the renderer | +| Blocking | Timeout does not release; request failure beats release failure | +| Cookie | Binary round-trip, strict envelope shape, expiry, malformed input | +| File GC | Pathname is always string; false deletion is not counted; temp path is isolated | +| File write | Complete byte count succeeds; false and short writes fail and preserve Store state | +| Database direct write | Existing row selects update; constructed and cloned reused identities reset and insert | +| Redis config | default/custom/zero/null/empty prefix, clone isolation, invalid store | +| Parity | Store `exists`/`has`/`hasAny` behavior and enum keys | +| Metadata | Complete direct split dependencies | +| Filesystem | Existing array and string deletion behavior remains | + +### Full gate + +After focused tests are green, run only the authoritative combined gate: + +```bash +composer fix +``` + +Do not redundantly run PHP CS Fixer or PHPStan immediately before it. The gate +owns formatting, both PHPStan configurations, the full parallel suite, and both +Testbench suites. + +## Fresh self-review requirements + +After the full gate, review the complete diff without trusting this plan: + +1. Trace every Store key read/write and verify no fixed state key remains. +2. Trace construction in root, request, copied child, and reused-object-ID + contexts. +3. Trace JSON/PHP load and save through live/persisted error-bag precedence, + encrypted/plain, handler failure, encoding failure, retry, and consecutive + success. +4. Trace middleware startup, route/render/save exceptions, after-response + callbacks, retry failures, precognition, and lock acquire/release failures. +5. Trace every cookie envelope shape through CookieJar and Symfony. +6. Trace direct and normal Database writes across fresh and reused-object-ID + existence state. +7. Verify Redis connection and prefix mutation remains confined to the clone + and adds no command or checkout. +8. Compare all ported methods/tests/config prose with the current upstream + default branches and originating changes. +9. Search for removed constants, `setConnection()` callers, stale call-site + defaults, unmatched PHPStan ignores, old GC wording, and omitted metadata. +10. Check hot-path allocations, context lookups, container resolutions, I/O, + locks, and retained worker state. +11. Remove any dead helper, stale comment, workaround, duplicated decision, or + abstraction that does not solve a verified requirement. + +Unexpected bugs, same-family omissions, Swoole defects, or design +contradictions return to focused investigation and second-opinion consensus +before implementation continues. + +## Expected API, performance, and complexity result + +- Laravel-shaped Session APIs remain intact. `getId()` becomes truthfully + non-null; the unused unsafe Hypervel-only Database handler mutator is removed. +- Current Laravel Redis-prefix and Store collection behavior are present. +- Hypervel's application config deliberately adds a dedicated Session prefix + default and current Laravel's JSON default. +- Normal Store state operations retain one context lookup using a precomputed + key. Three short strings are allocated once per manager-cached Store. +- `getId()` adds only an absent-slot branch; ordinary calls remain one lookup. +- JSON error-bag startup transforms the decoded storage array and avoids + repeated context reads and writes. +- Save adds no lock, retry, container lookup, yield, or I/O. PHP copy-on-write + copies only modified arrays; one strict success check follows the existing + handler I/O. +- Blocked routes allocate one callback beside existing lock I/O; unblocked + requests are unchanged. +- Redis setup adds one construction-time type check and prefix assignment, with + no network command or pool checkout. +- The database direct-write cold path adds one memory lookup and removes an + exception plus extra database work. +- Database existence state resets only at construction and cloning; normal + lookup work remains unchanged and negligible beside SQL I/O. +- Cookie and file changes occur beside existing serialization/filesystem work; + file write validation compares the already-returned byte count and adds no + I/O. +- No registry, WeakMap, counter, state machine, serializer abstraction, + compatibility decoder, retry mechanism, new worker cache, or unbounded state + is introduced. + +The final code should read as if Store identity, persistence commit, config +ownership, and handler contracts were designed this way from the beginning. From db5d1611a00855fd1b747d6f8169d93fd73c9001 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:43:10 +0000 Subject: [PATCH 02/16] fix(session): isolate store state and publish committed saves Give each Store instance its own coroutine-scoped identity, attributes, and started slots while keeping the active request store on the established fixed context key. Synchronize only that active identity through the HTTP testing bridge and restore the non-null Laravel session ID contract. Make JSON error-bag marshalling and flash aging pure, write serialized snapshots before publishing live state, and reject false handler writes without corrupting retry state. Cover sibling stores, reused object IDs, copied request contexts, serialization failures, cache write failures, encrypted stores, and consecutive JSON saves. --- src/contracts/src/Session/Session.php | 5 +- .../Testing/Concerns/InteractsWithSession.php | 7 - .../Testing/Concerns/MakesHttpRequests.php | 36 +- src/session/src/Store.php | 151 ++++-- src/support/src/Facades/Session.php | 4 +- .../Concerns/MakesHttpRequestsTest.php | 46 +- .../Session/CacheBasedSessionHandlerTest.php | 54 ++- tests/Session/EncryptedSessionStoreTest.php | 4 +- tests/Session/SessionStoreTest.php | 433 +++++++++++++++--- 9 files changed, 594 insertions(+), 146 deletions(-) diff --git a/src/contracts/src/Session/Session.php b/src/contracts/src/Session/Session.php index cc551233b..59032d689 100644 --- a/src/contracts/src/Session/Session.php +++ b/src/contracts/src/Session/Session.php @@ -4,6 +4,7 @@ namespace Hypervel\Contracts\Session; +use Hypervel\Http\Request; use SessionHandlerInterface; use UnitEnum; @@ -22,7 +23,7 @@ public function setName(string $name): void; /** * Get the current session ID. */ - public function getId(): ?string; + public function getId(): string; /** * Set the session ID. @@ -177,5 +178,5 @@ public function handlerNeedsRequest(): bool; /** * Set the request on the handler instance. */ - public function setRequestOnHandler(\Hypervel\Http\Request $request): void; + public function setRequestOnHandler(Request $request): void; } diff --git a/src/foundation/src/Testing/Concerns/InteractsWithSession.php b/src/foundation/src/Testing/Concerns/InteractsWithSession.php index 3f023ef47..9c7089409 100644 --- a/src/foundation/src/Testing/Concerns/InteractsWithSession.php +++ b/src/foundation/src/Testing/Concerns/InteractsWithSession.php @@ -36,13 +36,6 @@ public function session(array $data): static protected function startSession(): static { if (! $this->app['session']->isStarted()) { - // Ensure a session ID exists before starting. In production, the - // StartSession middleware sets the ID from the request cookie. - // In tests, we generate one if none exists. - if ($this->app['session']->getId() === null) { - $this->app['session']->setId(null); - } - $this->app['session']->start(); } diff --git a/src/foundation/src/Testing/Concerns/MakesHttpRequests.php b/src/foundation/src/Testing/Concerns/MakesHttpRequests.php index 48ac92639..61550c355 100644 --- a/src/foundation/src/Testing/Concerns/MakesHttpRequests.php +++ b/src/foundation/src/Testing/Concerns/MakesHttpRequests.php @@ -5,6 +5,7 @@ namespace Hypervel\Foundation\Testing\Concerns; use BackedEnum; +use Hypervel\Context\CoroutineContext; use Hypervel\Context\RequestContext; use Hypervel\Contracts\Http\Kernel as HttpKernel; use Hypervel\Cookie\CookieValuePrefix; @@ -581,12 +582,13 @@ protected function sessionContextSnapshot(Request $request): array /** @var SessionStore $session */ $session = $request->session(); + [$startedKey, $idKey, $attributesKey] = $this->sessionStoreContextKeys($session); return [ SessionStore::CONTEXT_KEY => $session, - SessionStore::STARTED_CONTEXT_KEY => $session->isStarted(), - SessionStore::ID_CONTEXT_KEY => $session->getId(), - SessionStore::ATTRIBUTES_CONTEXT_KEY => $session->all(), + $startedKey => $session->isStarted(), + $idKey => $session->getId(), + $attributesKey => $session->all(), ]; } @@ -597,11 +599,31 @@ protected function sessionContextSnapshot(Request $request): array */ protected function sessionContextKeys(): array { + $keys = [SessionStore::CONTEXT_KEY]; + + // Read the copied child Context before synchronizing a request without a session. + $session = CoroutineContext::get(SessionStore::CONTEXT_KEY); + + if ($session instanceof SessionStore) { + array_push($keys, ...$this->sessionStoreContextKeys($session)); + } + + return $keys; + } + + /** + * Get the Context keys for the given session store. + * + * @return array + */ + protected function sessionStoreContextKeys(SessionStore $session): array + { + $suffix = (string) spl_object_id($session); + return [ - SessionStore::CONTEXT_KEY, - SessionStore::STARTED_CONTEXT_KEY, - SessionStore::ID_CONTEXT_KEY, - SessionStore::ATTRIBUTES_CONTEXT_KEY, + SessionStore::STARTED_CONTEXT_KEY_PREFIX . $suffix, + SessionStore::ID_CONTEXT_KEY_PREFIX . $suffix, + SessionStore::ATTRIBUTES_CONTEXT_KEY_PREFIX . $suffix, ]; } diff --git a/src/session/src/Store.php b/src/session/src/Store.php index b23e1e69d..f51e669f5 100644 --- a/src/session/src/Store.php +++ b/src/session/src/Store.php @@ -20,6 +20,7 @@ use Hypervel\Support\Traits\Macroable; use Hypervel\Support\Uri; use Hypervel\Support\ViewErrorBag; +use InvalidArgumentException; use RuntimeException; use SessionHandlerInterface; use stdClass; @@ -39,23 +40,43 @@ class Store implements Session /** * Context key for whether the session has been started. */ - public const STARTED_CONTEXT_KEY = '__session.store.started'; + public const STARTED_CONTEXT_KEY_PREFIX = '__session.store.started.'; /** * Context key for the session attributes. */ - public const ATTRIBUTES_CONTEXT_KEY = '__session.store.attributes'; + public const ATTRIBUTES_CONTEXT_KEY_PREFIX = '__session.store.attributes.'; /** * Context key for the session ID. */ - public const ID_CONTEXT_KEY = '__session.store.id'; + public const ID_CONTEXT_KEY_PREFIX = '__session.store.id.'; + + /** + * The supported session serialization strategies. + */ + protected const SUPPORTED_SERIALIZATIONS = ['json', 'php']; /** * The length of session ID strings. */ protected const SESSION_ID_LENGTH = 40; + /** + * The context key for whether this session has been started. + */ + protected readonly string $startedContextKey; + + /** + * The context key for this session's attributes. + */ + protected readonly string $attributesContextKey; + + /** + * The context key for this session's ID. + */ + protected readonly string $idContextKey; + /** * Create a new session instance. * @@ -69,6 +90,23 @@ public function __construct( ?string $id = null, protected string $serialization = 'php' ) { + if (! in_array($serialization, self::SUPPORTED_SERIALIZATIONS, true)) { + throw new InvalidArgumentException(sprintf( + 'Session serialization [%s] is not supported. Supported: "%s".', + $serialization, + implode('", "', self::SUPPORTED_SERIALIZATIONS), + )); + } + + $suffix = (string) spl_object_id($this); + + $this->startedContextKey = self::STARTED_CONTEXT_KEY_PREFIX . $suffix; + $this->attributesContextKey = self::ATTRIBUTES_CONTEXT_KEY_PREFIX . $suffix; + $this->idContextKey = self::ID_CONTEXT_KEY_PREFIX . $suffix; + + CoroutineContext::set($this->startedContextKey, false); + CoroutineContext::set($this->attributesContextKey, []); + $this->setId($id); } @@ -83,7 +121,7 @@ public function start(): bool $this->regenerateToken(); } - return CoroutineContext::set(self::STARTED_CONTEXT_KEY, true); + return CoroutineContext::set($this->startedContextKey, true); } /** @@ -91,7 +129,7 @@ public function start(): bool */ protected function getAttributes(): array { - return CoroutineContext::get(self::ATTRIBUTES_CONTEXT_KEY, []); + return CoroutineContext::get($this->attributesContextKey, []); } /** @@ -99,7 +137,7 @@ protected function getAttributes(): array */ protected function setAttributes(array $attributes): void { - CoroutineContext::set(self::ATTRIBUTES_CONTEXT_KEY, $attributes); + CoroutineContext::set($this->attributesContextKey, $attributes); } /** @@ -108,8 +146,8 @@ protected function setAttributes(array $attributes): void protected function replaceAttributes(array $attributes): void { CoroutineContext::set( - self::ATTRIBUTES_CONTEXT_KEY, - array_replace(CoroutineContext::get(self::ATTRIBUTES_CONTEXT_KEY, []), $attributes) + $this->attributesContextKey, + array_replace(CoroutineContext::get($this->attributesContextKey, []), $attributes) ); } @@ -118,9 +156,9 @@ protected function replaceAttributes(array $attributes): void */ protected function loadSession(): void { - $this->replaceAttributes($this->readFromHandler()); - - $this->marshalErrorBag(); + // Marshal the decoded payload before merging: marshalling the merged result + // would iterate an already-live ViewErrorBag and replace it with an empty one. + $this->replaceAttributes($this->marshalErrorBagIn($this->readFromHandler())); } /** @@ -152,23 +190,25 @@ protected function prepareForUnserialize(string $data): string } /** - * Marshal the ViewErrorBag when using JSON serialization for sessions. + * Marshal the ViewErrorBag in the given session attributes. */ - protected function marshalErrorBag(): void + protected function marshalErrorBagIn(array $attributes): array { - if ($this->serialization !== 'json' || $this->missing('errors')) { - return; + if ($this->serialization !== 'json' || ! array_key_exists('errors', $attributes)) { + return $attributes; } $errorBag = new ViewErrorBag; - foreach ($this->get('errors') as $key => $value) { + foreach ($attributes['errors'] as $key => $value) { $messageBag = new MessageBag($value['messages']); $errorBag->put($key, $messageBag->setFormat($value['format'])); } - $this->put('errors', $errorBag); + $attributes['errors'] = $errorBag; + + return $attributes; } /** @@ -176,36 +216,49 @@ protected function marshalErrorBag(): void */ public function save(): void { - $this->ageFlashData(); + // Publish the aged attributes only after the handler commits, so a failed + // write leaves the live flash data and error bag intact for the retry. + $attributes = $this->ageFlashDataIn($this->getAttributes()); + $attributesForStorage = $this->prepareErrorBagForSerialization($attributes); + + $serialized = $this->serialization === 'json' + ? json_encode($attributesForStorage, JSON_THROW_ON_ERROR) + : serialize($attributesForStorage); - $this->prepareErrorBagForSerialization(); + $written = $this->handler->write( + $this->getId(), + $this->prepareForStorage($serialized) + ); - $this->handler->write($this->getId(), $this->prepareForStorage( - $this->serialization === 'json' ? json_encode($this->getAttributes()) : serialize($this->getAttributes()) - )); + if ($written === false) { + throw new RuntimeException('Unable to write the session data.'); + } - CoroutineContext::set(self::STARTED_CONTEXT_KEY, false); + $this->setAttributes($attributes); + CoroutineContext::set($this->startedContextKey, false); } /** - * Prepare the ViewErrorBag instance for JSON serialization. + * Prepare the ViewErrorBag in the given session attributes for JSON serialization. */ - protected function prepareErrorBagForSerialization(): void + protected function prepareErrorBagForSerialization(array $attributes): array { - if ($this->serialization !== 'json' || $this->missing('errors')) { - return; + if ($this->serialization !== 'json' || ! array_key_exists('errors', $attributes)) { + return $attributes; } $errors = []; - foreach ($this->getAttributes()['errors']->getBags() as $key => $value) { + foreach ($attributes['errors']->getBags() as $key => $value) { $errors[$key] = [ 'format' => $value->getFormat(), 'messages' => $value->getMessages(), ]; } - $this->replaceAttributes(['errors' => $errors]); + $attributes['errors'] = $errors; + + return $attributes; } /** @@ -221,11 +274,19 @@ protected function prepareForStorage(string $data): string */ public function ageFlashData(): void { - $this->forget($this->get('_flash.old', [])); + $this->setAttributes($this->ageFlashDataIn($this->getAttributes())); + } - $this->put('_flash.old', $this->get('_flash.new', [])); + /** + * Age the flash data in the given session attributes. + */ + protected function ageFlashDataIn(array $attributes): array + { + Arr::forget($attributes, Arr::get($attributes, '_flash.old', [])); + Arr::set($attributes, '_flash.old', Arr::get($attributes, '_flash.new', [])); + Arr::set($attributes, '_flash.new', []); - $this->put('_flash.new', []); + return $attributes; } /** @@ -259,7 +320,7 @@ public function exists(array|UnitEnum|string $key): bool { $placeholder = new stdClass; - return ! collect(is_array($key) ? $key : func_get_args())->contains(function ($key) use ($placeholder) { + return collect(is_array($key) ? $key : func_get_args())->doesntContain(function ($key) use ($placeholder) { return $this->get($key, $placeholder) === $placeholder; }); } @@ -277,7 +338,7 @@ public function missing(array|UnitEnum|string $key): bool */ public function has(array|UnitEnum|string $key): bool { - return ! collect(is_array($key) ? $key : func_get_args())->contains(function ($key) { + return collect(is_array($key) ? $key : func_get_args())->doesntContain(function ($key) { return is_null($this->get($key)); }); } @@ -287,9 +348,9 @@ public function has(array|UnitEnum|string $key): bool */ public function hasAny(array|UnitEnum|string $key): bool { - return collect(is_array($key) ? $key : func_get_args())->filter(function ($key) { + return collect(is_array($key) ? $key : func_get_args())->contains(function ($key) { return ! is_null($this->get($key)); - })->count() >= 1; + }); } /** @@ -555,7 +616,7 @@ public function migrate(bool $destroy = false): bool */ public function isStarted(): bool { - return CoroutineContext::get(self::STARTED_CONTEXT_KEY, false); + return CoroutineContext::get($this->startedContextKey, false); } /** @@ -580,7 +641,7 @@ public function setName(string $name): void /** * Get the current session ID. */ - public function id(): ?string + public function id(): string { return $this->getId(); } @@ -588,9 +649,17 @@ public function id(): ?string /** * Get the current session ID. */ - public function getId(): ?string + public function getId(): string { - return CoroutineContext::get(self::ID_CONTEXT_KEY, null); + /** @var null|string $id */ + $id = CoroutineContext::get($this->idContextKey); + + if ($id === null) { + $id = $this->generateSessionId(); + CoroutineContext::set($this->idContextKey, $id); + } + + return $id; } /** @@ -599,7 +668,7 @@ public function getId(): ?string public function setId(?string $id): void { CoroutineContext::set( - self::ID_CONTEXT_KEY, + $this->idContextKey, $this->isValidId($id) ? $id : $this->generateSessionId() ); } diff --git a/src/support/src/Facades/Session.php b/src/support/src/Facades/Session.php index 99f2d3173..60dff5bac 100644 --- a/src/support/src/Facades/Session.php +++ b/src/support/src/Facades/Session.php @@ -54,8 +54,8 @@ * @method static void flushState() * @method static string getName() * @method static void setName(string $name) - * @method static string|null id() - * @method static string|null getId() + * @method static string id() + * @method static string getId() * @method static void setId(string|null $id) * @method static bool isValidId(string|null $id) * @method static void setExists(bool $value) diff --git a/tests/Foundation/Testing/Concerns/MakesHttpRequestsTest.php b/tests/Foundation/Testing/Concerns/MakesHttpRequestsTest.php index 50c392b6f..dc199aeb9 100644 --- a/tests/Foundation/Testing/Concerns/MakesHttpRequestsTest.php +++ b/tests/Foundation/Testing/Concerns/MakesHttpRequestsTest.php @@ -4,6 +4,7 @@ namespace Hypervel\Tests\Foundation\Testing\Concerns; +use Hypervel\Context\CoroutineContext; use Hypervel\Contracts\Routing\Registrar; use Hypervel\Coroutine\Coroutine; use Hypervel\Foundation\Http\Middleware\HandlePrecognitiveRequests; @@ -329,6 +330,40 @@ public function testCallPropagatesFlashedInputToParentCoroutine() $this->assertSame('test-old-value', old('name')); } + public function testRequestWithoutSessionClearsPriorSessionContext(): void + { + $router = $this->app->make(Router::class); + $router->get('/with-session', function () { + session()->put('name', 'Taylor'); + + return 'session'; + })->middleware('web'); + $router->get('/without-session', fn () => 'no session'); + + $this->get('/with-session')->assertOk(); + + $session = CoroutineContext::get(Store::CONTEXT_KEY); + $this->assertInstanceOf(Store::class, $session); + + $suffix = (string) spl_object_id($session); + $sessionKeys = [ + Store::CONTEXT_KEY, + Store::STARTED_CONTEXT_KEY_PREFIX . $suffix, + Store::ID_CONTEXT_KEY_PREFIX . $suffix, + Store::ATTRIBUTES_CONTEXT_KEY_PREFIX . $suffix, + ]; + + foreach ($sessionKeys as $sessionKey) { + $this->assertTrue(CoroutineContext::has($sessionKey)); + } + + $this->get('/without-session')->assertOk(); + + foreach ($sessionKeys as $sessionKey) { + $this->assertFalse(CoroutineContext::has($sessionKey)); + } + } + public function testAssertSessionHasErrors() { $this->app->instance('session.store', $store = new Store('test-session', new ArraySessionHandler(1))); @@ -346,9 +381,10 @@ public function testAssertSessionHasErrors() $response->assertSessionHasErrors(['foo']); } - public function testAssertJsonSerializedSessionHasErrors() + public function testAssertJsonSerializedSessionHasErrors(): void { - $this->app->instance('session.store', $store = new Store('test-session', new ArraySessionHandler(1))); + $handler = new ArraySessionHandler(1); + $store = new Store('test-session', $handler, serialization: 'json'); $store->put('errors', $errorBag = new ViewErrorBag); @@ -358,7 +394,11 @@ public function testAssertJsonSerializedSessionHasErrors() ], ])); - $store->save(); // Required to serialize error bag to JSON + $store->save(); + + $store = new Store('test-session', $handler, $store->getId(), 'json'); + $store->start(); + $this->app->instance('session.store', $store); $response = TestResponse::fromBaseResponse(new Response); diff --git a/tests/Session/CacheBasedSessionHandlerTest.php b/tests/Session/CacheBasedSessionHandlerTest.php index b002ff81b..86c1fde07 100644 --- a/tests/Session/CacheBasedSessionHandlerTest.php +++ b/tests/Session/CacheBasedSessionHandlerTest.php @@ -6,8 +6,10 @@ use Hypervel\Contracts\Cache\Repository as CacheContract; use Hypervel\Session\CacheBasedSessionHandler; +use Hypervel\Session\Store; use Hypervel\Tests\TestCase; use Mockery as m; +use RuntimeException; class CacheBasedSessionHandlerTest extends TestCase { @@ -23,19 +25,19 @@ protected function setUp(): void $this->sessionHandler = new CacheBasedSessionHandler($this->cacheMock, 10); } - public function testOpen() + public function testOpen(): void { $result = $this->sessionHandler->open('path', 'session_name'); $this->assertTrue($result); } - public function testClose() + public function testClose(): void { $result = $this->sessionHandler->close(); $this->assertTrue($result); } - public function testReadReturnsDataFromCache() + public function testReadReturnsDataFromCache(): void { $this->cacheMock->shouldReceive('get')->once()->with('session_id', '')->andReturn('session_data'); @@ -43,7 +45,7 @@ public function testReadReturnsDataFromCache() $this->assertSame('session_data', $data); } - public function testReadReturnsEmptyStringIfNoData() + public function testReadReturnsEmptyStringIfNoData(): void { $this->cacheMock->shouldReceive('get')->once()->with('some_id', '')->andReturn(''); @@ -51,7 +53,7 @@ public function testReadReturnsEmptyStringIfNoData() $this->assertSame('', $data); } - public function testWriteStoresDataInCache() + public function testWriteStoresDataInCache(): void { $this->cacheMock->shouldReceive('put')->once()->with('session_id', 'session_data', 600) ->andReturn(true); @@ -61,7 +63,7 @@ public function testWriteStoresDataInCache() $this->assertTrue($result); } - public function testDestroyRemovesDataFromCache() + public function testDestroyRemovesDataFromCache(): void { $this->cacheMock->shouldReceive('forget')->once()->with('session_id')->andReturn(true); @@ -70,15 +72,51 @@ public function testDestroyRemovesDataFromCache() $this->assertTrue($result); } - public function testGcReturnsZero() + public function testGcReturnsZero(): void { $result = $this->sessionHandler->gc(120); $this->assertSame(0, $result); } - public function testGetCacheReturnsCacheInstance() + public function testGetCacheReturnsCacheInstance(): void { $this->assertSame($this->cacheMock, $this->sessionHandler->getCache()); } + + public function testFalseCacheWriteLeavesLiveSessionStateForRetry(): void + { + $this->cacheMock->shouldReceive('get')->once()->andReturn(serialize([])); + + $payloads = []; + $this->cacheMock->shouldReceive('put') + ->twice() + ->withArgs(function (string $sessionId, string $data, int $seconds) use (&$payloads): bool { + $payloads[] = unserialize($data); + + return $sessionId === str_repeat('a', 40) && $seconds === 600; + }) + ->andReturn(false, true); + + $session = new Store('name', $this->sessionHandler, str_repeat('a', 40)); + $session->start(); + $session->flash('status', 'saved'); + + try { + $session->save(); + + $this->fail('Expected the failed cache write to reject the session save.'); + } catch (RuntimeException $exception) { + $this->assertSame('Unable to write the session data.', $exception->getMessage()); + } + + $this->assertTrue($session->isStarted()); + $this->assertSame(['status'], $session->get('_flash.new')); + + $session->save(); + + $this->assertFalse($session->isStarted()); + $this->assertSame(['status'], $session->get('_flash.old')); + $this->assertSame($payloads[0], $payloads[1]); + } } diff --git a/tests/Session/EncryptedSessionStoreTest.php b/tests/Session/EncryptedSessionStoreTest.php index 5261920e5..38c7588d1 100644 --- a/tests/Session/EncryptedSessionStoreTest.php +++ b/tests/Session/EncryptedSessionStoreTest.php @@ -12,7 +12,7 @@ class EncryptedSessionStoreTest extends TestCase { - public function testSessionIsProperlyEncrypted() + public function testSessionIsProperlyEncrypted(): void { $session = $this->getSession(); $session->getEncrypter()->shouldReceive('decrypt')->once()->with(serialize([]))->andReturn(serialize([])); @@ -34,7 +34,7 @@ public function testSessionIsProperlyEncrypted() $session->getHandler()->shouldReceive('write')->once()->with( $this->getSessionId(), $serialized - ); + )->andReturnTrue(); $session->save(); $this->assertFalse($session->isStarted()); diff --git a/tests/Session/SessionStoreTest.php b/tests/Session/SessionStoreTest.php index e33c084d3..b5bec1f1d 100644 --- a/tests/Session/SessionStoreTest.php +++ b/tests/Session/SessionStoreTest.php @@ -5,6 +5,7 @@ namespace Hypervel\Tests\Session; use Hypervel\Container\Container; +use Hypervel\Context\CoroutineContext; use Hypervel\Contracts\Auth\Factory as AuthFactory; use Hypervel\Http\Request; use Hypervel\Session\CookieSessionHandler; @@ -14,13 +15,18 @@ use Hypervel\Support\Uri; use Hypervel\Support\ViewErrorBag; use Hypervel\Tests\TestCase; +use InvalidArgumentException; +use JsonException; use Mockery as m; use RuntimeException; use SessionHandlerInterface; +use UnitEnum; + +use function Hypervel\Coroutine\parallel; class SessionStoreTest extends TestCase { - public function testSessionIsLoadedFromHandler() + public function testSessionIsLoadedFromHandler(): void { $session = $this->getSession(); $session->getHandler()->shouldReceive('read')->once()->with($this->getSessionId())->andReturn(serialize(['foo' => 'bar', 'bagged' => ['name' => 'taylor'], '123' => 'bax'])); @@ -38,7 +44,7 @@ public function testSessionIsLoadedFromHandler() $this->assertTrue($session->has('baz')); } - public function testSessionMigration() + public function testSessionMigration(): void { $session = $this->getSession(); $oldId = $session->getId(); @@ -53,7 +59,7 @@ public function testSessionMigration() $this->assertNotEquals($oldId, $session->getId()); } - public function testSessionRegeneration() + public function testSessionRegeneration(): void { $session = $this->getSession(); $oldId = $session->getId(); @@ -62,7 +68,7 @@ public function testSessionRegeneration() $this->assertNotEquals($oldId, $session->getId()); } - public function testCantSetInvalidId() + public function testCantSetInvalidId(): void { $session = $this->getSession(); $this->assertTrue($session->isValidId($session->getId())); @@ -75,7 +81,81 @@ public function testCantSetInvalidId() $this->assertNotSame('wrong', $session->getId()); } - public function testSessionInvalidate() + public function testStoresUseIndependentCoroutineState(): void + { + $handler = m::mock(SessionHandlerInterface::class); + $handler->shouldReceive('read')->once()->andReturn(serialize([])); + + $first = new Store('first', $handler, str_repeat('a', 40)); + $first->start(); + $first->put('name', 'first'); + + $second = new Store('second', $handler, str_repeat('b', 40)); + $second->put('name', 'second'); + + $this->assertSame(str_repeat('a', 40), $first->getId()); + $this->assertSame(str_repeat('b', 40), $second->getId()); + $this->assertSame('first', $first->get('name')); + $this->assertSame('second', $second->get('name')); + $this->assertTrue($first->isStarted()); + $this->assertFalse($second->isStarted()); + } + + public function testConstructionClearsStaleObjectSpecificCoroutineState(): void + { + $handler = m::mock(SessionHandlerInterface::class); + + $session = new class('name', $handler, str_repeat('b', 40)) extends Store { + public function __construct(string $name, SessionHandlerInterface $handler, ?string $id = null) + { + // Model stale slots from a released Store whose object ID PHP reused. + $suffix = (string) spl_object_id($this); + + CoroutineContext::set(self::STARTED_CONTEXT_KEY_PREFIX . $suffix, true); + CoroutineContext::set(self::ATTRIBUTES_CONTEXT_KEY_PREFIX . $suffix, ['name' => 'stale']); + CoroutineContext::set(self::ID_CONTEXT_KEY_PREFIX . $suffix, str_repeat('a', 40)); + + parent::__construct($name, $handler, $id); + } + }; + + $this->assertSame(str_repeat('b', 40), $session->getId()); + $this->assertSame([], $session->all()); + $this->assertFalse($session->isStarted()); + } + + public function testStoreLazilyCreatesAnIdInAFreshCoroutine(): void + { + $session = new Store('name', m::mock(SessionHandlerInterface::class), str_repeat('a', 40)); + + [$id] = parallel([ + fn (): string => $session->getId(), + ]); + + $this->assertSame(40, strlen($id)); + $this->assertTrue($session->isValidId($id)); + $this->assertNotSame(str_repeat('a', 40), $id); + } + + public function testInvalidSerializationFailsBeforeWritingContext(): void + { + $context = CoroutineContext::captureFrom(); + + try { + new Store('name', m::mock(SessionHandlerInterface::class), serialization: 'yaml'); + + $this->fail('Expected invalid session serialization to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'Session serialization [yaml] is not supported. Supported: "json", "php".', + $exception->getMessage(), + ); + } + + $this->assertSame($context, CoroutineContext::captureFrom()); + } + + public function testSessionInvalidate(): void { $session = $this->getSession(); $oldId = $session->getId(); @@ -94,7 +174,7 @@ public function testSessionInvalidate() $this->assertCount(0, $session->all()); } - public function testBrandNewSessionIsProperlySaved() + public function testBrandNewSessionIsProperlySaved(): void { $session = $this->getSession(); $session->getHandler()->shouldReceive('read')->once()->andReturn(serialize([])); @@ -113,13 +193,13 @@ public function testBrandNewSessionIsProperlySaved() 'old' => ['baz'], ], ]) - ); + )->andReturnTrue(); $session->save(); $this->assertFalse($session->isStarted()); } - public function testSessionIsProperlyUpdated() + public function testSessionIsProperlyUpdated(): void { $session = $this->getSession(); $session->getHandler()->shouldReceive('read')->once()->andReturn(serialize([ @@ -143,14 +223,14 @@ public function testSessionIsProperlyUpdated() 'old' => [], ], ]) - ); + )->andReturnTrue(); $session->save(); $this->assertFalse($session->isStarted()); } - public function testSessionIsReSavedWhenNothingHasChanged() + public function testSessionIsReSavedWhenNothingHasChanged(): void { $session = $this->getSession(); $session->getHandler()->shouldReceive('read')->once()->andReturn(serialize([ @@ -175,14 +255,14 @@ public function testSessionIsReSavedWhenNothingHasChanged() 'old' => [], ], ]) - ); + )->andReturnTrue(); $session->save(); $this->assertFalse($session->isStarted()); } - public function testSessionIsReSavedWhenNothingHasChangedExceptSessionId() + public function testSessionIsReSavedWhenNothingHasChangedExceptSessionId(): void { $session = $this->getSession(); $oldId = $session->getId(); @@ -215,14 +295,57 @@ public function testSessionIsReSavedWhenNothingHasChangedExceptSessionId() 'old' => [], ], ]) + )->andReturnTrue(); + + $session->save(); + + $this->assertFalse($session->isStarted()); + } + + public function testFailedSaveDoesNotPublishAgedFlashDataBeforeRetry(): void + { + $handler = m::mock(SessionHandlerInterface::class); + $handler->shouldReceive('read')->once()->andReturn(serialize([])); + + $attempts = 0; + $payloads = []; + $handler->shouldReceive('write')->twice()->andReturnUsing( + function (string $sessionId, string $data) use (&$attempts, &$payloads): bool { + $payloads[] = unserialize($data); + + if (++$attempts === 1) { + throw new RuntimeException('Unable to persist the session.'); + } + + return true; + } ); + $session = new Store('name', $handler, $this->getSessionId()); + $session->start(); + $session->flash('status', 'saved'); + + try { + $session->save(); + + $this->fail('Expected the first session write to fail.'); + } catch (RuntimeException $exception) { + $this->assertSame('Unable to persist the session.', $exception->getMessage()); + } + + $this->assertTrue($session->isStarted()); + $this->assertSame(['status'], $session->get('_flash.new')); + $this->assertSame([], $session->get('_flash.old')); + $session->save(); $this->assertFalse($session->isStarted()); + $this->assertSame([], $session->get('_flash.new')); + $this->assertSame(['status'], $session->get('_flash.old')); + $this->assertSame($payloads[0], $payloads[1]); } - public function testOldInputFlashing() + public function testOldInputFlashing(): void { $session = $this->getSession(); $session->put('boom', 'baz'); @@ -244,7 +367,7 @@ public function testOldInputFlashing() $this->assertNull($session->getOldInput('name', 'default')); } - public function testDataFlashing() + public function testDataFlashing(): void { $session = $this->getSession(); $session->flash('foo', 'bar'); @@ -268,7 +391,7 @@ public function testDataFlashing() $this->assertNull($session->get('foo')); } - public function testDataFlashingNow() + public function testDataFlashingNow(): void { $session = $this->getSession(); $session->now('foo', 'bar'); @@ -284,7 +407,7 @@ public function testDataFlashingNow() $this->assertNull($session->get('foo')); } - public function testDataMergeNewFlashes() + public function testDataMergeNewFlashes(): void { $session = $this->getSession(); $session->flash('foo', 'bar'); @@ -299,7 +422,7 @@ public function testDataMergeNewFlashes() $this->assertFalse(array_search('qu', $session->get('_flash.old'))); } - public function testReflash() + public function testReflash(): void { $session = $this->getSession(); $session->flash('foo', 'bar'); @@ -309,7 +432,7 @@ public function testReflash() $this->assertFalse(array_search('foo', $session->get('_flash.old'))); } - public function testReflashWithNow() + public function testReflashWithNow(): void { $session = $this->getSession(); $session->now('foo', 'bar'); @@ -318,7 +441,7 @@ public function testReflashWithNow() $this->assertFalse(array_search('foo', $session->get('_flash.old'))); } - public function testOnly() + public function testOnly(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -327,7 +450,7 @@ public function testOnly() $this->assertEquals(['qu' => 'ux'], $session->only(['qu'])); } - public function testExcept() + public function testExcept(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -338,7 +461,7 @@ public function testExcept() $this->assertEquals(['bar' => 'baz', 'qu' => 'ux'], $session->except(['foo'])); } - public function testReplace() + public function testReplace(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -348,7 +471,7 @@ public function testReplace() $this->assertSame('ux', $session->get('qu')); } - public function testRemove() + public function testRemove(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -357,7 +480,7 @@ public function testRemove() $this->assertSame('bar', $pulled); } - public function testClear() + public function testClear(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -371,7 +494,7 @@ public function testClear() $this->assertFalse($session->has('foo')); } - public function testIncrement() + public function testIncrement(): void { $session = $this->getSession(); @@ -388,7 +511,7 @@ public function testIncrement() $this->assertEquals(1, $session->get('bar')); } - public function testDecrement() + public function testDecrement(): void { $session = $this->getSession(); @@ -405,7 +528,7 @@ public function testDecrement() $this->assertEquals(-1, $session->get('bar')); } - public function testHasOldInputWithoutKey() + public function testHasOldInputWithoutKey(): void { $session = $this->getSession(); $session->flash('boom', 'baz'); @@ -415,7 +538,7 @@ public function testHasOldInputWithoutKey() $this->assertTrue($session->hasOldInput()); } - public function testHandlerNeedsRequest() + public function testHandlerNeedsRequest(): void { $session = $this->getSession(); $this->assertFalse($session->handlerNeedsRequest()); @@ -428,7 +551,7 @@ public function testHandlerNeedsRequest() $session->setRequestOnHandler(new Request); } - public function testToken() + public function testToken(): void { $session = $this->getSession(); $this->assertNull($session->token()); @@ -437,7 +560,7 @@ public function testToken() $this->assertEquals($session->token(), $session->token()); } - public function testRegenerateToken() + public function testRegenerateToken(): void { $session = $this->getSession(); $token = $session->token(); @@ -445,7 +568,7 @@ public function testRegenerateToken() $this->assertNotEquals($token, $session->token()); } - public function testName() + public function testName(): void { $session = $this->getSession(); $this->assertEquals($session->getName(), $this->getSessionName()); @@ -453,7 +576,7 @@ public function testName() $this->assertSame('foo', $session->getName()); } - public function testForget() + public function testForget(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -468,7 +591,7 @@ public function testForget() $this->assertFalse($session->has('bar')); } - public function testSetPreviousUrl() + public function testSetPreviousUrl(): void { $session = $this->getSession(); $session->setPreviousUrl('https://example.com/foo/bar'); @@ -507,7 +630,7 @@ public function testPasswordConfirmedResolvesCurrentGuardWhenNoneGiven(): void } } - public function testKeyPush() + public function testKeyPush(): void { $session = $this->getSession(); $session->put('language', ['PHP' => ['Laravel']]); @@ -516,7 +639,7 @@ public function testKeyPush() $this->assertEquals(['PHP' => ['Laravel', 'Symfony']], $session->get('language')); } - public function testKeyPull() + public function testKeyPull(): void { $session = $this->getSession(); $session->put('name', 'Taylor'); @@ -526,7 +649,7 @@ public function testKeyPull() $this->assertNull($session->pull('name')); } - public function testKeyHas() + public function testKeyHas(): void { $session = $this->getSession(); $session->put('first_name', 'Mehdi'); @@ -541,7 +664,7 @@ public function testKeyHas() $this->assertFalse($session->has('foo', 'bar')); } - public function testKeyHasAny() + public function testKeyHasAny(): void { $session = $this->getSession(); $session->put('first_name', 'Mahmoud'); @@ -557,7 +680,28 @@ public function testKeyHasAny() $this->assertFalse($session->hasAny(['foo', 'bar'])); } - public function testKeyExists() + public function testHasAnyStopsAfterTheFirstPresentKey(): void + { + $session = new class('name', m::mock(SessionHandlerInterface::class), $this->getSessionId()) extends Store { + public int $getCalls = 0; + + public function get(UnitEnum|string $key, mixed $default = null): mixed + { + ++$this->getCalls; + + if ($key === 'first') { + return 'value'; + } + + throw new RuntimeException('The second key should not be read.'); + } + }; + + $this->assertTrue($session->hasAny(['first', 'second'])); + $this->assertSame(1, $session->getCalls); + } + + public function testKeyExists(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -573,7 +717,7 @@ public function testKeyExists() $this->assertFalse($session->exists(['hulk.two'])); } - public function testKeyMissing() + public function testKeyMissing(): void { $session = $this->getSession(); $session->put('foo', 'bar'); @@ -589,7 +733,7 @@ public function testKeyMissing() $this->assertTrue($session->missing(['hulk.two'])); } - public function testBackedEnumKeyPut() + public function testBackedEnumKeyPut(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -598,7 +742,7 @@ public function testBackedEnumKeyPut() $this->assertSame('Taylor', $session->get(SessionTestKey::User)); } - public function testBackedEnumKeyGet() + public function testBackedEnumKeyGet(): void { $session = $this->getSession(); $session->put('user', 'Taylor'); @@ -607,7 +751,7 @@ public function testBackedEnumKeyGet() $this->assertSame('default', $session->get(SessionTestKey::Settings, 'default')); } - public function testBackedEnumKeyHas() + public function testBackedEnumKeyHas(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -619,7 +763,7 @@ public function testBackedEnumKeyHas() $this->assertFalse($session->has(SessionTestKey::Preference)); } - public function testBackedEnumKeyHasAny() + public function testBackedEnumKeyHasAny(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -636,7 +780,7 @@ public function testBackedEnumKeyHasAny() $this->assertFalse($session->hasAny([SessionTestKey::Preference, 'foo'])); } - public function testBackedEnumKeyExists() + public function testBackedEnumKeyExists(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -650,7 +794,7 @@ public function testBackedEnumKeyExists() $this->assertFalse($session->exists('preference')); } - public function testBackedEnumKeyMissing() + public function testBackedEnumKeyMissing(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -664,7 +808,7 @@ public function testBackedEnumKeyMissing() $this->assertTrue($session->missing('preference')); } - public function testBackedEnumKeyForget() + public function testBackedEnumKeyForget(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -680,7 +824,7 @@ public function testBackedEnumKeyForget() $this->assertFalse($session->has('settings')); } - public function testBackedEnumKeyPull() + public function testBackedEnumKeyPull(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -690,7 +834,7 @@ public function testBackedEnumKeyPull() $this->assertSame('default', $session->pull(SessionTestKey::User, 'default')); } - public function testBackedEnumKeyRemember() + public function testBackedEnumKeyRemember(): void { $session = $this->getSession(); @@ -701,7 +845,7 @@ public function testBackedEnumKeyRemember() $this->assertSame('Taylor', $session->remember(SessionTestKey::User, fn () => 'Otwell')); } - public function testBackedEnumKeyPush() + public function testBackedEnumKeyPush(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, ['Taylor']); @@ -710,7 +854,7 @@ public function testBackedEnumKeyPush() $this->assertSame(['Taylor', 'Otwell'], $session->get('user')); } - public function testBackedEnumKeyIncrement() + public function testBackedEnumKeyIncrement(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 5); @@ -722,7 +866,7 @@ public function testBackedEnumKeyIncrement() $this->assertSame(10, $session->get('user')); } - public function testBackedEnumKeyDecrement() + public function testBackedEnumKeyDecrement(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 5); @@ -731,7 +875,7 @@ public function testBackedEnumKeyDecrement() $this->assertSame(4, $session->get('user')); } - public function testBackedEnumKeyRemove() + public function testBackedEnumKeyRemove(): void { $session = $this->getSession(); $session->put(SessionTestKey::User, 'Taylor'); @@ -740,21 +884,21 @@ public function testBackedEnumKeyRemove() $this->assertFalse($session->has('user')); } - public function testBackedEnumKeyFlash() + public function testBackedEnumKeyFlash(): void { $session = $this->getSession(); $session->flash(SessionTestKey::User, 'Taylor'); $this->assertTrue($session->has(SessionTestKey::User)); } - public function testBackedEnumKeyNow() + public function testBackedEnumKeyNow(): void { $session = $this->getSession(); $session->now(SessionTestKey::User, 'Taylor'); $this->assertTrue($session->has(SessionTestKey::User)); } - public function testRememberMethodCallsPutAndReturnsDefault() + public function testRememberMethodCallsPutAndReturnsDefault(): void { $session = $this->getSession(); $session->getHandler()->shouldReceive('get')->andReturn(null); @@ -765,7 +909,7 @@ public function testRememberMethodCallsPutAndReturnsDefault() $this->assertSame('bar', $result); } - public function testRememberMethodReturnsPreviousValueIfItAlreadySets() + public function testRememberMethodReturnsPreviousValueIfItAlreadySets(): void { $session = $this->getSession(); $session->put('key', 'foo'); @@ -776,20 +920,12 @@ public function testRememberMethodReturnsPreviousValueIfItAlreadySets() $this->assertSame('foo', $result); } - public function testValidationErrorsCanBeSerializedAsJson() + public function testValidationErrorsCanBeSerializedAsJson(): void { $session = $this->getSession('json'); - $session->getHandler()->shouldReceive('read')->once()->andReturn(serialize([])); + $session->getHandler()->shouldReceive('read')->once()->andReturn(json_encode([])); $session->start(); - $session->put('errors', $errorBag = new ViewErrorBag); - $messageBag = new MessageBag([ - 'first_name' => [ - 'Your first name is required', - 'Your first name must be at least 1 character', - ], - ]); - $messageBag->setFormat('

:message

'); - $errorBag->put('default', $messageBag); + $session->put('errors', $this->getErrorBag()); $session->getHandler()->shouldReceive('write')->once()->with( $this->getSessionId(), @@ -811,13 +947,149 @@ public function testValidationErrorsCanBeSerializedAsJson() 'new' => [], ], ]) + )->andReturnTrue(); + $session->save(); + + $this->assertFalse($session->isStarted()); + } + + public function testFailedJsonSaveKeepsLiveErrorBagAndFlashUntilRetry(): void + { + $handler = m::mock(SessionHandlerInterface::class); + $handler->shouldReceive('read')->once()->andReturn(json_encode([])); + + $attempts = 0; + $payloads = []; + $handler->shouldReceive('write')->twice()->andReturnUsing( + function (string $sessionId, string $data) use (&$attempts, &$payloads): bool { + $payloads[] = json_decode($data, true, flags: JSON_THROW_ON_ERROR); + + if (++$attempts === 1) { + throw new RuntimeException('Unable to persist the session.'); + } + + return true; + } ); + + $session = new Store('name', $handler, $this->getSessionId(), 'json'); + $session->start(); + $session->put('errors', $this->getErrorBag()); + $session->flash('status', 'saved'); + + try { + $session->save(); + + $this->fail('Expected the first session write to fail.'); + } catch (RuntimeException $exception) { + $this->assertSame('Unable to persist the session.', $exception->getMessage()); + } + + $this->assertInstanceOf(ViewErrorBag::class, $session->get('errors')); + $this->assertSame(['status'], $session->get('_flash.new')); + $this->assertSame([], $session->get('_flash.old')); + $this->assertTrue($session->isStarted()); + $session->save(); + $this->assertInstanceOf(ViewErrorBag::class, $session->get('errors')); + $this->assertSame([], $session->get('_flash.new')); + $this->assertSame(['status'], $session->get('_flash.old')); $this->assertFalse($session->isStarted()); + $this->assertSame($payloads[0], $payloads[1]); + } + + public function testConsecutiveJsonSavesKeepTheLiveErrorBag(): void + { + $handler = m::mock(SessionHandlerInterface::class); + $handler->shouldReceive('read')->once()->andReturn(json_encode([])); + $handler->shouldReceive('write')->twice()->andReturnTrue(); + + $session = new Store('name', $handler, $this->getSessionId(), 'json'); + $session->start(); + $session->put('errors', $this->getErrorBag()); + + $session->save(); + $session->save(); + + $this->assertInstanceOf(ViewErrorBag::class, $session->get('errors')); + } + + public function testStartingJsonSessionRetainsLiveErrorBagWhenStorageHasNone(): void + { + $handler = m::mock(SessionHandlerInterface::class); + $handler->shouldReceive('read')->once()->andReturn(json_encode([])); + + $session = new Store('name', $handler, $this->getSessionId(), 'json'); + $errorBag = $this->getErrorBag(); + $session->put('errors', $errorBag); + + $session->start(); + + $this->assertSame($errorBag, $session->get('errors')); + $this->assertSame([ + 'first_name' => [ + 'Your first name is required', + 'Your first name must be at least 1 character', + ], + ], $errorBag->getBag('default')->getMessages()); + } + + public function testPersistedJsonErrorBagOverridesLiveErrorBagOnStart(): void + { + $handler = m::mock(SessionHandlerInterface::class); + $handler->shouldReceive('read')->once()->andReturn(json_encode([ + 'errors' => [ + 'persisted' => [ + 'format' => ':message', + 'messages' => [ + 'email' => ['The email address is invalid.'], + ], + ], + ], + ])); + + $session = new Store('name', $handler, $this->getSessionId(), 'json'); + $liveErrorBag = $this->getErrorBag(); + $session->put('errors', $liveErrorBag); + + $session->start(); + + $errorBag = $session->get('errors'); + + $this->assertInstanceOf(ViewErrorBag::class, $errorBag); + $this->assertNotSame($liveErrorBag, $errorBag); + $this->assertFalse($errorBag->hasBag('default')); + $this->assertSame( + ['email' => ['The email address is invalid.']], + $errorBag->getBag('persisted')->getMessages() + ); } - public function testValidationErrorsCanBeReadAsJson() + public function testJsonEncodingFailureLeavesLiveStateUntouched(): void + { + $handler = m::mock(SessionHandlerInterface::class); + $handler->shouldReceive('read')->once()->andReturn(json_encode([])); + $handler->shouldReceive('write')->never(); + + $session = new Store('name', $handler, $this->getSessionId(), 'json'); + $session->start(); + + $recursive = []; + $recursive['self'] = &$recursive; + $session->put('recursive', $recursive); + + try { + $session->save(); + + $this->fail('Expected recursive session data to fail JSON encoding.'); + } catch (JsonException) { + $this->assertTrue($session->isStarted()); + $this->assertTrue($session->has('recursive')); + } + } + + public function testValidationErrorsCanBeReadAsJson(): void { $session = $this->getSession('json'); $session->getHandler()->shouldReceive('read')->once()->with($this->getSessionId())->andReturn(json_encode([ @@ -846,7 +1118,7 @@ public function testValidationErrorsCanBeReadAsJson() ]], $errors->getBags()['default']->getMessages()); } - public function testItIsMacroable() + public function testItIsMacroable(): void { $this->getSession()->macro('foo', function () { return 'macroable'; @@ -855,7 +1127,7 @@ public function testItIsMacroable() $this->assertSame('macroable', $this->getSession()->foo()); } - public function testFlushStateClearsMacros() + public function testFlushStateClearsMacros(): void { Store::macro('foo', function () { return 'macroable'; @@ -868,7 +1140,7 @@ public function testFlushStateClearsMacros() $this->assertFalse(Store::hasMacro('foo')); } - public function testSessionIdLengthConstant() + public function testSessionIdLengthConstant(): void { $session = $this->getSession(); $id = $session->getId(); @@ -878,7 +1150,7 @@ public function testSessionIdLengthConstant() $this->assertFalse($session->isValidId(str_repeat('a', 41))); } - public function testPreviousUri() + public function testPreviousUri(): void { $session = $this->getSession(); $session->setPreviousUrl('https://example.com/foo'); @@ -888,7 +1160,7 @@ public function testPreviousUri() $this->assertSame('https://example.com/foo', (string) $uri); } - public function testPreviousUriThrowsWhenNoPreviousUrl() + public function testPreviousUriThrowsWhenNoPreviousUrl(): void { $session = $this->getSession(); @@ -898,7 +1170,7 @@ public function testPreviousUriThrowsWhenNoPreviousUrl() $session->previousUri(); } - public function testPreviousRoute() + public function testPreviousRoute(): void { $session = $this->getSession(); $this->assertNull($session->previousRoute()); @@ -907,7 +1179,7 @@ public function testPreviousRoute() $this->assertSame('home.index', $session->previousRoute()); } - public function testSetPreviousRoute() + public function testSetPreviousRoute(): void { $session = $this->getSession(); $session->setPreviousRoute('dashboard'); @@ -917,6 +1189,19 @@ public function testSetPreviousRoute() $this->assertNull($session->get('_previous.route')); } + protected function getErrorBag(): ViewErrorBag + { + $messageBag = new MessageBag([ + 'first_name' => [ + 'Your first name is required', + 'Your first name must be at least 1 character', + ], + ]); + $messageBag->setFormat('

:message

'); + + return (new ViewErrorBag)->put('default', $messageBag); + } + public function getSession(string $serialization = 'php'): Store { return new Store( From 90f9cb5baacd18ebdf6837b89f00a2d61c5c9a69 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:43:18 +0000 Subject: [PATCH 03/16] fix(session): make middleware commit and lock ownership explicit Use the request session as the startup commit flag so failed reads cannot schedule an empty after-response write. Preserve the primary rendered failure when a retry also fails, while retaining persistence after route and render failures. Delegate blocked-route acquisition and cleanup to Cache lock callback ownership so timeout paths never release an unacquired lock and request failures remain primary over release failures. Add focused middleware and Testbench-backed persistence regressions for every failure boundary. --- src/session/src/Middleware/StartSession.php | 62 ++-- .../Session/SessionPersistenceTest.php | 47 ++- tests/Session/Middleware/StartSessionTest.php | 305 +++++++++++++++++- 3 files changed, 365 insertions(+), 49 deletions(-) diff --git a/src/session/src/Middleware/StartSession.php b/src/session/src/Middleware/StartSession.php index a1377c76f..5d3172a13 100644 --- a/src/session/src/Middleware/StartSession.php +++ b/src/session/src/Middleware/StartSession.php @@ -51,7 +51,7 @@ public function handle(Request $request, Closure $next): Response $session = $this->getSession($request); if ($this->manager->shouldBlock() - || ($request->route() instanceof Route && $request->route()->locksFor())) { // @phpstan-ignore instanceof.alwaysTrue + || ($request->route() instanceof Route && $request->route()->locksFor())) { return $this->handleRequestWhileBlocking($request, $session, $next); } @@ -63,30 +63,24 @@ public function handle(Request $request, Closure $next): Response */ protected function handleRequestWhileBlocking(Request $request, Session $session, Closure $next): Response { - if (! $request->route() instanceof Route) { // @phpstan-ignore instanceof.alwaysTrue + if (! $request->route() instanceof Route) { return $this->handleStatefulRequest($request, $session, $next); } $lockFor = $request->route()->locksFor() ?: $this->manager->defaultRouteBlockLockSeconds(); - /** @var \Hypervel\Contracts\Cache\Repository&LockProvider $store */ // @phpstan-ignore varTag.nativeType + /** @var \Hypervel\Contracts\Cache\Repository&LockProvider $store */ $store = $this->cache->store($this->manager->blockDriver()); $lock = $store ->lock('session:' . $session->getId(), (int) $lockFor) ->betweenBlockedAttemptsSleepFor(50); - try { - $lock->block( - ! is_null($request->route()->waitsFor()) - ? $request->route()->waitsFor() - : $this->manager->defaultRouteBlockWaitSeconds() - ); - - return $this->handleStatefulRequest($request, $session, $next); - } finally { - $lock->release(); - } + return $lock->block( + $request->route()->waitsFor() + ?? $this->manager->defaultRouteBlockWaitSeconds(), + fn (): Response => $this->handleStatefulRequest($request, $session, $next), + ); } /** @@ -120,12 +114,18 @@ protected function handleStatefulRequest(Request $request, Session $session, Clo $this->saveSession($request); return $response; - } catch (Throwable $e) { - $this->exceptionHandler->afterResponse( - fn () => $this->saveSession($request) - ); + } catch (Throwable $throwable) { + if ($request->hasSession()) { + $this->exceptionHandler->afterResponse(function () use ($request): void { + try { + $this->saveSession($request); + } catch (Throwable) { + // The request failure stays primary; a retry failure must not escape the exception renderer. + } + }); + } - throw $e; + throw $throwable; } } @@ -180,7 +180,7 @@ protected function configHitsLottery(array $config): bool protected function storeCurrentUrl(Request $request, Session $session): void { if ($request->isMethod('GET') - && $request->route() instanceof Route // @phpstan-ignore instanceof.alwaysTrue + && $request->route() instanceof Route && ! $request->ajax() && ! $request->prefetch() && ! $request->isPrecognitive()) { @@ -218,17 +218,17 @@ protected function addCookieToResponse(Request $request, Response $response, Ses /** * Get the session cookie configuration. * - * @return array{path: string, domain: string, secure: ?bool, http_only: bool, same_site: ?string, partitioned: bool} + * @return array{path: string, domain: ?string, secure: ?bool, http_only: bool, same_site: ?string, partitioned: bool} */ protected function resolveSessionCookieConfig(Request $request, array $config): array { $cookieConfig = [ - 'path' => $config['path'] ?? '/', - 'domain' => $config['domain'] ?? '', - 'secure' => $config['secure'] ?? null, - 'http_only' => $config['http_only'] ?? true, - 'same_site' => $config['same_site'] ?? null, - 'partitioned' => $config['partitioned'] ?? false, + 'path' => $config['path'], + 'domain' => $config['domain'], + 'secure' => $config['secure'], + 'http_only' => $config['http_only'], + 'same_site' => $config['same_site'], + 'partitioned' => $config['partitioned'], ]; foreach (static::$sessionCookieCallbacks as $callback) { @@ -268,7 +268,7 @@ protected function saveSession(Request $request): void */ protected function getSessionLifetimeInSeconds(): int { - return ($this->manager->getSessionConfig()['lifetime'] ?? null) * 60; + return $this->manager->getSessionConfig()['lifetime'] * 60; } /** @@ -288,7 +288,7 @@ protected function getCookieExpirationDate(): DateTimeInterface|int */ protected function sessionConfigured(): bool { - return ! is_null($this->manager->getSessionConfig()['driver'] ?? null); + return ! is_null($this->manager->getSessionConfig()['driver']); } /** @@ -296,9 +296,9 @@ protected function sessionConfigured(): bool */ protected function sessionIsPersistent(?array $config = null): bool { - $config = $config ?: $this->manager->getSessionConfig(); + $config ??= $this->manager->getSessionConfig(); - return ! is_null($config['driver'] ?? null); + return ! is_null($config['driver']); } /** diff --git a/tests/Integration/Session/SessionPersistenceTest.php b/tests/Integration/Session/SessionPersistenceTest.php index b2ab875e0..097fce7cd 100644 --- a/tests/Integration/Session/SessionPersistenceTest.php +++ b/tests/Integration/Session/SessionPersistenceTest.php @@ -4,19 +4,18 @@ namespace Hypervel\Tests\Integration\Session; -use Hypervel\Contracts\Debug\ExceptionHandler; -use Hypervel\Http\Response; use Hypervel\Session\NullSessionHandler; use Hypervel\Session\TokenMismatchException; +use Hypervel\Support\Facades\Exceptions; use Hypervel\Support\Facades\Route; use Hypervel\Support\Facades\Session; use Hypervel\Support\Str; use Hypervel\Testbench\TestCase; -use Mockery as m; +use RuntimeException; class SessionPersistenceTest extends TestCase { - public function testSessionIsPersistedEvenIfExceptionIsThrownFromRoute() + public function testSessionIsPersistedEvenIfExceptionIsThrownFromRoute(): void { $handler = new FakeNullSessionHandler; $this->assertFalse($handler->written); @@ -33,15 +32,29 @@ public function testSessionIsPersistedEvenIfExceptionIsThrownFromRoute() $this->assertTrue($handler->written); } - protected function defineEnvironment($app): void + public function testPersistentSaveFailureIsRenderedWithoutRetryFailureEscaping(): void { - $app->instance( - ExceptionHandler::class, - $handler = m::mock(ExceptionHandler::class)->shouldIgnoreMissing() - ); + $handler = new FailingNullSessionHandler; + + Session::extend('failing-null', fn () => $handler); + + Route::get('/', fn () => 'response')->middleware('web'); + + $this->app->make('config')->set('session.driver', 'failing-null'); + Exceptions::fake(); - $handler->shouldReceive('render')->andReturn(new Response); + $response = $this->get('/'); + $response->assertInternalServerError(); + Exceptions::assertReported( + fn (RuntimeException $exception): bool => $exception->getMessage() === 'Unable to persist the session.' + ); + Exceptions::assertReportedCount(1); + $this->assertSame(2, $handler->writeCount); + } + + protected function defineEnvironment($app): void + { $app['config']->set('app.key', Str::random(32)); $app['config']->set('session.driver', 'fake-null'); $app['config']->set('session.expire_on_close', true); @@ -52,10 +65,22 @@ class FakeNullSessionHandler extends NullSessionHandler { public bool $written = false; - public function write($sessionId, $data): bool + public function write(string $sessionId, string $data): bool { $this->written = true; return true; } } + +class FailingNullSessionHandler extends NullSessionHandler +{ + public int $writeCount = 0; + + public function write(string $sessionId, string $data): bool + { + ++$this->writeCount; + + throw new RuntimeException('Unable to persist the session.'); + } +} diff --git a/tests/Session/Middleware/StartSessionTest.php b/tests/Session/Middleware/StartSessionTest.php index 048ab229d..92e595fbb 100644 --- a/tests/Session/Middleware/StartSessionTest.php +++ b/tests/Session/Middleware/StartSessionTest.php @@ -4,10 +4,23 @@ namespace Hypervel\Tests\Session\Middleware; +use Closure; +use Hypervel\Cache\Lock; +use Hypervel\Contracts\Cache\Factory as CacheFactoryContract; +use Hypervel\Contracts\Cache\LockProvider; +use Hypervel\Contracts\Cache\LockTimeoutException; +use Hypervel\Contracts\Cache\Repository; +use Hypervel\Contracts\Debug\ExceptionHandler as ExceptionHandlerContract; +use Hypervel\Contracts\Session\Session; use Hypervel\Http\Request; +use Hypervel\Routing\Route; use Hypervel\Session\Middleware\StartSession; +use Hypervel\Session\SessionManager; use Hypervel\Support\ClassInvoker; use Hypervel\Tests\TestCase; +use Mockery as m; +use RuntimeException; +use Symfony\Component\HttpFoundation\Response; class StartSessionTest extends TestCase { @@ -15,10 +28,14 @@ public function testResolveSessionCookieConfigReturnsDefaults(): void { $middleware = $this->createStartSessionMock(); - $config = $this->invokeResolveSessionCookieConfig($middleware, Request::create('/'), []); + $config = $this->invokeResolveSessionCookieConfig( + $middleware, + Request::create('/'), + $this->defaultCookieConfig(), + ); $this->assertSame('/', $config['path']); - $this->assertSame('', $config['domain']); + $this->assertNull($config['domain']); $this->assertNull($config['secure']); $this->assertTrue($config['http_only']); $this->assertNull($config['same_site']); @@ -30,6 +47,7 @@ public function testResolveSessionCookieConfigReturnsConfiguredValues(): void $middleware = $this->createStartSessionMock(); $config = $this->invokeResolveSessionCookieConfig($middleware, Request::create('/'), [ + ...$this->defaultCookieConfig(), 'path' => '/app', 'domain' => '.example.com', 'secure' => true, @@ -57,7 +75,7 @@ public function testSessionCookieConfigCanBeConfiguredUsingCallback(): void }); $config = $this->invokeResolveSessionCookieConfig($middleware, Request::create('/'), [ - 'path' => '/', + ...$this->defaultCookieConfig(), 'domain' => '.example.com', ]); @@ -78,7 +96,7 @@ public function testSessionCookieConfigCallbacksReceiveRequest(): void $config = $this->invokeResolveSessionCookieConfig( $middleware, Request::create('https://tenant.example.com'), - [] + $this->defaultCookieConfig(), ); $this->assertSame('.tenant.example.com', $config['domain']); @@ -100,7 +118,11 @@ public function testSessionCookieConfigCallbacksComposeInRegistrationOrder(): vo return $cookie; }); - $config = $this->invokeResolveSessionCookieConfig($middleware, Request::create('/'), []); + $config = $this->invokeResolveSessionCookieConfig( + $middleware, + Request::create('/'), + $this->defaultCookieConfig(), + ); $this->assertSame('.first.example.com', $config['domain']); $this->assertSame('lax', $config['same_site']); @@ -118,12 +140,198 @@ public function testFlushStateClearsSessionCookieCallbacks(): void $this->assertSame( '.custom.example.com', - $this->invokeResolveSessionCookieConfig($middleware, Request::create('/'), [])['domain'] + $this->invokeResolveSessionCookieConfig( + $middleware, + Request::create('/'), + $this->defaultCookieConfig(), + )['domain'] ); StartSession::flushState(); - $this->assertSame('', $this->invokeResolveSessionCookieConfig($middleware, Request::create('/'), [])['domain']); + $this->assertNull($this->invokeResolveSessionCookieConfig( + $middleware, + Request::create('/'), + $this->defaultCookieConfig(), + )['domain']); + } + + public function testFailedSessionStartupDoesNotRegisterPersistenceRetry(): void + { + $request = Request::create('/'); + $failure = new RuntimeException('read failure'); + $manager = m::mock(SessionManager::class); + $cache = m::mock(CacheFactoryContract::class); + $exceptionHandler = m::mock(ExceptionHandlerContract::class); + $session = m::mock(Session::class); + $middleware = new StartSession($manager, $cache, $exceptionHandler); + + $session->shouldReceive('setRequestOnHandler')->once()->with($request); + $session->shouldReceive('start')->once()->andThrow($failure); + $exceptionHandler->shouldNotReceive('afterResponse'); + + try { + (new ClassInvoker($middleware))->handleStatefulRequest( + $request, + $session, + fn () => $this->fail('The request pipeline should not run after session startup fails.'), + ); + + $this->fail('Expected session startup to fail.'); + } catch (RuntimeException $exception) { + $this->assertSame($failure, $exception); + } + + $this->assertFalse($request->hasSession()); + } + + public function testFailureAfterSuccessfulStartupRegistersPersistenceRetry(): void + { + $request = Request::create('/'); + $failure = new RuntimeException('route failure'); + $manager = m::mock(SessionManager::class); + $cache = m::mock(CacheFactoryContract::class); + $exceptionHandler = m::mock(ExceptionHandlerContract::class); + $session = m::mock(Session::class); + $middleware = new StartSession($manager, $cache, $exceptionHandler); + $afterResponse = null; + + $session->shouldReceive('setRequestOnHandler')->once()->with($request); + $session->shouldReceive('start')->once()->andReturnTrue(); + $manager->shouldReceive('getSessionConfig')->once()->andReturn([ + 'lottery' => [0, 1], + ]); + $exceptionHandler->shouldReceive('afterResponse')->once()->andReturnUsing( + function (callable $callback) use (&$afterResponse): void { + $afterResponse = $callback; + } + ); + $manager->shouldReceive('driver')->once()->andReturn($session); + $session->shouldReceive('save')->once(); + + try { + (new ClassInvoker($middleware))->handleStatefulRequest( + $request, + $session, + fn () => throw $failure, + ); + + $this->fail('Expected the route failure to be rethrown.'); + } catch (RuntimeException $exception) { + $this->assertSame($failure, $exception); + } + + $this->assertTrue($request->hasSession()); + $this->assertIsCallable($afterResponse); + + $afterResponse(); + } + + public function testPersistenceRetryFailureDoesNotReplacePrimaryFailure(): void + { + $request = Request::create('/'); + $failure = new RuntimeException('write failure'); + $manager = m::mock(SessionManager::class); + $cache = m::mock(CacheFactoryContract::class); + $exceptionHandler = m::mock(ExceptionHandlerContract::class); + $session = m::mock(Session::class); + $middleware = new StartSession($manager, $cache, $exceptionHandler); + $afterResponse = null; + + $session->shouldReceive('setRequestOnHandler')->once()->with($request); + $session->shouldReceive('start')->once()->andReturnTrue(); + $manager->shouldReceive('getSessionConfig')->twice()->andReturn([ + 'lottery' => [0, 1], + 'driver' => null, + ]); + $manager->shouldReceive('driver')->twice()->andReturn($session); + $session->shouldReceive('save')->twice()->andThrow($failure); + $exceptionHandler->shouldReceive('afterResponse')->once()->andReturnUsing( + function (callable $callback) use (&$afterResponse): void { + $afterResponse = $callback; + } + ); + + try { + (new ClassInvoker($middleware))->handleStatefulRequest( + $request, + $session, + fn () => new Response, + ); + + $this->fail('Expected session persistence to fail.'); + } catch (RuntimeException $exception) { + $this->assertSame($failure, $exception); + } + + $this->assertIsCallable($afterResponse); + + $afterResponse(); + $this->addToAssertionCount(1); + } + + public function testBlockingPreservesRequestFailureWhenReleaseAlsoFails(): void + { + $request = Request::create('/'); + $route = (new Route('GET', '/', fn () => new Response))->block(10, 0); + $request->setRouteResolver(fn (): Route => $route); + $manager = m::mock(SessionManager::class); + $cache = m::mock(CacheFactoryContract::class); + $exceptionHandler = m::mock(ExceptionHandlerContract::class); + $session = m::mock(Session::class); + $store = m::mock(Repository::class, LockProvider::class); + $lock = new StartSessionFailingReleaseLock; + $middleware = new StartSessionThrowingMiddleware($manager, $cache, $exceptionHandler); + + $manager->shouldReceive('blockDriver')->once()->andReturn('array'); + $cache->shouldReceive('store')->once()->with('array')->andReturn($store); + $session->shouldReceive('getId')->once()->andReturn('session-id'); + $store->shouldReceive('lock')->once()->with('session:session-id', 10)->andReturn($lock); + + try { + (new ClassInvoker($middleware))->handleRequestWhileBlocking( + $request, + $session, + fn () => new Response, + ); + + $this->fail('Expected the request pipeline to fail.'); + } catch (RuntimeException $exception) { + $this->assertSame('request failure', $exception->getMessage()); + } + + $this->assertTrue($lock->released); + } + + public function testBlockingTimeoutDoesNotReleaseUnacquiredLock(): void + { + $request = Request::create('/'); + $route = (new Route('GET', '/', fn () => new Response))->block(10, 0); + $request->setRouteResolver(fn (): Route => $route); + $manager = m::mock(SessionManager::class); + $cache = m::mock(CacheFactoryContract::class); + $exceptionHandler = m::mock(ExceptionHandlerContract::class); + $session = m::mock(Session::class); + $store = m::mock(Repository::class, LockProvider::class); + $lock = new StartSessionUnacquirableLock; + $middleware = new StartSession($manager, $cache, $exceptionHandler); + + $manager->shouldReceive('blockDriver')->once()->andReturn('array'); + $cache->shouldReceive('store')->once()->with('array')->andReturn($store); + $session->shouldReceive('getId')->once()->andReturn('session-id'); + $store->shouldReceive('lock')->once()->with('session:session-id', 10)->andReturn($lock); + + try { + (new ClassInvoker($middleware))->handleRequestWhileBlocking( + $request, + $session, + fn () => $this->fail('The request pipeline should not run without the lock.'), + ); + + $this->fail('Expected lock acquisition to time out.'); + } catch (LockTimeoutException) { + $this->assertFalse($lock->released); + } } private function createStartSessionMock(): StartSession @@ -135,6 +343,21 @@ private function invokeResolveSessionCookieConfig(StartSession $middleware, Requ { return (new ClassInvoker($middleware))->resolveSessionCookieConfig($request, $config); } + + /** + * @return array{path: string, domain: ?string, secure: ?bool, http_only: bool, same_site: ?string, partitioned: bool} + */ + private function defaultCookieConfig(): array + { + return [ + 'path' => '/', + 'domain' => null, + 'secure' => null, + 'http_only' => true, + 'same_site' => null, + 'partitioned' => false, + ]; + } } class TestStartSession extends StartSession @@ -144,3 +367,71 @@ public function __construct() // Skip parent constructor for testing. } } + +class StartSessionThrowingMiddleware extends StartSession +{ + protected function handleStatefulRequest(Request $request, Session $session, Closure $next): Response + { + throw new RuntimeException('request failure'); + } +} + +class StartSessionFailingReleaseLock extends Lock +{ + public bool $released = false; + + public function __construct() + { + parent::__construct('session', 10, 'owner'); + } + + public function acquire(): bool + { + return true; + } + + public function release(): bool + { + $this->released = true; + + throw new RuntimeException('release failure'); + } + + public function forceRelease(): void + { + } + + protected function getCurrentOwner(): ?string + { + return $this->owner; + } +} + +class StartSessionUnacquirableLock extends Lock +{ + public bool $released = false; + + public function __construct() + { + parent::__construct('session', 10, 'owner'); + } + + public function acquire(): bool + { + return false; + } + + public function release(): bool + { + return $this->released = true; + } + + public function forceRelease(): void + { + } + + protected function getCurrentOwner(): ?string + { + return null; + } +} From 1cf6617ba8b771505edf537a6b340d02c3a9e636 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:43:28 +0000 Subject: [PATCH 04/16] fix(session): make cookie persistence binary safe Replace the JSON transport envelope with private PHP serialization so arbitrary serialized session bytes round-trip without UTF-8 loss. Decode untrusted cookies with classes disabled and accept only the exact string payload and integer expiry shape. Cover binary data, expiration, malformed frames, object rejection, strict field types, real CookieJar transport, coroutine isolation, and current test typing without adding a compatibility decoder or a second framing layer. --- src/session/src/CookieSessionHandler.php | 19 +++--- .../Session/CookieSessionHandlerTest.php | 4 +- ...ookieSessionHandlerCoroutineSafetyTest.php | 4 +- tests/Session/CookieSessionHandlerTest.php | 58 +++++++++++++++++++ 4 files changed, 72 insertions(+), 13 deletions(-) create mode 100644 tests/Session/CookieSessionHandlerTest.php diff --git a/src/session/src/CookieSessionHandler.php b/src/session/src/CookieSessionHandler.php index 92741da13..31c540978 100644 --- a/src/session/src/CookieSessionHandler.php +++ b/src/session/src/CookieSessionHandler.php @@ -46,21 +46,22 @@ public function close(): bool public function read(string $sessionId): false|string { $value = $this->getRequest()->cookies->get($sessionId) ?: ''; - - if (! is_null($decoded = json_decode($value, true)) - && is_array($decoded) - && isset($decoded['expires']) - && $this->currentTime() <= $decoded['expires'] - ) { - return $decoded['data']; + $decoded = @unserialize($value, ['allowed_classes' => false]); + + if (! is_array($decoded) + || ! isset($decoded['data'], $decoded['expires']) + || ! is_string($decoded['data']) + || ! is_int($decoded['expires']) + || $this->currentTime() > $decoded['expires']) { + return ''; } - return ''; + return $decoded['data']; } public function write(string $sessionId, string $data): bool { - $this->cookie->queue($sessionId, json_encode([ + $this->cookie->queue($sessionId, serialize([ 'data' => $data, 'expires' => $this->availableAt($this->minutes * 60), ]), $this->expireOnClose ? 0 : $this->minutes); diff --git a/tests/Integration/Session/CookieSessionHandlerTest.php b/tests/Integration/Session/CookieSessionHandlerTest.php index d75cb1ea0..7a3676907 100644 --- a/tests/Integration/Session/CookieSessionHandlerTest.php +++ b/tests/Integration/Session/CookieSessionHandlerTest.php @@ -10,7 +10,7 @@ class CookieSessionHandlerTest extends TestCase { - public function testCookieSessionDriverCookiesCanExpireOnClose() + public function testCookieSessionDriverCookiesCanExpireOnClose(): void { Route::get('/', fn () => '')->middleware('web'); @@ -22,7 +22,7 @@ public function testCookieSessionDriverCookiesCanExpireOnClose() $this->assertEquals(0, $sessionValueCookie->getExpiresTime()); } - public function testCookieSessionInheritsRequestSecureState() + public function testCookieSessionInheritsRequestSecureState(): void { Route::get('/', fn () => '')->middleware('web'); diff --git a/tests/Session/CookieSessionHandlerCoroutineSafetyTest.php b/tests/Session/CookieSessionHandlerCoroutineSafetyTest.php index 38f38d378..6f2f2fa46 100644 --- a/tests/Session/CookieSessionHandlerCoroutineSafetyTest.php +++ b/tests/Session/CookieSessionHandlerCoroutineSafetyTest.php @@ -24,7 +24,7 @@ public function testCookieSessionHandlerRequestIsCoroutineIsolated(): void [$resultA, $resultB] = parallel([ function () use ($handler): string { $handler->setRequest(Request::create('/', 'GET', [], [ - 'session-a' => json_encode([ + 'session-a' => serialize([ 'data' => 'payload-a', 'expires' => time() + 60, ]), @@ -38,7 +38,7 @@ function () use ($handler): string { usleep(2500); $handler->setRequest(Request::create('/', 'GET', [], [ - 'session-b' => json_encode([ + 'session-b' => serialize([ 'data' => 'payload-b', 'expires' => time() + 60, ]), diff --git a/tests/Session/CookieSessionHandlerTest.php b/tests/Session/CookieSessionHandlerTest.php new file mode 100644 index 000000000..067960dc5 --- /dev/null +++ b/tests/Session/CookieSessionHandlerTest.php @@ -0,0 +1,58 @@ + "\x00\xff\x10"]); + $cookieValue = null; + + $cookie->shouldReceive('queue') + ->once() + ->withArgs(function (string $name, string $value, int $minutes) use (&$cookieValue): bool { + $this->assertSame('session-id', $name); + $this->assertSame(120, $minutes); + $cookieValue = $value; + + return true; + }); + + $this->assertTrue($handler->write('session-id', $payload)); + $this->assertIsString($cookieValue); + + $handler->setRequest(Request::create('/', cookies: ['session-id' => $cookieValue])); + + $this->assertSame($payload, $handler->read('session-id')); + } + + public function testInvalidCookieEnvelopesReturnEmptyString(): void + { + $handler = new CookieSessionHandler(m::mock(QueueingFactory::class), 120); + + foreach ([ + 'garbage' => 'not serialized data', + 'top-level object' => serialize(new stdClass), + 'missing data' => serialize(['expires' => time() + 60]), + 'non-string data' => serialize(['data' => ['payload'], 'expires' => time() + 60]), + 'non-integer expiry' => serialize(['data' => 'payload', 'expires' => (string) (time() + 60)]), + 'expired' => serialize(['data' => 'payload', 'expires' => time() - 1]), + ] as $description => $cookieValue) { + $handler->setRequest(Request::create('/', cookies: ['session-id' => $cookieValue])); + + $this->assertSame('', $handler->read('session-id'), $description); + } + } +} From 38eaa362ac1a231872428a7432b46c6f789b71e8 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:43:36 +0000 Subject: [PATCH 05/16] fix(session): validate file persistence and cleanup results Require Filesystem writes to return the complete byte count before reporting a successful session save. Use Finder pathnames directly during garbage collection and count only files that were actually deleted. Align the concrete Filesystem delete signature with its existing contract and sibling implementations, keep destroy idempotent, and cover successful, false, partial, empty, and garbage-collection paths in isolated temporary storage. --- src/filesystem/src/Filesystem.php | 4 +- src/session/src/FileSessionHandler.php | 13 ++-- tests/Session/FileSessionHandlerTest.php | 83 ++++++++++++++++++------ 3 files changed, 69 insertions(+), 31 deletions(-) diff --git a/src/filesystem/src/Filesystem.php b/src/filesystem/src/Filesystem.php index e67ae7077..0813dd0e6 100644 --- a/src/filesystem/src/Filesystem.php +++ b/src/filesystem/src/Filesystem.php @@ -306,11 +306,9 @@ public function chmod(string $path, ?int $mode = null): string|bool /** * Delete the file at a given path. * - * @param array|string $paths - * * @phpstan-impure */ - public function delete($paths): bool + public function delete(array|string $paths): bool { $paths = is_array($paths) ? $paths : func_get_args(); diff --git a/src/session/src/FileSessionHandler.php b/src/session/src/FileSessionHandler.php index d575cf67c..5076efffc 100644 --- a/src/session/src/FileSessionHandler.php +++ b/src/session/src/FileSessionHandler.php @@ -21,8 +21,8 @@ class FileSessionHandler implements SessionHandlerInterface */ public function __construct( protected Filesystem $files, - protected $path, - protected $minutes + protected string $path, + protected int $minutes ) { } @@ -53,9 +53,7 @@ public function read(string $sessionId): false|string public function write(string $sessionId, string $data): bool { - $this->files->put($this->path . '/' . $sessionId, $data, true); - - return true; + return $this->files->put($this->path . '/' . $sessionId, $data, true) === strlen($data); } public function destroy(string $sessionId): bool @@ -76,8 +74,9 @@ public function gc(int $lifetime): int $deletedSessions = 0; foreach ($files as $file) { - $this->files->delete($file->getRealPath()); - ++$deletedSessions; + if ($this->files->delete($file->getPathname())) { + ++$deletedSessions; + } } return $deletedSessions; diff --git a/tests/Session/FileSessionHandlerTest.php b/tests/Session/FileSessionHandlerTest.php index 2ca503cec..9022716b1 100644 --- a/tests/Session/FileSessionHandlerTest.php +++ b/tests/Session/FileSessionHandlerTest.php @@ -7,9 +7,12 @@ use Hypervel\Contracts\Filesystem\FileNotFoundException; use Hypervel\Filesystem\Filesystem; use Hypervel\Session\FileSessionHandler; +use Hypervel\Session\Store; use Hypervel\Support\CarbonImmutable; +use Hypervel\Testing\ParallelTesting; use Hypervel\Tests\TestCase; use Mockery as m; +use RuntimeException; use function Hypervel\Filesystem\join_paths; @@ -27,12 +30,12 @@ protected function setUp(): void $this->sessionHandler = new FileSessionHandler($this->files, '/path/to/sessions', 30); } - public function testOpen() + public function testOpen(): void { $this->assertTrue($this->sessionHandler->open('/path/to/sessions', 'session_name')); } - public function testClose() + public function testClose(): void { $this->assertTrue($this->sessionHandler->close()); } @@ -71,7 +74,7 @@ public function testReadReturnsEmptyWhenFileExistsButExpired(): void $this->assertSame('', $result); } - public function testReadReturnsEmptyStringWhenFileDoesNotExist() + public function testReadReturnsEmptyStringWhenFileDoesNotExist(): void { $sessionId = 'non_existing_session_id'; $path = '/path/to/sessions/' . $sessionId; @@ -98,7 +101,7 @@ public function testReadReturnsEmptyStringWhenTheSessionFileDisappearsBeforeRead $this->assertSame('', $this->sessionHandler->read($sessionId)); } - public function testWriteStoresData() + public function testWriteStoresData(): void { $sessionId = 'session_id'; $data = 'session_data'; @@ -110,7 +113,47 @@ public function testWriteStoresData() $this->assertTrue($result); } - public function testDestroyDeletesSessionFile() + public function testWriteRejectsFalseAndShortFilesystemWrites(): void + { + $data = 'session_data'; + + foreach ([false, strlen($data) - 1] as $written) { + $files = m::mock(Filesystem::class); + $files->shouldReceive('put') + ->once() + ->with('/path/to/sessions/session_id', $data, true) + ->andReturn($written); + + $handler = new FileSessionHandler($files, '/path/to/sessions', 30); + + $this->assertFalse($handler->write('session_id', $data)); + } + } + + public function testFailedFileWriteLeavesLiveSessionStateUntouched(): void + { + $sessionId = str_repeat('a', 40); + $path = '/path/to/sessions/' . $sessionId; + $this->files->shouldReceive('isFile')->once()->with($path)->andReturnFalse(); + $this->files->shouldReceive('put')->once()->with($path, m::type('string'), true)->andReturnFalse(); + + $session = new Store('name', $this->sessionHandler, $sessionId); + $session->start(); + $session->flash('status', 'saved'); + + try { + $session->save(); + + $this->fail('Expected the failed file write to reject the session save.'); + } catch (RuntimeException $exception) { + $this->assertSame('Unable to write the session data.', $exception->getMessage()); + } + + $this->assertTrue($session->isStarted()); + $this->assertSame(['status'], $session->get('_flash.new')); + } + + public function testDestroyDeletesSessionFile(): void { $sessionId = 'session_id'; @@ -121,26 +164,24 @@ public function testDestroyDeletesSessionFile() $this->assertTrue($result); } - public function testGcDeletesOldSessionFiles() + public function testGcDeletesOldSessionFiles(): void { - $session = new FileSessionHandler($this->files, join_paths(__DIR__, 'tmp'), 30); - - $this->files->shouldReceive('delete')->with(join_paths(__DIR__, 'tmp', 'a2'))->once()->andReturn(false); - $this->files->shouldReceive('delete')->with(join_paths(__DIR__, 'tmp', 'a3'))->once()->andReturn(true); - - mkdir(__DIR__ . '/tmp'); - touch(__DIR__ . '/tmp/a1', time() - 3); // last modified: 3 sec ago - touch(__DIR__ . '/tmp/a2', time() - 5); // last modified: 5 sec ago - touch(__DIR__ . '/tmp/a3', time() - 7); // last modified: 7 sec ago + $tempDir = ParallelTesting::tempDir('FileSessionHandlerTest'); + mkdir($tempDir, 0777, true); - $count = $session->gc(5); + try { + $session = new FileSessionHandler($this->files, $tempDir, 30); - $this->assertSame(2, $count); + $this->files->shouldReceive('delete')->with(join_paths($tempDir, 'a2'))->once()->andReturn(false); + $this->files->shouldReceive('delete')->with(join_paths($tempDir, 'a3'))->once()->andReturn(true); - unlink(__DIR__ . '/tmp/a1'); - unlink(__DIR__ . '/tmp/a2'); - unlink(__DIR__ . '/tmp/a3'); + touch(join_paths($tempDir, 'a1'), time() - 3); // last modified: 3 sec ago + touch(join_paths($tempDir, 'a2'), time() - 5); // last modified: 5 sec ago + touch(join_paths($tempDir, 'a3'), time() - 7); // last modified: 7 sec ago - rmdir(__DIR__ . '/tmp'); + $this->assertSame(1, $session->gc(5)); + } finally { + (new Filesystem)->deleteDirectory($tempDir); + } } } From af7e97c4c735ab23c1c9696c77a22a2a02a0d0b2 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:43:43 +0000 Subject: [PATCH 06/16] fix(session): reset database handler existence state Refresh the local existence decision after a cold read so direct writes update existing rows instead of attempting duplicate inserts. Reset the object-specific existence slot at construction and cloning so reused PHP object IDs cannot silently update zero rows. Remove the unsafe unused connection mutator, retain fresh pooled connection resolution, document the shared container mutator as boot/test-only, and cover direct update plus deterministic constructor and clone identity reuse against the database harness. --- src/session/src/DatabaseSessionHandler.php | 31 +++--- .../Session/DatabaseSessionHandlerTest.php | 98 ++++++++++++++++++- 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/src/session/src/DatabaseSessionHandler.php b/src/session/src/DatabaseSessionHandler.php index 56b560c8a..2e75eebe3 100644 --- a/src/session/src/DatabaseSessionHandler.php +++ b/src/session/src/DatabaseSessionHandler.php @@ -44,6 +44,7 @@ public function __construct( protected int $minutes, protected ?Container $container = null ) { + $this->setExists(false); } public function open(string $savePath, string $sessionName): bool @@ -88,8 +89,11 @@ public function write(string $sessionId, string $data): bool { $payload = $this->getDefaultPayload($data); - if (! $exists = $this->getExists()) { + $exists = $this->getExists(); + + if (! $exists) { $this->read($sessionId); + $exists = $this->getExists(); } if ($exists) { @@ -228,18 +232,12 @@ public function connection(): ConnectionInterface return $this->resolver->connection($this->connection); } - /** - * Set the connection name to be used. - */ - public function setConnection(?string $connection): static - { - $this->connection = $connection; - - return $this; - } - /** * Set the application instance used by the handler. + * + * Boot or tests only. Mutating the container on a shared handler during + * request handling can expose the wrong request or authentication state + * to concurrent coroutines. */ public function setContainer(Container $container): static { @@ -265,4 +263,15 @@ public function getExists(): bool { return CoroutineContext::get(self::DATABASE_EXISTS_CONTEXT_KEY_PREFIX . spl_object_id($this), false); } + + /** + * Reset this handler's existence state when it is cloned. + * + * PHP reuses freed object IDs, so a clone can land on a released handler's + * ID and must not inherit its existence state. + */ + public function __clone(): void + { + $this->setExists(false); + } } diff --git a/tests/Integration/Session/DatabaseSessionHandlerTest.php b/tests/Integration/Session/DatabaseSessionHandlerTest.php index 9473e32d4..43d06a11c 100644 --- a/tests/Integration/Session/DatabaseSessionHandlerTest.php +++ b/tests/Integration/Session/DatabaseSessionHandlerTest.php @@ -4,7 +4,9 @@ namespace Hypervel\Tests\Integration\Session; +use Hypervel\Context\CoroutineContext; use Hypervel\Context\RequestContext; +use Hypervel\Database\ConnectionResolverInterface; use Hypervel\Http\Request; use Hypervel\Session\DatabaseSessionHandler; use Hypervel\Support\CarbonImmutable; @@ -87,7 +89,7 @@ public function testGarbageCollector(): void $this->assertEquals(0, $connection->table('sessions')->count()); } - public function testDestroy() + public function testDestroy(): void { $resolver = $this->app->make('db'); $connection = $this->app['db']->connection(); @@ -108,7 +110,7 @@ public function testDestroy() $this->assertEquals(1, $connection->table('sessions')->where('id', 'id_2')->count()); } - public function testItCanWorkWithoutContainer() + public function testItCanWorkWithoutContainer(): void { $resolver = $this->app->make('db'); $connection = $this->app['db']->connection(); @@ -124,4 +126,96 @@ public function testItCanWorkWithoutContainer() $this->assertNull($session->ip_address); $this->assertNull($session->user_id); } + + public function testDirectWriteUpdatesAnExistingSessionWithoutAttemptingDuplicateInsert(): void + { + $resolver = $this->app->make('db'); + $connection = $resolver->connection(); + $connection->table('sessions')->insert([ + 'id' => 'existing-session', + 'payload' => base64_encode('old data'), + 'last_activity' => time(), + ]); + $handler = new TrackingDatabaseSessionHandler($resolver, null, 'sessions', 120); + + $this->assertTrue($handler->write('existing-session', 'new data')); + $this->assertSame(0, $handler->insertCount); + $this->assertSame(1, $handler->updateCount); + $this->assertSame('new data', $handler->read('existing-session')); + } + + public function testConstructionClearsStaleObjectSpecificExistenceState(): void + { + $resolver = $this->app->make('db'); + $handler = new class($resolver) extends TrackingDatabaseSessionHandler { + public function __construct(ConnectionResolverInterface $resolver) + { + CoroutineContext::set( + self::DATABASE_EXISTS_CONTEXT_KEY_PREFIX . spl_object_id($this), + true + ); + + parent::__construct($resolver, null, 'sessions', 120); + } + }; + + $this->assertFalse($handler->getExists()); + $this->assertTrue($handler->write('new-session', 'new data')); + $this->assertSame(1, $handler->insertCount); + $this->assertSame(0, $handler->updateCount); + $this->assertSame('new data', $handler->read('new-session')); + } + + public function testCloningClearsStaleObjectSpecificExistenceStateWithoutChangingSource(): void + { + $resolver = $this->app->make('db'); + $source = new class($resolver) extends TrackingDatabaseSessionHandler { + public function __construct(ConnectionResolverInterface $resolver) + { + parent::__construct($resolver, null, 'sessions', 120); + } + + public function __clone(): void + { + CoroutineContext::set( + self::DATABASE_EXISTS_CONTEXT_KEY_PREFIX . spl_object_id($this), + true + ); + + parent::__clone(); + } + }; + $source->setExists(true); + + $clone = clone $source; + + $this->assertTrue($source->getExists()); + $this->assertFalse($clone->getExists()); + $this->assertTrue($clone->write('cloned-session', 'cloned data')); + $this->assertSame(1, $clone->insertCount); + $this->assertSame(0, $clone->updateCount); + $this->assertSame('cloned data', $clone->read('cloned-session')); + $this->assertTrue($source->getExists()); + } +} + +class TrackingDatabaseSessionHandler extends DatabaseSessionHandler +{ + public int $insertCount = 0; + + public int $updateCount = 0; + + protected function performInsert(string $sessionId, array $payload): ?bool + { + ++$this->insertCount; + + return parent::performInsert($sessionId, $payload); + } + + protected function performUpdate(string $sessionId, array $payload): int + { + ++$this->updateCount; + + return parent::performUpdate($sessionId, $payload); + } } From d6a0b31c9bdc1aec86c30745acc886397f675009 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:43:53 +0000 Subject: [PATCH 07/16] feat(session): own serialization blocking and Redis configuration Declare the complete Session configuration surface at its framework owner, including the JSON application default, route-block settings, and a dedicated application-scoped Redis session prefix. Apply Redis connection and prefix changes only to the cloned cache store, preserve explicit null, empty, and zero prefix behavior, reject incompatible stores clearly, remove drift-prone call-site defaults, and delete the dead unsupported cache-driver wrapper. Cover configuration conversion, direct PHP construction, clone isolation, and every prefix boundary. --- src/foundation/config/session.php | 48 +++++++++ src/session/src/SessionManager.php | 40 +++++--- tests/Session/SessionConfigTest.php | 40 +++++++- tests/Session/SessionManagerTest.php | 146 +++++++++++++++++++++++++++ 4 files changed, 257 insertions(+), 17 deletions(-) diff --git a/src/foundation/config/session.php b/src/foundation/config/session.php index a757dcddd..a7cd75a4d 100644 --- a/src/foundation/config/session.php +++ b/src/foundation/config/session.php @@ -101,6 +101,38 @@ 'store' => env('SESSION_STORE'), + /* + |-------------------------------------------------------------------------- + | Session Redis Prefix + |-------------------------------------------------------------------------- + | + | When using the "redis" session driver, you may define the prefix used + | for session keys. This keeps session data separate from other values + | stored on the same Redis connection. + | + */ + + 'prefix' => env('SESSION_PREFIX', app_id() . '_session:'), + + /* + |-------------------------------------------------------------------------- + | Session Blocking + |-------------------------------------------------------------------------- + | + | Session blocking prevents concurrent requests for the same session + | from executing at the same time. You may configure the cache store + | and time limits used to acquire and maintain the session lock. + | + */ + + 'block' => (bool) env('SESSION_BLOCK', false), + + 'block_store' => env('SESSION_BLOCK_STORE'), + + 'block_lock_seconds' => (int) env('SESSION_BLOCK_LOCK_SECONDS', 10), + + 'block_wait_seconds' => (int) env('SESSION_BLOCK_WAIT_SECONDS', 10), + /* |-------------------------------------------------------------------------- | Session Sweeping Lottery @@ -208,4 +240,20 @@ */ 'partitioned' => env('SESSION_PARTITIONED_COOKIE', false), + + /* + |-------------------------------------------------------------------------- + | Session Serialization + |-------------------------------------------------------------------------- + | + | This value controls the serialization strategy for session data, which + | is JSON by default. Setting this to "php" allows the storage of PHP + | objects in the session but can make an application vulnerable to + | "gadget chain" serialization attacks if the APP_KEY is leaked. + | + | Supported: "json", "php" + | + */ + + 'serialization' => 'json', ]; diff --git a/src/session/src/SessionManager.php b/src/session/src/SessionManager.php index 44c31bb6a..411cdff48 100644 --- a/src/session/src/SessionManager.php +++ b/src/session/src/SessionManager.php @@ -4,8 +4,10 @@ namespace Hypervel\Session; +use Hypervel\Cache\RedisStore; use Hypervel\Contracts\Encryption\Encrypter; use Hypervel\Support\Manager; +use InvalidArgumentException; use SessionHandlerInterface; use UnitEnum; @@ -100,22 +102,30 @@ protected function createDatabaseDriver(): Store protected function createRedisDriver(): Store { $handler = $this->createCacheHandler('redis'); - $connection = $this->config->get('session.connection'); + $store = $handler->getCache()->getStore(); - $handler->getCache()->getStore()->setConnection( // @phpstan-ignore method.notFound (RedisStore::setConnection — always Redis here) - $connection ?? 'session' + if (! $store instanceof RedisStore) { + throw new InvalidArgumentException( + 'The [session.driver] value [redis] requires [session.store] to reference a Redis cache store.' + ); + } + + $store->setConnection( + $this->config->get('session.connection') ?? 'session' ); + $prefix = $this->config->get('session.prefix'); + + if ($prefix !== null && $prefix !== '') { + $store->setPrefix($prefix); + } + return $this->buildSession($handler); } - /** - * Create an instance of a cache driven driver. - */ - protected function createCacheBased(string $driver): Store - { - return $this->buildSession($this->createCacheHandler($driver)); - } + // Laravel's apc/memcached/dynamodb drivers and their shared createCacheBased() + // wrapper are intentionally omitted; Hypervel has no matching cache stores. + // Register cache-backed handlers with Session::extend(). /** * Create the cache based session handler instance. @@ -142,7 +152,7 @@ protected function buildSession(SessionHandlerInterface $handler): Store $this->config->string('session.cookie'), $handler, null, - $this->config->string('session.serialization', 'php') + $this->config->string('session.serialization') ); } @@ -156,7 +166,7 @@ protected function buildEncryptedSession(SessionHandlerInterface $handler): Encr $handler, $this->container->make(Encrypter::class), null, - $this->config->string('session.serialization', 'php'), + $this->config->string('session.serialization'), ); } @@ -165,7 +175,7 @@ protected function buildEncryptedSession(SessionHandlerInterface $handler): Encr */ public function shouldBlock(): bool { - return $this->config->boolean('session.block', false); + return $this->config->boolean('session.block'); } /** @@ -181,7 +191,7 @@ public function blockDriver(): ?string */ public function defaultRouteBlockLockSeconds(): int { - return $this->config->integer('session.block_lock_seconds', 10); + return $this->config->integer('session.block_lock_seconds'); } /** @@ -189,7 +199,7 @@ public function defaultRouteBlockLockSeconds(): int */ public function defaultRouteBlockWaitSeconds(): int { - return $this->config->integer('session.block_wait_seconds', 10); + return $this->config->integer('session.block_wait_seconds'); } /** diff --git a/tests/Session/SessionConfigTest.php b/tests/Session/SessionConfigTest.php index 13c068980..c822769e7 100644 --- a/tests/Session/SessionConfigTest.php +++ b/tests/Session/SessionConfigTest.php @@ -16,6 +16,9 @@ public function testBooleanOptionsAreLoadedAsBooleansFromEnvironment(): void $originalValues = $this->setEnvironmentVariables([ 'SESSION_ENCRYPT' => '1', 'SESSION_EXPIRE_ON_CLOSE' => '0', + 'SESSION_BLOCK' => '1', + 'SESSION_BLOCK_LOCK_SECONDS' => '45', + 'SESSION_BLOCK_WAIT_SECONDS' => '12', ]); try { @@ -26,6 +29,39 @@ public function testBooleanOptionsAreLoadedAsBooleansFromEnvironment(): void $this->assertTrue($config['encrypt']); $this->assertFalse($config['expire_on_close']); + $this->assertTrue($config['block']); + $this->assertSame(45, $config['block_lock_seconds']); + $this->assertSame(12, $config['block_wait_seconds']); + } finally { + $this->restoreEnvironmentVariables($originalValues); + Env::flushRepository(); + Container::setInstance(null); + } + } + + public function testSessionConfigurationDeclaresCanonicalDefaults(): void + { + $originalValues = $this->setEnvironmentVariables([ + 'APP_ID' => 'session_config_test', + 'SESSION_PREFIX' => null, + 'SESSION_BLOCK' => null, + 'SESSION_BLOCK_STORE' => null, + 'SESSION_BLOCK_LOCK_SECONDS' => null, + 'SESSION_BLOCK_WAIT_SECONDS' => null, + ]); + + try { + Env::flushRepository(); + new Application(dirname(__DIR__, 2)); + + $config = require dirname(__DIR__, 2) . '/src/foundation/config/session.php'; + + $this->assertSame('session_config_test_session:', $config['prefix']); + $this->assertFalse($config['block']); + $this->assertNull($config['block_store']); + $this->assertSame(10, $config['block_lock_seconds']); + $this->assertSame(10, $config['block_wait_seconds']); + $this->assertSame('json', $config['serialization']); } finally { $this->restoreEnvironmentVariables($originalValues); Env::flushRepository(); @@ -36,7 +72,7 @@ public function testBooleanOptionsAreLoadedAsBooleansFromEnvironment(): void /** * Set the given environment variables. * - * @param array $values + * @param array $values * @return array */ private function setEnvironmentVariables(array $values): array @@ -53,7 +89,7 @@ private function setEnvironmentVariables(array $values): array ]; unset($_SERVER[$key], $_ENV[$key]); - putenv("{$key}={$value}"); + $value === null ? putenv($key) : putenv("{$key}={$value}"); } return $originalValues; diff --git a/tests/Session/SessionManagerTest.php b/tests/Session/SessionManagerTest.php index 6ef83570f..3c64ec13e 100644 --- a/tests/Session/SessionManagerTest.php +++ b/tests/Session/SessionManagerTest.php @@ -4,17 +4,22 @@ namespace Hypervel\Tests\Session; +use Hypervel\Cache\ArrayStore; use Hypervel\Cache\RedisStore; use Hypervel\Cache\Repository as CacheRepository; use Hypervel\Config\Repository as ConfigRepository; use Hypervel\Container\Container; use Hypervel\Contracts\Container\Container as ContainerContract; use Hypervel\Contracts\Encryption\Encrypter; +use Hypervel\Contracts\Redis\Factory as RedisFactory; use Hypervel\Database\ConnectionResolverInterface; +use Hypervel\Session\CacheBasedSessionHandler; use Hypervel\Session\DatabaseSessionHandler; +use Hypervel\Session\EncryptedStore; use Hypervel\Session\SessionManager; use Hypervel\Session\Store; use Hypervel\Tests\TestCase; +use InvalidArgumentException; use Mockery as m; use ReflectionProperty; use SessionHandlerInterface; @@ -29,6 +34,7 @@ public function testEnumDefaultDriverIsNormalizedWithoutTreatingZeroAsAbsent(): 'lifetime' => 120, 'cookie' => 'session', 'encrypt' => false, + 'serialization' => 'php', ], ])); @@ -48,6 +54,7 @@ public function testDatabaseDriverLeavesConnectionUnsetByDefault(): void 'session.lifetime' => 120, 'session.cookie' => 'session', 'session.encrypt' => false, + 'session.serialization' => 'php', ])); $store = $manager->driver(); @@ -61,6 +68,7 @@ public function testRedisDriverDefaultsToSessionConnectionWhenUnset(): void { $store = m::mock(RedisStore::class); $store->shouldReceive('setConnection')->once()->with('session'); + $store->shouldReceive('setPrefix')->once()->with('application_session:'); $repository = new CacheRepository($store); @@ -74,6 +82,8 @@ public function testRedisDriverDefaultsToSessionConnectionWhenUnset(): void 'session.lifetime' => 120, 'session.cookie' => 'session', 'session.encrypt' => false, + 'session.serialization' => 'php', + 'session.prefix' => 'application_session:', ]); $container->instance('cache', $cacheManager); @@ -102,6 +112,8 @@ public function testCacheBackedSessionsPreserveZeroStoreAndEmptyFallback(): void 'session.lifetime' => 120, 'session.cookie' => 'session', 'session.encrypt' => false, + 'session.serialization' => 'php', + 'session.prefix' => null, ]); $container->instance('cache', $cacheManager); @@ -118,6 +130,7 @@ public function testExplicitSessionConnectionOverridesBothDrivers(): void 'session.lifetime' => 120, 'session.cookie' => 'session', 'session.encrypt' => false, + 'session.serialization' => 'php', ])); $databaseStore = $databaseManager->driver(); @@ -142,6 +155,8 @@ public function testExplicitSessionConnectionOverridesBothDrivers(): void 'session.lifetime' => 120, 'session.cookie' => 'session', 'session.encrypt' => false, + 'session.serialization' => 'php', + 'session.prefix' => null, ]); $container->instance('cache', $cacheManager); @@ -149,6 +164,123 @@ public function testExplicitSessionConnectionOverridesBothDrivers(): void $this->assertInstanceOf(Store::class, (new SessionManager($container))->driver()); } + public function testRedisDriverAppliesSessionPrefixWithoutMutatingSharedCacheStore(): void + { + foreach ([ + ['custom:', 'custom:'], + ['0', '0'], + [null, 'cache:'], + ['', 'cache:'], + ] as [$configuredPrefix, $expectedPrefix]) { + $redis = m::mock(RedisFactory::class); + $sharedStore = new RedisStore($redis, 'cache:', 'cache'); + $repository = new CacheRepository($sharedStore); + $cacheManager = m::mock(); + $cacheManager->shouldReceive('store')->once()->with('redis')->andReturn($repository); + + $container = $this->getContainer([ + 'session.driver' => 'redis', + 'session.connection' => 'session', + 'session.store' => null, + 'session.lifetime' => 120, + 'session.cookie' => 'session', + 'session.encrypt' => false, + 'session.serialization' => 'php', + 'session.prefix' => $configuredPrefix, + ]); + $container->instance('cache', $cacheManager); + + $sessionStore = (new SessionManager($container))->driver(); + $handler = $this->handlerFromStore($sessionStore); + + $this->assertInstanceOf(CacheBasedSessionHandler::class, $handler); + + $sessionRedisStore = $handler->getCache()->getStore(); + + $this->assertInstanceOf(RedisStore::class, $sessionRedisStore); + $this->assertNotSame($sharedStore, $sessionRedisStore); + $this->assertSame($expectedPrefix, $sessionRedisStore->getPrefix()); + $this->assertSame('session', $this->redisConnectionFromStore($sessionRedisStore)); + $this->assertSame('cache:', $sharedStore->getPrefix()); + $this->assertSame('cache', $this->redisConnectionFromStore($sharedStore)); + } + } + + public function testRedisDriverRejectsNonRedisCacheStore(): void + { + $cacheManager = m::mock(); + $cacheManager->shouldReceive('store') + ->once() + ->with('array') + ->andReturn(new CacheRepository(new ArrayStore)); + + $container = $this->getContainer([ + 'session.driver' => 'redis', + 'session.connection' => null, + 'session.store' => 'array', + 'session.lifetime' => 120, + 'session.cookie' => 'session', + 'session.encrypt' => false, + 'session.serialization' => 'php', + 'session.prefix' => 'application_session:', + ]); + $container->instance('cache', $cacheManager); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage( + 'The [session.driver] value [redis] requires [session.store] to reference a Redis cache store.' + ); + + (new SessionManager($container))->driver(); + } + + public function testSessionSerializationUsesConfiguredStrategy(): void + { + foreach (['json', 'php'] as $serialization) { + $manager = new SessionManager($this->getContainer([ + 'session.driver' => 'array', + 'session.lifetime' => 120, + 'session.cookie' => 'session', + 'session.encrypt' => false, + 'session.serialization' => $serialization, + ])); + + $this->assertSame($serialization, $this->serializationFromStore($manager->driver())); + } + } + + public function testEncryptedSessionSerializationUsesConfiguredStrategy(): void + { + $manager = new SessionManager($this->getContainer([ + 'session.driver' => 'array', + 'session.lifetime' => 120, + 'session.cookie' => 'session', + 'session.encrypt' => true, + 'session.serialization' => 'json', + ])); + + $store = $manager->driver(); + + $this->assertInstanceOf(EncryptedStore::class, $store); + $this->assertSame('json', $this->serializationFromStore($store)); + } + + public function testBlockingConfigurationUsesDeclaredValues(): void + { + $manager = new SessionManager($this->getContainer([ + 'session.driver' => 'array', + 'session.block' => true, + 'session.block_store' => 'locks', + 'session.block_lock_seconds' => 30, + 'session.block_wait_seconds' => 15, + ])); + + $this->assertTrue($manager->shouldBlock()); + $this->assertSame('locks', $manager->blockDriver()); + $this->assertSame(30, $manager->defaultRouteBlockLockSeconds()); + $this->assertSame(15, $manager->defaultRouteBlockWaitSeconds()); + } + protected function getContainer(array $config): Container { $container = new Container; @@ -175,6 +307,20 @@ protected function databaseConnectionFromHandler(DatabaseSessionHandler $handler return $property->getValue($handler); } + + protected function redisConnectionFromStore(RedisStore $store): string + { + $property = new ReflectionProperty($store, 'connection'); + + return $property->getValue($store); + } + + protected function serializationFromStore(Store $store): string + { + $property = new ReflectionProperty($store, 'serialization'); + + return $property->getValue($store); + } } enum SessionIntegerIdentifier: int From 792038335b1c19f099e9d21aac8b7fc4af223f3e Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:44:01 +0000 Subject: [PATCH 08/16] fix(session): complete native handler contracts Bring the Array and Null session handlers into line with SessionHandlerInterface and the already typed Cookie, File, Database, and Cache handlers. Add only the missing parameter and return types, preserving valid covariance and existing runtime behavior without widening the public surface or performing a broad unrelated typing sweep. --- src/session/src/ArraySessionHandler.php | 10 +++++----- src/session/src/NullSessionHandler.php | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/session/src/ArraySessionHandler.php b/src/session/src/ArraySessionHandler.php index 1bdc65f82..21530ae39 100644 --- a/src/session/src/ArraySessionHandler.php +++ b/src/session/src/ArraySessionHandler.php @@ -26,7 +26,7 @@ public function __construct( ) { } - public function open($savePath, $sessionName): bool + public function open(string $savePath, string $sessionName): bool { return true; } @@ -36,7 +36,7 @@ public function close(): bool return true; } - public function read($sessionId): false|string + public function read(string $sessionId): false|string { if (! isset($this->storage[$sessionId])) { return ''; @@ -53,7 +53,7 @@ public function read($sessionId): false|string return ''; } - public function write($sessionId, $data): bool + public function write(string $sessionId, string $data): bool { $this->storage[$sessionId] = [ 'data' => $data, @@ -63,7 +63,7 @@ public function write($sessionId, $data): bool return true; } - public function destroy($sessionId): bool + public function destroy(string $sessionId): bool { if (isset($this->storage[$sessionId])) { unset($this->storage[$sessionId]); @@ -72,7 +72,7 @@ public function destroy($sessionId): bool return true; } - public function gc($lifetime): int + public function gc(int $lifetime): int { $expiration = $this->calculateExpiration($lifetime); diff --git a/src/session/src/NullSessionHandler.php b/src/session/src/NullSessionHandler.php index abab19f1c..730aeb2b3 100644 --- a/src/session/src/NullSessionHandler.php +++ b/src/session/src/NullSessionHandler.php @@ -23,12 +23,12 @@ public function read(string $sessionId): string return ''; } - public function write($sessionId, $data): bool + public function write(string $sessionId, string $data): bool { return true; } - public function destroy($sessionId): bool + public function destroy(string $sessionId): bool { return true; } From 72edb242660286c5d8aed566260dd58cb6348458 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:44:08 +0000 Subject: [PATCH 09/16] refactor(session): remove redundant wiring and stale suppressions Rely on Hypervel auto-singletoning for StartSession, remove the empty provider boot hook, and keep only the canonical manager and store bindings. Delete verified unmatched PHPStan suppressions from Session authentication and Filesystem adapter creation while preserving the real route and magic-proxy guards. Update the touched authentication coverage to the repository typing convention. --- src/filesystem/src/FilesystemManager.php | 2 -- .../src/Middleware/AuthenticateSession.php | 10 +++++----- src/session/src/SessionServiceProvider.php | 10 ---------- .../Middleware/AuthenticateSessionTest.php | 16 ++++++++-------- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/filesystem/src/FilesystemManager.php b/src/filesystem/src/FilesystemManager.php index 2d47265f9..88e9c3982 100644 --- a/src/filesystem/src/FilesystemManager.php +++ b/src/filesystem/src/FilesystemManager.php @@ -635,12 +635,10 @@ private function expandScopedConfigRecursively(array $config, array $diskStack): protected function createFlysystem(FlysystemAdapter $adapter, array $config): FilesystemOperator { if ($config['read-only'] ?? false) { - /* @phpstan-ignore-next-line */ $adapter = new ReadOnlyFilesystemAdapter($adapter); } if (! empty($config['prefix'])) { - /* @phpstan-ignore-next-line */ $adapter = new PathPrefixedAdapter($adapter, $config['prefix']); } diff --git a/src/session/src/Middleware/AuthenticateSession.php b/src/session/src/Middleware/AuthenticateSession.php index df8d4267a..007de0507 100644 --- a/src/session/src/Middleware/AuthenticateSession.php +++ b/src/session/src/Middleware/AuthenticateSession.php @@ -45,11 +45,11 @@ public function handle(Request $request, Closure $next): mixed } } - if (! $request->session()->has('password_hash_' . $this->auth->getDefaultDriver())) { // @phpstan-ignore method.notFound (proxied via AuthManager::__call) + if (! $request->session()->has('password_hash_' . $this->auth->getDefaultDriver())) { $this->storePasswordHashInSession($request); } - $sessionPasswordHash = $request->session()->get('password_hash_' . $this->auth->getDefaultDriver()); // @phpstan-ignore method.notFound + $sessionPasswordHash = $request->session()->get('password_hash_' . $this->auth->getDefaultDriver()); if (! $this->validatePasswordHash($request->user()->getAuthPassword(), $sessionPasswordHash)) { $this->logout($request); @@ -76,7 +76,7 @@ protected function storePasswordHashInSession(Request $request): void $passwordHash = $this->guard()->hashPasswordForCookie($passwordHash); // @phpstan-ignore method.notFound $request->session()->put([ - 'password_hash_' . $this->auth->getDefaultDriver() => $passwordHash, // @phpstan-ignore method.notFound + 'password_hash_' . $this->auth->getDefaultDriver() => $passwordHash, ]); } @@ -105,8 +105,8 @@ protected function logout(Request $request): void throw new AuthenticationException( 'Unauthenticated.', - [$this->auth->getDefaultDriver()], // @phpstan-ignore method.notFound - $this->redirectTo($request) // @phpstan-ignore method.notFound + [$this->auth->getDefaultDriver()], + $this->redirectTo($request) ); } diff --git a/src/session/src/SessionServiceProvider.php b/src/session/src/SessionServiceProvider.php index f98070b57..119d86c98 100644 --- a/src/session/src/SessionServiceProvider.php +++ b/src/session/src/SessionServiceProvider.php @@ -4,7 +4,6 @@ namespace Hypervel\Session; -use Hypervel\Session\Middleware\StartSession; use Hypervel\Support\ServiceProvider; class SessionServiceProvider extends ServiceProvider @@ -17,20 +16,11 @@ public function register(): void $this->registerSessionManager(); $this->registerSessionDriver(); - $this->app->singleton(StartSession::class); - $this->commands([ Console\SessionTableCommand::class, ]); } - /** - * Bootstrap the service provider. - */ - public function boot(): void - { - } - /** * Register the session manager instance. */ diff --git a/tests/Session/Middleware/AuthenticateSessionTest.php b/tests/Session/Middleware/AuthenticateSessionTest.php index b86910563..6115daffa 100644 --- a/tests/Session/Middleware/AuthenticateSessionTest.php +++ b/tests/Session/Middleware/AuthenticateSessionTest.php @@ -17,7 +17,7 @@ class AuthenticateSessionTest extends TestCase { - public function testHandleWithoutSession() + public function testHandleWithoutSession(): void { $request = new Request; $next = fn () => 'next-1'; @@ -30,7 +30,7 @@ public function testHandleWithoutSession() $this->assertEquals('next-1', $response); } - public function testHandleWithSessionWithoutRequestUser() + public function testHandleWithSessionWithoutRequestUser(): void { $request = new Request; @@ -46,7 +46,7 @@ public function testHandleWithSessionWithoutRequestUser() $this->assertEquals('next-2', $response); } - public function testHandleWithSessionWithoutAuthPassword() + public function testHandleWithSessionWithoutAuthPassword(): void { $user = new class { public function getAuthPassword() @@ -72,7 +72,7 @@ public function getAuthPassword() $this->assertEquals('next-3', $response); } - public function testHandleWithSessionWithUserAuthPasswordOnRequestViaRememberFalse() + public function testHandleWithSessionWithUserAuthPasswordOnRequestViaRememberFalse(): void { $user = new class { public function getAuthPassword() @@ -101,7 +101,7 @@ public function getAuthPassword() $this->assertEquals('next-4', $response); } - public function testHandleWithInvalidPasswordHash() + public function testHandleWithInvalidPasswordHash(): void { $user = new class { public function getAuthPassword() @@ -189,7 +189,7 @@ public function getAuthPassword(): string $this->fail('AuthenticationException was not thrown.'); } - public function testHandleWithInvalidIncookiePasswordHashViaRememberTrue() + public function testHandleWithInvalidIncookiePasswordHashViaRememberTrue(): void { $user = new class { public function getAuthPassword() @@ -232,7 +232,7 @@ public function getAuthPassword() $this->assertNull($session->get('b')); } - public function testHandleWithValidIncookieInvalidInsessionHashViaRememberTrue() + public function testHandleWithValidIncookieInvalidInsessionHashViaRememberTrue(): void { $user = new class { public function getAuthPassword() @@ -276,7 +276,7 @@ public function getAuthPassword() $this->assertNull($session->get('b')); } - public function testHandleWithValidPasswordInSessionCookieIsEmptyGuardHasUser() + public function testHandleWithValidPasswordInSessionCookieIsEmptyGuardHasUser(): void { $user = new class { public function getAuthPassword() From a794ddf050964ca070cf516abf009b663b33d5a8 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:44:14 +0000 Subject: [PATCH 10/16] build(session): declare complete split dependencies Declare the extensions, Container, and Symfony components used directly by the Session split package instead of relying on transitive installation. Add package metadata coverage for the complete direct runtime dependency set so future source changes cannot silently drift the standalone manifest. --- src/session/composer.json | 8 +++- tests/Session/PackageMetadataTest.php | 53 +++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tests/Session/PackageMetadataTest.php diff --git a/src/session/composer.json b/src/session/composer.json index 4b4a6dd09..240124343 100644 --- a/src/session/composer.json +++ b/src/session/composer.json @@ -30,11 +30,14 @@ }, "require": { "php": "^8.4", + "ext-ctype": "*", + "ext-mbstring": "*", "ext-session": "*", "hypervel/auth": "^0.4", "hypervel/cache": "^0.4", "hypervel/collections": "^0.4", "hypervel/console": "^0.4", + "hypervel/container": "^0.4", "hypervel/context": "^0.4", "hypervel/contracts": "^0.4", "hypervel/cookie": "^0.4", @@ -43,7 +46,10 @@ "hypervel/http": "^0.4", "hypervel/macroable": "^0.4", "hypervel/routing": "^0.4", - "hypervel/support": "^0.4" + "hypervel/support": "^0.4", + "symfony/console": "^8.1", + "symfony/finder": "^8.1", + "symfony/http-foundation": "^8.1" }, "config": { "sort-packages": true diff --git a/tests/Session/PackageMetadataTest.php b/tests/Session/PackageMetadataTest.php new file mode 100644 index 000000000..21b40725e --- /dev/null +++ b/tests/Session/PackageMetadataTest.php @@ -0,0 +1,53 @@ +assertArrayHasKey($dependency, $composer['require']); + $this->assertIsString($composer['require'][$dependency]); + $this->assertNotSame('', trim($composer['require'][$dependency])); + } + } +} From b8817e4c985fc684dddfab1981cf5b050a922ef5 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:44:23 +0000 Subject: [PATCH 11/16] docs(session): explain persistence and runtime boundaries Document JSON serialization, deliberate PHP object-session opt-in, worker-local array sessions, Redis session prefixes, blocking configuration, and truthful custom handler write and garbage-collection contracts in the existing task-oriented guide style. Record the intentionally omitted unsupported Laravel cache-backed drivers and point custom implementations to Session::extend() so future upstream synchronization does not reintroduce the deleted dead wrapper. --- src/boost/docs/session.md | 12 +++++++++--- src/session/README.md | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/boost/docs/session.md b/src/boost/docs/session.md index 09bfa46af..5c36d546e 100644 --- a/src/boost/docs/session.md +++ b/src/boost/docs/session.md @@ -41,7 +41,9 @@ The session `driver` configuration option defines where session data will be sto > [!NOTE] -> The array driver is primarily used during [testing](/docs/{{version}}/testing) and prevents the data stored in the session from being persisted. +> The array driver stores sessions only in the memory of a single Swoole worker. It is useful during [testing](/docs/{{version}}/testing), but should not be used for production sessions. + +By default, Hypervel serializes session data as JSON, which is suitable for scalar and array values. If your application needs to store PHP objects in the session, you may set the `serialization` option in your `session.php` configuration file to `php`. PHP serialization should only be enabled when necessary, since deserializing objects increases the impact of compromised session data or encryption keys. Routes assigned to the `web` middleware group already include the `Hypervel\Session\Middleware\StartSession` middleware. If you need session state on routes outside of the `web` middleware group, you should apply the middleware to those routes. @@ -69,6 +71,8 @@ Before using Redis sessions with Hypervel, you will need to install the [PhpRedi > [!NOTE] > The `SESSION_CONNECTION` environment variable, or the `connection` option in the `session.php` configuration file, may be used to specify which Redis connection is used for session storage. +Redis session keys use the `SESSION_PREFIX` environment variable and default to your application ID followed by `_session:`. The session prefix is separate from `SESSION_CONNECTION`: the connection selects where sessions are stored, while the prefix separates session keys from other data on that connection. Any prefix configured on the Redis connection will also be applied. + ## Interacting With the Session @@ -311,6 +315,8 @@ For more information on Hypervel's cache methods, consult the [cache documentati By default, Hypervel allows requests using the same session to execute concurrently. So, for example, if you use a JavaScript HTTP library to make two HTTP requests to your application, they will both execute at the same time. For many applications, this is not a problem; however, session data loss can occur in a small subset of applications that make concurrent requests to two different application endpoints which both write data to the session. +To enable session blocking for every route that uses session middleware, set the `SESSION_BLOCK` environment variable to `true`. You may use `SESSION_BLOCK_STORE` to select the cache store used for locks, and `SESSION_BLOCK_LOCK_SECONDS` and `SESSION_BLOCK_WAIT_SECONDS` to change the default lock and wait times. + To mitigate this, Hypervel provides functionality that allows you to limit concurrent requests for a given session. To get started, you may simply chain the `block` method onto your route definition. In this example, an incoming request to the `/profile` endpoint would acquire a session lock. While this lock is being held, any incoming requests to the `/profile` or `/order` endpoints which share the same session ID will wait for the first request to finish executing before continuing their execution: ```php @@ -430,9 +436,9 @@ Since the purpose of these methods is not readily understandable, here is an ove - The `open` method would typically be used in file based session store systems. Since Hypervel ships with a `file` session driver, you will rarely need to put anything in this method. You can simply return `true`. - The `close` method, like the `open` method, can also usually be disregarded. For most drivers, it is not needed. - The `read` method should return the string version of the session data associated with the given `$sessionId`. There is no need to do any serialization or other encoding when retrieving or storing session data in your driver, as Hypervel will perform the serialization for you. -- The `write` method should write the given `$data` string associated with the `$sessionId` to a persistent storage system of your choice. Again, you should not perform any serialization - Hypervel will have already handled that for you. +- The `write` method should write the given `$data` string associated with the `$sessionId` to a persistent storage system of your choice and return `true` when the write succeeds or `false` when it fails. A failed write will cause the request to fail rather than accepting the loss of session data. Again, you should not perform any serialization - Hypervel will have already handled that for you. - The `destroy` method should remove the data associated with the `$sessionId` from persistent storage. -- The `gc` method should destroy all session data that is older than the given `$lifetime`, which is a UNIX timestamp. For self-expiring systems like Redis, this method may return `0`. +- The `gc` method should destroy all session data older than the given `$lifetime` in seconds. For self-expiring systems like Redis, this method may return `0`. diff --git a/src/session/README.md b/src/session/README.md index 5c34476f0..5f8b68e92 100644 --- a/src/session/README.md +++ b/src/session/README.md @@ -7,3 +7,4 @@ Session for Hypervel - `Store::passwordConfirmed(?string $guard = null)` stamps a guard-scoped key (`auth.password_confirmed_at_{guard}`) instead of Laravel's single shared key, resolving the current guard when none is given. - Password-hash session artifacts are HMAC-only. Laravel's raw-hash fallback for legacy sessions is intentionally omitted because Hypervel 0.4 has no released legacy sessions. +- Laravel's `apc`, `memcached`, and `dynamodb` session drivers and the shared `SessionManager::createCacheBased()` wrapper are intentionally omitted because Hypervel ships no matching cache stores. Register cache-backed handlers with `Session::extend()`. From 4cf3d076c13cb63a088298df760490386b6697e8 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:44:29 +0000 Subject: [PATCH 12/16] test(pool): name flush teardown regression accurately Rename the heartbeat connection regression for the explicit Pool::flush() close protocol it exercises. The Pool lifecycle deliberately removed unreachable destructors in favor of deterministic close ownership, so retaining a destructor-named test made the supported behavior harder to verify and search. --- tests/Pool/HeartbeatConnectionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Pool/HeartbeatConnectionTest.php b/tests/Pool/HeartbeatConnectionTest.php index 1d2d352ca..5a64d7967 100644 --- a/tests/Pool/HeartbeatConnectionTest.php +++ b/tests/Pool/HeartbeatConnectionTest.php @@ -173,7 +173,7 @@ public function testHeartbeatFailureFallsBackToThePhpErrorLogWithoutALogger(): v } } - public function testConnectionDestruct(): void + public function testConnectionCloseProtocolRunsOnPoolFlush(): void { $container = $this->getContainer(); $pool = $container->make(HeartbeatPoolStub::class); From ff2837980682739cc22b2f101ecdd9b7e74ed87b Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:44:39 +0000 Subject: [PATCH 13/16] docs(static-analysis): track stale suppression audit Record the owner-approved framework-wide review of unmatched inline ignores and global patterns with reportUnmatchedIgnoredErrors enabled. Require each suppression to be traced before removal, prohibit runtime contortions or wider types merely to satisfy PHPStan, and leave the permanent strict-setting decision with that dedicated audit rather than expanding Session work. --- docs/todo.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/todo.md b/docs/todo.md index 1134806db..3732142ed 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -25,6 +25,7 @@ - Convert container array access to `make()` across `src/`. About 40 files use `$app['...']` (e.g. `LogManager`, `ViewServiceProvider`, `TranslationServiceProvider`), carried over from upstream Laravel. `offsetGet()` always returns `mixed`, while `make()` has class-string generics phpstan can follow, so the conversion makes static analysis strictly more useful. Approved modernization per the Porting Packages policy in `AGENTS.md`; new code already follows the rule. - Investigate where requiring and directly using a PHP extension would make framework code significantly faster than its current pure-PHP implementation. The framework already declares bundled extensions it depends on, so the question is which hot paths are doing in PHP what a C extension does natively. The worked example is `ext-gmp` for identifier encoding: UUID and ULID string conversion and any base32/base58/base62 short-id work exceed 64 bits, so `ramsey/uuid` and `symfony/uid` convert them digit by digit in PHP, while `gmp_init()`/`gmp_strval()` do arbitrary-base conversion natively — a hand-rolled base-36 UUID conversion measured 14.6 µs against 0.5 µs for the GMP equivalent with byte-identical output. Anything that fits in a 64-bit int (snowflakes, timestamps, counters) needs no extension, and hashing, encryption, and signatures are already C. Measure `Str::uuid()`/`Str::ulid()` and the other candidates before adding a requirement, and weigh each new extension against installation cost. - Convert untyped `$config->get()` calls across `src/` to the typed getters (`string()`, `integer()`, `float()`, `boolean()`, `array()`) without call-site defaults, for every key that isn't genuinely nullable. Defaults live in the merged config files — declare any key currently defaulted only at a call site in its package's config file as part of the conversion. Typed getters throw `InvalidArgumentException` naming the key on misconfiguration instead of letting a wrong type propagate silently, and give phpstan real return types. Bootstrap code that runs before config merging keeps its call-site defaults. Approved modernization per the Porting Packages policy in `AGENTS.md`; new code already follows the rule. +- Audit unmatched PHPStan inline ignores and global patterns with `reportUnmatchedIgnoredErrors` enabled — currently 196 unmatched inline ignores across 99 files plus 5 unmatched global patterns. Remove only suppressions that no longer match after tracing the underlying code; do not replace correct source with runtime branches or wider types merely to keep static analysis green. Decide as part of the work whether `phpstan.neon.dist` should then set `reportUnmatchedIgnoredErrors: true` permanently, since leaving it `false` lets the suppressions rot again. ## Documentation From e909ff8d394c800bbe63cfc1d73df37a7eae1dae Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:44:46 +0000 Subject: [PATCH 14/16] docs(audit): record Session lifecycle completion Record session-01 through session-22 and filesystem-12 with their final owners, regressions, rejected alternatives, performance profile, Laravel-facing result, and completed validation. Mark Session complete, route the next standalone Queue audit through its exact prerequisite work units, close verified Cache, Log, Pool, Redis, Support, and Filesystem revalidation markers, and keep the dependency index grouped in completion order. --- ...amework-coroutine-state-lifecycle-audit.md | 25 +++++------ ...-coroutine-state-lifecycle-audit-ledger.md | 43 ++++++++++++++++++- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md b/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md index 602369297..eba49e880 100644 --- a/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md +++ b/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md @@ -990,8 +990,8 @@ An exceptionally large shared work unit may receive its own linked detail plan w This compact index routes the completed-work history that must be consulted with the full plan after compaction. Detailed history remains in the [companion ledger](2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md). -- **Active package or work unit:** `session` -- **Ledger entries required for the active work:** `Normalize framework enum identifiers at string boundaries`; `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; `Complete Cache parity, cleanup, permanence, and tagged ownership`. +- **Active package or work unit:** `queue` +- **Ledger entries required for the active work:** `Harden framework contracts and request-scoped state`; `Consolidate reflection metadata and correct callable inference`; `Correct event dispatch, queued-consumer isolation, and queue interoperability`; `Normalize framework enum identifiers at string boundaries`; `Make Bus dispatch, batches, and unique payloads lifecycle-safe`; `Complete Foundation runtime lifecycles and safe publication`; `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`. - **Pending revalidation carried into the active work:** None. Update these three lines when a package starts, completes, or gains a cross-package dependency. Name exact work-unit headings or shared finding IDs from the companion ledger; never use “see recent entries” or require a full-ledger reread. @@ -1017,8 +1017,8 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `container-05` | `container` | `context` (revalidation complete) | `Coordinate shared container construction and complete current contextual resolution`; finding `container-05` | | `container-06` | `container` | `context` (revalidation complete) | `Coordinate shared container construction and complete current contextual resolution`; finding `container-06` | | `container-08` | `container` | `auth`, `cache`, `log`, `routing`, `support`; later full consumer audits | `Coordinate shared container construction and complete current contextual resolution`; finding `container-08` | -| `container-09` | `auth`, `cache`, `log` | `container` (revalidation complete); later full `auth`, `cache`, and `log` audits | `Coordinate shared container construction and complete current contextual resolution`; finding `container-09` | -| `container-10` | `log` | `container` (revalidation complete); later full `log` audit | `Coordinate shared container construction and complete current contextual resolution`; finding `container-10` | +| `container-09` | `auth`, `cache`, `log` | `container`, `cache`, and `log` (revalidation complete); later full `auth` audit | `Coordinate shared container construction and complete current contextual resolution`; finding `container-09` | +| `container-10` | `log` | `container` and `log` (revalidation complete) | `Coordinate shared container construction and complete current contextual resolution`; finding `container-10` | | `context-01` | `context` | `container` and `foundation` (revalidation complete) | `Correct explicit coroutine context targeting`; finding `context-01` | | `context-04` | `context` | `foundation` and `database` (revalidation complete) | `Correct explicit coroutine context targeting`; finding `context-04` | | `coroutine-05` | `coroutine`, `filesystem` | `filesystem` (revalidation complete) | `Make coroutine creation and copied context failure-safe`; finding `coroutine-05` | @@ -1028,8 +1028,8 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `concurrency-01` | `concurrency`, `foundation`, `testbench` | `foundation` (revalidation complete); later full `testbench` audit | `Make process concurrency transport lossless and reconstruct failures safely`; finding `concurrency-01` | | `concurrency-02` | `concurrency`, `testbench` | later full `testbench` audit | `Make process concurrency transport lossless and reconstruct failures safely`; finding `concurrency-02` | | `concurrency-03` | `concurrency`, `foundation`, `testbench` | `foundation` (revalidation complete); later full `testbench` audit | `Make process concurrency transport lossless and reconstruct failures safely`; finding `concurrency-03` | -| `pool-01` | `pool` | `coordinator` (revalidation complete); later full `pool` audit | `Release cleared coordinator timers deterministically`; finding `pool-01` | -| `pool-02` | `pool` | later full `pool` audit | `Release cleared coordinator timers deterministically`; finding `pool-02` | +| `pool-01` | `pool` | `coordinator` and `pool` (revalidation complete) | `Release cleared coordinator timers deterministically`; finding `pool-01` | +| `pool-02` | `pool` | `pool` (revalidation complete) | `Release cleared coordinator timers deterministically`; finding `pool-02` | | `pool-04` | `pool`, `database`, `redis` | `database` and `redis` (revalidation complete) | `Bound pool resources and connection progress deterministically`; finding `pool-04` | | `pool-05` | `pool` | `database` and `redis` (revalidation complete) | `Bound pool resources and connection progress deterministically`; finding `pool-05` | | `database-02` | `database` | `pool` and `database` (revalidation complete) | `Bound pool resources and connection progress deterministically`; finding `database-02` | @@ -1040,7 +1040,7 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `di-02` | `di` | `foundation` (revalidation complete); later full `sentry` and `telescope` audits | `Correct AOP proxy generation and publication`; finding `di-02` | | `filesystem-02` | `filesystem` | `di` and `filesystem` (revalidation complete) | `Correct AOP proxy generation and publication`; finding `filesystem-02` | | `filesystem-03` | `filesystem` | `encryption`, `support`, and `filesystem` (revalidation complete) | `Harden encryption rotation, key publication, and global lifecycle state`; finding `filesystem-03` | -| `filesystem-04` | `filesystem` | `cache`; later full `cache` audit | `Harden filesystem I/O, streaming, and response teardown`; finding `filesystem-04` | +| `filesystem-04` | `filesystem` | `cache` (revalidation complete) | `Harden filesystem I/O, streaming, and response teardown`; finding `filesystem-04` | | `http-02` | `http` | `filesystem`, `foundation`, and `http-server` (revalidation complete); later full `http` audit | `Harden filesystem I/O, streaming, and response teardown`; finding `http-02` | | `filesystem-07` | `filesystem`, `foundation`, `http-server` | `filesystem`, `foundation`, and `http-server` (revalidation complete); later full `http` audit | `Harden filesystem I/O, streaming, and response teardown`; finding `filesystem-07` | | `foundation-04` | `foundation` | `filesystem`, `foundation`, and `http-server` (revalidation complete) | `Harden filesystem I/O, streaming, and response teardown`; finding `foundation-04` | @@ -1052,7 +1052,7 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `queue-11` | `queue` | `events` (revalidation complete), `broadcasting`; later full `queue` and `broadcasting` audits | `Correct event dispatch, queued-consumer isolation, and queue interoperability`; finding `queue-11` | | `queue-12` | `bus`, `queue` | `events` and `bus` (revalidation complete), `broadcasting`; later full `queue` and `broadcasting` audits | `Correct event dispatch, queued-consumer isolation, and queue interoperability`; finding `queue-12` | | `foundation-01` | `foundation` | `support` and `foundation` (revalidation complete) | `Correct event dispatch, queued-consumer isolation, and queue interoperability`; finding `foundation-01` | -| `support-02` | `support` | `auth`, `broadcasting`, `bus` (revalidation complete), `cache`, `concurrency`, `console` (revalidation complete), `container`, `contracts`, `cookie`, `database` (revalidation complete), `events`, `filesystem` (revalidation complete), `foundation` (revalidation complete), `hashing` (revalidation complete), `horizon`, `inertia`, `jwt`, `log`, `mail`, `notifications`, `permission`, `pipeline`, `queue`, `redis` (revalidation complete), `reverb`, `routing`, `sanctum`, `scout`, `session`, `socialite`, `telescope`, `testbench`, `translation`; later full consumer audits | `Normalize framework enum identifiers at string boundaries`; finding `support-02`; sibling findings `translation-01` and `reverb-03`; linked detail plan `2026-07-15-0920-framework-enum-identifier-contracts.md` | +| `support-02` | `support` | `auth`, `broadcasting`, `bus` (revalidation complete), `cache` (revalidation complete), `concurrency`, `console` (revalidation complete), `container`, `contracts`, `cookie`, `database` (revalidation complete), `events`, `filesystem` (revalidation complete), `foundation` (revalidation complete), `hashing` (revalidation complete), `horizon`, `inertia`, `jwt`, `log`, `mail`, `notifications`, `permission`, `pipeline`, `queue`, `redis` (revalidation complete), `reverb`, `routing`, `sanctum`, `scout`, `session` (revalidation complete), `socialite`, `telescope`, `testbench`, `translation`; later full consumer audits | `Normalize framework enum identifiers at string boundaries`; finding `support-02`; sibling findings `translation-01` and `reverb-03`; linked detail plan `2026-07-15-0920-framework-enum-identifier-contracts.md` | | `auth-01` | `support`, `auth` | later full `auth` audit | `Correct Support utility boundaries and authentication timing isolation`; finding `auth-01` | | `encryption-03` | `encryption` | `contracts`, `support`, `filesystem`, and `foundation` (revalidation complete) | `Harden encryption rotation, key publication, and global lifecycle state`; finding `encryption-03` | | `sanctum-01` | `sanctum` | `encryption`; later full `sanctum` audit | `Harden encryption rotation, key publication, and global lifecycle state`; finding `sanctum-01` | @@ -1089,11 +1089,11 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `redis-06` | `redis` | `redis` (revalidation complete) | `Complete Database persistence lifecycles and current Laravel parity`; finding `redis-06` | | `redis-07` | `redis` | `redis` (revalidation complete) | `Complete Database persistence lifecycles and current Laravel parity`; finding `redis-07` | | `redis-08` | `redis`, `pool` | `redis` (revalidation complete) | `Complete Database persistence lifecycles and current Laravel parity`; finding `redis-08` | -| `redis-09` | `redis` | `cache`; later full `cache` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-09` | +| `redis-09` | `redis` | `cache` (revalidation complete) | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-09` | | `redis-10` | `redis` | `reverb` (revalidation complete); later full `reverb` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-10` | | `redis-11` | `redis` | `reverb` (revalidation complete); later full `reverb` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-11` | -| `redis-12` | `redis`, `cache` | `redis` (revalidation complete); later full `cache` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-12` | -| `redis-13` | `redis` | `horizon` (revalidation complete), `cache`, `queue`, `session`, and `broadcasting`; later full consumer audits | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-13` | +| `redis-12` | `redis`, `cache` | `redis` and `cache` (revalidation complete) | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-12` | +| `redis-13` | `redis` | `horizon` (revalidation complete), `cache` (revalidation complete), `queue`, `session` (revalidation complete), and `broadcasting`; later full `horizon`, `queue`, and `broadcasting` audits | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-13` | | `reverb-05` | `reverb` | `redis` (revalidation complete); later full `reverb` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `reverb-05` | | `redis-15` | `redis` | `telescope` and `sentry` (revalidation complete); later full consumer audits | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `redis-15` | | `horizon-01` | `horizon` | `redis` (revalidation complete); later full `horizon` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `horizon-01` | @@ -1101,6 +1101,7 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `telescope-02` | `telescope` | `redis` (revalidation complete); later full `telescope` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `telescope-02` | | `sentry-01` | `sentry` | `redis` (revalidation complete); later full `sentry` audit | `Complete Redis pooling, subscriber transport, topology, parity, and lifecycle safety`; finding `sentry-01` | | `cache-04` | `cache` | `auth`, `sanctum`, and `testbench` (revalidation complete); later full consumer audits | `Complete Cache parity, cleanup, permanence, and tagged ownership`; finding `cache-04` | +| `filesystem-12` | `filesystem` | `session` (revalidation complete) | `Complete Session lifecycles, persistence, and current Laravel parity`; finding `filesystem-12` | ## Package checklist @@ -1172,7 +1173,7 @@ The order is lower-level first where practical. Hypervel has cross-cutting depen - [x] `database` - [x] `redis` - [x] `cache` -- [ ] `session` +- [x] `session` - [ ] `queue` - [ ] `horizon` - [ ] `reverb` diff --git a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md index fd0a06485..0270bc25f 100644 --- a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md +++ b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md @@ -416,7 +416,7 @@ Append package entries in checklist order. Keep each entry compact but complete - **Construction and native-boundary design:** Built-in single, daily, and emergency channels construct safe framework stream classes directly. The monolog driver swaps only the exact configured vendor `StreamHandler` and `RotatingFileHandler` class strings before container construction, forwarding the existing `with` arguments; subclasses pass through untouched. UID normalization resolves the configured processor first and replaces only an exact standard instance, preserving its length with `strlen(getUid())`. Native failures use deterministic operation/path diagnostics and immediate result checks, never `set_error_handler()` or `error_get_last()` around a yielding operation. PHP's `@` suppression is coroutine-local under the current Swoole hooks, but process-global error handlers and `error_get_last()` can observe another coroutine's warnings; the local open, write, lock, permission, close, inode, directory, and rotation catches are therefore required ownership boundaries rather than optional warning cleanup. The stream implementations preserve one reopen retry, best-effort locking and permissions, chunk size, inode handling, rotation, and retention behavior. - **State ownership design:** One logger-family state object contains visible context and recursion depth; replication copies visible context and resets depth. `withoutContext()` mutates only the visible-context field, including for a full clear, because removing the combined slot during a handler-triggered re-entrant log would reset loop detection and orphan the outer frame's depth bookkeeping. Fingers-crossed replication starts with an empty buffer and initial buffering state so a child neither races on nor duplicates its parent's history. UID replication copies the current request identifier. The exception-reporting slot is an array containing coroutine ID plus exception so a copied child cannot mistake an inherited parent report for its own. The logger-family counter is a worker-lifetime identity generator, not resettable state; resetting it can reintroduce context aliasing. - **Queue timing and error contract:** SyncQueue creates a serialized payload at the execution scheduling boundary and delegates to `executePayload()`. Background and Deferred queues schedule only that immutable string; delayed timers capture it immediately, while after-commit callbacks continue to create it only when the commit callback runs. Serialization failures now throw synchronously to the dispatcher or commit callback, matching persistent queues; configured exception callbacks handle only asynchronous execution failures. A payload without log context flushes an already-created repository but does not allocate one solely to hydrate null. -- **Cross-package implications:** Log owns logger, handler, processor, manager, context-listener, provenance, and logging-guide changes. Foundation owns exception-reporting state and the `logs()` helper. Support owns callback rebinding, with matching direct manager consumers in Auth, Broadcasting, Cache, Filesystem, and Log. The shared `Manager` correction also restores Laravel's custom-driver callback contract across its eight Reverb, JWT, Session, Socialite, Notifications, Foundation maintenance-mode, and Hashing subclasses; stale Socialite and JWT test creators now use explicit container or local captures rather than relying on an external anonymous-closure receiver. Queue owns payload timing and serialized execution. Sentry removes its workaround and delegates action-level wrapping to Log. Later full Foundation, Support, Queue, and Sentry audits must retain and revalidate these boundaries. +- **Cross-package implications:** Log owns logger, handler, processor, manager, context-listener, provenance, and logging-guide changes. Foundation owns exception-reporting state and the `logs()` helper. Support owns callback rebinding, with matching direct manager consumers in Auth, Broadcasting, Cache, Filesystem, and Log. The shared `Manager` correction also restores Laravel's custom-driver callback contract across its eight Reverb, JWT, Session, Socialite, Notifications, Foundation maintenance-mode, and Hashing subclasses; stale Socialite and JWT test creators now use explicit container or local captures rather than relying on an external anonymous-closure receiver. Queue owns payload timing and serialized execution. Sentry removes its workaround and delegates action-level wrapping to Log. The completed `container-09` and `container-10` boundaries remain intact through enum-channel and named-logger coverage. Later full Foundation, Support, Queue, and Sentry audits must retain and revalidate these boundaries. - **Regression strategy:** Deterministically cover object-ID reuse, concurrent recursion depth, context clearing during a bounded re-entrant write without losing the loop warning, named stacks, current Laravel callback forms plus the eight-subclass Manager contract, direct and currently-reported JSON exceptions including copied-child context, zero-named channels, coroutine-isolated fingers-crossed buffers and buffering state, stream warning isolation and native false/retry/rotation boundaries, exact-class versus subclass/custom normalization, request UID isolation and replication, on-demand cache/taps, queue dispatch/delay/after-commit capture and fail-fast serialization, null-context repository flushing, and one safe Sentry wrapper. Port current Laravel tests where applicable and retain exact vendor `instanceof` compatibility. - **Performance and complexity:** The approved state lookups occur only on handled log records and only for the relevant configured feature. Safe stream handlers add no lock, yield, retry beyond Monolog's existing single retry, or coroutine coordination. Callback reflection is boot-only; JSON enrichment is exception-only; payload serialization still occurs once and merely moves to the correct earlier boundary; on-demand construction retains less state. Separate state holders and exact-class adapters replace process-global vendor state without a generic abstraction or request-wide handler rebuild. - **Laravel-facing result:** Public methods, signatures, config keys, and supported custom extension shapes remain intact. Most changes restore current Laravel behavior or add Swoole ownership behind vendor-compatible types. Observable corrections are that build-time taps run, dynamic loggers are not cached as a named channel, serialization failures surface at dispatch/commit time, and queued payloads snapshot state at that boundary. No Laravel public API is removed or renamed. @@ -706,6 +706,7 @@ Append package entries in checklist order. Keep each entry compact but complete - **Performance and compatibility:** Keepalive cleanup is failure-only. Native timeout translation occurs at pool construction or native connection creation and adds no borrow/release work. Monotonic conversion keeps the existing number of clock reads and measured no regression. Frequency adds only the corrected integer window bound inside existing bucket maintenance. The approved deletions reduce dependency and retained-object surface. Removing the Hypervel-specific low-frequency constructor requirement broadens implementation freedom and adds no runtime work; no repository caller constructs strategies through that interface. No Laravel-facing API or configuration changes are involved; the existing Hypervel-specific timeout key and behavior are retained and corrected. - **Regression strategy:** Prove Keepalive spawn and heartbeat failures close exactly once and leave disconnected state; prove native timeout propagation and explicit precedence for Database and Redis; prove SimplePool activity uses the monotonic domain; keep every injected lifecycle timestamp in that domain; revalidate Database query-duration thresholds and Redis success/failure event durations; prove the eleventh frequency bucket no longer affects the public result; run every changed test file immediately, all affected Pool/Database/Redis groups, `composer fix`, a fresh caller/callee and clock-domain review, and independent code review. - **Implementation:** Keepalive creation and heartbeat failures now reuse the deterministic close boundary without replacing their primary failure. Database maps the validated connect timeout to native MySQL/MariaDB PDO options or the PostgreSQL DSN, Redis maps it to the native client timeout, and explicit native configuration wins. Pool, Database, and Redis lifecycle and live duration measurements now use one monotonic domain. Frequency retains exactly its configured ten buckets. Pool directly declares `psr/log`; the unused Context helper and Context dependency, Frequency-to-Pool constructor cycle, false low-frequency constructor contract, and empty Redis Frequency alias are removed. Pool provenance, ownership, timeout, and managed-count trim semantics are documented at the package, contract, option, and Database/Redis guide surfaces. +- **Cross-package revalidation:** The completed `pool-01` and `pool-02` lifecycle boundaries remain intact: Pool close still clears `ConstantFrequency` deterministically, and Keepalive teardown remains owned by explicit `close()` without a dead destructor. - **Regression tests:** Focused coverage proves Keepalive creation and heartbeat failures close exactly once, every native timeout translation and precedence branch, monotonic SimplePool activity and callback-resource reuse, monotonic Database query duration, non-negative Redis success/failure event duration, lifecycle generation bounds, and exclusion of the expired eleventh frequency bucket. Every changed test file and the affected Pool, Database, and Redis groups are green; the final SimplePool review addition passes with one test and six assertions. - **Validation and review:** Final `composer fix` completed with zero formatter changes, both PHPStan configurations green, 23,195 component tests and 66,046 assertions passing with 1,600 expected skips, 346 Testbench contract tests and 1,029 assertions passing with 3 expected skips, and 4 dogfood tests and 7 assertions passing. `composer validate --strict src/pool/composer.json`, `git diff --check`, broad deleted-symbol/implementer/clock-domain/documentation sweeps, and the package-checklist parity check are clean. Fresh self-review corrected the over-strong idle-floor documentation model and every same-family wording artifact. Independent post-implementation review traced the complete diff, requested the focused SimplePool reuse assertion, re-reviewed it after the final full gate, and signed off with no remaining concern. - **Laravel-facing result:** No Laravel public API, configuration key or structure, documented behavior, or conventional extension pattern changes. Pool is Hyperf-derived infrastructure, and Hyperf API parity is not required. The existing Hypervel-specific `pool.connect_timeout` key now enforces its documented connection-establishment bound; the owner-approved low-frequency constructor removal broadens the Hypervel-specific extension contract without a compatibility shim. @@ -1228,3 +1229,43 @@ Append package entries in checklist order. Keep each entry compact but complete - **Validation and review:** Every changed test file, the complete Cache suite, and the affected Auth, Redis/Valkey, and Testbench paths passed. The authoritative `composer fix` gate changed no formatting, both PHPStan configurations passed, and the complete parallel components, Testbench package, and dogfood suites passed. Split-manifest validation, `git diff --check`, stale-reference scans, and fresh caller/callee, event, serialization, pool-ownership, failure-precedence, hot-path, retained-state, documentation, and overengineering review are complete. Independent code review is signed off with every final re-review correction incorporated. - **Laravel-facing result:** Current supported Laravel Cache APIs, facade metadata, tests, documentation, and configuration behavior are restored while Hypervel preserves nullable values, the Swoole store, failover/stack stores, Redis pooling, transforms, tag modes, SafeScan behavior, and the explicit pinned-connection API. Hypervel additionally declares the documented nullable `cache.limiter` key through `CACHE_LIMITER`; the owner approved this configuration-structure difference because it makes existing provider behavior explicit without changing the unset default. Other intentional differences remain limited to verified Swoole/pooling requirements and the documented atomic any-mode add event behavior. - **Assessment:** Every accepted Cache finding and carried lower-level assumption is implemented at its lowest owner. The result removes callback-held leases, duplicate remember machinery, stale operation containers, year-long pseudo-permanence, partial cleanup, repeated factory resolution, and event gaps while adding only the approved noise-level local checks and events beside existing I/O. It contains no registry, retry loop, state machine, compatibility shim, runtime serializer wrapper, hot-path synchronization, unresolved accepted defect, or stale superseded path. + +### Complete Session lifecycles, persistence, and current Laravel parity + +- **Architecture and inspected risk surfaces:** Session is a Laravel-derived manager, Store, middleware, and handler surface adapted to worker-cached drivers and coroutine-local request state. The audit covered every Session source and test file; Foundation configuration and HTTP-test context synchronization; Filesystem write/delete contracts; Contracts and facade metadata; Cache locks and cloned Redis stores; Auth password-confirmation state; Routing block settings; current Laravel framework/application source, tests, documentation, and originating pull requests; and every carried Support, Redis, and Cache assumption. The detailed design is recorded in [`2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md`](2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md). + +| ID | Category | Severity | Confidence | Failure and owning boundary | Final decision | +|---|---|---|---|---|---| +| `session-01` | Defect | Major | High | Fixed Store context keys make concurrent or sibling Store objects share IDs, attributes, and started state | Derive three per-object context keys at construction, initialize every slot, and synchronize only the active Store through Foundation testing | +| `session-02` | Defect and upstream defect | Major | High | Save mutates flash and JSON error-bag state before persistence commits, so a failed write corrupts the live retry state | Age and marshal pure snapshots, serialize and write, reject false, then publish the aged live snapshot | +| `session-03` | Defect | Major | High | Failed session startup registers a later empty write against a cookie-derived ID | Use `Request::hasSession()` as the existing startup commit flag before registering a retry | +| `session-04` | Defect | Major | High | Manual lock release can replace the request failure or release after acquisition timeout | Delegate ownership and cleanup to Cache's `Lock::block(..., $callback)` boundary | +| `session-05` | Defect and upstream defect | Major | High | JSON cookie envelopes reject binary serialized payloads and accept malformed decoded shapes | Use a private PHP-serialized envelope with classes disabled and strict data/expiry validation | +| `session-06` | Defect | Minor | High | File GC passes a false-capable real path and counts failed deletions | Delete Finder pathnames and count only successful deletions | +| `session-07` | Defect | Minor | High | A direct Database handler write retains its pre-read false existence value and attempts a duplicate insert | Refresh local existence once after the cold read | +| `session-08` | Current Laravel parity and configuration improvement | Minor | High | Redis sessions cannot own a distinct prefix and reuse cache naming by default | Configure the cloned Redis store with a dedicated application-scoped Session prefix and preserve null/empty fallback | +| `session-09` | Current Laravel parity | Improvement | High | Store collection predicates lag current Laravel and `hasAny()` scans every key | Port current `doesntContain()` / `contains()` implementations and early exit | +| `session-10` | Configuration and security defect | Major | High | Supported blocking/serialization keys are undeclared, defaults are duplicated, and invalid serialization silently selects PHP | Declare canonical config, default applications to JSON, retain direct-construction PHP defaults, and reject unsupported strategies | +| `session-11` | Dead-code cleanup | Improvement | High | Redundant provider registration, an empty boot method, and an unused cache-driver wrapper obscure the real extension boundary | Delete them and record the intentionally omitted unsupported Laravel drivers with `Session::extend()` as the alternative | +| `session-12` | Userland footgun and API cleanup | Minor | High | Unused Hypervel-only `setConnection()` can mutate a worker-cached Database handler during requests | Remove it and document retained `setContainer()` as boot/test-only | +| `session-13` | Metadata defect | Minor | High | The Session split manifest omits direct runtime dependencies | Declare the complete dependency set and enforce it with package metadata coverage | +| `session-14` | Documentation defect and upstream defect | Minor | High | Custom-driver documentation describes the GC lifetime in seconds as a Unix timestamp | Correct the public driver contract wording | +| `session-15` | Contract defect | Major | High | Nullable Store IDs violate Laravel, Symfony, handler, and guard string boundaries | Lazily create and return a non-null ID per Store and coroutine | +| `session-16` | Type-consistency improvement | Improvement | High | Array and Null handlers lag the native SessionHandlerInterface signatures already used by sibling handlers | Complete only the missing native parameter types without widening valid read covariance | +| `session-17` | Static-analysis maintenance defect | Minor | High | Unmatched Session and Filesystem ignores can hide later real errors | Remove only the verified stale suppressions and retain genuine magic-proxy ignores | +| `session-18` | Intentional runtime difference | Minor | High | Array sessions are worker-local and unsuitable for production persistence | Document the runtime boundary concisely | +| `session-19` | Defect | Major | High | A failed after-response save retry can escape the exception renderer and replace the primary request failure | Preserve reporting/rendering of the first failure and contain only the retry failure | +| `session-20` | Defect and upstream defect | Major | High | JSON startup marshals the merged live attributes and can replace an existing validation error bag with an empty bag | Marshal only decoded storage data before merging it into live attributes | +| `session-21` | Defect | Major | High | A reused Database handler object ID can inherit `exists=true`, update no row, and falsely report success | Reset the dynamic object-specific existence slot at construction and cloning | +| `session-22` | Defect | Major | High | File sessions report success after false or partial filesystem writes | Require the complete returned byte count while keeping destroy idempotent | +| `filesystem-12` | Type-consistency improvement | Improvement | High | Concrete `Filesystem::delete()` omits the contract and sibling implementations' native `array|string` type | Add the native union without changing runtime behavior | + +- **Approved owner gates and intentional differences:** The owner approved current Laravel Session APIs/configuration, the JSON application default, the dedicated `app_id() . '_session:'` Redis prefix, non-null IDs, false-write rejection, after-response retry containment, collection parity, handler type completion, dead-code removal, metadata/documentation completion, and every source-proven noise-level cost. Direct `Store` construction retains Laravel's PHP serialization default. Unsupported `apc`, `memcached`, and `dynamodb` session drivers and their shared wrapper remain intentionally omitted because Hypervel has no matching Cache stores; the README and source point custom cache-backed handlers to `Session::extend()`. +- **Important rejected concerns:** Do not add a Store clone lifecycle, context registry, `WeakMap`, serializer service/enum/registry, persistence transaction object, retry loop, failure event/logger, cookie compatibility decoder or base64 frame, Database upsert/extra existence query/exception classifier, per-request driver clone, broad handler read-type sweep, defensive reserved-error-key guard, or broad PHPStan cleanup inside Session. Store cloning has no supported consumer; direct Database writes retain the one necessary cold-read refresh; and deliberate framework escape hatches remain escape hatches. A separate owner-approved framework-wide todo records unmatched PHPStan suppression cleanup without expanding this work unit. +- **Implementation:** Store state now uses precomputed per-object context keys and lazy non-null IDs, while Foundation testing synchronizes only the active Store's dynamic slots. JSON load marshals decoded storage before merging; save uses pure flash/error-bag snapshots and publishes only after an exact successful handler write. Middleware uses the request session as its startup commit flag, Cache's callback lock owner, and contained after-response retry failure. Cookie envelopes are binary-safe and strict; File and Database handlers have truthful write and object-lifecycle behavior; Redis sessions configure only the cloned cache store. Current Store predicates, declared configuration, direct split dependencies, facade/contracts, intentional omission records, public documentation, and handler types are complete. Redundant bindings, methods, call-site defaults, suppressions, and stale wording are removed. +- **Regression tests:** Deterministic coverage proves independent and reused Store identities, fresh-coroutine IDs, active-context synchronization and removal, pure JSON error-bag precedence, throwing/false/encoding save failure and successful retry, consecutive JSON saves, failed startup and contained retry, lock timeout/failure precedence, strict binary cookie envelopes, successful/false/partial file writes, file GC accounting, Database direct update plus constructor/clone identity reset, cloned Redis prefix/connection isolation, null/empty/zero prefix semantics, incompatible Redis store failure, canonical config, metadata, current collection behavior, and unchanged public handler contracts. All touched Session tests use native `void` return types. +- **Cross-package revalidation:** `support-02` remains correct at Session enum/default-driver boundaries. `redis-13` is completed for Session by configuring normalized connection and prefix state only on the cloned Redis store. Cache's callback lock failure precedence and cloned Repository behavior remain intact. Foundation owns the HTTP-test context bridge, JSON application config, and rendered-exception lifecycle; Filesystem owns `filesystem-12` and the exact byte/delete results consumed by the handlers. Contracts and Support facade metadata now expose the truthful non-null ID. No completed package assumption is weakened. +- **Performance and complexity:** Ordinary Store state operations remain one context lookup against a key allocated once per manager-cached Store. `getId()` adds only an absent-slot branch. Save adds no lock, retry, I/O, container resolution, yield, or retained state beyond existing handler work; PHP copy-on-write touches only changed snapshots. Unblocked requests are unchanged, while blocked routes add one callback beside existing lock I/O. Redis prefix/type setup occurs once at driver construction with no command or pool checkout. Direct Database cold writes add one memory lookup and avoid an exception/extra database work. File writes compare an already-returned byte count. No registry, state machine, compatibility path, worker cache, or hot-path synchronization is introduced. +- **Laravel-facing result:** Current supported Laravel Session APIs, collection behavior, handler contracts, Redis prefix support, configuration, tests, and documentation are restored or preserved. `getId()` regains Laravel's truthful string contract; the removed `setConnection()` was unused and Hypervel-only. Hypervel intentionally adds a dedicated Session prefix default, retains worker/coroutine adaptations and cloned cache-store isolation, fixes two shared upstream persistence defects, and records unsupported cache-backed drivers at every required future-sync surface. +- **Validation and review:** Every changed test file and affected Session, Foundation testing, Filesystem, configuration, and integration group passed during implementation. The authoritative `composer fix` gate changed no formatting, both PHPStan configurations passed, and the complete parallel components, Testbench package, and dogfood suites passed. `git diff --check`, dependency/stale-reference/suppression scans, current-upstream comparison, and fresh caller/callee, context-lifecycle, persistence, retry, lock, handler, API, documentation, hot-path, retained-state, and overengineering review are complete. Independent code review re-ran the gates, verified the final constructor/clone and file-write amendments, and signed off with no remaining finding. +- **Assessment:** Every accepted Session finding and `filesystem-12` is fixed at its lowest owner. The result removes cross-Store state sharing, pre-commit mutation, false persistence success, unsafe retry/lock cleanup, malformed cookie framing, stale object-ID state, dead APIs/bindings/defaults/suppressions, and metadata/documentation drift while preserving all useful Laravel and Hypervel capabilities. It contains no workaround, speculative abstraction, compatibility shim, meaningful hot-path regression, unresolved accepted defect, or stale superseded path. From cb2847797b8d11ae5a0068f560ee4ab57723a16e Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 06:44:37 +0000 Subject: [PATCH 15/16] docs(audit): repair union types in markdown tables Escape union separators only where inline code appears inside Markdown table cells. Raw pipes were being parsed as additional columns, truncating the Session, Filesystem, Bus, and Console audit records in rendered documentation. Leave identical notation in normal prose untouched, where escaping would render a visible backslash. The five repaired rows now retain their expected column counts and code spans through GitHub's GFM renderer. --- ...0915-framework-coroutine-state-lifecycle-audit-ledger.md | 6 +++--- ...sion-lifecycle-persistence-and-current-laravel-parity.md | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md index 0270bc25f..62e51b09f 100644 --- a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md +++ b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md @@ -895,7 +895,7 @@ Append package entries in checklist order. Keep each entry compact but complete |---|---|---|---|---|---| | `bus-01` | Defect | Major | High | The direct `QueueingDispatcher` alias bypasses a `Bus::fake()` swap and resolves the real dispatcher | Chain the queueing contract alias through the base dispatcher contract and prove both resolve the fake | | `bus-02` | Defect | Minor | High | Bus lacks current Laravel bulk dispatch, fake support, facade metadata, tests, and documentation; upstream's delimiter-concatenated grouping key silently merges supported colon-bearing connection and queue routes | Port current bulk and immediate-dispatch behavior with a nested connection/queue route map; require `bulk()` on Hypervel's `QueueingDispatcher` because every conforming queue dispatcher must provide the facade capability | -| `bus-03` | Defect | Minor | High | `PreparesForDispatch` rejects valid void implementations, and pending dispatch cannot disable a previously selected after-response mode | Restore Laravel's `bool|void` contract and `afterResponse(bool)` toggle with current integration and Conditionable coverage | +| `bus-03` | Defect | Minor | High | `PreparesForDispatch` rejects valid void implementations, and pending dispatch cannot disable a previously selected after-response mode | Restore Laravel's `bool\|void` contract and `afterResponse(bool)` toggle with current integration and Conditionable coverage | | `bus-04` | Defect | Minor | High | Batch started/canceled events, cancellation exceptions, first-job detection, explicit chain routing, and finished-state batching differ from current Laravel | Port the complete current lifecycle and guard optional observational events with `hasListeners()` | | `bus-05` | Defect | Minor | High | Truthiness drops public/custom batch ID `"0"` in `Batchable`, its fake, and paginated repository reads | Use the exact null/empty sentinels at each existing boundary without a normalizer | | `bus-06` | Defect | Major | High | Concurrent batch deletion or unfinished pruning makes atomic count updates read absent fields, while a later callback refresh can pass null to a typed Batch callback | Return nullable updated counts from the locked repository boundary and stop completion/failure/callback processing when the batch no longer exists | @@ -998,7 +998,7 @@ Append package entries in checklist order. Keep each entry compact but complete | `console-06` | Defect | Major | High | Scheduled Event failures can skip output, callback, or mutex cleanup and later cleanup can mask the earliest throwable | Attempt every independent terminal phase, preserve the earliest throwable, and clear local mutex ownership in `finally` | | `console-07` | Defect | Major | High | Completed scheduled system processes remain retained for the scheduler coroutine lifetime | Retain the exact process only through output/after callbacks and forget its object-keyed context slot on every terminal path | | `console-08` | Defect | Major | High | Scheduled output ignores failed/partial writes and uses TOCTOU native reads that can violate string contracts | Require exact write lengths and centralize checked reads through Filesystem | -| `console-09` | Parity defect | Minor | High | Current Laravel scheduled callback dependency injection is absent and invokable filter callbacks are rejected | Port current Event callback parameter routing and widen filter callbacks to `bool|callable` | +| `console-09` | Parity defect | Minor | High | Current Laravel scheduled callback dependency injection is absent and invokable filter callbacks are rejected | Port current Event callback parameter routing and widen filter callbacks to `bool\|callable` | | `console-10` | Defect | Major | High | Isolated commands re-resolve their mutex for release, one terminal event failure skips cleanup, and setup-hook failure can retain signal handlers | Retain the exact mutex, make terminal phases exhaustive, and include supported trait setup in the command's signal-cleanup boundary | | `console-11` | Defect | Minor | High | `CacheCommandMutex::exists()` reverses the answer while acquiring or releasing locks it does not own | Match CacheEventMutex's non-destructive closure-based probe | | `console-12` | Defect | Major | High | Nested commands overwrite the parent command's process-global prompt configuration and restore only output on success | Reconfigure every parent prompt setting in `finally` and delete the partial restore helper | @@ -1258,7 +1258,7 @@ Append package entries in checklist order. Keep each entry compact but complete | `session-20` | Defect and upstream defect | Major | High | JSON startup marshals the merged live attributes and can replace an existing validation error bag with an empty bag | Marshal only decoded storage data before merging it into live attributes | | `session-21` | Defect | Major | High | A reused Database handler object ID can inherit `exists=true`, update no row, and falsely report success | Reset the dynamic object-specific existence slot at construction and cloning | | `session-22` | Defect | Major | High | File sessions report success after false or partial filesystem writes | Require the complete returned byte count while keeping destroy idempotent | -| `filesystem-12` | Type-consistency improvement | Improvement | High | Concrete `Filesystem::delete()` omits the contract and sibling implementations' native `array|string` type | Add the native union without changing runtime behavior | +| `filesystem-12` | Type-consistency improvement | Improvement | High | Concrete `Filesystem::delete()` omits the contract and sibling implementations' native `array\|string` type | Add the native union without changing runtime behavior | - **Approved owner gates and intentional differences:** The owner approved current Laravel Session APIs/configuration, the JSON application default, the dedicated `app_id() . '_session:'` Redis prefix, non-null IDs, false-write rejection, after-response retry containment, collection parity, handler type completion, dead-code removal, metadata/documentation completion, and every source-proven noise-level cost. Direct `Store` construction retains Laravel's PHP serialization default. Unsupported `apc`, `memcached`, and `dynamodb` session drivers and their shared wrapper remain intentionally omitted because Hypervel has no matching Cache stores; the README and source point custom cache-backed handlers to `Session::extend()`. - **Important rejected concerns:** Do not add a Store clone lifecycle, context registry, `WeakMap`, serializer service/enum/registry, persistence transaction object, retry loop, failure event/logger, cookie compatibility decoder or base64 frame, Database upsert/extra existence query/exception classifier, per-request driver clone, broad handler read-type sweep, defensive reserved-error-key guard, or broad PHPStan cleanup inside Session. Store cloning has no supported consumer; direct Database writes retain the one necessary cold-read refresh; and deliberate framework escape hatches remain escape hatches. A separate owner-approved framework-wide todo records unmatched PHPStan suppression cleanup without expanding this work unit. diff --git a/docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md b/docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md index 6c795418b..94a9a1113 100644 --- a/docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md +++ b/docs/plans/2026-07-27-0530-session-lifecycle-persistence-and-current-laravel-parity.md @@ -140,7 +140,7 @@ must make that code-owned compatibility decision in configuration. | `session-03` | Defect | Major | Failed startup registers a later empty write against the cookie-derived ID | `Request::hasSession()` commit flag | | `session-04` | Defect | Major | Manual lock release can replace request failure or run after failed acquire | Cache lock callback form | | `session-05` | Defect and upstream defect | Major | JSON cookie envelope rejects binary serialized data and accepts invalid decoded shapes | Private PHP-serialized envelope | -| `session-06` | Defect | Minor | File GC passes `string|false` and counts failed deletes | Finder pathname and successful-delete count | +| `session-06` | Defect | Minor | File GC passes `string\|false` and counts failed deletes | Finder pathname and successful-delete count | | `session-07` | Defect | Minor | Direct database write caches stale false existence after `read()` updates context | Refresh local existence once | | `session-08` | Current Laravel parity and configuration improvement | Minor | Redis sessions cannot own a distinct prefix | Truthful RedisStore setup and declared default | | `session-09` | Current Laravel parity | Improvement | Store collection checks lag current upstream and `hasAny()` scans all keys | Current `doesntContain()` / `contains()` shape | @@ -157,7 +157,7 @@ must make that code-owned compatibility decision in configuration. | `session-20` | Defect and upstream defect | Major | Starting a JSON Store can replace an already-live validation error bag with an empty one | Marshal only the decoded storage payload before merging | | `session-21` | Defect | Major | A reused Database handler object ID can inherit `exists=true`, update zero rows, and report a silently lost write | Initialize object-specific state on construction and cloning | | `session-22` | Defect | Major | The file handler reports success after false or partial filesystem writes | Require the complete byte count | -| `filesystem-12` | Type-consistency improvement | Improvement | Concrete `Filesystem::delete()` alone omits its contract's native union | Add `array|string` | +| `filesystem-12` | Type-consistency improvement | Improvement | Concrete `Filesystem::delete()` alone omits its contract's native union | Add `array\|string` | ## Owner decisions From b25b012978bcc54ff49af5ac35dfe449f16d5d72 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 27 Jul 2026 08:08:32 +0000 Subject: [PATCH 16/16] fix(cache): preserve literal session cache keys Store the session cache collection as a flat map so dotted cache keys are no longer interpreted as Session attribute paths. This prevents neighboring keys from nesting into, overwriting, or deleting one another while preserving expiration metadata during increments. Document that the session cache inherits the configured Session serializer and record the JSON-backed store's object-fidelity limitation without adding another encoding or validation layer. Add focused regressions for dotted-key coexistence, deletion, enumeration, and expiration-preserving increments, and update the audit ledger with the final boundaries. --- ...-coroutine-state-lifecycle-audit-ledger.md | 14 ++++--- src/boost/docs/cache.md | 2 + src/boost/docs/session.md | 2 + src/cache/src/SessionStore.php | 37 ++++++++++++------- tests/Cache/CacheSessionStoreTest.php | 31 ++++++++++++++++ 5 files changed, 67 insertions(+), 19 deletions(-) diff --git a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md index 62e51b09f..a3f518f43 100644 --- a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md +++ b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md @@ -1258,14 +1258,16 @@ Append package entries in checklist order. Keep each entry compact but complete | `session-20` | Defect and upstream defect | Major | High | JSON startup marshals the merged live attributes and can replace an existing validation error bag with an empty bag | Marshal only decoded storage data before merging it into live attributes | | `session-21` | Defect | Major | High | A reused Database handler object ID can inherit `exists=true`, update no row, and falsely report success | Reset the dynamic object-specific existence slot at construction and cloning | | `session-22` | Defect | Major | High | File sessions report success after false or partial filesystem writes | Require the complete returned byte count while keeping destroy idempotent | +| `session-23` | Defect and upstream defect | Major | High | Session cache sends valid dotted cache keys through Session dot notation, so entries can nest, overwrite, or delete one another | Manage the `_cache` collection as a flat map of literal cache keys | +| `session-24` | Known PSR-16 deviation and documentation defect | Minor | High | A JSON-backed session cache returns PHP objects as decoded JSON values instead of preserving their type and value | Document the inherited serializer and exact-value limitation without adding validation or another encoding layer | | `filesystem-12` | Type-consistency improvement | Improvement | High | Concrete `Filesystem::delete()` omits the contract and sibling implementations' native `array\|string` type | Add the native union without changing runtime behavior | -- **Approved owner gates and intentional differences:** The owner approved current Laravel Session APIs/configuration, the JSON application default, the dedicated `app_id() . '_session:'` Redis prefix, non-null IDs, false-write rejection, after-response retry containment, collection parity, handler type completion, dead-code removal, metadata/documentation completion, and every source-proven noise-level cost. Direct `Store` construction retains Laravel's PHP serialization default. Unsupported `apc`, `memcached`, and `dynamodb` session drivers and their shared wrapper remain intentionally omitted because Hypervel has no matching Cache stores; the README and source point custom cache-backed handlers to `Session::extend()`. -- **Important rejected concerns:** Do not add a Store clone lifecycle, context registry, `WeakMap`, serializer service/enum/registry, persistence transaction object, retry loop, failure event/logger, cookie compatibility decoder or base64 frame, Database upsert/extra existence query/exception classifier, per-request driver clone, broad handler read-type sweep, defensive reserved-error-key guard, or broad PHPStan cleanup inside Session. Store cloning has no supported consumer; direct Database writes retain the one necessary cold-read refresh; and deliberate framework escape hatches remain escape hatches. A separate owner-approved framework-wide todo records unmatched PHPStan suppression cleanup without expanding this work unit. -- **Implementation:** Store state now uses precomputed per-object context keys and lazy non-null IDs, while Foundation testing synchronizes only the active Store's dynamic slots. JSON load marshals decoded storage before merging; save uses pure flash/error-bag snapshots and publishes only after an exact successful handler write. Middleware uses the request session as its startup commit flag, Cache's callback lock owner, and contained after-response retry failure. Cookie envelopes are binary-safe and strict; File and Database handlers have truthful write and object-lifecycle behavior; Redis sessions configure only the cloned cache store. Current Store predicates, declared configuration, direct split dependencies, facade/contracts, intentional omission records, public documentation, and handler types are complete. Redundant bindings, methods, call-site defaults, suppressions, and stale wording are removed. -- **Regression tests:** Deterministic coverage proves independent and reused Store identities, fresh-coroutine IDs, active-context synchronization and removal, pure JSON error-bag precedence, throwing/false/encoding save failure and successful retry, consecutive JSON saves, failed startup and contained retry, lock timeout/failure precedence, strict binary cookie envelopes, successful/false/partial file writes, file GC accounting, Database direct update plus constructor/clone identity reset, cloned Redis prefix/connection isolation, null/empty/zero prefix semantics, incompatible Redis store failure, canonical config, metadata, current collection behavior, and unchanged public handler contracts. All touched Session tests use native `void` return types. +- **Approved owner gates and intentional differences:** The owner approved current Laravel Session APIs/configuration, the JSON application default, the dedicated `app_id() . '_session:'` Redis prefix, non-null IDs, false-write rejection, after-response retry containment, collection parity, handler type completion, literal dotted session-cache keys, dead-code removal, metadata/documentation completion, and every source-proven noise-level cost. Direct `Store` construction retains Laravel's PHP serialization default. Unsupported `apc`, `memcached`, and `dynamodb` session drivers and their shared wrapper remain intentionally omitted because Hypervel has no matching Cache stores; the README and source point custom cache-backed handlers to `Session::extend()`. +- **Important rejected concerns:** Do not add a Store clone lifecycle, context registry, `WeakMap`, serializer service/enum/registry, persistence transaction object, retry loop, failure event/logger, cookie compatibility decoder or base64 frame, Database upsert/extra existence query/exception classifier, per-request driver clone, broad handler read-type sweep, defensive reserved-error-key guard, session-cache-only serializer or recursive object validation, or broad PHPStan cleanup inside Session. Store cloning has no supported consumer; direct Database writes retain the one necessary cold-read refresh; JSON session cache inherits the selected Session serializer; and deliberate framework escape hatches remain escape hatches. A separate owner-approved framework-wide todo records unmatched PHPStan suppression cleanup without expanding this work unit. +- **Implementation:** Store state now uses precomputed per-object context keys and lazy non-null IDs, while Foundation testing synchronizes only the active Store's dynamic slots. JSON load marshals decoded storage before merging; save uses pure flash/error-bag snapshots and publishes only after an exact successful handler write. Middleware uses the request session as its startup commit flag, Cache's callback lock owner, and contained after-response retry failure. Cookie envelopes are binary-safe and strict; File and Database handlers have truthful write and object-lifecycle behavior; Redis sessions configure only the cloned cache store. Session cache stores its entries in one flat literal-keyed map and documents the JSON object-fidelity boundary. Current Store predicates, declared configuration, direct split dependencies, facade/contracts, intentional omission records, public documentation, and handler types are complete. Redundant bindings, methods, call-site defaults, suppressions, and stale wording are removed. +- **Regression tests:** Deterministic coverage proves independent and reused Store identities, fresh-coroutine IDs, active-context synchronization and removal, pure JSON error-bag precedence, throwing/false/encoding save failure and successful retry, consecutive JSON saves, failed startup and contained retry, lock timeout/failure precedence, strict binary cookie envelopes, successful/false/partial file writes, file GC accounting, Database direct update plus constructor/clone identity reset, cloned Redis prefix/connection isolation, null/empty/zero prefix semantics, incompatible Redis store failure, literal dotted session-cache keys across retrieval, enumeration, expiration-preserving increment, and deletion, canonical config, metadata, current collection behavior, and unchanged public handler contracts. All touched Session tests use native `void` return types. - **Cross-package revalidation:** `support-02` remains correct at Session enum/default-driver boundaries. `redis-13` is completed for Session by configuring normalized connection and prefix state only on the cloned Redis store. Cache's callback lock failure precedence and cloned Repository behavior remain intact. Foundation owns the HTTP-test context bridge, JSON application config, and rendered-exception lifecycle; Filesystem owns `filesystem-12` and the exact byte/delete results consumed by the handlers. Contracts and Support facade metadata now expose the truthful non-null ID. No completed package assumption is weakened. -- **Performance and complexity:** Ordinary Store state operations remain one context lookup against a key allocated once per manager-cached Store. `getId()` adds only an absent-slot branch. Save adds no lock, retry, I/O, container resolution, yield, or retained state beyond existing handler work; PHP copy-on-write touches only changed snapshots. Unblocked requests are unchanged, while blocked routes add one callback beside existing lock I/O. Redis prefix/type setup occurs once at driver construction with no command or pool checkout. Direct Database cold writes add one memory lookup and avoid an exception/extra database work. File writes compare an already-returned byte count. No registry, state machine, compatibility path, worker cache, or hot-path synchronization is introduced. -- **Laravel-facing result:** Current supported Laravel Session APIs, collection behavior, handler contracts, Redis prefix support, configuration, tests, and documentation are restored or preserved. `getId()` regains Laravel's truthful string contract; the removed `setConnection()` was unused and Hypervel-only. Hypervel intentionally adds a dedicated Session prefix default, retains worker/coroutine adaptations and cloned cache-store isolation, fixes two shared upstream persistence defects, and records unsupported cache-backed drivers at every required future-sync surface. +- **Performance and complexity:** Ordinary Store state operations remain one context lookup against a key allocated once per manager-cached Store. `getId()` adds only an absent-slot branch. Save adds no lock, retry, I/O, container resolution, yield, or retained state beyond existing handler work; PHP copy-on-write touches only changed snapshots. Unblocked requests are unchanged, while blocked routes add one callback beside existing lock I/O. Redis prefix/type setup occurs once at driver construction with no command or pool checkout. Direct Database cold writes add one memory lookup and avoid an exception/extra database work. File writes compare an already-returned byte count. Session-cache reads remove a duplicate Session lookup; writes read and replace only the local cache map, adding no serialization, backend I/O, network round trip, or retained state. No registry, state machine, compatibility path, worker cache, or hot-path synchronization is introduced. +- **Laravel-facing result:** Current supported Laravel Session APIs, collection behavior, handler contracts, Redis prefix support, configuration, tests, and documentation are restored or preserved. `getId()` regains Laravel's truthful string contract; the removed `setConnection()` was unused and Hypervel-only. Hypervel intentionally adds a dedicated Session prefix default, retains worker/coroutine adaptations and cloned cache-store isolation, fixes shared upstream persistence and dotted-key defects, documents the JSON-backed session cache's PSR-16 limitation, and records unsupported cache-backed drivers at every required future-sync surface. - **Validation and review:** Every changed test file and affected Session, Foundation testing, Filesystem, configuration, and integration group passed during implementation. The authoritative `composer fix` gate changed no formatting, both PHPStan configurations passed, and the complete parallel components, Testbench package, and dogfood suites passed. `git diff --check`, dependency/stale-reference/suppression scans, current-upstream comparison, and fresh caller/callee, context-lifecycle, persistence, retry, lock, handler, API, documentation, hot-path, retained-state, and overengineering review are complete. Independent code review re-ran the gates, verified the final constructor/clone and file-write amendments, and signed off with no remaining finding. - **Assessment:** Every accepted Session finding and `filesystem-12` is fixed at its lowest owner. The result removes cross-Store state sharing, pre-commit mutation, false persistence success, unsafe retry/lock cleanup, malformed cookie framing, stale object-ID state, dead APIs/bindings/defaults/suppressions, and metadata/documentation drift while preserving all useful Laravel and Hypervel capabilities. It contains no workaround, speculative abstraction, compatibility shim, meaningful hot-path regression, unresolved accepted defect, or stale superseded path. diff --git a/src/boost/docs/cache.md b/src/boost/docs/cache.md index 4c89ef9ca..b024bc314 100644 --- a/src/boost/docs/cache.md +++ b/src/boost/docs/cache.md @@ -229,6 +229,8 @@ The `session` cache driver stores cache values inside the active session store. ], ``` +Session cache values use the serialization strategy configured for the session. With the default `json` strategy, cached PHP objects do not retain their type or value across requests, so this store does not provide PSR-16's exact-value guarantee for objects. If your application needs to cache PHP objects in the session, set the `serialization` option in `config/session.php` to `php` and review the [security considerations](/docs/{{version}}/session#configuration). + ### Cache Failover diff --git a/src/boost/docs/session.md b/src/boost/docs/session.md index 5c36d546e..4b155d213 100644 --- a/src/boost/docs/session.md +++ b/src/boost/docs/session.md @@ -305,6 +305,8 @@ $request->session()->cache()->put( By default, session cache values are stored under the `_cache` key within the user's session data. You may change this key using the `SESSION_CACHE_KEY` environment variable or the `key` option of the `session` cache store. +Session cache values use the session's configured serialization strategy. With the default `json` strategy, cached PHP objects do not retain their type or value across requests, so the session cache does not provide PSR-16's exact-value guarantee for objects. If you need to retrieve cached PHP objects in their original form, use PHP serialization as described in the [configuration section](#configuration). + For more information on Hypervel's cache methods, consult the [cache documentation](/docs/{{version}}/cache). diff --git a/src/cache/src/SessionStore.php b/src/cache/src/SessionStore.php index bd267f3bf..e8227e80d 100644 --- a/src/cache/src/SessionStore.php +++ b/src/cache/src/SessionStore.php @@ -38,13 +38,14 @@ public function all(): array */ public function get(string $key): mixed { - if (! $this->session->exists($this->itemKey($key))) { + $items = $this->all(); + + if (! array_key_exists($key, $items)) { return null; } - $item = $this->session->get($this->itemKey($key)); - - $expiresAt = $item['expiresAt'] ?? 0.0; + $item = $items[$key]; + $expiresAt = $item['expiresAt']; if ($this->isExpired($expiresAt)) { $this->forget($key); @@ -68,10 +69,13 @@ protected function isExpired(int|float $expiresAt): bool */ public function put(string $key, mixed $value, int $seconds): bool { - $this->session->put($this->itemKey($key), [ + $items = $this->all(); + $items[$key] = [ 'value' => $value, 'expiresAt' => $this->toTimestamp($seconds), - ]); + ]; + + $this->session->put($this->key, $items); return true; } @@ -90,9 +94,12 @@ protected function toTimestamp(int $seconds): float public function increment(string $key, int $value = 1): int { if (! is_null($existing = $this->get($key))) { - return tap(((int) $existing) + $value, function ($incremented) use ($key) { - $this->session->put($this->itemKey("{$key}.value"), $incremented); - }); + $incremented = ((int) $existing) + $value; + $items = $this->all(); + $items[$key]['value'] = $incremented; + $this->session->put($this->key, $items); + + return $incremented; } $this->forever($key, $value); @@ -137,13 +144,17 @@ public function touch(string $key, int $seconds): bool */ public function forget(string $key): bool { - if ($this->session->exists($this->itemKey($key))) { - $this->session->forget($this->itemKey($key)); + $items = $this->all(); - return true; + if (! array_key_exists($key, $items)) { + return false; } - return false; + unset($items[$key]); + + $this->session->put($this->key, $items); + + return true; } /** diff --git a/tests/Cache/CacheSessionStoreTest.php b/tests/Cache/CacheSessionStoreTest.php index 365904d1d..ffd227098 100755 --- a/tests/Cache/CacheSessionStoreTest.php +++ b/tests/Cache/CacheSessionStoreTest.php @@ -21,6 +21,22 @@ public function testItemsCanBeSetAndRetrieved() $this->assertSame('bar', $store->get('foo')); } + public function testDottedKeysAreStoredLiterallyAndIndependently(): void + { + $store = new SessionStore(self::getSession()); + + $store->put('form', 'first', 10); + $store->put('form.value', 'second', 10); + + $this->assertSame('first', $store->get('form')); + $this->assertSame('second', $store->get('form.value')); + $this->assertSame(['form', 'form.value'], array_keys($store->all())); + + $this->assertTrue($store->forget('form')); + $this->assertNull($store->get('form')); + $this->assertSame('second', $store->get('form.value')); + } + public function testCacheTtl() { $store = new SessionStore(self::getSession()); @@ -112,6 +128,21 @@ public function testValuesCanBeIncremented() $this->assertEquals(4, $store->get('foo')); } + public function testDottedKeysCanBeIncrementedWithoutChangingTheirExpiration(): void + { + CarbonImmutable::setTestNow('2000-01-01 00:00:00'); + + $store = new SessionStore(self::getSession()); + $store->put('counter.value', 1, 10); + + $expiresAt = $store->all()['counter.value']['expiresAt']; + + $this->assertSame(2, $store->increment('counter.value')); + $this->assertSame(2, $store->get('counter.value')); + $this->assertSame($expiresAt, $store->all()['counter.value']['expiresAt']); + $this->assertSame(['counter.value'], array_keys($store->all())); + } + public function testValuesGetCastedByIncrementOrDecrement() { $store = new SessionStore(self::getSession());