From 2e100caa807fb3ee19a6d368f8825d7fdf4122cb Mon Sep 17 00:00:00 2001 From: Arif Hoque Date: Wed, 6 May 2026 13:07:52 +0600 Subject: [PATCH] Fix: per-request singleton state leakage in worker mode --- src/Phaseolies/Application.php | 31 ++++++- src/Phaseolies/DI/Container.php | 11 +++ tests/Application/ApplicationTest.php | 113 ++++++++++++++++++++++++++ tests/Application/ContainerTest.php | 32 +++++++- 4 files changed, 182 insertions(+), 5 deletions(-) diff --git a/src/Phaseolies/Application.php b/src/Phaseolies/Application.php index c673266f..79cd234f 100644 --- a/src/Phaseolies/Application.php +++ b/src/Phaseolies/Application.php @@ -2,6 +2,7 @@ namespace Phaseolies; +use Phaseolies\Auth\ActorManager; use Phaseolies\Support\Router; use Phaseolies\Providers\ServiceProvider; use Phaseolies\Http\DispatchResult; @@ -871,12 +872,32 @@ public function terminating(callable $callback): self */ public function terminate(Request $request, ?Response $response = null, ?\Throwable $exception = null): void { - if (empty($this->terminatingCallbacks)) { - return; + try { + foreach ($this->terminatingCallbacks as $callback) { + $this->callTerminatingCallback($callback, $request, $response, $exception); + } + } finally { + $this->cleanupRequestScopedServices(); + } + } + + /** + * Drop resolved request-scoped services while preserving their bindings. + * + * @return void + */ + protected function cleanupRequestScopedServices(): void + { + if ($this->has('auth')) { + $auth = $this->make('auth'); + + if ($auth instanceof ActorManager) { + $auth->forgetActors(); + } } - foreach ($this->terminatingCallbacks as $callback) { - $this->callTerminatingCallback($callback, $request, $response, $exception); + foreach (['session', 'request', 'response', 'redirect'] as $abstract) { + $this->forgetResolved($abstract); } } @@ -1012,6 +1033,8 @@ public function handle(Request $request): Response public function dispatch($request): DispatchResult { try { + $this->instance('request', $request); + $response = $this->handle($request); $response->prepare($request)->send(); diff --git a/src/Phaseolies/DI/Container.php b/src/Phaseolies/DI/Container.php index c4e2d5d6..50c77b02 100644 --- a/src/Phaseolies/DI/Container.php +++ b/src/Phaseolies/DI/Container.php @@ -398,6 +398,17 @@ public function hasInstance(string $key): bool return isset(self::$instances[$key]) && self::$instances[$key] !== null; } + /** + * Forget a resolved instance while keeping its binding intact. + * + * @param string $abstract + * @return void + */ + public function forgetResolved(string $abstract): void + { + unset(self::$instances[$abstract]); + } + /** * Flush the container of all instances and bindings. * diff --git a/tests/Application/ApplicationTest.php b/tests/Application/ApplicationTest.php index c912f7b3..4e26e9aa 100644 --- a/tests/Application/ApplicationTest.php +++ b/tests/Application/ApplicationTest.php @@ -5,13 +5,16 @@ use ReflectionClass; use Tests\Support\Kernel; use Phaseolies\Application; +use Phaseolies\Auth\ActorManager; use Phaseolies\DI\Container; use Phaseolies\Http\DispatchResult; use Phaseolies\Http\Request; use Phaseolies\Http\Response; use Phaseolies\Http\Exceptions\HttpException; use Phaseolies\Config\Config; +use Phaseolies\Http\Response\RedirectResponse; use Phaseolies\Support\Router; +use Phaseolies\Support\Session; use Phaseolies\Console\Console; use PHPUnit\Framework\TestCase; use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; @@ -408,6 +411,45 @@ public function testDispatchStillTerminatesWhenResultIsIgnored(): void $this->assertNull($captured[2]); } + public function testDispatchRebindsCurrentRequestBeforeResolvingRoutes(): void + { + $oldRequest = new Request(); + $newRequest = new Request(); + + $this->app->instance('request', $oldRequest); + + $response = $this->getMockBuilder(Response::class) + ->disableOriginalConstructor() + ->onlyMethods(['prepare', 'send']) + ->getMock(); + + $response->expects($this->once()) + ->method('prepare') + ->with($newRequest) + ->willReturnSelf(); + + $response->expects($this->once()) + ->method('send') + ->with() + ->willReturnSelf(); + + $router = $this->createMock(Router::class); + $router->expects($this->once()) + ->method('resolve') + ->with($this->app, $newRequest) + ->willReturnCallback(function () use ($oldRequest, $newRequest, $response) { + $this->assertSame($newRequest, app('request')); + $this->assertSame($newRequest, app(Request::class)); + $this->assertNotSame($oldRequest, app('request')); + + return $response; + }); + + $this->app->router = $router; + + $this->app->dispatch($newRequest); + } + public function testDispatchResultDoesNotTerminateTwiceAfterExplicitTermination(): void { $request = new Request(); @@ -444,4 +486,75 @@ public function testTerminateProvidesLifecycleContextToNamedParameters(): void $this->assertSame($response, $captured['response']); $this->assertSame($exception, $captured['exception']); } + + public function testTerminateCleansRequestScopedServicesAfterCallbacks(): void + { + $_SESSION = []; + + $request = new Request(); + $response = new Response('ok'); + $session = new Session(); + $redirect = new RedirectResponse(); + + $auth = $this->getMockBuilder(ActorManager::class) + ->onlyMethods(['forgetActors']) + ->getMock(); + + $auth->expects($this->once()) + ->method('forgetActors'); + + $this->app->instance('request', $request); + $this->app->instance('response', $response); + $this->app->instance('session', $session); + $this->app->instance('redirect', $redirect); + $this->app->instance('auth', $auth); + + $seenInsideCallback = []; + + $this->app->terminating(function () use (&$seenInsideCallback): void { + $seenInsideCallback = [ + 'request' => $this->app->hasInstance('request'), + 'response' => $this->app->hasInstance('response'), + 'session' => $this->app->hasInstance('session'), + 'redirect' => $this->app->hasInstance('redirect'), + 'auth' => $this->app->hasInstance('auth'), + ]; + }); + + $this->app->terminate($request, $response); + + $this->assertSame([ + 'request' => true, + 'response' => true, + 'session' => true, + 'redirect' => true, + 'auth' => true, + ], $seenInsideCallback); + + $this->assertFalse($this->app->hasInstance('request')); + $this->assertFalse($this->app->hasInstance('response')); + $this->assertFalse($this->app->hasInstance('session')); + $this->assertFalse($this->app->hasInstance('redirect')); + $this->assertTrue($this->app->hasInstance('auth')); + } + + public function testTerminateStillCleansRequestScopedServicesWithoutCallbacks(): void + { + $_SESSION = []; + + $request = new Request(); + $response = new Response('ok'); + + $this->app->instance('request', $request); + $this->app->instance('response', $response); + $this->app->instance('session', new Session()); + $this->app->instance('redirect', new RedirectResponse()); + + $this->app->terminate($request, $response); + + $this->assertFalse($this->app->hasInstance('request')); + $this->assertFalse($this->app->hasInstance('response')); + $this->assertFalse($this->app->hasInstance('session')); + $this->assertFalse($this->app->hasInstance('redirect')); + } } diff --git a/tests/Application/ContainerTest.php b/tests/Application/ContainerTest.php index 12c1c127..33a8aebf 100644 --- a/tests/Application/ContainerTest.php +++ b/tests/Application/ContainerTest.php @@ -884,6 +884,36 @@ public function testResetClearsAll() $this->assertFalse($this->container->hasInstance('service2')); } + public function testForgetResolvedClearsResolvedSingletonInstanceButKeepsBinding() + { + $this->container->singleton('service', fn() => new \stdClass()); + $this->container->get('service'); + + $this->container->forgetResolved('service'); + + $this->assertTrue($this->container->has('service')); + $this->assertFalse($this->container->hasInstance('service')); + $this->assertFalse($this->container->resolved('service')); + } + + public function testForgetResolvedAllowsSingletonToBeResolvedFresh() + { + $this->container->singleton('service', fn() => new \stdClass()); + + $first = $this->container->get('service'); + $this->container->forgetResolved('service'); + $second = $this->container->get('service'); + + $this->assertNotSame($first, $second); + } + + public function testForgetResolvedForUnknownBindingDoesNotThrow() + { + $this->container->forgetResolved('missing-service'); + + $this->assertFalse($this->container->hasInstance('missing-service')); + } + public function testFlushAllowsRebinding() { $this->container->bind('service', fn() => 'value1'); @@ -2983,4 +3013,4 @@ public function testMailerServiceReadsWorkAfterFreeze() function config_mock(string $key, mixed $default = null): mixed { return $default; -} \ No newline at end of file +}