Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.4] Fix output buffering #7500

Merged
merged 4 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions system/CodeIgniter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -164,6 +165,11 @@ class CodeIgniter
*/
protected bool $returnResponse = false;

/**
* Application output buffering level
*/
protected int $bufferLevel;

/**
* Constructor.
*/
Expand Down Expand Up @@ -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());

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand All @@ -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();
}
}

kenjis marked this conversation as resolved.
Show resolved Hide resolved
$this->response = $returned;

return;
Expand Down Expand Up @@ -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;
}
}
4 changes: 0 additions & 4 deletions system/Debug/BaseExceptionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions system/Debug/Exceptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions system/HTTP/DownloadResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
22 changes: 0 additions & 22 deletions system/Test/FeatureTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
18 changes: 10 additions & 8 deletions tests/system/CodeIgniterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ protected function tearDown(): void
{
parent::tearDown();

if (ob_get_level() > 1) {
ob_end_clean();
}

$this->resetServices();
}

Expand All @@ -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'];
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions tests/system/Test/FeatureTestTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.4.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ Deprecations
Bugs Fixed
**********

- **Output Buffering:** Bug fix with output buffering.

See the repo's
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.