From 96a0d4f303347a7f1c881fe7d09fb8a2478e61f2 Mon Sep 17 00:00:00 2001 From: Andrey Pyzhikov <5071@mail.ru> Date: Mon, 15 May 2023 14:58:30 +0800 Subject: [PATCH 1/4] fix: output buffering --- system/CodeIgniter.php | 56 ++++++++++++--------- system/Debug/BaseExceptionHandler.php | 4 -- system/Debug/Exceptions.php | 4 -- tests/system/CodeIgniterTest.php | 18 ++++--- user_guide_src/source/changelogs/v4.4.0.rst | 2 + 5 files changed, 45 insertions(+), 39 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 5e6c932aa2ea..54d89cce6eaf 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -36,6 +36,7 @@ use Kint\Renderer\RichRenderer; use Locale; use LogicException; +use Throwable; /** * This class is the core of the framework, and will analyse the @@ -164,6 +165,11 @@ class CodeIgniter */ protected bool $returnResponse = false; + /** + * Application output buffering level + */ + protected int $bufferLevel; + /** * Constructor. */ @@ -367,6 +373,7 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon try { return $this->handleRequest($routes, $cacheConfig, $returnResponse); } catch (RedirectException $e) { + $this->outputBufferingEnd(); $logger = Services::logger(); $logger->info('REDIRECTED ROUTE at ' . $e->getMessage()); @@ -389,6 +396,10 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon if ($return instanceof ResponseInterface) { return $return; } + } catch (Throwable $e) { + $this->outputBufferingEnd(); + + throw $e; } } @@ -810,7 +821,7 @@ protected function tryToRouteIt(?RouteCollectionInterface $routes = null) $this->benchmark->stop('bootstrap'); $this->benchmark->start('routing'); - ob_start(); + $this->outputBufferingStart(); $this->controller = $this->router->handle($path); $this->method = $this->router->methodName(); @@ -979,15 +990,8 @@ protected function display404errors(PageNotFoundException $e) // Display 404 Errors $this->response->setStatusCode($e->getCode()); - if (ENVIRONMENT !== 'testing') { - if (ob_get_level() > 0) { - ob_end_flush(); // @codeCoverageIgnore - } - } - // When testing, one is for phpunit, another is for test case. - elseif (ob_get_level() > 2) { - ob_end_flush(); // @codeCoverageIgnore - } + echo $this->outputBufferingEnd(); + flush(); // Throws new PageNotFoundException and remove exception message on production. throw PageNotFoundException::forPageNotFound( @@ -1006,21 +1010,9 @@ protected function display404errors(PageNotFoundException $e) */ protected function gatherOutput(?Cache $cacheConfig = null, $returned = null) { - $this->output = ob_get_contents(); - // If buffering is not null. - // Clean (erase) the output buffer and turn off output buffering - if (ob_get_length()) { - ob_end_clean(); - } + $this->output = $this->outputBufferingEnd(); if ($returned instanceof DownloadResponse) { - // Turn off output buffering completely, even if php.ini output_buffering is not off - if (ENVIRONMENT !== 'testing') { - while (ob_get_level() > 0) { - ob_end_clean(); - } - } - $this->response = $returned; return; @@ -1150,4 +1142,22 @@ public function setContext(string $context) return $this; } + + protected function outputBufferingStart(): void + { + $this->bufferLevel = ob_get_level(); + ob_start(); + } + + protected function outputBufferingEnd(): string + { + $buffer = ''; + + while (ob_get_level() > $this->bufferLevel) { + $buffer = ob_get_contents(); + ob_end_clean(); + } + + return $buffer; + } } diff --git a/system/Debug/BaseExceptionHandler.php b/system/Debug/BaseExceptionHandler.php index a4a4c72946b1..576149b8bf68 100644 --- a/system/Debug/BaseExceptionHandler.php +++ b/system/Debug/BaseExceptionHandler.php @@ -215,10 +215,6 @@ protected function render(Throwable $exception, int $statusCode, $viewFile = nul exit(1); } - if (ob_get_level() > $this->obLevel + 1) { - ob_end_clean(); - } - echo(function () use ($exception, $statusCode, $viewFile): string { $vars = $this->collectVars($exception, $statusCode); extract($vars, EXTR_SKIP); diff --git a/system/Debug/Exceptions.php b/system/Debug/Exceptions.php index 9d2ada166aee..8979febf4d46 100644 --- a/system/Debug/Exceptions.php +++ b/system/Debug/Exceptions.php @@ -296,10 +296,6 @@ protected function render(Throwable $exception, int $statusCode) exit(1); } - if (ob_get_level() > $this->ob_level + 1) { - ob_end_clean(); - } - echo(function () use ($exception, $statusCode, $viewFile): string { $vars = $this->collectVars($exception, $statusCode); extract($vars, EXTR_SKIP); diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 2871b3ffdb52..cef482bb4e34 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -54,10 +54,6 @@ protected function tearDown(): void { parent::tearDown(); - if (ob_get_level() > 1) { - ob_end_clean(); - } - $this->resetServices(); } @@ -73,6 +69,16 @@ public function testRunEmptyDefaultRoute() $this->assertStringContainsString('Welcome to CodeIgniter', $output); } + public function testOutputBufferingControl() + { + ob_start(); + $this->codeigniter->run(); + ob_get_clean(); + + // 1 phpunit output buffering level + $this->assertSame(1, ob_get_level()); + } + public function testRunEmptyDefaultRouteReturnResponse() { $_SERVER['argv'] = ['index.php']; @@ -828,10 +834,6 @@ public function testPageCacheWithCacheQueryString( $output = ob_get_clean(); $this->assertSame($string, $output); - - if (ob_get_level() > 1) { - ob_end_clean(); - } } // Calculate how much cached items exist in the cache after the test requests diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index 652f4f03c579..ebfa65ede545 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -142,6 +142,8 @@ Deprecations Bugs Fixed ********** +- **Output Buffering:** Bug fix with output buffering. + See the repo's `CHANGELOG.md `_ for a complete list of bugs fixed. From 2033cea9b21087e4e308e7c00c2a542682ae0201 Mon Sep 17 00:00:00 2001 From: Andrey Pyzhikov <5071@mail.ru> Date: Mon, 15 May 2023 19:33:45 +0800 Subject: [PATCH 2/4] testing and bug fixes. --- system/CodeIgniter.php | 2 +- system/Test/FeatureTestTrait.php | 22 ---------------------- tests/system/Test/FeatureTestTraitTest.php | 18 ++++++++++++++++++ 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 54d89cce6eaf..6f01b96518e6 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -1154,7 +1154,7 @@ protected function outputBufferingEnd(): string $buffer = ''; while (ob_get_level() > $this->bufferLevel) { - $buffer = ob_get_contents(); + $buffer .= ob_get_contents(); ob_end_clean(); } diff --git a/system/Test/FeatureTestTrait.php b/system/Test/FeatureTestTrait.php index e6e8492ea466..fd12efd4962b 100644 --- a/system/Test/FeatureTestTrait.php +++ b/system/Test/FeatureTestTrait.php @@ -144,14 +144,6 @@ public function skipEvents() */ public function call(string $method, string $path, ?array $params = null) { - $buffer = \ob_get_level(); - - // Clean up any open output buffers - // not relevant to unit testing - if (\ob_get_level() > 0 && (! isset($this->clean) || $this->clean === true)) { - \ob_end_clean(); // @codeCoverageIgnore - } - // Simulate having a blank session $_SESSION = []; $_SERVER['REQUEST_METHOD'] = $method; @@ -180,23 +172,9 @@ public function call(string $method, string $path, ?array $params = null) ->setRequest($request) ->run($routes, true); - $output = \ob_get_contents(); - if (empty($response->getBody()) && ! empty($output)) { - $response->setBody($output); - } - // Reset directory if it has been set Services::router()->setDirectory(null); - // Ensure the output buffer is identical so no tests are risky - while (\ob_get_level() > $buffer) { - \ob_end_clean(); // @codeCoverageIgnore - } - - while (\ob_get_level() < $buffer) { - \ob_start(); // @codeCoverageIgnore - } - return new TestResponse($response); } diff --git a/tests/system/Test/FeatureTestTraitTest.php b/tests/system/Test/FeatureTestTraitTest.php index 4636d58d3927..e6c8aef13e2c 100644 --- a/tests/system/Test/FeatureTestTraitTest.php +++ b/tests/system/Test/FeatureTestTraitTest.php @@ -74,6 +74,24 @@ public function testCallSimpleGet() $this->assertSame(200, $response->response()->getStatusCode()); } + public function testClosureWithEcho() + { + $this->withRoutes([ + [ + 'get', + 'home', + static function () { echo 'test echo'; }, + ], + ]); + + $response = $this->get('home'); + $this->assertInstanceOf(TestResponse::class, $response); + $this->assertInstanceOf(Response::class, $response->response()); + $this->assertTrue($response->isOK()); + $this->assertSame('test echo', $response->response()->getBody()); + $this->assertSame(200, $response->response()->getStatusCode()); + } + public function testCallPost() { $this->withRoutes([ From f6d971024f6e96e50c25e800898a4a9254347998 Mon Sep 17 00:00:00 2001 From: Andrey Pyzhikov <5071@mail.ru> Date: Mon, 15 May 2023 20:02:27 +0800 Subject: [PATCH 3/4] trigger GitHub actions From ff38f5c581a3de79f286e7f74bfb8d5816b44ae5 Mon Sep 17 00:00:00 2001 From: Andrey Pyzhikov <5071@mail.ru> Date: Mon, 22 May 2023 11:28:42 +0800 Subject: [PATCH 4/4] Disabling output buffering in the DownloadResponse::send method --- system/HTTP/DownloadResponse.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/system/HTTP/DownloadResponse.php b/system/HTTP/DownloadResponse.php index 19e3c657795e..e83f9453455d 100644 --- a/system/HTTP/DownloadResponse.php +++ b/system/HTTP/DownloadResponse.php @@ -252,6 +252,13 @@ public function setCache(array $options = []) */ public function send() { + // Turn off output buffering completely, even if php.ini output_buffering is not off + if (ENVIRONMENT !== 'testing') { + while (ob_get_level() > 0) { + ob_end_clean(); + } + } + $this->buildHeaders(); $this->sendHeaders(); $this->sendBody();