From b3bc1c0b489b813b13e600172b736d879d161b6c Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Thu, 30 Jul 2026 13:22:41 -0400 Subject: [PATCH 01/19] FOUR-32465: [Octane] CRITICAL Data Leaks Between Requests "$uid2id" --- .../Nayra/Repositories/EntityRepository.php | 10 +-- .../Repositories/EntityRepositoryTest.php | 66 +++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php diff --git a/ProcessMaker/Nayra/Repositories/EntityRepository.php b/ProcessMaker/Nayra/Repositories/EntityRepository.php index 4b10af1a02..2f84e55b7e 100644 --- a/ProcessMaker/Nayra/Repositories/EntityRepository.php +++ b/ProcessMaker/Nayra/Repositories/EntityRepository.php @@ -11,7 +11,7 @@ abstract class EntityRepository { - private static $uid2id = ['requests' =>[], 'tokens' =>[]]; + private $uid2id = ['requests' =>[], 'tokens' =>[]]; abstract public function create(array $transaction): ? Model; @@ -41,16 +41,16 @@ public function resolveId(string $uid): int } // Get record if is not stored previously - if (!isset(self::$uid2id[$type][$uid])) { + if (!isset($this->uid2id[$type][$uid])) { $record = $instance->select('id')->where('uuid', $uid)->first(); if ($record) { - self::$uid2id[$type][$uid] = $record->getKey(); + $this->uid2id[$type][$uid] = $record->getKey(); } else { throw new Exception("The uid {$uid} does not exist in the database"); } } - return self::$uid2id[$type][$uid] ?? 0; + return $this->uid2id[$type][$uid] ?? 0; } /** @@ -71,6 +71,6 @@ public function storeUid(string $uid, int $id): void break; } - self::$uid2id[$type][$uid] = $id; + $this->uid2id[$type][$uid] = $id; } } diff --git a/tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php b/tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php new file mode 100644 index 0000000000..29986e2f77 --- /dev/null +++ b/tests/unit/ProcessMaker/Nayra/Repositories/EntityRepositoryTest.php @@ -0,0 +1,66 @@ +assertFalse( + $reflection->isStatic(), + 'uid2id must NOT be static to prevent data leaks between requests in Octane' + ); + } + + /** + * Test that $uid2id is a private property. + */ + public function test_uid2id_is_private(): void + { + $reflection = new ReflectionProperty(EntityRepository::class, 'uid2id'); + + $this->assertTrue( + $reflection->isPrivate(), + 'uid2id should be private' + ); + } +} From 214ac429b5ed38f593f8ccef33bb620dc2d6b6fe Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 09:36:14 -0400 Subject: [PATCH 02/19] feat: refactor redirect handling by introducing RedirectToEventService --- ProcessMaker/Jobs/BpmnAction.php | 6 +- .../Listeners/HandleRedirectListener.php | 35 ++------- .../Providers/ProcessMakerServiceProvider.php | 3 + .../Services/RedirectToEventService.php | 75 +++++++++++++++++++ 4 files changed, 89 insertions(+), 30 deletions(-) create mode 100644 ProcessMaker/Services/RedirectToEventService.php diff --git a/ProcessMaker/Jobs/BpmnAction.php b/ProcessMaker/Jobs/BpmnAction.php index f78ddaf647..eefc44a149 100644 --- a/ProcessMaker/Jobs/BpmnAction.php +++ b/ProcessMaker/Jobs/BpmnAction.php @@ -14,10 +14,10 @@ use Illuminate\Support\Facades\Log; use ProcessMaker\BpmnEngine; use ProcessMaker\Exception\HttpABTestingException; -use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Models\Process as Definitions; use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Models\ProcessRequestLock; +use ProcessMaker\Services\RedirectToEventService; use Throwable; abstract class BpmnAction implements ShouldQueue @@ -60,6 +60,7 @@ abstract class BpmnAction implements ShouldQueue public function handle() { $response = null; + $redirectToEventService = app(RedirectToEventService::class); try { extract($this->loadContext()); $this->engine = $engine; @@ -74,7 +75,7 @@ public function handle() // (e.g. completed, assigned, process completed, etc) // excluding system process (non_persistent_process) if ($this->processId !== 'non_persistent_process') { - HandleRedirectListener::sendRedirectToEvent(); + $redirectToEventService->sendRedirectToEvent(); } } catch (HttpABTestingException $exception) { Log::error($exception->getMessage()); @@ -87,6 +88,7 @@ public function handle() $request->logError($exception, $element); } } finally { + $redirectToEventService->reset(); $this->unlock(); } diff --git a/ProcessMaker/Listeners/HandleRedirectListener.php b/ProcessMaker/Listeners/HandleRedirectListener.php index 78491809a4..490235af17 100644 --- a/ProcessMaker/Listeners/HandleRedirectListener.php +++ b/ProcessMaker/Listeners/HandleRedirectListener.php @@ -2,40 +2,19 @@ namespace ProcessMaker\Listeners; -use ProcessMaker\Events\RedirectToEvent; use ProcessMaker\Models\ProcessRequest; +use ProcessMaker\Services\RedirectToEventService; class HandleRedirectListener { - private static $processRequest = null; - - protected static $redirectionMethod = ''; - - private static $redirectionParams = []; - - protected function setRedirectTo(ProcessRequest $processRequest, string $method, ...$params): void - { - self::$processRequest = $processRequest; - self::$redirectionMethod = $method; - self::$redirectionParams = $params; + public function __construct( + private ?RedirectToEventService $redirectToEventService = null + ) { } - public static function sendRedirectToEvent() + protected function setRedirectTo(ProcessRequest $processRequest, string $method, ...$params): void { - $method = self::$redirectionMethod; - $params = self::$redirectionParams; - $processRequest = self::$processRequest; - - // Only get active tokens if there is a valid process request - if ($processRequest !== null) { - $params['activeTokens'] = ProcessRequest::getActiveTokens($processRequest); - $event = new RedirectToEvent($processRequest, $method, $params); - event($event); - - // Clean params to prevent sending the same redirect multiple times - self::$redirectionParams = []; - self::$redirectionMethod = ''; - self::$processRequest = null; - } + $this->redirectToEventService ??= app(RedirectToEventService::class); + $this->redirectToEventService->setRedirectTo($processRequest, $method, ...$params); } } diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 72c71c2a46..603b116c3d 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -53,6 +53,7 @@ use ProcessMaker\Providers\PermissionServiceProvider; use ProcessMaker\Repositories\SettingsConfigRepository; use ProcessMaker\Services\ConditionalRedirectService; +use ProcessMaker\Services\RedirectToEventService; use RuntimeException; use Spatie\Multitenancy\Events\MadeTenantCurrentEvent; use Spatie\Multitenancy\Events\TenantNotFoundForRequestEvent; @@ -243,6 +244,8 @@ public function register(): void $this->app->instance('tenant-resolved', false); + $this->app->scoped(RedirectToEventService::class); + /** * Conditional Redirect Service * This service is used to evaluate the conditional redirect property of a process request token. diff --git a/ProcessMaker/Services/RedirectToEventService.php b/ProcessMaker/Services/RedirectToEventService.php new file mode 100644 index 0000000000..4dd67eb8c8 --- /dev/null +++ b/ProcessMaker/Services/RedirectToEventService.php @@ -0,0 +1,75 @@ +processRequest = $processRequest; + $this->redirectionMethod = $method; + $this->redirectionParams = $params; + } + + /** + * Dispatch the pending redirect, including the request's active token IDs. + * + * This method is a no-op when no redirect is pending. Pending state is + * consumed before querying tokens or dispatching the event so an exception + * cannot cause stale request data to be retried or leaked into later work. + * + * @throws \Throwable If active-token retrieval or event dispatch fails + */ + public function sendRedirectToEvent(): void + { + if ($this->processRequest === null) { + return; + } + + $processRequest = $this->processRequest; + $method = $this->redirectionMethod; + $params = $this->redirectionParams; + + // Consume the pending redirect before doing work that may throw. + $this->reset(); + + $params['activeTokens'] = ProcessRequest::getActiveTokens($processRequest); + event(new RedirectToEvent($processRequest, $method, $params)); + } + + /** + * Discard all pending redirect state without dispatching an event. + */ + public function reset(): void + { + $this->processRequest = null; + $this->redirectionMethod = ''; + $this->redirectionParams = []; + } +} From 7f405b46f66a6be76b56f4c40268782b2ae1efff Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 10:01:09 -0400 Subject: [PATCH 03/19] test: add unit tests for BpmnAction and RedirectToEventService functionality --- .../Jobs/BpmnActionRedirectCleanupTest.php | 25 ++ .../Services/RedirectToEventServiceTest.php | 230 ++++++++++++++++++ 2 files changed, 255 insertions(+) create mode 100644 tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php create mode 100644 tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php diff --git a/tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php b/tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php new file mode 100644 index 0000000000..ceb1b27262 --- /dev/null +++ b/tests/unit/ProcessMaker/Jobs/BpmnActionRedirectCleanupTest.php @@ -0,0 +1,25 @@ +shouldReceive('sendRedirectToEvent')->never(); + $redirectToEventService->shouldReceive('reset')->once(); + app()->instance(RedirectToEventService::class, $redirectToEventService); + + $job = new class extends BpmnAction { + protected $definitionsId = -1; + }; + + $this->assertNull($job->handle()); + } +} diff --git a/tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php b/tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php new file mode 100644 index 0000000000..e24d3fa3c1 --- /dev/null +++ b/tests/unit/ProcessMaker/Services/RedirectToEventServiceTest.php @@ -0,0 +1,230 @@ +create(); + $secondRequest = ProcessRequest::factory()->create(); + $service = app(RedirectToEventService::class); + + $service->setRedirectTo($firstRequest, 'firstRedirect', [ + 'requestId' => $firstRequest->id, + ]); + $service->setRedirectTo($secondRequest, 'secondRedirect', [ + 'requestId' => $secondRequest->id, + ]); + + $service->sendRedirectToEvent(); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($secondRequest) { + return $event->method === 'secondRedirect' + && $event->params[0]['requestId'] === $secondRequest->id + && $event->params['activeTokens'] === [] + && $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $secondRequest->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); + } + + public function test_scoped_binding_does_not_leak_pending_redirect_between_operations(): void + { + Event::fake([RedirectToEvent::class]); + + $firstRequest = ProcessRequest::factory()->create(); + $firstScope = app(RedirectToEventService::class); + $this->assertSame($firstScope, app(RedirectToEventService::class)); + $firstScope->setRedirectTo($firstRequest, 'staleRedirect'); + + app()->forgetScopedInstances(); + + $secondScope = app(RedirectToEventService::class); + $this->assertNotSame($firstScope, $secondScope); + + $secondScope->sendRedirectToEvent(); + Event::assertNotDispatched(RedirectToEvent::class); + + $secondRequest = ProcessRequest::factory()->create(); + $secondScope->setRedirectTo($secondRequest, 'currentRedirect', [ + 'requestId' => $secondRequest->id, + ]); + $secondScope->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($secondRequest) { + return $event->method === 'currentRedirect' + && $event->params[0]['requestId'] === $secondRequest->id + && $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $secondRequest->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); + } + + public function test_reset_discards_pending_redirect(): void + { + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $service->setRedirectTo(ProcessRequest::factory()->create(), 'discardedRedirect'); + + $service->reset(); + $service->sendRedirectToEvent(); + + Event::assertNotDispatched(RedirectToEvent::class); + } + + public function test_activity_completed_listener_and_dispatcher_share_the_same_scoped_state(): void + { + Event::fake([RedirectToEvent::class]); + + $processRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $activeToken = ProcessRequestToken::factory()->create([ + 'process_id' => $processRequest->process_id, + 'process_request_id' => $processRequest->id, + 'status' => 'ACTIVE', + ]); + $activeToken->setInstance($processRequest); + + app(HandleActivityCompletedRedirect::class)->handle(new ActivityCompleted($activeToken)); + app(RedirectToEventService::class)->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ( + $activeToken, + $processRequest + ) { + return $event->method === 'processUpdated' + && $event->params[0]['tokenId'] === $activeToken->id + && $event->params[0]['requestStatus'] === $processRequest->status + && $event->params['activeTokens'] === [$activeToken->id]; + }); + } + + public function test_active_tokens_exclude_closed_and_unrelated_request_tokens(): void + { + Event::fake([RedirectToEvent::class]); + + $processRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $unrelatedRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $activeToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $processRequest->id, + 'status' => 'ACTIVE', + ]); + $closedToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $processRequest->id, + 'status' => 'CLOSED', + ]); + $unrelatedToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $unrelatedRequest->id, + 'status' => 'ACTIVE', + ]); + + $service = app(RedirectToEventService::class); + $service->setRedirectTo($processRequest, 'isolatedRedirect'); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ( + $activeToken, + $closedToken, + $unrelatedToken + ) { + return $event->params['activeTokens'] === [$activeToken->id] + && !in_array($closedToken->id, $event->params['activeTokens'], true) + && !in_array($unrelatedToken->id, $event->params['activeTokens'], true); + }); + } + + public function test_active_tokens_include_all_active_tokens_in_the_same_collaboration(): void + { + Event::fake([RedirectToEvent::class]); + + $processRequest = ProcessRequest::factory()->create(); + $collaboratingRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => $processRequest->process_collaboration_id, + ]); + $unrelatedRequest = ProcessRequest::factory()->create(); + $firstToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $processRequest->id, + 'status' => 'ACTIVE', + ]); + $collaboratingToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $collaboratingRequest->id, + 'status' => 'ACTIVE', + ]); + $unrelatedToken = ProcessRequestToken::factory()->create([ + 'process_request_id' => $unrelatedRequest->id, + 'status' => 'ACTIVE', + ]); + + $service = app(RedirectToEventService::class); + $service->setRedirectTo($processRequest, 'collaborationRedirect'); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ( + $firstToken, + $collaboratingToken, + $unrelatedToken + ) { + $activeTokens = $event->params['activeTokens']; + sort($activeTokens); + + $expectedTokens = [$firstToken->id, $collaboratingToken->id]; + sort($expectedTokens); + + return $activeTokens === $expectedTokens + && !in_array($unrelatedToken->id, $activeTokens, true); + }); + } + + public function test_pending_redirect_is_consumed_when_event_dispatch_throws(): void + { + $processRequest = ProcessRequest::factory()->create([ + 'process_collaboration_id' => null, + ]); + $service = app(RedirectToEventService::class); + $service->setRedirectTo($processRequest, 'failingRedirect'); + + $originalDispatcher = Event::getFacadeRoot(); + $failingDispatcher = Mockery::mock(Dispatcher::class); + $failingDispatcher->shouldReceive('dispatch') + ->once() + ->with(Mockery::type(RedirectToEvent::class)) + ->andThrow(new RuntimeException('Broadcast failed')); + Event::swap($failingDispatcher); + + try { + try { + $service->sendRedirectToEvent(); + $this->fail('The event dispatcher should have thrown an exception.'); + } catch (RuntimeException $exception) { + $this->assertSame('Broadcast failed', $exception->getMessage()); + } + + // A retry without a new redirect must not dispatch the failed event again. + $service->sendRedirectToEvent(); + } finally { + Event::swap($originalDispatcher); + } + } +} From 2ff685cd1be44a5ea7082c9e61c1402e9a9e682f Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Fri, 31 Jul 2026 10:55:30 -0400 Subject: [PATCH 04/19] feat(FOUR-32473): [Octane] CRITICAL Data Leaks Between Requests "$redirectionParams" --- .../Listeners/HandleRedirectListener.php | 4 +- .../Listeners/HandleRedirectListenerTest.php | 235 ++++++++++++++++++ 2 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php diff --git a/ProcessMaker/Listeners/HandleRedirectListener.php b/ProcessMaker/Listeners/HandleRedirectListener.php index 7679a73572..2d2850fad8 100644 --- a/ProcessMaker/Listeners/HandleRedirectListener.php +++ b/ProcessMaker/Listeners/HandleRedirectListener.php @@ -44,9 +44,7 @@ public static function sendRedirectToEvent() event($event); // Clean params to prevent sending the same redirect multiple times - self::$redirectionParams = []; - self::$redirectionMethod = ''; - self::$processRequest = null; + self::reset(); } } } diff --git a/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php new file mode 100644 index 0000000000..0e3e86cf4e --- /dev/null +++ b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php @@ -0,0 +1,235 @@ +setRedirectTo($processRequest, $method, ...$params); + } + }; + } + + /** + * Read a private static property from HandleRedirectListener. + */ + private function readStaticProperty(string $property): mixed + { + $reflection = new ReflectionProperty(HandleRedirectListener::class, $property); + $reflection->setAccessible(true); + + return $reflection->getValue(); + } + + /** + * Assert that all 3 static properties are in their default/clean state. + */ + private function assertStateIsClean(): void + { + $this->assertNull($this->readStaticProperty('processRequest')); + $this->assertSame('', $this->readStaticProperty('redirectionMethod')); + $this->assertSame([], $this->readStaticProperty('redirectionParams')); + } + + /** + * Test that reset() clears the static $processRequest property. + */ + public function test_reset_clears_process_request(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated'); + + HandleRedirectListener::reset(); + + $this->assertNull($this->readStaticProperty('processRequest')); + } + + /** + * Test that reset() clears the static $redirectionMethod property. + */ + public function test_reset_clears_redirection_method(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processCompletedRedirect'); + + HandleRedirectListener::reset(); + + $this->assertSame('', $this->readStaticProperty('redirectionMethod')); + } + + /** + * Test that reset() clears the static $redirectionParams property. + */ + public function test_reset_clears_redirection_params(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['key' => 'value']); + + HandleRedirectListener::reset(); + + $this->assertSame([], $this->readStaticProperty('redirectionParams')); + } + + /** + * Critical test for Octane: verify that reset() prevents data leaks. + * After reset(), the stored redirect data should be gone. + */ + public function test_reset_prevents_stale_redirect_from_leaking(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['tokenId' => 123]); + + // Simulate Octane reset between requests + HandleRedirectListener::reset(); + + // sendRedirectToEvent should NOT dispatch RedirectToEvent after reset + $this->expectNotToPerformAssertions(); + HandleRedirectListener::sendRedirectToEvent(); + } + + /** + * Test that reset() can be called multiple times safely. + */ + public function test_reset_can_be_called_multiple_times(): void + { + HandleRedirectListener::reset(); + HandleRedirectListener::reset(); + HandleRedirectListener::reset(); + + // Should not throw any errors + $this->assertStateIsClean(); + } + + /** + * Test that sendRedirectToEvent dispatches the event and clears state. + */ + public function test_send_redirect_to_event_dispatches_and_clears_state(): void + { + \Illuminate\Support\Facades\Event::fake([RedirectToEvent::class]); + + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated'); + + HandleRedirectListener::sendRedirectToEvent(); + + // Assert the event was dispatched + \Illuminate\Support\Facades\Event::assertDispatched(RedirectToEvent::class); + + // After dispatch, the state should be cleared + $this->assertNull($this->readStaticProperty('processRequest')); + } + + /** + * CRITICAL: Simulate the full Octane request cycle to guarantee no data leak. + * + * Flow: + * 1. Request A stores data with different values + * 2. Reset (simulating Octane's RequestTerminated event) + * 3. Verify ALL 3 properties are clean + * 4. Request B stores NEW data with different values + * 5. Verify Request B's data is correct (not contaminated by Request A) + * 6. Reset again + * 7. Verify clean again + */ + public function test_full_octane_cycle_guarantees_no_data_leak(): void + { + // === Request A === + $requestA = ProcessRequest::factory()->create(); + $probeA = $this->createProbe(); + $probeA->queue($requestA, 'processCompletedRedirect', ['tokenA' => 111]); + + // Verify Request A data is stored (setRedirectTo uses ...$params, so it's nested) + $this->assertSame($requestA->getKey(), $this->readStaticProperty('processRequest')->getKey()); + $this->assertSame('processCompletedRedirect', $this->readStaticProperty('redirectionMethod')); + $this->assertSame([['tokenA' => 111]], $this->readStaticProperty('redirectionParams')); + + // === Octane reset after Request A === + HandleRedirectListener::reset(); + + // === Verify ALL properties are clean after reset === + $this->assertStateIsClean(); + + // === Request B (simulating a DIFFERENT user/request) === + $requestB = ProcessRequest::factory()->create(); + $probeB = $this->createProbe(); + $probeB->queue($requestB, 'processUpdated', ['tokenB' => 222, 'userId' => 999]); + + // Verify Request B's data is correct (NOT contaminated by Request A) + $this->assertSame($requestB->getKey(), $this->readStaticProperty('processRequest')->getKey()); + $this->assertSame('processUpdated', $this->readStaticProperty('redirectionMethod')); + $this->assertSame([['tokenB' => 222, 'userId' => 999]], $this->readStaticProperty('redirectionParams')); + + // Verify Request A's data is GONE (no leak) + $this->assertNotSame($requestA->getKey(), $this->readStaticProperty('processRequest')?->getKey()); + + // === Octane reset after Request B === + HandleRedirectListener::reset(); + + // === Verify clean again === + $this->assertStateIsClean(); + } + + /** + * CRITICAL: Verify that ResetRequestState orchestrator triggers the reset correctly. + */ + public function test_reset_request_state_triggers_handle_redirect_reset(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['tokenId' => 456]); + + // Verify state is dirty before reset + $this->assertNotNull($this->readStaticProperty('processRequest')); + + // Execute the orchestrator (same as Octane's RequestTerminated listener) + $resetState = new ResetRequestState(); + $resetState->handle(); + + // Verify orchestrator cleaned everything + $this->assertStateIsClean(); + } + + /** + * CRITICAL: Simulate the scenario where sendRedirectToEvent() fails, + * but reset() still cleans up (edge case in Octane). + */ + public function test_reset_cleans_up_even_when_send_redirect_fails(): void + { + $probe = $this->createProbe(); + $request = ProcessRequest::factory()->create(); + $probe->queue($request, 'processUpdated', ['data' => 'sensitive']); + + // Simulate that sendRedirectToEvent is NEVER called (e.g., error in BPMN flow) + // But Octane's RequestTerminated event still fires and calls reset() + + // This should NOT be called in this scenario: + // HandleRedirectListener::sendRedirectToEvent(); + + // Octane reset still happens + HandleRedirectListener::reset(); + + // Verify no sensitive data leaked + $this->assertStateIsClean(); + } +} From f0ba6183b8e2452bd0309f083e4765a9e6542996 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Fri, 31 Jul 2026 10:55:30 -0400 Subject: [PATCH 05/19] FOUR-32474 [Octane] CRITICAL Data Leaks Between Requests "$landlordValues" Description: Fix Octane data leak by storing landlord config in request-scoped Context Replace SwitchTenant static $landlordValues with Laravel Context to prevent tenant config snapshots from persisting across Octane requests. Remove unused duplicate property from ProcessMakerServiceProvider. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32474 --- ProcessMaker/Multitenancy/SwitchTenant.php | 10 ++-- .../Providers/ProcessMakerServiceProvider.php | 3 -- .../Multitenancy/SwitchTenantTest.php | 53 +++++++++++++++++++ 3 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php diff --git a/ProcessMaker/Multitenancy/SwitchTenant.php b/ProcessMaker/Multitenancy/SwitchTenant.php index e85c1421f7..34e7a5be60 100644 --- a/ProcessMaker/Multitenancy/SwitchTenant.php +++ b/ProcessMaker/Multitenancy/SwitchTenant.php @@ -6,6 +6,7 @@ use Illuminate\Contracts\Routing\UrlGenerator; use Illuminate\Support\Arr; use Illuminate\Support\Env; +use Illuminate\Support\Facades\Context; use Monolog\Handler\RotatingFileHandler; use ProcessMaker\Application; use ProcessMaker\Multitenancy\Broadcasting\TenantAwareBroadcastManager; @@ -17,7 +18,7 @@ class SwitchTenant implements SwitchTenantTask { use UsesMultitenancyConfig; - public static $landlordValues = null; + private const LANDLORD_VALUES_CONTEXT_KEY = 'multitenancy.landlord_values'; /** * Make the given tenant current. @@ -31,9 +32,8 @@ public function makeCurrent(IsTenant $tenant): void \Log::debug('SwitchTenant: ' . $tenant->id, ['domain' => request()->getHost()]); - // Save the landlord values for later use - if (!self::$landlordValues) { - self::$landlordValues = $app->make('config')->all(); + if (!Context::has(self::LANDLORD_VALUES_CONTEXT_KEY)) { + Context::add(self::LANDLORD_VALUES_CONTEXT_KEY, $app->make('config')->all()); } // Set the tenant's domain in the request headers. Used for things like the global url() helper. @@ -70,7 +70,7 @@ public function forgetCurrent(): void private function landlordConfig($key) { - return Arr::get(self::$landlordValues, $key); + return Arr::get(Context::get(self::LANDLORD_VALUES_CONTEXT_KEY), $key); } private function setConfig($key, $value) diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 72c71c2a46..3706dac585 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -75,9 +75,6 @@ class ProcessMakerServiceProvider extends ServiceProvider // Track the query time for each request private static $queryTime = 0; - // Track the landlord values for multitenancy - private static $landlordValues = null; - public function boot(): void { // Track the start time for service providers boot diff --git a/tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php b/tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php new file mode 100644 index 0000000000..e19db9daa0 --- /dev/null +++ b/tests/unit/ProcessMaker/Multitenancy/SwitchTenantTest.php @@ -0,0 +1,53 @@ + 'https://landlord.example.com']); + Context::add(self::LANDLORD_VALUES_KEY, config()->all()); + + config(['app.url' => 'https://tenant-modified.example.com']); + + $this->assertSame( + 'https://landlord.example.com', + $this->landlordConfig('app.url') + ); + } + + public function test_landlord_values_are_not_reused_across_requests(): void + { + Context::add(self::LANDLORD_VALUES_KEY, ['app' => ['url' => 'https://tenant-a.example.com']]); + Context::forget(self::LANDLORD_VALUES_KEY); + + config(['app.url' => 'https://tenant-b.example.com']); + Context::add(self::LANDLORD_VALUES_KEY, config()->all()); + + $this->assertSame( + 'https://tenant-b.example.com', + Context::get(self::LANDLORD_VALUES_KEY)['app']['url'] + ); + } + + private function landlordConfig(string $key): mixed + { + $method = new \ReflectionMethod(SwitchTenant::class, 'landlordConfig'); + + return $method->invoke(new SwitchTenant(), $key); + } +} From 6775f5eeab5ea5fcf397c645ba460cc61e0a12f5 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Fri, 31 Jul 2026 11:43:04 -0400 Subject: [PATCH 06/19] FOUR-32475 [Octane] CRITICAL Data Leaks Between Requests "AnonymousUser::class" Description: Replace AnonymousUser singleton with a scoped binding and add resolve() to load the user from the database per request. Prevents stale anonymous user data from leaking across Octane requests while keeping the same behavior in PHP-FPM and queue workers. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32475 --- ProcessMaker/Models/AnonymousUser.php | 6 ++ .../Providers/ProcessMakerServiceProvider.php | 5 +- .../ProcessMaker/Models/AnonymousUserTest.php | 64 +++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/unit/ProcessMaker/Models/AnonymousUserTest.php diff --git a/ProcessMaker/Models/AnonymousUser.php b/ProcessMaker/Models/AnonymousUser.php index b78c65e1df..3e15c87e08 100644 --- a/ProcessMaker/Models/AnonymousUser.php +++ b/ProcessMaker/Models/AnonymousUser.php @@ -12,6 +12,12 @@ class AnonymousUser extends User protected $table = 'users'; + public static function resolve(): self + { + return static::where('username', '=', static::ANONYMOUS_USERNAME) + ->firstOrFail(); + } + public $isAnonymous = true; public function receivesBroadcastNotificationsOn($notification) diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 72c71c2a46..f11d725e55 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -188,9 +188,8 @@ public function register(): void return new Managers\GlobalScriptsManager(); }); - $this->app->singleton(Models\AnonymousUser::class, function ($app) { - return Models\AnonymousUser::where('username', '=', Models\AnonymousUser::ANONYMOUS_USERNAME) - ->firstOrFail(); + $this->app->scoped(Models\AnonymousUser::class, function ($app) { + return Models\AnonymousUser::resolve(); }); $this->app->singleton(PolicyExtension::class, function ($app) { diff --git a/tests/unit/ProcessMaker/Models/AnonymousUserTest.php b/tests/unit/ProcessMaker/Models/AnonymousUserTest.php new file mode 100644 index 0000000000..864e094c9a --- /dev/null +++ b/tests/unit/ProcessMaker/Models/AnonymousUserTest.php @@ -0,0 +1,64 @@ +app->forgetScopedInstances(); + + parent::tearDown(); + } + + public function test_resolve_returns_anonymous_user_from_database(): void + { + $user = AnonymousUser::resolve(); + + $this->assertInstanceOf(AnonymousUser::class, $user); + $this->assertSame(AnonymousUser::ANONYMOUS_USERNAME, $user->username); + } + + public function test_container_binding_returns_same_instance_within_request(): void + { + $first = app(AnonymousUser::class); + $second = app(AnonymousUser::class); + + $this->assertSame($first, $second); + } + + public function test_container_binding_is_not_reused_across_requests(): void + { + $first = app(AnonymousUser::class); + + $this->app->forgetScopedInstances(); + + $second = app(AnonymousUser::class); + + $this->assertNotSame($first, $second); + $this->assertSame($first->id, $second->id); + } + + public function test_container_binding_reflects_database_changes_after_flush(): void + { + $original = app(AnonymousUser::class); + $originalEmail = $original->email; + + User::where('username', AnonymousUser::ANONYMOUS_USERNAME) + ->update(['email' => 'updated-anon@example.com']); + + $this->app->forgetScopedInstances(); + + $refreshed = app(AnonymousUser::class); + + $this->assertSame('updated-anon@example.com', $refreshed->email); + $this->assertNotSame($originalEmail, $refreshed->email); + + User::where('username', AnonymousUser::ANONYMOUS_USERNAME) + ->update(['email' => $originalEmail]); + } +} From 9d0d012b476ddc865bbc913b1f015c9863ae448e Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 14:05:13 -0400 Subject: [PATCH 07/19] feat: refactor ResetRequestState to use RedirectToEventService for handling redirects --- ProcessMaker/Octane/ResetRequestState.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ProcessMaker/Octane/ResetRequestState.php b/ProcessMaker/Octane/ResetRequestState.php index 45071e97ad..8bde301049 100644 --- a/ProcessMaker/Octane/ResetRequestState.php +++ b/ProcessMaker/Octane/ResetRequestState.php @@ -4,14 +4,19 @@ namespace ProcessMaker\Octane; -use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Providers\ProcessMakerServiceProvider; +use ProcessMaker\Services\RedirectToEventService; final class ResetRequestState { + public function __construct( + private readonly RedirectToEventService $redirectToEventService + ) { + } + public function handle(): void { ProcessMakerServiceProvider::beginRequestTiming(); - HandleRedirectListener::reset(); + $this->redirectToEventService->reset(); } } From 51a44447a4482064473c01c0290c2d7c5502f583 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 14:05:22 -0400 Subject: [PATCH 08/19] test: update ResetRequestStateTest to utilize app() for dependency resolution and ensure proper redirect handling --- tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php index 7895d8519f..1ec4a124aa 100644 --- a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php +++ b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php @@ -13,6 +13,7 @@ use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Octane\ResetRequestState; use ProcessMaker\Providers\ProcessMakerServiceProvider; +use ProcessMaker\Services\RedirectToEventService; use Symfony\Component\HttpFoundation\Response; use Tests\TestCase; @@ -24,7 +25,7 @@ public function test_it_clears_request_timing_before_the_next_request(): void $this->assertGreaterThan(0, ProcessMakerServiceProvider::getQueryTime()); - $listener = new ResetRequestState(); + $listener = app(ResetRequestState::class); $listener->handle(); $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); @@ -37,10 +38,10 @@ public function test_it_prevents_redirect_state_from_leaking_into_the_next_reque $redirectListener = new RedirectStateProbe(); $redirectListener->queue(ProcessRequest::factory()->create()); - $listener = new ResetRequestState(); + $listener = app(ResetRequestState::class); $listener->handle(); - HandleRedirectListener::sendRedirectToEvent(); + app(RedirectToEventService::class)->sendRedirectToEvent(); Event::assertNotDispatched(RedirectToEvent::class); } @@ -59,7 +60,7 @@ public function test_octane_request_termination_automatically_resets_request_sta new Response() )); - HandleRedirectListener::sendRedirectToEvent(); + app(RedirectToEventService::class)->sendRedirectToEvent(); Event::assertNotDispatched(RedirectToEvent::class); } From 0c58e630e3926e2fe8b88b40e09da3c6321ad753 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 15:28:33 -0400 Subject: [PATCH 09/19] test: enhance ResetRequestStateTest with query duration tracking and error response handling --- .../Octane/ResetRequestStateTest.php | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php index 7895d8519f..22e50f8959 100644 --- a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php +++ b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php @@ -4,6 +4,7 @@ namespace Tests\Unit\ProcessMaker\Octane; +use Illuminate\Database\Events\QueryExecuted; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; @@ -18,6 +19,20 @@ class ResetRequestStateTest extends TestCase { + protected function tearDown(): void + { + ProcessMakerServiceProvider::beginRequestTiming(); + + parent::tearDown(); + } + + private function recordQueryDuration(float $milliseconds): void + { + $connection = DB::connection(); + + event(new QueryExecuted('SELECT 1', [], $milliseconds, $connection)); + } + public function test_it_clears_request_timing_before_the_next_request(): void { DB::select('SELECT 1'); @@ -49,6 +64,13 @@ public function test_octane_request_termination_automatically_resets_request_sta { Event::fake([RedirectToEvent::class]); + ProcessMakerServiceProvider::beginRequestTiming(); + $this->recordQueryDuration(5000); + + $firstRequestQueryTime = ProcessMakerServiceProvider::getQueryTime(); + + $this->assertSame(5000.0, $firstRequestQueryTime); + $redirectListener = new RedirectStateProbe(); $redirectListener->queue(ProcessRequest::factory()->create()); @@ -59,9 +81,33 @@ public function test_octane_request_termination_automatically_resets_request_sta new Response() )); + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); + HandleRedirectListener::sendRedirectToEvent(); Event::assertNotDispatched(RedirectToEvent::class); + + $this->recordQueryDuration(10); + + $nextRequestQueryTime = ProcessMakerServiceProvider::getQueryTime(); + + $this->assertSame(10.0, $nextRequestQueryTime); + } + + public function test_octane_request_termination_resets_timing_after_an_error_response(): void + { + DB::select('SELECT 1'); + + $this->assertGreaterThan(0, ProcessMakerServiceProvider::getQueryTime()); + + event(new RequestTerminated( + $this->app, + $this->app, + Request::create('/failed-request'), + new Response(status: 500) + )); + + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); } } From ee0318330474acba38a450dad8f546072a3b55b1 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 15:28:42 -0400 Subject: [PATCH 10/19] test: add query duration tracking and isolate metrics in ServerTimingMiddlewareTest --- tests/Feature/ServerTimingMiddlewareTest.php | 67 ++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/Feature/ServerTimingMiddlewareTest.php b/tests/Feature/ServerTimingMiddlewareTest.php index 040401d227..9654a44854 100644 --- a/tests/Feature/ServerTimingMiddlewareTest.php +++ b/tests/Feature/ServerTimingMiddlewareTest.php @@ -2,8 +2,11 @@ namespace Tests\Feature; +use Illuminate\Database\Events\QueryExecuted; +use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Route; +use Laravel\Octane\ApplicationGateway; use ProcessMaker\Http\Middleware\ServerTimingMiddleware; use ProcessMaker\Models\User; use ProcessMaker\Providers\ProcessMakerServiceProvider; @@ -15,6 +18,13 @@ class ServerTimingMiddlewareTest extends TestCase { use RequestHelper; + protected function tearDown(): void + { + ProcessMakerServiceProvider::beginRequestTiming(); + + parent::tearDown(); + } + private function getHeader($response, $header) { $headers = $response->headers->all(); @@ -22,6 +32,24 @@ private function getHeader($response, $header) return $headers[$header]; } + private function getMetricDuration($response, string $metric): float + { + $serverTiming = implode(',', $this->getHeader($response, 'server-timing')); + + preg_match('/(?:^|,)\\s*' . preg_quote($metric, '/') . ';dur=([\\d.]+)/', $serverTiming, $matches); + + $this->assertArrayHasKey(1, $matches, "The {$metric} metric was not present in the Server-Timing header."); + + return (float) $matches[1]; + } + + private function recordQueryDuration(float $milliseconds): void + { + $connection = DB::connection(); + + event(new QueryExecuted('SELECT 1', [], $milliseconds, $connection)); + } + public function testServerTimingHeaderIncludesAllMetrics() { Route::middleware(ServerTimingMiddleware::class)->get('/test', function () { @@ -85,6 +113,45 @@ public function testQueryTimeIsMeasured() $this->assertGreaterThanOrEqual(200, (float) $dbTime); } + public function testOctaneGatewayIsolatesQueryTimingAcrossConsecutiveRequests() + { + Route::middleware(ServerTimingMiddleware::class)->get('/octane-query/slow', function () { + $this->recordQueryDuration(5000); + + return response()->json(['request' => 'slow']); + }); + + Route::middleware(ServerTimingMiddleware::class)->get('/octane-query/fast', function () { + $this->recordQueryDuration(10); + + return response()->json(['request' => 'fast']); + }); + + $gateway = new ApplicationGateway($this->app, $this->app); + + $firstRequest = Request::create('/octane-query/slow'); + $firstResponse = $gateway->handle($firstRequest); + $firstRequestQueryTime = $this->getMetricDuration($firstResponse, 'db'); + + $this->assertGreaterThanOrEqual(5000, $firstRequestQueryTime); + + $gateway->terminate($firstRequest, $firstResponse); + + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); + + $secondRequest = Request::create('/octane-query/fast'); + $secondResponse = $gateway->handle($secondRequest); + $secondRequestQueryTime = $this->getMetricDuration($secondResponse, 'db'); + + $this->assertGreaterThanOrEqual(10, $secondRequestQueryTime); + $this->assertLessThan(5000, $secondRequestQueryTime); + $this->assertLessThan($firstRequestQueryTime, $secondRequestQueryTime); + + $gateway->terminate($secondRequest, $secondResponse); + + $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); + } + public function testServiceProviderTimeIsMeasured() { // Mock a route From fde54105434211e654b837acfc9e25ad56d4625a Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Fri, 31 Jul 2026 15:40:33 -0400 Subject: [PATCH 11/19] =?UTF-8?q?FOUR-32498=20[Octane]=20MEDIUM=20?= =?UTF-8?q?=E2=80=94=20Accumulating=20State=20"addons"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description: Fix Octane state leak by moving controller addons from static trait property to singleton registry Replace `HasControllerAddons` static `$addons` with `ControllerAddonsRegistry` to prevent addon accumulation across Octane requests while keeping the existing `registerAddon()` and `getPluginAddons()` API unchanged. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32498 --- .../Managers/ControllerAddonsRegistry.php | 45 ++++++ .../Providers/ProcessMakerServiceProvider.php | 4 + ProcessMaker/Traits/HasControllerAddons.php | 29 +--- .../Managers/ControllerAddonsRegistryTest.php | 136 ++++++++++++++++++ 4 files changed, 189 insertions(+), 25 deletions(-) create mode 100644 ProcessMaker/Managers/ControllerAddonsRegistry.php create mode 100644 tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php diff --git a/ProcessMaker/Managers/ControllerAddonsRegistry.php b/ProcessMaker/Managers/ControllerAddonsRegistry.php new file mode 100644 index 0000000000..2159b56523 --- /dev/null +++ b/ProcessMaker/Managers/ControllerAddonsRegistry.php @@ -0,0 +1,45 @@ +addons[] = $config; + } + + /** + * Get configured addons for a controller. + */ + public function getAddons(string $scope, string $method, array $data): array + { + $addons = []; + + foreach ($this->addons as $addon) { + if ($addon['method'] !== $method || $addon['scope'] !== $scope) { + continue; + } + + if (isset($addon['data']) && is_callable($addon['data'])) { + $data = call_user_func($addon['data'], $data); + } + + $addon['content'] = isset($addon['view']) && !isset($addon['content']) + ? view($addon['view'], $data)->render() + : (isset($addon['content']) ? $addon['content'] : ''); + $addon['script'] = isset($addon['script']) && is_string($addon['script']) + ? view($addon['script'], $data)->render() + : ''; + $addons[] = $addon; + } + + return $addons; + } +} diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 55ef2cfa55..04d81fbeda 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -136,6 +136,10 @@ public function register(): void return new Managers\LoginManager(); }); + $this->app->singleton(Managers\ControllerAddonsRegistry::class, function () { + return new Managers\ControllerAddonsRegistry(); + }); + /* * Maps our Index Manager as a singleton. The Index Manager is used * to manage customizations to the search indexer. diff --git a/ProcessMaker/Traits/HasControllerAddons.php b/ProcessMaker/Traits/HasControllerAddons.php index 0889ac1d0d..374bb9c36d 100644 --- a/ProcessMaker/Traits/HasControllerAddons.php +++ b/ProcessMaker/Traits/HasControllerAddons.php @@ -2,10 +2,10 @@ namespace ProcessMaker\Traits; +use ProcessMaker\Managers\ControllerAddonsRegistry; + trait HasControllerAddons { - private static $addons = []; - /** * Get configured addons for this controller * @@ -16,26 +16,7 @@ trait HasControllerAddons */ protected function getPluginAddons($method, array $data) { - if (!isset(static::$addons)) { - return; - } - - $addons = []; - foreach (static::$addons as $addon) { - // The addon must have the requested method and must be associated to the current controller - if ($addon['method'] === $method && $addon['scope'] === get_class($this)) { - if (isset($addon['data']) && is_callable($addon['data'])) { - $data = call_user_func($addon['data'], $data); - } - $addon['content'] = isset($addon['view']) && !isset($addon['content']) - ? view($addon['view'], $data)->render() : (isset($addon['content']) - ? $addon['content'] : ''); - $addon['script'] = isset($addon['script']) && is_string($addon['script']) ? view($addon['script'], $data)->render() : ''; - $addons[] = $addon; - } - } - - return $addons; + return app(ControllerAddonsRegistry::class)->getAddons(static::class, $method, $data); } /** @@ -47,8 +28,6 @@ protected function getPluginAddons($method, array $data) */ public static function registerAddon(array $config) { - // Add the controller to which the addon is attached - $config['scope'] = static::class; - static::$addons[] = $config; + app(ControllerAddonsRegistry::class)->register(static::class, $config); } } diff --git a/tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php b/tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php new file mode 100644 index 0000000000..bc5d018391 --- /dev/null +++ b/tests/unit/ProcessMaker/Managers/ControllerAddonsRegistryTest.php @@ -0,0 +1,136 @@ +registry = new ControllerAddonsRegistry(); + $this->bindRegistryInContainer(); + } + + protected function tearDown(): void + { + Container::setInstance($this->previousContainer); + + parent::tearDown(); + } + + public function test_register_addon_is_retrieved_for_matching_scope_and_method(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'test-addon', + 'method' => 'edit', + 'title' => 'Test Addon', + 'content' => 'addon-content', + ]); + + $addons = $this->registry->getAddons(UserController::class, 'edit', []); + + $this->assertCount(1, $addons); + $this->assertSame('test-addon', $addons[0]['id']); + $this->assertSame('addon-content', $addons[0]['content']); + } + + public function test_addons_from_other_controllers_are_not_returned(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'user-addon', + 'method' => 'edit', + 'content' => 'user-content', + ]); + $this->registry->register('Other\\Controller', [ + 'id' => 'other-addon', + 'method' => 'edit', + 'content' => 'other-content', + ]); + + $addons = $this->registry->getAddons(UserController::class, 'edit', []); + + $this->assertCount(1, $addons); + $this->assertSame('user-addon', $addons[0]['id']); + } + + public function test_get_plugin_addons_does_not_mutate_registered_addons(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'test-addon', + 'method' => 'edit', + 'content' => 'original-content', + ]); + + $this->registry->getAddons(UserController::class, 'edit', []); + $this->registry->getAddons(UserController::class, 'edit', []); + + $addons = $this->registry->getAddons(UserController::class, 'edit', []); + + $this->assertCount(1, $addons); + $this->assertSame('original-content', $addons[0]['content']); + } + + public function test_register_addon_static_method_delegates_to_registry(): void + { + UserController::registerAddon([ + 'id' => 'static-addon', + 'method' => 'edit.settings', + 'content' => 'settings-content', + ]); + + $controller = new UserController(); + $addons = $this->invokeGetPluginAddons($controller, 'edit.settings', []); + + $this->assertCount(1, $addons); + $this->assertSame('static-addon', $addons[0]['id']); + } + + public function test_callable_data_modifier_is_applied_when_resolving_addons(): void + { + $this->registry->register(UserController::class, [ + 'id' => 'callable-addon', + 'method' => 'edit', + 'content' => 'content', + 'data' => fn (array $data) => array_merge($data, ['extra' => 'value']), + ]); + + $addons = $this->registry->getAddons(UserController::class, 'edit', ['base' => 'data']); + + $this->assertCount(1, $addons); + } + + private function bindRegistryInContainer(): void + { + $this->previousContainer = Container::getInstance(); + + $container = new Container(); + $container->singleton(ControllerAddonsRegistry::class, fn () => $this->registry); + Container::setInstance($container); + + if (!function_exists('app')) { + require_once dirname(__DIR__, 4) . '/vendor/laravel/framework/src/Illuminate/Foundation/helpers.php'; + } + } + + /** + * @return array> + */ + private function invokeGetPluginAddons(object $controller, string $method, array $data): array + { + $reflection = new \ReflectionMethod($controller, 'getPluginAddons'); + + return $reflection->invoke($controller, $method, $data); + } +} From 3392225cdb1573786efc66df371c4c200758c7d9 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Fri, 31 Jul 2026 18:13:25 -0400 Subject: [PATCH 12/19] test: enhance HandleRedirectListenerTest to utilize RedirectToEventService for improved redirect handling and state management --- .../Listeners/HandleRedirectListenerTest.php | 285 ++++++++---------- 1 file changed, 131 insertions(+), 154 deletions(-) diff --git a/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php index 0e3e86cf4e..9f6ff1ed13 100644 --- a/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php +++ b/tests/unit/ProcessMaker/Listeners/HandleRedirectListenerTest.php @@ -4,21 +4,28 @@ namespace Tests\Unit\ProcessMaker\Listeners; +use Illuminate\Http\Request; +use Illuminate\Support\Facades\Event; +use Laravel\Octane\Events\RequestTerminated; +use Mockery; use ProcessMaker\Events\RedirectToEvent; use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Octane\ResetRequestState; -use ReflectionProperty; +use ProcessMaker\Services\RedirectToEventService; +use Symfony\Component\HttpFoundation\Response; use Tests\TestCase; class HandleRedirectListenerTest extends TestCase { /** - * Create a test subclass that exposes the protected setRedirectTo method. + * Create a test listener that exposes the protected setRedirectTo method. */ - private function createProbe(): HandleRedirectListener + private function createProbe(?RedirectToEventService $service = null): HandleRedirectListener { - return new class extends HandleRedirectListener { + $service ??= app(RedirectToEventService::class); + + return new class ($service) extends HandleRedirectListener { public function queue(ProcessRequest $processRequest, string $method, ...$params): void { $this->setRedirectTo($processRequest, $method, ...$params); @@ -26,210 +33,180 @@ public function queue(ProcessRequest $processRequest, string $method, ...$params }; } - /** - * Read a private static property from HandleRedirectListener. - */ - private function readStaticProperty(string $property): mixed - { - $reflection = new ReflectionProperty(HandleRedirectListener::class, $property); - $reflection->setAccessible(true); - - return $reflection->getValue(); - } - - /** - * Assert that all 3 static properties are in their default/clean state. - */ - private function assertStateIsClean(): void - { - $this->assertNull($this->readStaticProperty('processRequest')); - $this->assertSame('', $this->readStaticProperty('redirectionMethod')); - $this->assertSame([], $this->readStaticProperty('redirectionParams')); - } - - /** - * Test that reset() clears the static $processRequest property. - */ public function test_reset_clears_process_request(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated'); - - HandleRedirectListener::reset(); - - $this->assertNull($this->readStaticProperty('processRequest')); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $probe = $this->createProbe($service); + $staleRequest = ProcessRequest::factory()->create(); + $currentRequest = ProcessRequest::factory()->create(); + + $probe->queue($staleRequest, 'staleRedirect'); + $service->reset(); + $probe->queue($currentRequest, 'currentRedirect'); + $service->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($currentRequest) { + return $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $currentRequest->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); } - /** - * Test that reset() clears the static $redirectionMethod property. - */ public function test_reset_clears_redirection_method(): void { - $probe = $this->createProbe(); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $probe = $this->createProbe($service); $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processCompletedRedirect'); - HandleRedirectListener::reset(); + $probe->queue($request, 'staleRedirect'); + $service->reset(); + $probe->queue($request, 'currentRedirect'); + $service->sendRedirectToEvent(); - $this->assertSame('', $this->readStaticProperty('redirectionMethod')); + Event::assertDispatched( + RedirectToEvent::class, + fn (RedirectToEvent $event) => $event->method === 'currentRedirect' + ); } - /** - * Test that reset() clears the static $redirectionParams property. - */ public function test_reset_clears_redirection_params(): void { - $probe = $this->createProbe(); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $probe = $this->createProbe($service); $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['key' => 'value']); - HandleRedirectListener::reset(); + $probe->queue($request, 'processUpdated', ['secret' => 'stale']); + $service->reset(); + $probe->queue($request, 'processUpdated', ['tokenId' => 222]); + $service->sendRedirectToEvent(); - $this->assertSame([], $this->readStaticProperty('redirectionParams')); + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) { + return $event->params[0] === ['tokenId' => 222] + && !array_key_exists('secret', $event->params[0]); + }); } - /** - * Critical test for Octane: verify that reset() prevents data leaks. - * After reset(), the stored redirect data should be gone. - */ public function test_reset_prevents_stale_redirect_from_leaking(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['tokenId' => 123]); + Event::fake([RedirectToEvent::class]); + + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue( + ProcessRequest::factory()->create(), + 'processUpdated', + ['tokenId' => 123] + ); - // Simulate Octane reset between requests - HandleRedirectListener::reset(); + $service->reset(); + $service->sendRedirectToEvent(); - // sendRedirectToEvent should NOT dispatch RedirectToEvent after reset - $this->expectNotToPerformAssertions(); - HandleRedirectListener::sendRedirectToEvent(); + Event::assertNotDispatched(RedirectToEvent::class); } - /** - * Test that reset() can be called multiple times safely. - */ public function test_reset_can_be_called_multiple_times(): void { - HandleRedirectListener::reset(); - HandleRedirectListener::reset(); - HandleRedirectListener::reset(); + Event::fake([RedirectToEvent::class]); - // Should not throw any errors - $this->assertStateIsClean(); + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue(ProcessRequest::factory()->create(), 'processUpdated'); + + $service->reset(); + $service->reset(); + $service->reset(); + $service->sendRedirectToEvent(); + + Event::assertNotDispatched(RedirectToEvent::class); } - /** - * Test that sendRedirectToEvent dispatches the event and clears state. - */ public function test_send_redirect_to_event_dispatches_and_clears_state(): void { - \Illuminate\Support\Facades\Event::fake([RedirectToEvent::class]); - - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated'); + Event::fake([RedirectToEvent::class]); - HandleRedirectListener::sendRedirectToEvent(); + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue(ProcessRequest::factory()->create(), 'processUpdated'); - // Assert the event was dispatched - \Illuminate\Support\Facades\Event::assertDispatched(RedirectToEvent::class); + $service->sendRedirectToEvent(); + $service->sendRedirectToEvent(); - // After dispatch, the state should be cleared - $this->assertNull($this->readStaticProperty('processRequest')); + Event::assertDispatched( + RedirectToEvent::class, + fn (RedirectToEvent $event) => $event->method === 'processUpdated' + ); + Event::assertDispatched(RedirectToEvent::class, 1); } - /** - * CRITICAL: Simulate the full Octane request cycle to guarantee no data leak. - * - * Flow: - * 1. Request A stores data with different values - * 2. Reset (simulating Octane's RequestTerminated event) - * 3. Verify ALL 3 properties are clean - * 4. Request B stores NEW data with different values - * 5. Verify Request B's data is correct (not contaminated by Request A) - * 6. Reset again - * 7. Verify clean again - */ public function test_full_octane_cycle_guarantees_no_data_leak(): void { - // === Request A === + Event::fake([RedirectToEvent::class]); + $requestA = ProcessRequest::factory()->create(); - $probeA = $this->createProbe(); - $probeA->queue($requestA, 'processCompletedRedirect', ['tokenA' => 111]); + $scopeA = app(RedirectToEventService::class); + $this->createProbe($scopeA)->queue( + $requestA, + 'processCompletedRedirect', + ['tokenA' => 111] + ); - // Verify Request A data is stored (setRedirectTo uses ...$params, so it's nested) - $this->assertSame($requestA->getKey(), $this->readStaticProperty('processRequest')->getKey()); - $this->assertSame('processCompletedRedirect', $this->readStaticProperty('redirectionMethod')); - $this->assertSame([['tokenA' => 111]], $this->readStaticProperty('redirectionParams')); + app(ResetRequestState::class)->handle(); + $scopeA->sendRedirectToEvent(); + Event::assertNotDispatched(RedirectToEvent::class); - // === Octane reset after Request A === - HandleRedirectListener::reset(); + app()->forgetScopedInstances(); - // === Verify ALL properties are clean after reset === - $this->assertStateIsClean(); + $scopeB = app(RedirectToEventService::class); + $this->assertNotSame($scopeA, $scopeB); - // === Request B (simulating a DIFFERENT user/request) === $requestB = ProcessRequest::factory()->create(); - $probeB = $this->createProbe(); - $probeB->queue($requestB, 'processUpdated', ['tokenB' => 222, 'userId' => 999]); - - // Verify Request B's data is correct (NOT contaminated by Request A) - $this->assertSame($requestB->getKey(), $this->readStaticProperty('processRequest')->getKey()); - $this->assertSame('processUpdated', $this->readStaticProperty('redirectionMethod')); - $this->assertSame([['tokenB' => 222, 'userId' => 999]], $this->readStaticProperty('redirectionParams')); - - // Verify Request A's data is GONE (no leak) - $this->assertNotSame($requestA->getKey(), $this->readStaticProperty('processRequest')?->getKey()); - - // === Octane reset after Request B === - HandleRedirectListener::reset(); - - // === Verify clean again === - $this->assertStateIsClean(); + $this->createProbe($scopeB)->queue( + $requestB, + 'processUpdated', + ['tokenB' => 222, 'userId' => 999] + ); + $scopeB->sendRedirectToEvent(); + + Event::assertDispatched(RedirectToEvent::class, function (RedirectToEvent $event) use ($requestB) { + return $event->method === 'processUpdated' + && $event->params[0] === ['tokenB' => 222, 'userId' => 999] + && $event->broadcastOn()[0]->name === + 'private-ProcessMaker.Models.ProcessRequest.' . $requestB->id; + }); + Event::assertDispatched(RedirectToEvent::class, 1); } - /** - * CRITICAL: Verify that ResetRequestState orchestrator triggers the reset correctly. - */ - public function test_reset_request_state_triggers_handle_redirect_reset(): void + public function test_reset_request_state_triggers_redirect_service_reset(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['tokenId' => 456]); - - // Verify state is dirty before reset - $this->assertNotNull($this->readStaticProperty('processRequest')); - - // Execute the orchestrator (same as Octane's RequestTerminated listener) - $resetState = new ResetRequestState(); - $resetState->handle(); + $service = Mockery::mock(RedirectToEventService::class); + $service->shouldReceive('reset')->once(); - // Verify orchestrator cleaned everything - $this->assertStateIsClean(); + (new ResetRequestState($service))->handle(); } - /** - * CRITICAL: Simulate the scenario where sendRedirectToEvent() fails, - * but reset() still cleans up (edge case in Octane). - */ - public function test_reset_cleans_up_even_when_send_redirect_fails(): void + public function test_octane_termination_cleans_up_when_redirect_is_never_sent(): void { - $probe = $this->createProbe(); - $request = ProcessRequest::factory()->create(); - $probe->queue($request, 'processUpdated', ['data' => 'sensitive']); + Event::fake([RedirectToEvent::class]); - // Simulate that sendRedirectToEvent is NEVER called (e.g., error in BPMN flow) - // But Octane's RequestTerminated event still fires and calls reset() + $service = app(RedirectToEventService::class); + $this->createProbe($service)->queue( + ProcessRequest::factory()->create(), + 'processUpdated', + ['data' => 'sensitive'] + ); - // This should NOT be called in this scenario: - // HandleRedirectListener::sendRedirectToEvent(); + event(new RequestTerminated( + $this->app, + $this->app, + Request::create('/first-request'), + new Response() + )); - // Octane reset still happens - HandleRedirectListener::reset(); + app(RedirectToEventService::class)->sendRedirectToEvent(); - // Verify no sensitive data leaked - $this->assertStateIsClean(); + Event::assertNotDispatched(RedirectToEvent::class); } } From 743dde4fc51c6ef225ab3cb86e199be9ea4cb055 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Mon, 3 Aug 2026 11:12:26 -0400 Subject: [PATCH 13/19] feat: update SettingObserver to use instance property for artisan cache refresh flag --- ProcessMaker/Observers/SettingObserver.php | 6 +++--- ProcessMaker/Providers/ProcessMakerServiceProvider.php | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ProcessMaker/Observers/SettingObserver.php b/ProcessMaker/Observers/SettingObserver.php index e4144ab3fb..1a5a6ed240 100644 --- a/ProcessMaker/Observers/SettingObserver.php +++ b/ProcessMaker/Observers/SettingObserver.php @@ -10,7 +10,7 @@ class SettingObserver { - private static $added_refresh_artisan_caches = false; + private bool $addedRefreshArtisanCaches = false; /** * Handle the setting "created" event. @@ -95,7 +95,7 @@ private function invalidateSettingCache(Setting $setting) // Check to see if we already added the refresh to the app's terminating queue. // This is important for install commands when multiple settings are being created/updated. - if (self::$added_refresh_artisan_caches) { + if ($this->addedRefreshArtisanCaches) { return; } @@ -106,6 +106,6 @@ private function invalidateSettingCache(Setting $setting) RefreshArtisanCaches::dispatchSync(); }); - self::$added_refresh_artisan_caches = true; + $this->addedRefreshArtisanCaches = true; } } diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index fe32f28b43..a67e07bb52 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -200,6 +200,8 @@ public function register(): void return Models\AnonymousUser::resolve(); }); + $this->app->scoped(Observers\SettingObserver::class); + $this->app->singleton(PolicyExtension::class, function ($app) { return new PolicyExtension(); }); From c6fee3d1aee54acf9ed5d8d0738f9688ca2cb0dd Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Mon, 3 Aug 2026 11:44:36 -0400 Subject: [PATCH 14/19] test: add unit tests to validate callback scheduling and cache invalidation --- .../Observers/SettingObserverTest.php | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 tests/unit/ProcessMaker/Observers/SettingObserverTest.php diff --git a/tests/unit/ProcessMaker/Observers/SettingObserverTest.php b/tests/unit/ProcessMaker/Observers/SettingObserverTest.php new file mode 100644 index 0000000000..612d15c3b4 --- /dev/null +++ b/tests/unit/ProcessMaker/Observers/SettingObserverTest.php @@ -0,0 +1,145 @@ +app->forgetScopedInstances(); + + $observer = app(SettingObserver::class); + $callbackCount = $this->terminatingCallbackCount(); + + $observer->saving($this->setting('first-setting')); + $observer->saving($this->setting('second-setting')); + $observer->deleted($this->setting('third-setting')); + + $this->assertSame($observer, app(SettingObserver::class)); + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_octane_termination_allows_a_refresh_callback_in_the_next_scope(): void + { + $this->app->forgetScopedInstances(); + + $firstObserver = app(SettingObserver::class); + $firstObserver->saving($this->setting('first-request-setting')); + $callbackCount = $this->terminatingCallbackCount(); + + (new FlushTemporaryContainerInstances())->handle(new RequestTerminated( + $this->app, + $this->app, + Request::create('/first-request'), + new Response() + )); + + $secondObserver = app(SettingObserver::class); + $secondObserver->saving($this->setting('second-request-setting')); + + $this->assertNotSame($firstObserver, $secondObserver); + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_eloquent_saves_updates_and_deletes_share_the_scoped_observer(): void + { + $this->app->forgetScopedInstances(); + $callbackCount = $this->terminatingCallbackCount(); + + $firstSetting = Setting::factory()->create([ + 'key' => 'four-32505-first-setting', + 'config' => 'first value', + 'format' => 'text', + ]); + Setting::factory()->create([ + 'key' => 'four-32505-second-setting', + 'config' => 'second value', + 'format' => 'text', + ]); + + $firstSetting->config = 'updated value'; + $firstSetting->save(); + $firstSetting->delete(); + + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_it_invalidates_every_setting_cache_entry_while_debouncing_the_refresh(): void + { + $this->app->forgetScopedInstances(); + + $observer = app(SettingObserver::class); + $settingCache = SettingCacheFactory::getSettingsCache(); + $settings = [ + $this->setting('cached-first-setting'), + $this->setting('cached-second-setting'), + $this->setting('cached-deleted-setting'), + ]; + + foreach ($settings as $setting) { + $settingCache->set($settingCache->createKey(['key' => $setting->key]), 'cached value'); + } + + $callbackCount = $this->terminatingCallbackCount(); + $observer->saving($settings[0]); + $observer->saving($settings[1]); + $observer->deleted($settings[2]); + + foreach ($settings as $setting) { + $this->assertTrue($settingCache->missing( + $settingCache->createKey(['key' => $setting->key]) + )); + } + + $this->assertSame($callbackCount + 1, $this->terminatingCallbackCount()); + } + + public function test_the_terminating_callback_dispatches_the_refresh_job_synchronously(): void + { + Bus::fake([RefreshArtisanCaches::class]); + $this->app->forgetScopedInstances(); + + $callbackCount = $this->terminatingCallbackCount(); + app(SettingObserver::class)->saving($this->setting('refresh-job-setting')); + + Bus::assertNotDispatched(RefreshArtisanCaches::class); + + $callbacks = $this->terminatingCallbacks(); + $this->app->call($callbacks[$callbackCount]); + + Bus::assertDispatchedSyncTimes(RefreshArtisanCaches::class, 1); + } + + private function setting(string $key): Setting + { + return new Setting([ + 'key' => $key, + 'config' => 'value', + 'format' => 'text', + ]); + } + + private function terminatingCallbackCount(): int + { + return count($this->terminatingCallbacks()); + } + + private function terminatingCallbacks(): array + { + return (new ReflectionProperty(Application::class, 'terminatingCallbacks')) + ->getValue($this->app); + } +} From 96fbfd3d35cfc495ff47df746a1e860e61e4e6e7 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Mon, 3 Aug 2026 13:22:33 -0400 Subject: [PATCH 15/19] =?UTF-8?q?FOUR-32507=20[Octane]=20MEDIUM=20?= =?UTF-8?q?=E2=80=94=20Accumulating=20State=20"ServerTimingMiddleware"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description: Fix Octane state leak in ServerTimingMiddleware by reading min_package_time from config per request instead of a static property. Add tests for package timing threshold and config refresh between requests. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32507 --- .../Middleware/ServerTimingMiddleware.php | 13 ++--- tests/Feature/ServerTimingMiddlewareTest.php | 53 +++++++++++++++++++ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php b/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php index 2a15e84a6d..aab562465f 100644 --- a/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php +++ b/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php @@ -9,18 +9,10 @@ class ServerTimingMiddleware { - // Minimum time in ms to include a package in the Server-Timing header - private static $minPackageTime; - - public function __construct() - { - self::$minPackageTime = config('app.server_timing.min_package_time'); - } - /** * Handle an incoming request. * - * @param \Closure(\Illuminate\Http\Request): (\Symfony\Component\HttpFoundation\Response) $next + * @param Closure(Request): (Response) $next */ public function handle(Request $request, Closure $next): Response { @@ -56,12 +48,13 @@ public function handle(Request $request, Closure $next): Response } $packageTimes = ProcessMakerServiceProvider::getPackageBootTiming(); + $minPackageTime = config('app.server_timing.min_package_time'); foreach ($packageTimes as $package => $timing) { $time = ($timing['end'] - $timing['start']) * 1000; // Only include packages that took more than MIN_PACKAGE_TIME ms - if ($time > self::$minPackageTime) { + if ($time > $minPackageTime) { $serverTiming[] = "{$package};dur={$time}"; } } diff --git a/tests/Feature/ServerTimingMiddlewareTest.php b/tests/Feature/ServerTimingMiddlewareTest.php index 040401d227..11ce958fd4 100644 --- a/tests/Feature/ServerTimingMiddlewareTest.php +++ b/tests/Feature/ServerTimingMiddlewareTest.php @@ -22,6 +22,11 @@ private function getHeader($response, $header) return $headers[$header]; } + private function getServerTimingHeaderValue($response): string + { + return implode(',', $this->getHeader($response, 'server-timing')); + } + public function testServerTimingHeaderIncludesAllMetrics() { Route::middleware(ServerTimingMiddleware::class)->get('/test', function () { @@ -162,6 +167,54 @@ public function testServerTimingOnLogin() $this->assertStringContainsString('db;dur=', $serverTiming[2]); } + public function testPackageTimingRespectsMinPackageTimeThreshold() + { + config([ + 'app.server_timing.enabled' => true, + 'app.server_timing.min_package_time' => 5, + ]); + + ProcessMakerServiceProvider::setPackageBootStart('foour32507-fast-package', 0.0); + ProcessMakerServiceProvider::setPackageBootedTime('foour32507-fast-package', 0.002); + + ProcessMakerServiceProvider::setPackageBootStart('foour32507-slow-package', 0.0); + ProcessMakerServiceProvider::setPackageBootedTime('foour32507-slow-package', 0.010); + + Route::middleware(ServerTimingMiddleware::class)->get('/package-threshold-test', function () { + return response()->json(['message' => 'Package threshold test']); + }); + + $response = $this->get('/package-threshold-test'); + $response->assertHeader('Server-Timing'); + + $serverTiming = $this->getServerTimingHeaderValue($response); + + $this->assertStringNotContainsString('foour32507-fast-package;dur=', $serverTiming); + $this->assertStringContainsString('foour32507-slow-package;dur=', $serverTiming); + } + + public function testMinPackageTimeReadsConfigPerRequest() + { + config(['app.server_timing.enabled' => true]); + + ProcessMakerServiceProvider::setPackageBootStart('foour32507-octane-package', 0.0); + ProcessMakerServiceProvider::setPackageBootedTime('foour32507-octane-package', 0.008); + + Route::middleware(ServerTimingMiddleware::class)->get('/octane-min-package-test', function () { + return response()->json(['message' => 'Octane min package test']); + }); + + config(['app.server_timing.min_package_time' => 10]); + $responseAboveThreshold = $this->get('/octane-min-package-test'); + $serverTimingAboveThreshold = $this->getServerTimingHeaderValue($responseAboveThreshold); + $this->assertStringNotContainsString('foour32507-octane-package;dur=', $serverTimingAboveThreshold); + + config(['app.server_timing.min_package_time' => 5]); + $responseBelowThreshold = $this->get('/octane-min-package-test'); + $serverTimingBelowThreshold = $this->getServerTimingHeaderValue($responseBelowThreshold); + $this->assertStringContainsString('foour32507-octane-package;dur=', $serverTimingBelowThreshold); + } + public function testServerTimingIfIsDisabled() { config(['app.server_timing.enabled' => false]); From 3d9f5b17b9cfb00daf7b772552ef5662873dbf26 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Mon, 3 Aug 2026 13:40:59 -0400 Subject: [PATCH 16/19] =?UTF-8?q?FOUR-32504=20[Octane]=20MEDIUM=20?= =?UTF-8?q?=E2=80=94=20Accumulating=20State=20"RetryProcessRequest"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description: Replace static $output and $taskTypes with per-instance state to prevent data leaking between requests under Octane. Add getOutput() and update UnblockRequest and ProcessRequestController callers. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32504 --- .../Console/Commands/UnblockRequest.php | 2 +- .../Api/ProcessRequestController.php | 2 +- ProcessMaker/RetryProcessRequest.php | 19 +-- .../ProcessMaker/RetryProcessRequestTest.php | 119 ++++++++++++++++++ 4 files changed, 133 insertions(+), 9 deletions(-) create mode 100644 tests/unit/ProcessMaker/RetryProcessRequestTest.php diff --git a/ProcessMaker/Console/Commands/UnblockRequest.php b/ProcessMaker/Console/Commands/UnblockRequest.php index f23aba0e6b..110ccf3f17 100644 --- a/ProcessMaker/Console/Commands/UnblockRequest.php +++ b/ProcessMaker/Console/Commands/UnblockRequest.php @@ -64,7 +64,7 @@ public function handle(): int $retryRequest->retry(); - foreach ($retryRequest::$output as $line) { + foreach ($retryRequest->getOutput() as $line) { $this->info($line); } diff --git a/ProcessMaker/Http/Controllers/Api/ProcessRequestController.php b/ProcessMaker/Http/Controllers/Api/ProcessRequestController.php index 15746d08cd..41e496e391 100644 --- a/ProcessMaker/Http/Controllers/Api/ProcessRequestController.php +++ b/ProcessMaker/Http/Controllers/Api/ProcessRequestController.php @@ -304,7 +304,7 @@ public function retry(ProcessRequest $request, Request $httpRequest): JsonRespon $retryRequest->retry(); return response()->json([ - 'message' => $retryRequest::$output, + 'message' => $retryRequest->getOutput(), 'success' => true, ]); } catch (Throwable $throwable) { diff --git a/ProcessMaker/RetryProcessRequest.php b/ProcessMaker/RetryProcessRequest.php index 677a514779..98b68c2ab7 100644 --- a/ProcessMaker/RetryProcessRequest.php +++ b/ProcessMaker/RetryProcessRequest.php @@ -21,9 +21,9 @@ class RetryProcessRequest { - public static array $output = []; + private array $output = []; - private static array $taskTypes = []; + private array $taskTypes = []; private ProcessRequest $processRequest; @@ -60,7 +60,7 @@ public function getRetriableTasks(): Collection public function hasNonRetriableTasks(): bool { - $currentTaskTypes = static::$taskTypes; + $currentTaskTypes = $this->taskTypes; $this->determineTaskTypes(true); @@ -102,7 +102,7 @@ public function retry(): void WorkflowManager::runServiceTask($task, $token); } - static::$output[] = $this->formatOutput($task, $element, $token); + $this->output[] = $this->formatOutput($task, $element, $token); }); $this->createRequestComment(); @@ -166,12 +166,17 @@ public function createRequestComment(): void $comment->save(); } + public function getOutput(): array + { + return $this->output; + } + private function determineTaskTypes(bool $all = false): void { if ($all || app()->runningInConsole()) { - static::$taskTypes = ['scriptTask', 'serviceTask', 'task']; + $this->taskTypes = ['scriptTask', 'serviceTask', 'task']; } else { - static::$taskTypes = ['scriptTask']; + $this->taskTypes = ['scriptTask']; } } @@ -181,7 +186,7 @@ public function retriableTasksQuery(): HasMany $tokensQuery->whereIn('status', ['FAILING', 'ACTIVE', 'ERROR']); - $tokensQuery->whereIn('element_type', static::$taskTypes); + $tokensQuery->whereIn('element_type', $this->taskTypes); return $tokensQuery; } diff --git a/tests/unit/ProcessMaker/RetryProcessRequestTest.php b/tests/unit/ProcessMaker/RetryProcessRequestTest.php new file mode 100644 index 0000000000..ec4adc89ab --- /dev/null +++ b/tests/unit/ProcessMaker/RetryProcessRequestTest.php @@ -0,0 +1,119 @@ +getProperty($property); + $propertyReflection->setAccessible(true); + + return $propertyReflection->getValue($object); + } + + private function setPrivateProperty(object $object, string $property, mixed $value): void + { + $reflection = new ReflectionClass($object); + $propertyReflection = $reflection->getProperty($property); + $propertyReflection->setAccessible(true); + $propertyReflection->setValue($object, $value); + } + + private function createRetryProcessRequest(): RetryProcessRequest + { + return RetryProcessRequest::for(ProcessRequest::factory()->create()); + } + + private function invokeDetermineTaskTypes(RetryProcessRequest $retry, bool $all = false): void + { + $reflection = new ReflectionClass($retry); + $method = $reflection->getMethod('determineTaskTypes'); + $method->setAccessible(true); + $method->invoke($retry, $all); + } + + private function withRunningInConsole(bool $runningInConsole, callable $callback): mixed + { + $originalApp = $this->app; + $mock = Mockery::mock($originalApp)->makePartial(); + $mock->shouldReceive('runningInConsole')->andReturn($runningInConsole); + $this->app = $mock; + Container::setInstance($mock); + + try { + return $callback(); + } finally { + $this->app = $originalApp; + Container::setInstance($originalApp); + } + } + + public function test_output_does_not_leak_between_instances(): void + { + $first = $this->createRetryProcessRequest(); + $second = $this->createRetryProcessRequest(); + + $this->setPrivateProperty($first, 'output', ['Retrying ScriptTask (node_1) for Request::1']); + + $this->assertSame(['Retrying ScriptTask (node_1) for Request::1'], $first->getOutput()); + $this->assertSame([], $second->getOutput()); + } + + public function test_task_types_do_not_leak_between_instances(): void + { + $first = $this->createRetryProcessRequest(); + $second = $this->createRetryProcessRequest(); + + $this->setPrivateProperty($first, 'taskTypes', ['scriptTask', 'serviceTask', 'task']); + $this->setPrivateProperty($second, 'taskTypes', ['scriptTask']); + + $this->assertSame(['scriptTask', 'serviceTask', 'task'], $this->getPrivateProperty($first, 'taskTypes')); + $this->assertSame(['scriptTask'], $this->getPrivateProperty($second, 'taskTypes')); + } + + public function test_determine_task_types_can_include_all_types_when_requested(): void + { + $retry = $this->createRetryProcessRequest(); + + $this->invokeDetermineTaskTypes($retry, true); + + $this->assertSame( + ['scriptTask', 'serviceTask', 'task'], + $this->getPrivateProperty($retry, 'taskTypes') + ); + } + + public function test_task_types_include_only_script_tasks_in_web_context(): void + { + $retry = $this->createRetryProcessRequest(); + + $this->withRunningInConsole(false, function () use ($retry) { + $this->invokeDetermineTaskTypes($retry); + }); + + $this->assertSame(['scriptTask'], $this->getPrivateProperty($retry, 'taskTypes')); + } + + public function test_has_non_retriable_tasks_does_not_leak_task_types_to_other_instances(): void + { + $first = $this->createRetryProcessRequest(); + $second = $this->createRetryProcessRequest(); + + $this->setPrivateProperty($second, 'taskTypes', ['scriptTask']); + + $first->hasNonRetriableTasks(); + + $this->assertSame(['scriptTask'], $this->getPrivateProperty($second, 'taskTypes')); + } +} From c583b0fa0fc217435e11baa7d81c957f9071c4c6 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Mon, 3 Aug 2026 15:33:31 -0400 Subject: [PATCH 17/19] feat: implement WorkerBootTimingService for improved boot timing tracking --- .../Middleware/ServerTimingMiddleware.php | 9 +- .../Providers/ProcessMakerServiceProvider.php | 75 ++------------- .../Services/WorkerBootTimingService.php | 95 +++++++++++++++++++ .../Traits/PluginServiceProviderTrait.php | 16 +--- 4 files changed, 114 insertions(+), 81 deletions(-) create mode 100644 ProcessMaker/Services/WorkerBootTimingService.php diff --git a/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php b/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php index aab562465f..958daa6707 100644 --- a/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php +++ b/ProcessMaker/Http/Middleware/ServerTimingMiddleware.php @@ -5,10 +5,15 @@ use Closure; use Illuminate\Http\Request; use ProcessMaker\Providers\ProcessMakerServiceProvider; +use ProcessMaker\Services\WorkerBootTimingService; use Symfony\Component\HttpFoundation\Response; class ServerTimingMiddleware { + public function __construct(private WorkerBootTimingService $workerBootTimingService) + { + } + /** * Handle an incoming request. * @@ -31,7 +36,7 @@ public function handle(Request $request, Closure $next): Response // Calculate execution times $controllerTime = (microtime(true) - $startController) * 1000; // Convert to ms // Fetch service provider boot time - $serviceProviderTime = ProcessMakerServiceProvider::getBootTime() ?? 0; + $serviceProviderTime = $this->workerBootTimingService->getProviderBootTime() ?? 0; // Fetch query time $queryTime = ProcessMakerServiceProvider::getQueryTime() ?? 0; @@ -47,7 +52,7 @@ public function handle(Request $request, Closure $next): Response array_unshift($serverTiming, "boot;dur={$bootTiming}"); } - $packageTimes = ProcessMakerServiceProvider::getPackageBootTiming(); + $packageTimes = $this->workerBootTimingService->getPackageBootTiming(); $minPackageTime = config('app.server_timing.min_package_time'); foreach ($packageTimes as $package => $timing) { diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index a67e07bb52..1e24c71f51 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -57,6 +57,7 @@ use ProcessMaker\Repositories\SettingsConfigRepository; use ProcessMaker\Services\ConditionalRedirectService; use ProcessMaker\Services\RedirectToEventService; +use ProcessMaker\Services\WorkerBootTimingService; use RuntimeException; use Spatie\Multitenancy\Events\MadeTenantCurrentEvent; use Spatie\Multitenancy\Events\TenantNotFoundForRequestEvent; @@ -67,22 +68,13 @@ */ class ProcessMakerServiceProvider extends ServiceProvider { - // Track the start time for service providers boot - private static $bootStart; - - // Track the boot time for service providers - private static $bootTime; - - // Track the boot time for each package - private static $packageBootTiming = []; - // Track the query time for each request private static $queryTime = 0; public function boot(): void { // Track the start time for service providers boot - self::$bootStart = microtime(true); + $bootStart = microtime(true); // Set the current tenant $this->setCurrentTenantForConsoleCommands(); @@ -111,11 +103,15 @@ public function boot(): void $this->registerOctaneListeners(); // Hook after service providers boot - self::$bootTime = (microtime(true) - self::$bootStart) * 1000; // Convert to milliseconds + $this->app->make(WorkerBootTimingService::class) + ->setProviderBootTime((microtime(true) - $bootStart) * 1000); } public function register(): void { + // Boot metrics live for the lifetime of the application worker. + $this->app->singleton(WorkerBootTimingService::class); + if (config('app.server_timing.enabled')) { // Listen to query events and accumulate query execution time DB::listen(function ($query) { @@ -509,16 +505,6 @@ public static function forceHttps(): void } } - /** - * Get the boot time for service providers. - * - * @return float|null - */ - public static function getBootTime(): ?float - { - return self::$bootTime; - } - /** * Reset per-request query timing metrics. */ @@ -537,53 +523,6 @@ public static function getQueryTime(): float return self::$queryTime; } - /** - * Set the boot time for service providers. - * - * @param string $package - * @param float $time - */ - public static function setPackageBootStart(string $package, float $time): void - { - if ($time < 0) { - Log::info("Server Timing: Invalid boot time for package: {$package}, time: {$time}"); - - $time = 0; - } - - self::$packageBootTiming[$package] = [ - 'start' => $time, - 'end' => null, - ]; - } - - /** - * Set the boot time for service providers. - * - * - * @param float $time - */ - public static function setPackageBootedTime(string $package, $time): void - { - if (!isset(self::$packageBootTiming[$package]) || $time < 0) { - Log::info("Server Timing: Invalid booted time for package: {$package}, time: {$time}"); - - return; - } - - self::$packageBootTiming[$package]['end'] = $time; - } - - /** - * Get the boot time for service providers. - * - * @return array - */ - public static function getPackageBootTiming(): array - { - return self::$packageBootTiming; - } - /** * Reset per-request static state between Octane requests. * diff --git a/ProcessMaker/Services/WorkerBootTimingService.php b/ProcessMaker/Services/WorkerBootTimingService.php new file mode 100644 index 0000000000..0e14ae3de9 --- /dev/null +++ b/ProcessMaker/Services/WorkerBootTimingService.php @@ -0,0 +1,95 @@ + + */ + private array $packageBootTiming = []; + + /** + * Store the ProcessMaker service provider boot duration for this worker. + * + * @param float $time Boot duration in milliseconds + */ + public function setProviderBootTime(float $time): void + { + $this->providerBootTime = $time; + } + + /** + * Get the ProcessMaker service provider boot duration for this worker. + * + * @return float|null Boot duration in milliseconds, or null before it is recorded + */ + public function getProviderBootTime(): ?float + { + return $this->providerBootTime; + } + + /** + * Record when a package service provider starts booting. + * + * Invalid negative timestamps are logged and stored as zero. + * Calling this method again for the same package replaces its prior timing. + * + * @param string $package Package name used in the Server-Timing header + * @param float $time Start timestamp in seconds, as returned by microtime(true) + */ + public function setPackageBootStart(string $package, float $time): void + { + if ($time < 0) { + Log::info("Server Timing: Invalid boot time for package: {$package}, time: {$time}"); + + $time = 0.0; + } + + $this->packageBootTiming[$package] = [ + 'start' => $time, + 'end' => null, + ]; + } + + /** + * Record when a package service provider finishes booting. + * + * Invalid negative timestamps and packages without a recorded start are + * logged and ignored. + * + * @param string $package Package name used in the Server-Timing header + * @param float $time End timestamp in seconds, as returned by microtime(true) + */ + public function setPackageBootedTime(string $package, float $time): void + { + if (!isset($this->packageBootTiming[$package]) || $time < 0) { + Log::info("Server Timing: Invalid booted time for package: {$package}, time: {$time}"); + + return; + } + + $this->packageBootTiming[$package]['end'] = $time; + } + + /** + * Get all package boot timestamps recorded for this worker. + * + * @return array + */ + public function getPackageBootTiming(): array + { + return $this->packageBootTiming; + } +} diff --git a/ProcessMaker/Traits/PluginServiceProviderTrait.php b/ProcessMaker/Traits/PluginServiceProviderTrait.php index b03160e575..f049697279 100644 --- a/ProcessMaker/Traits/PluginServiceProviderTrait.php +++ b/ProcessMaker/Traits/PluginServiceProviderTrait.php @@ -11,7 +11,7 @@ use ProcessMaker\Managers\IndexManager; use ProcessMaker\Managers\LoginManager; use ProcessMaker\Managers\PackageManager; -use ProcessMaker\Providers\ProcessMakerServiceProvider; +use ProcessMaker\Services\WorkerBootTimingService; /** * Add functionality to control a PM plug-in @@ -22,10 +22,6 @@ trait PluginServiceProviderTrait private $scriptBuilderScripts = []; - private static $bootStart = null; - - private static $bootTime; - public function __construct($app) { parent::__construct($app); @@ -48,15 +44,13 @@ protected function bootServerTiming(): void $package = $this->getPackageName(); $this->booting(function () use ($package) { - self::$bootStart = microtime(true); - - ProcessMakerServiceProvider::setPackageBootStart($package, self::$bootStart); + $this->app->make(WorkerBootTimingService::class) + ->setPackageBootStart($package, microtime(true)); }); $this->booted(function () use ($package) { - self::$bootTime = microtime(true); - - ProcessMakerServiceProvider::setPackageBootedTime($package, self::$bootTime); + $this->app->make(WorkerBootTimingService::class) + ->setPackageBootedTime($package, microtime(true)); }); } From 9b798c5eb24f2a4d93ae0c330a7071c18b419017 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Mon, 3 Aug 2026 16:21:39 -0400 Subject: [PATCH 18/19] test: add middleware request timing validation tests --- tests/Feature/ServerTimingMiddlewareTest.php | 58 ++++- .../Octane/ResetRequestStateTest.php | 21 ++ .../Services/WorkerBootTimingServiceTest.php | 214 ++++++++++++++++++ 3 files changed, 287 insertions(+), 6 deletions(-) create mode 100644 tests/unit/ProcessMaker/Services/WorkerBootTimingServiceTest.php diff --git a/tests/Feature/ServerTimingMiddlewareTest.php b/tests/Feature/ServerTimingMiddlewareTest.php index 63bf54f0bb..b6457dd9d7 100644 --- a/tests/Feature/ServerTimingMiddlewareTest.php +++ b/tests/Feature/ServerTimingMiddlewareTest.php @@ -10,6 +10,7 @@ use ProcessMaker\Http\Middleware\ServerTimingMiddleware; use ProcessMaker\Models\User; use ProcessMaker\Providers\ProcessMakerServiceProvider; +use ProcessMaker\Services\WorkerBootTimingService; use ReflectionClass; use Tests\Feature\Shared\RequestHelper; use Tests\TestCase; @@ -177,6 +178,49 @@ public function testServiceProviderTimeIsMeasured() $this->assertGreaterThanOrEqual(0, (float) $providersTime); } + public function testOctaneGatewayPreservesWorkerBootTimingAcrossConsecutiveRequests() + { + config(['app.server_timing.min_package_time' => 0]); + + $workerTiming = app(WorkerBootTimingService::class); + $workerTiming->setProviderBootTime(12.5); + $workerTiming->setPackageBootStart('four32501-worker-package', 10.0); + $workerTiming->setPackageBootedTime('four32501-worker-package', 10.025); + $expectedPackageTiming = $workerTiming->getPackageBootTiming(); + + Route::middleware(ServerTimingMiddleware::class)->get('/octane-worker-timing', function () { + return response()->json(['message' => 'Octane worker timing test']); + }); + + $firstRequest = Request::create('/octane-worker-timing'); + $firstGateway = new ApplicationGateway($this->app, clone $this->app); + $firstResponse = $firstGateway->handle($firstRequest); + + $this->assertSame(12.5, $this->getMetricDuration($firstResponse, 'provider')); + $this->assertEqualsWithDelta( + 25.0, + $this->getMetricDuration($firstResponse, 'four32501-worker-package'), + 0.001 + ); + + $firstGateway->terminate($firstRequest, $firstResponse); + + $secondRequest = Request::create('/octane-worker-timing'); + $secondGateway = new ApplicationGateway($this->app, clone $this->app); + $secondResponse = $secondGateway->handle($secondRequest); + + $this->assertSame(12.5, $this->getMetricDuration($secondResponse, 'provider')); + $this->assertEqualsWithDelta( + 25.0, + $this->getMetricDuration($secondResponse, 'four32501-worker-package'), + 0.001 + ); + + $secondGateway->terminate($secondRequest, $secondResponse); + + $this->assertSame($expectedPackageTiming, $workerTiming->getPackageBootTiming()); + } + public function testControllerTimingIsMeasuredCorrectly() { // Mock a route @@ -241,11 +285,12 @@ public function testPackageTimingRespectsMinPackageTimeThreshold() 'app.server_timing.min_package_time' => 5, ]); - ProcessMakerServiceProvider::setPackageBootStart('foour32507-fast-package', 0.0); - ProcessMakerServiceProvider::setPackageBootedTime('foour32507-fast-package', 0.002); + $workerTiming = app(WorkerBootTimingService::class); + $workerTiming->setPackageBootStart('foour32507-fast-package', 0.0); + $workerTiming->setPackageBootedTime('foour32507-fast-package', 0.002); - ProcessMakerServiceProvider::setPackageBootStart('foour32507-slow-package', 0.0); - ProcessMakerServiceProvider::setPackageBootedTime('foour32507-slow-package', 0.010); + $workerTiming->setPackageBootStart('foour32507-slow-package', 0.0); + $workerTiming->setPackageBootedTime('foour32507-slow-package', 0.010); Route::middleware(ServerTimingMiddleware::class)->get('/package-threshold-test', function () { return response()->json(['message' => 'Package threshold test']); @@ -264,8 +309,9 @@ public function testMinPackageTimeReadsConfigPerRequest() { config(['app.server_timing.enabled' => true]); - ProcessMakerServiceProvider::setPackageBootStart('foour32507-octane-package', 0.0); - ProcessMakerServiceProvider::setPackageBootedTime('foour32507-octane-package', 0.008); + $workerTiming = app(WorkerBootTimingService::class); + $workerTiming->setPackageBootStart('foour32507-octane-package', 0.0); + $workerTiming->setPackageBootedTime('foour32507-octane-package', 0.008); Route::middleware(ServerTimingMiddleware::class)->get('/octane-min-package-test', function () { return response()->json(['message' => 'Octane min package test']); diff --git a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php index 556f80d1cd..0af3ee25ad 100644 --- a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php +++ b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php @@ -15,6 +15,7 @@ use ProcessMaker\Octane\ResetRequestState; use ProcessMaker\Providers\ProcessMakerServiceProvider; use ProcessMaker\Services\RedirectToEventService; +use ProcessMaker\Services\WorkerBootTimingService; use Symfony\Component\HttpFoundation\Response; use Tests\TestCase; @@ -110,6 +111,26 @@ public function test_octane_request_termination_resets_timing_after_an_error_res $this->assertSame(0.0, ProcessMakerServiceProvider::getQueryTime()); } + + public function test_octane_request_termination_preserves_worker_boot_timing(): void + { + $workerTiming = app(WorkerBootTimingService::class); + $workerTiming->setProviderBootTime(12.5); + $workerTiming->setPackageBootStart('ExamplePackage', 10.0); + $workerTiming->setPackageBootedTime('ExamplePackage', 10.25); + $expectedPackageTiming = $workerTiming->getPackageBootTiming(); + + event(new RequestTerminated( + $this->app, + $this->app, + Request::create('/first-request'), + new Response() + )); + + $this->assertSame($workerTiming, app(WorkerBootTimingService::class)); + $this->assertSame(12.5, $workerTiming->getProviderBootTime()); + $this->assertSame($expectedPackageTiming, $workerTiming->getPackageBootTiming()); + } } final class RedirectStateProbe extends HandleRedirectListener diff --git a/tests/unit/ProcessMaker/Services/WorkerBootTimingServiceTest.php b/tests/unit/ProcessMaker/Services/WorkerBootTimingServiceTest.php new file mode 100644 index 0000000000..139211139c --- /dev/null +++ b/tests/unit/ProcessMaker/Services/WorkerBootTimingServiceTest.php @@ -0,0 +1,214 @@ +setProviderBootTime(12.5); + + $this->app->forgetScopedInstances(); + + $this->assertSame($service, app(WorkerBootTimingService::class)); + $this->assertSame(12.5, app(WorkerBootTimingService::class)->getProviderBootTime()); + $this->assertNotContains(WorkerBootTimingService::class, config('octane.flush')); + } + + public function test_octane_application_clone_shares_the_worker_timing_service(): void + { + $service = app(WorkerBootTimingService::class); + $service->setProviderBootTime(18.75); + + $sandbox = clone $this->app; + $sandboxService = $sandbox->make(WorkerBootTimingService::class); + + $this->assertSame($service, $sandboxService); + $this->assertSame(18.75, $sandboxService->getProviderBootTime()); + } + + public function test_it_records_package_boot_start_and_end_times(): void + { + $service = new WorkerBootTimingService(); + + $service->setPackageBootStart('ExamplePackage', 10.25); + $service->setPackageBootedTime('ExamplePackage', 10.75); + + $this->assertSame([ + 'ExamplePackage' => [ + 'start' => 10.25, + 'end' => 10.75, + ], + ], $service->getPackageBootTiming()); + } + + public function test_repeated_package_measurements_replace_the_existing_entry(): void + { + $service = new WorkerBootTimingService(); + + $service->setPackageBootStart('ExamplePackage', 10.0); + $service->setPackageBootedTime('ExamplePackage', 11.0); + $service->setPackageBootStart('ExamplePackage', 20.0); + $service->setPackageBootedTime('ExamplePackage', 20.5); + + $this->assertCount(1, $service->getPackageBootTiming()); + $this->assertSame([ + 'start' => 20.0, + 'end' => 20.5, + ], $service->getPackageBootTiming()['ExamplePackage']); + } + + public function test_many_repeated_measurements_remain_bounded_by_unique_package_names(): void + { + $service = new WorkerBootTimingService(); + + for ($index = 0; $index < 1000; $index++) { + $package = 'Package' . ($index % 5); + $service->setPackageBootStart($package, (float) $index); + $service->setPackageBootedTime($package, $index + 0.5); + } + + $this->assertCount(5, $service->getPackageBootTiming()); + $this->assertSame([ + 'start' => 999.0, + 'end' => 999.5, + ], $service->getPackageBootTiming()['Package4']); + } + + public function test_returned_package_timing_snapshot_cannot_mutate_worker_state(): void + { + $service = new WorkerBootTimingService(); + $service->setPackageBootStart('ExamplePackage', 10.0); + $service->setPackageBootedTime('ExamplePackage', 10.5); + + $snapshot = $service->getPackageBootTiming(); + $snapshot['ExamplePackage']['start'] = 999.0; + $snapshot['InjectedPackage'] = [ + 'start' => 20.0, + 'end' => 21.0, + ]; + + $this->assertSame([ + 'ExamplePackage' => [ + 'start' => 10.0, + 'end' => 10.5, + ], + ], $service->getPackageBootTiming()); + } + + public function test_separate_worker_services_do_not_share_timing_state(): void + { + $firstWorker = new WorkerBootTimingService(); + $secondWorker = new WorkerBootTimingService(); + + $firstWorker->setProviderBootTime(10.0); + $firstWorker->setPackageBootStart('FirstWorkerPackage', 1.0); + $firstWorker->setPackageBootedTime('FirstWorkerPackage', 1.5); + + $secondWorker->setProviderBootTime(20.0); + $secondWorker->setPackageBootStart('SecondWorkerPackage', 2.0); + $secondWorker->setPackageBootedTime('SecondWorkerPackage', 2.5); + + $this->assertSame(10.0, $firstWorker->getProviderBootTime()); + $this->assertSame(20.0, $secondWorker->getProviderBootTime()); + $this->assertArrayHasKey('FirstWorkerPackage', $firstWorker->getPackageBootTiming()); + $this->assertArrayNotHasKey('SecondWorkerPackage', $firstWorker->getPackageBootTiming()); + $this->assertArrayHasKey('SecondWorkerPackage', $secondWorker->getPackageBootTiming()); + $this->assertArrayNotHasKey('FirstWorkerPackage', $secondWorker->getPackageBootTiming()); + } + + public function test_invalid_package_start_time_is_logged_and_clamped_to_zero(): void + { + Log::spy(); + $service = new WorkerBootTimingService(); + + $service->setPackageBootStart('InvalidPackage', -1.5); + + $this->assertSame([ + 'start' => 0.0, + 'end' => null, + ], $service->getPackageBootTiming()['InvalidPackage']); + Log::shouldHaveReceived('info') + ->once() + ->with('Server Timing: Invalid boot time for package: InvalidPackage, time: -1.5'); + } + + public function test_invalid_package_end_time_is_logged_and_ignored(): void + { + Log::spy(); + $service = new WorkerBootTimingService(); + $service->setPackageBootStart('InvalidPackage', 5.0); + + $service->setPackageBootedTime('InvalidPackage', -2.5); + + $this->assertNull($service->getPackageBootTiming()['InvalidPackage']['end']); + Log::shouldHaveReceived('info') + ->once() + ->with('Server Timing: Invalid booted time for package: InvalidPackage, time: -2.5'); + } + + public function test_package_end_without_a_start_is_logged_and_does_not_create_state(): void + { + Log::spy(); + $service = new WorkerBootTimingService(); + + $service->setPackageBootedTime('UnstartedPackage', 10.5); + + $this->assertSame([], $service->getPackageBootTiming()); + Log::shouldHaveReceived('info') + ->once() + ->with('Server Timing: Invalid booted time for package: UnstartedPackage, time: 10.5'); + } + + public function test_plugin_service_provider_records_one_complete_package_interval(): void + { + config(['app.server_timing.enabled' => true]); + + $this->app->register(new WorkerBootTimingTestPluginServiceProvider($this->app)); + + $timing = app(WorkerBootTimingService::class)->getPackageBootTiming(); + $this->assertArrayHasKey('WorkerBootTimingTestPlugin', $timing); + $this->assertIsFloat($timing['WorkerBootTimingTestPlugin']['start']); + $this->assertIsFloat($timing['WorkerBootTimingTestPlugin']['end']); + $this->assertGreaterThanOrEqual( + $timing['WorkerBootTimingTestPlugin']['start'], + $timing['WorkerBootTimingTestPlugin']['end'] + ); + } + + public function test_plugin_service_provider_does_not_record_timing_when_disabled(): void + { + config(['app.server_timing.enabled' => false]); + + $this->app->register(new DisabledWorkerBootTimingTestPluginServiceProvider($this->app)); + + $timing = app(WorkerBootTimingService::class)->getPackageBootTiming(); + $this->assertArrayNotHasKey('DisabledWorkerBootTimingTestPlugin', $timing); + } +} + +final class WorkerBootTimingTestPluginServiceProvider extends ServiceProvider +{ + use PluginServiceProviderTrait; + + public const name = 'worker-boot-timing-test-plugin'; + + public function boot(): void + { + usleep(1000); + } +} + +final class DisabledWorkerBootTimingTestPluginServiceProvider extends ServiceProvider +{ + use PluginServiceProviderTrait; + + public const name = 'disabled-worker-boot-timing-test-plugin'; +} From 46afd1b5b0f521dc33858004b24a790e9fb675a7 Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Mon, 3 Aug 2026 17:09:00 -0400 Subject: [PATCH 19/19] =?UTF-8?q?FOUR-32502=20[Octane]=20MEDIUM=20?= =?UTF-8?q?=E2=80=94=20Accumulating=20State=20"Manifest"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description: Reset request-scoped Manifest statics ($parents, $logger) in ResetRequestState after each Octane request. Keep $tableColumns as a schema cache. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32502 --- ProcessMaker/ImportExport/Manifest.php | 6 +++++ ProcessMaker/Octane/ResetRequestState.php | 2 ++ tests/Feature/ImportExport/ManifestTest.php | 7 +++++ .../Octane/ResetRequestStateTest.php | 27 +++++++++++++++++++ 4 files changed, 42 insertions(+) diff --git a/ProcessMaker/ImportExport/Manifest.php b/ProcessMaker/ImportExport/Manifest.php index ac5372b777..9f2bce95bb 100644 --- a/ProcessMaker/ImportExport/Manifest.php +++ b/ProcessMaker/ImportExport/Manifest.php @@ -21,6 +21,12 @@ class Manifest private static $logger = null; + public static function resetRequestState(): void + { + self::$parents = null; + self::$logger = null; + } + public function has(string $uuid) { return array_key_exists($uuid, $this->manifest); diff --git a/ProcessMaker/Octane/ResetRequestState.php b/ProcessMaker/Octane/ResetRequestState.php index 8bde301049..5dc89ace40 100644 --- a/ProcessMaker/Octane/ResetRequestState.php +++ b/ProcessMaker/Octane/ResetRequestState.php @@ -4,6 +4,7 @@ namespace ProcessMaker\Octane; +use ProcessMaker\ImportExport\Manifest; use ProcessMaker\Providers\ProcessMakerServiceProvider; use ProcessMaker\Services\RedirectToEventService; @@ -17,6 +18,7 @@ public function __construct( public function handle(): void { ProcessMakerServiceProvider::beginRequestTiming(); + Manifest::resetRequestState(); $this->redirectToEventService->reset(); } } diff --git a/tests/Feature/ImportExport/ManifestTest.php b/tests/Feature/ImportExport/ManifestTest.php index c42ac2da83..66ba4782be 100644 --- a/tests/Feature/ImportExport/ManifestTest.php +++ b/tests/Feature/ImportExport/ManifestTest.php @@ -22,6 +22,13 @@ class ManifestTest extends TestCase { use HelperTrait; + protected function tearDown(): void + { + Manifest::resetRequestState(); + + parent::tearDown(); + } + private function mockExporter($dependents) { return $this->mock(ScreenExporter::class, function ($mock) use ($dependents) { diff --git a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php index 556f80d1cd..b1ac40d268 100644 --- a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php +++ b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php @@ -10,6 +10,8 @@ use Illuminate\Support\Facades\Event; use Laravel\Octane\Events\RequestTerminated; use ProcessMaker\Events\RedirectToEvent; +use ProcessMaker\ImportExport\Manifest; +use ProcessMaker\ImportExport\Options; use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Octane\ResetRequestState; @@ -23,6 +25,7 @@ class ResetRequestStateTest extends TestCase protected function tearDown(): void { ProcessMakerServiceProvider::beginRequestTiming(); + Manifest::resetRequestState(); parent::tearDown(); } @@ -34,6 +37,30 @@ private function recordQueryDuration(float $milliseconds): void event(new QueryExecuted('SELECT 1', [], $milliseconds, $connection)); } + public function test_octane_request_termination_resets_manifest_request_state(): void + { + Manifest::buildParentModeMap([ + 'parent-uuid' => [ + 'dependents' => [ + ['uuid' => 'child-uuid'], + ], + ], + ], new Options([ + 'parent-uuid' => ['mode' => 'update'], + ])); + + $this->assertNotNull(Manifest::$parents); + + event(new RequestTerminated( + $this->app, + $this->app, + Request::create('/import-request'), + new Response() + )); + + $this->assertNull(Manifest::$parents); + } + public function test_it_clears_request_timing_before_the_next_request(): void { DB::select('SELECT 1');