From 783d001ae6a2a73e9fe449f74f1cb5993a524371 Mon Sep 17 00:00:00 2001 From: Tom Mitchelmore Date: Tue, 24 Mar 2026 13:36:05 +0000 Subject: [PATCH 1/4] Introduce a dedicated ResponseEmitter class to safely handle response emission and refactor core classes to use it via dependency injection --- src/Application.php | 18 +++- .../RegisterExceptionHandler.php | 9 +- src/Http/ResponseEmitter.php | 28 ++++++ tests/Unit/ApplicationTest.php | 27 +++++- .../RegisterExceptionHandlerTest.php | 32 +++++-- tests/Unit/Http/ResponseEmitterTest.php | 86 +++++++++++++++++++ 6 files changed, 181 insertions(+), 19 deletions(-) create mode 100644 src/Http/ResponseEmitter.php create mode 100644 tests/Unit/Http/ResponseEmitterTest.php diff --git a/src/Application.php b/src/Application.php index 901bf41..6af29c6 100644 --- a/src/Application.php +++ b/src/Application.php @@ -5,9 +5,9 @@ use Closure; use DI\Container; use Illuminate\Support\Collection; -use Laminas\HttpHandlerRunner\Emitter\SapiEmitter; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseInterface; +use Rareloop\Lumberjack\Http\ResponseEmitter; class Application implements ContainerInterface { @@ -282,9 +282,23 @@ public function shutdown(?ResponseInterface $response = null) // If we're handling a WordPressController response at this point then WordPress will already have // sent headers as it happens earlier in the lifecycle. For this scenario we need to do a bit more // work to make sure that duplicate headers are not sent back. - (new SapiEmitter())->emit($this->removeSentHeadersAndMoveIntoResponse($response)); + $responseToSend = $this->removeSentHeadersAndMoveIntoResponse($response); + + $this->get(ResponseEmitter::class)->emit($responseToSend); } + $this->terminate(); + } + + /** + * Terminate the application execution. + * + * This method is a wrapper around die() to allow for easier unit testing by mocking. + * + * @return void + */ + protected function terminate() + { die(); } diff --git a/src/Bootstrappers/RegisterExceptionHandler.php b/src/Bootstrappers/RegisterExceptionHandler.php index 64e946d..8c9d364 100644 --- a/src/Bootstrappers/RegisterExceptionHandler.php +++ b/src/Bootstrappers/RegisterExceptionHandler.php @@ -5,12 +5,11 @@ use Error; use ErrorException; use DI\NotFoundException; -use function Http\Response\send; use Rareloop\Router\Responsable; use Rareloop\Lumberjack\Application; use Psr\Http\Message\ResponseInterface; use Laminas\Diactoros\ServerRequestFactory; -use Laminas\HttpHandlerRunner\Emitter\SapiEmitter; +use Rareloop\Lumberjack\Http\ResponseEmitter; use Rareloop\Lumberjack\Exceptions\HandlerInterface; use Symfony\Component\Debug\Exception\FatalErrorException; use Symfony\Component\ErrorHandler\Error\FatalError; @@ -25,12 +24,10 @@ class RegisterExceptionHandler { - private $app; + public function __construct(private Application $app, private ResponseEmitter $emitter) {} public function bootstrap(Application $app) { - $this->app = $app; - if (is_admin()) { return; } @@ -72,7 +69,7 @@ public function handleException($e) public function send(ResponseInterface $response) { - @(new SapiEmitter())->emit($response); + $this->emitter->emit($response); } protected function getExceptionHandler(): HandlerInterface diff --git a/src/Http/ResponseEmitter.php b/src/Http/ResponseEmitter.php new file mode 100644 index 0000000..53d443e --- /dev/null +++ b/src/Http/ResponseEmitter.php @@ -0,0 +1,28 @@ +getBody(); + return; + } + + (new SapiEmitter())->emit($response); + } +} diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 7175ed5..65b55da 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -8,9 +8,10 @@ use Rareloop\Lumberjack\Application; use Rareloop\Lumberjack\Providers\ServiceProvider; use Rareloop\Lumberjack\Test\Unit\BrainMonkeyPHPUnitIntegration; -use phpmock\Mock; -use phpmock\MockBuilder; use Brain\Monkey; +use Laminas\Diactoros\Response\TextResponse; +use Psr\Http\Message\ResponseInterface; +use Rareloop\Lumberjack\Http\ResponseEmitter; class ApplicationTest extends TestCase { @@ -548,6 +549,28 @@ public function calling_detectWhenRequestHasNotBeenHandled_adds_actions() $this->assertTrue(has_action('wp_footer')); $this->assertTrue(has_action('shutdown')); } + + /** + * @test + */ + public function shutdown_should_use_emitter_to_output_response() + { + $wp = Mockery::mock('WP'); + $wp->shouldReceive('send_headers')->once(); + $GLOBALS['wp'] = $wp; + + $response = new TextResponse('Hello World'); + + $emitter = Mockery::mock(ResponseEmitter::class); + $emitter->shouldReceive('emit')->once()->with(Mockery::type(\Psr\Http\Message\ResponseInterface::class)); + + $app = Mockery::mock(Application::class . '[terminate]'); + $app->shouldAllowMockingProtectedMethods(); + $app->shouldReceive('terminate')->once(); + $app->bind(ResponseEmitter::class, $emitter); + + $app->shutdown($response); + } } class BootstrapperBootstrapTester diff --git a/tests/Unit/Bootstrappers/RegisterExceptionHandlerTest.php b/tests/Unit/Bootstrappers/RegisterExceptionHandlerTest.php index ff38c42..3e5ed8c 100644 --- a/tests/Unit/Bootstrappers/RegisterExceptionHandlerTest.php +++ b/tests/Unit/Bootstrappers/RegisterExceptionHandlerTest.php @@ -10,6 +10,7 @@ use Rareloop\Lumberjack\Application; use Rareloop\Lumberjack\Bootstrappers\RegisterExceptionHandler; use Rareloop\Lumberjack\Config; +use Rareloop\Lumberjack\Http\ResponseEmitter; use Rareloop\Lumberjack\Exceptions\Handler; use Rareloop\Lumberjack\Exceptions\HandlerInterface; use Rareloop\Lumberjack\Test\Unit\BrainMonkeyPHPUnitIntegration; @@ -37,7 +38,7 @@ public function errors_are_converted_to_exceptions() $app->bind('config', $config); $app->bind(Config::class, $config); - $bootstrapper = new RegisterExceptionHandler(); + $bootstrapper = new RegisterExceptionHandler($app, new ResponseEmitter()); $bootstrapper->bootstrap($app); $bootstrapper->handleError(E_USER_ERROR, 'Test Error'); } @@ -60,7 +61,7 @@ public function E_USER_NOTICE_errors_are_not_converted_to_exceptions() return $e->getSeverity() === E_USER_NOTICE && $e->getMessage() === 'Test Error'; })); - $bootstrapper = new RegisterExceptionHandler(); + $bootstrapper = new RegisterExceptionHandler($app, new ResponseEmitter()); $bootstrapper->bootstrap($app); $bootstrapper->handleError(E_USER_NOTICE, 'Test Error'); } @@ -83,7 +84,7 @@ public function E_USER_DEPRECATED_errors_are_not_converted_to_exceptions() return $e->getSeverity() === E_USER_DEPRECATED && $e->getMessage() === 'Test Error'; })); - $bootstrapper = new RegisterExceptionHandler(); + $bootstrapper = new RegisterExceptionHandler($app, new ResponseEmitter()); $bootstrapper->bootstrap($app); $bootstrapper->handleError(E_USER_DEPRECATED, 'Test Error'); } @@ -106,7 +107,7 @@ public function E_DEPRECATED_errors_are_not_converted_to_exceptions() return $e->getSeverity() === E_DEPRECATED && $e->getMessage() === 'Test Error'; })); - $bootstrapper = new RegisterExceptionHandler(); + $bootstrapper = new RegisterExceptionHandler($app, new ResponseEmitter()); $bootstrapper->bootstrap($app); $bootstrapper->handleError(E_DEPRECATED, 'Test Error'); } @@ -131,7 +132,7 @@ public function custom_error_level_can_be_set_for_report_only() return $e->getSeverity() === E_USER_ERROR && $e->getMessage() === 'Test Error'; })); - $bootstrapper = new RegisterExceptionHandler(); + $bootstrapper = new RegisterExceptionHandler($app, new ResponseEmitter()); $bootstrapper->bootstrap($app); $bootstrapper->handleError(E_USER_ERROR, 'Test Error'); $bootstrapper->handleError(E_USER_DEPRECATED, 'Test Error'); @@ -153,7 +154,7 @@ public function handle_exception_should_call_handlers_report_and_render_methods( $handler->shouldReceive('render')->with($request, $exception)->once()->andReturn(new Response()); $app->bind(HandlerInterface::class, $handler); - $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]'); + $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]', [$app, new ResponseEmitter()]); $bootstrapper->shouldReceive('send')->once(); $bootstrapper->bootstrap($app); @@ -176,7 +177,7 @@ public function handle_exception_should_call_handlers_report_and_render_methods_ $handler->shouldReceive('render')->with($request, Mockery::type(\ErrorException::class))->once()->andReturn(new Response()); $app->bind(HandlerInterface::class, $handler); - $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]'); + $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]', [$app, new ResponseEmitter()]); $bootstrapper->shouldReceive('send')->once(); $bootstrapper->bootstrap($app); @@ -197,7 +198,7 @@ public function handle_exception_should_call_handlers_report_and_render_methods_ $handler->shouldReceive('render')->with(Mockery::type(ServerRequest::class), $exception)->once()->andReturn(new Response()); $app->bind(HandlerInterface::class, $handler); - $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]'); + $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]', [$app, new ResponseEmitter()]); $bootstrapper->shouldReceive('send')->once(); $bootstrapper->bootstrap($app); @@ -222,12 +223,25 @@ public function handle_exception_should_not_call_render_methods_when_exception_i $handler->shouldNotReceive('render'); $app->bind(HandlerInterface::class, $handler); - $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]'); + $bootstrapper = Mockery::mock(RegisterExceptionHandler::class . '[send]', [$app, new ResponseEmitter()]); $bootstrapper->shouldReceive('send')->once(); $bootstrapper->bootstrap($app); $bootstrapper->handleException($exception); } + + /** @test */ + public function send_should_use_the_emitter() + { + $app = new Application; + $response = new TextResponse('Hello World'); + $emitter = Mockery::mock(ResponseEmitter::class); + $emitter->shouldReceive('emit')->once()->with($response); + + $bootstrapper = new RegisterExceptionHandler($app, $emitter); + + $bootstrapper->send($response); + } } class ResponsableException extends \Exception implements Responsable diff --git a/tests/Unit/Http/ResponseEmitterTest.php b/tests/Unit/Http/ResponseEmitterTest.php new file mode 100644 index 0000000..c8d0919 --- /dev/null +++ b/tests/Unit/Http/ResponseEmitterTest.php @@ -0,0 +1,86 @@ +setNamespace('Rareloop\Lumberjack\Http') + ->setName('headers_sent') + ->setFunction(function () { + return true; + }); + $mock = $builder->build(); + $mock->enable(); + + $response = new TextResponse('Hello World'); + $emitter = new ResponseEmitter(); + + ob_start(); + $emitter->emit($response); + $output = ob_get_clean(); + + $this->assertSame('Hello World', $output); + + $mock->disable(); + } + + /** @test */ + public function emit_should_use_sapi_emitter_when_headers_not_sent() + { + $builder = new MockBuilder(); + $builder->setNamespace('Rareloop\Lumberjack\Http') + ->setName('headers_sent') + ->setFunction(function () { + return false; + }); + $mock = $builder->build(); + $mock->enable(); + + // SapiEmitter uses the global header() function. + // We mock it in the SapiEmitter's namespace to prevent it from actually sending headers + $headerBuilder = new MockBuilder(); + $headerBuilder->setNamespace('Laminas\HttpHandlerRunner\Emitter') + ->setName('header') + ->setFunction(function () { + }); + $headerMock = $headerBuilder->build(); + $headerMock->enable(); + + // SapiEmitterTrait checks for namespaced headers_sent + $emitterHeadersSentBuilder = new MockBuilder(); + $emitterHeadersSentBuilder->setNamespace('Laminas\HttpHandlerRunner\Emitter') + ->setName('headers_sent') + ->setFunction(function () { + return false; + }); + $emitterHeadersSentMock = $emitterHeadersSentBuilder->build(); + $emitterHeadersSentMock->enable(); + + $response = new TextResponse('Hello World'); + $emitter = new ResponseEmitter(); + + ob_start(); + $emitter->emit($response); + $output = ob_get_clean(); + + // SapiEmitter echoes the body, so we should see 'Hello World' + $this->assertSame('Hello World', $output); + + $mock->disable(); + $headerMock->disable(); + $emitterHeadersSentMock->disable(); + } +} From 623c48fea5462240cc0d123f4e3474f2df76cab1 Mon Sep 17 00:00:00 2001 From: Tom Mitchelmore Date: Tue, 24 Mar 2026 13:53:13 +0000 Subject: [PATCH 2/4] Restore missing phpmock imports and simplify shutdown test by mocking ResponseEmitter --- tests/Unit/ApplicationTest.php | 60 ++++++++++------------------------ 1 file changed, 17 insertions(+), 43 deletions(-) diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 65b55da..c15c04d 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -7,10 +7,10 @@ use PHPUnit\Framework\TestCase; use Rareloop\Lumberjack\Application; use Rareloop\Lumberjack\Providers\ServiceProvider; -use Rareloop\Lumberjack\Test\Unit\BrainMonkeyPHPUnitIntegration; +use phpmock\Mock; +use phpmock\MockBuilder; use Brain\Monkey; use Laminas\Diactoros\Response\TextResponse; -use Psr\Http\Message\ResponseInterface; use Rareloop\Lumberjack\Http\ResponseEmitter; class ApplicationTest extends TestCase @@ -550,7 +550,7 @@ public function calling_detectWhenRequestHasNotBeenHandled_adds_actions() $this->assertTrue(has_action('shutdown')); } - /** + /** * @test */ public function shutdown_should_use_emitter_to_output_response() @@ -560,7 +560,7 @@ public function shutdown_should_use_emitter_to_output_response() $GLOBALS['wp'] = $wp; $response = new TextResponse('Hello World'); - + $emitter = Mockery::mock(ResponseEmitter::class); $emitter->shouldReceive('emit')->once()->with(Mockery::type(\Psr\Http\Message\ResponseInterface::class)); @@ -575,16 +575,12 @@ public function shutdown_should_use_emitter_to_output_response() class BootstrapperBootstrapTester { - public function __construct(public $callback) - { - } + public function __construct(public $callback) {} } abstract class TestBootstrapperBase { - public function __construct(private BootstrapperBootstrapTester $tester) - { - } + public function __construct(private BootstrapperBootstrapTester $tester) {} public function bootstrap(Application $app) { @@ -592,58 +588,36 @@ public function bootstrap(Application $app) } } -class TestBootstrapper1 extends TestBootstrapperBase -{ -} +class TestBootstrapper1 extends TestBootstrapperBase {} -class TestBootstrapper2 extends TestBootstrapperBase -{ -} +class TestBootstrapper2 extends TestBootstrapperBase {} -interface TestInterface -{ -} +interface TestInterface {} -class TestInterfaceImplementation implements TestInterface -{ -} +class TestInterfaceImplementation implements TestInterface {} class TestInterfaceImplementationWithConstructorParams implements TestInterface { - public function __construct(TestServiceProvider $provider) - { - } + public function __construct(TestServiceProvider $provider) {} } -interface TestSubInterface -{ -} +interface TestSubInterface {} -class TestSubInterfaceImplementation implements TestSubInterface -{ -} +class TestSubInterfaceImplementation implements TestSubInterface {} class TestServiceProvider extends ServiceProvider { - public function register() - { - } - public function boot() - { - } + public function register() {} + public function boot() {} } -class EmptyServiceProvider extends ServiceProvider -{ -} +class EmptyServiceProvider extends ServiceProvider {} class TestBootServiceProvider extends ServiceProvider { private $bootCallback; - public function register() - { - } + public function register() {} public function boot(Application $app, TestInterface $test) { From 025dc4d0e54bdc778b1b4f521ae430fdb835ce83 Mon Sep 17 00:00:00 2001 From: Tom Mitchelmore Date: Tue, 24 Mar 2026 13:55:22 +0000 Subject: [PATCH 3/4] Resolve phpcs --- src/Bootstrappers/RegisterExceptionHandler.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Bootstrappers/RegisterExceptionHandler.php b/src/Bootstrappers/RegisterExceptionHandler.php index 8c9d364..7dc8131 100644 --- a/src/Bootstrappers/RegisterExceptionHandler.php +++ b/src/Bootstrappers/RegisterExceptionHandler.php @@ -24,7 +24,10 @@ class RegisterExceptionHandler { - public function __construct(private Application $app, private ResponseEmitter $emitter) {} + public function __construct(private Application $app, private ResponseEmitter $emitter) + { + // + } public function bootstrap(Application $app) { From a1b3553943d3b7fae3de19da1e1e74e2ec806e3f Mon Sep 17 00:00:00 2001 From: Tom Mitchelmore Date: Tue, 24 Mar 2026 14:22:42 +0000 Subject: [PATCH 4/4] Make header modification instances resilient to early headers --- src/Application.php | 3 +++ src/Providers/SessionServiceProvider.php | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Application.php b/src/Application.php index 6af29c6..0f41a49 100644 --- a/src/Application.php +++ b/src/Application.php @@ -304,6 +304,9 @@ protected function terminate() protected function removeSentHeadersAndMoveIntoResponse(ResponseInterface $response): ResponseInterface { + if (headers_sent()) { + return $response; + } // 1. Format the previously sent headers into an array of [key, value] // 2. Remove all headers from the output that we find // 3. Filter out any headers that would clash with those already in the response diff --git a/src/Providers/SessionServiceProvider.php b/src/Providers/SessionServiceProvider.php index 2bedef1..99b1285 100644 --- a/src/Providers/SessionServiceProvider.php +++ b/src/Providers/SessionServiceProvider.php @@ -32,7 +32,7 @@ public function boot() $cookieSet = false; add_action('send_headers', function () use (&$cookieSet) { - if (!$cookieSet) { + if (!$cookieSet && !headers_sent()) { $cookieOptions = [ 'lifetime' => Config::get('session.lifetime', 120), 'path' => Config::get('session.path', '/'),