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

fix: exceptionHandler may return invalid HTTP status code #6228

Merged
merged 10 commits into from
Aug 6, 2022
13 changes: 6 additions & 7 deletions system/Database/Exceptions/DatabaseException.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@

namespace CodeIgniter\Database\Exceptions;

use CodeIgniter\Exceptions\HasExitCodeInterface;
use Error;

class DatabaseException extends Error implements ExceptionInterface
class DatabaseException extends Error implements ExceptionInterface, HasExitCodeInterface
{
/**
* Exit status code
*
* @var int
*/
protected $code = EXIT_DATABASE;
public function getExitCode(): int
{
return EXIT_DATABASE;
}
}
19 changes: 9 additions & 10 deletions system/Debug/Exceptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
namespace CodeIgniter\Debug;

use CodeIgniter\API\ResponseTrait;
use CodeIgniter\Exceptions\HasExitCodeInterface;
use CodeIgniter\Exceptions\HTTPExceptionInterface;
use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\HTTP\CLIRequest;
use CodeIgniter\HTTP\Exceptions\HTTPException;
Expand Down Expand Up @@ -312,18 +314,15 @@ protected function maskSensitiveData(&$trace, array $keysToMask, string $path =
*/
protected function determineCodes(Throwable $exception): array
{
$statusCode = abs($exception->getCode());
$statusCode = 500;
$exitStatus = EXIT_ERROR;

if ($statusCode < 100 || $statusCode > 599) {
$exitStatus = $statusCode + EXIT__AUTO_MIN;

if ($exitStatus > EXIT__AUTO_MAX) {
$exitStatus = EXIT_ERROR;
}
if ($exception instanceof HTTPExceptionInterface) {
$statusCode = $exception->getCode();
}

$statusCode = 500;
} else {
$exitStatus = EXIT_ERROR;
if ($exception instanceof HasExitCodeInterface) {
$exitStatus = $exception->getExitCode();
}

return [$statusCode, $exitStatus];
Expand Down
12 changes: 7 additions & 5 deletions system/Entity/Exceptions/CastException.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,18 @@
namespace CodeIgniter\Entity\Exceptions;

use CodeIgniter\Exceptions\FrameworkException;
use CodeIgniter\Exceptions\HasExitCodeInterface;

/**
* CastException is thrown for invalid cast initialization and management.
*
* @TODO CodeIgniter\Exceptions\CastException is deprecated and this class is used.
* CodeIgniter\Exceptions\CastException has the property $code = EXIT_CONFIG,
* but this class does not have the code.
*/
class CastException extends FrameworkException
class CastException extends FrameworkException implements HasExitCodeInterface
{
public function getExitCode(): int
{
return EXIT_CONFIG;
}

/**
* Thrown when the cast class does not extends BaseCast.
*
Expand Down
12 changes: 5 additions & 7 deletions system/Exceptions/CastException.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,14 @@
*
* @codeCoverageIgnore
*/
class CastException extends CriticalError
class CastException extends CriticalError implements HasExitCodeInterface
{
use DebugTraceableTrait;

/**
* Exit status code
*
* @var int
*/
protected $code = EXIT_CONFIG;
public function getExitCode(): int
{
return EXIT_CONFIG;
}

public static function forInvalidJsonFormatException(int $error)
{
Expand Down
12 changes: 5 additions & 7 deletions system/Exceptions/ConfigException.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,14 @@
/**
* Exception for automatic logging.
*/
class ConfigException extends CriticalError
class ConfigException extends CriticalError implements HasExitCodeInterface
{
use DebugTraceableTrait;

/**
* Exit status code
*
* @var int
*/
protected $code = EXIT_CONFIG;
public function getExitCode(): int
{
return EXIT_CONFIG;
}

public static function forDisabledMigrations()
{
Expand Down
19 changes: 19 additions & 0 deletions system/Exceptions/HTTPExceptionInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\Exceptions;

/**
* Interface for Exceptions that has exception code as HTTP status code.
*/
interface HTTPExceptionInterface
{
}
23 changes: 23 additions & 0 deletions system/Exceptions/HasExitCodeInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\Exceptions;

/**
* Interface for Exceptions that has exception code as exit code.
*/
interface HasExitCodeInterface
{
/**
* Returns exit status code.
*/
public function getExitCode(): int;
}
2 changes: 1 addition & 1 deletion system/Exceptions/PageNotFoundException.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Config\Services;
use OutOfBoundsException;

class PageNotFoundException extends OutOfBoundsException implements ExceptionInterface
class PageNotFoundException extends OutOfBoundsException implements ExceptionInterface, HTTPExceptionInterface
{
use DebugTraceableTrait;

Expand Down
3 changes: 2 additions & 1 deletion system/Router/Exceptions/RedirectException.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@

namespace CodeIgniter\Router\Exceptions;

use CodeIgniter\Exceptions\HTTPExceptionInterface;
use Exception;

/**
* RedirectException
*/
class RedirectException extends Exception
class RedirectException extends Exception implements HTTPExceptionInterface
{
/**
* HTTP status code for redirects
Expand Down
15 changes: 6 additions & 9 deletions tests/system/Debug/ExceptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,13 @@ public function testDetermineCodes(): void
{
$determineCodes = $this->getPrivateMethodInvoker($this->exception, 'determineCodes');

$this->assertSame([500, EXIT__AUTO_MIN], $determineCodes(new RuntimeException('This.')));
$this->assertSame([500, EXIT_ERROR], $determineCodes(new RuntimeException('This.')));
$this->assertSame([500, EXIT_ERROR], $determineCodes(new RuntimeException('That.', 600)));
$this->assertSame([404, EXIT_ERROR], $determineCodes(new RuntimeException('There.', 404)));
$this->assertSame([167, EXIT_ERROR], $determineCodes(new RuntimeException('This.', 167)));
// @TODO This exit code should be EXIT_CONFIG.
$this->assertSame([500, 12], $determineCodes(new ConfigException('This.')));
// @TODO This exit code should be EXIT_CONFIG.
$this->assertSame([500, 9], $determineCodes(new CastException('This.')));
// @TODO This exit code should be EXIT_DATABASE.
$this->assertSame([500, 17], $determineCodes(new DatabaseException('This.')));
$this->assertSame([500, EXIT_ERROR], $determineCodes(new RuntimeException('There.', 404)));
$this->assertSame([500, EXIT_ERROR], $determineCodes(new RuntimeException('This.', 167)));
$this->assertSame([500, EXIT_CONFIG], $determineCodes(new ConfigException('This.')));
$this->assertSame([500, EXIT_CONFIG], $determineCodes(new CastException('This.')));
$this->assertSame([500, EXIT_DATABASE], $determineCodes(new DatabaseException('This.')));
}

public function testRenderBacktrace(): void
Expand Down
14 changes: 14 additions & 0 deletions user_guide_src/source/changelogs/v4.3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ Exceptions when Database Errors Occur
- The exceptions thrown by the database connection classes have been changed to ``CodeIgniter\Database\Exceptions\DatabaseException``. Previously, different database drivers threw different exception classes, but these have been unified into ``DatabaseException``.
- The exceptions thrown by the ``execute()`` method of Prepared Queries have been changed to ``DatabaseException``. Previously, different database drivers might throw different exception classes or did not throw exceptions, but these have been unified into ``DatabaseException``.

HTTP Status Code and Exit Code when Exception Occurs
----------------------------------------------------

Previously, CodeIgniter's Exception Handler used the *Exception code* as the *HTTP status code* in some cases, and calculated the *Exit code* based on the Exception code. However there should be no logical connection with Exception code and HTTP Status Code or Exit code.

- Now the Exception Handler sets HTTP status code to ``500`` and set Exit code to the constant ``EXIT_ERROR`` (= ``1``) by default.
- You can change HTTP status code or Exit code to implement ``HTTPExceptionInterface`` or ``HasExitCodeInterface`` in your Exception class. See :ref:`error-specify-http-status-code` and :ref:`error-specify-exit-code`.

For example, the Exit code has been changed like the following:

- If an uncaught ``ConfigException`` occurs, the Exit code is ``EXIT_CONFIG`` (= ``3``) instead of ``12``.
- If an uncaught ``CastException`` occurs, the Exit code is ``EXIT_CONFIG`` (= ``3``) instead of ``9``.
- If an uncaught ``DatabaseException`` occurs, the Exit code is ``EXIT_DATABASE`` (= ``8``) instead of ``17``.

Others
------

Expand Down
30 changes: 25 additions & 5 deletions user_guide_src/source/general/errors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ To ignore logging on other status codes, you can set the status code to ignore i
.. note:: It is possible that logging still will not happen for exceptions if your current Log settings
are not set up to log **critical** errors, which all exceptions are logged as.

Custom Exceptions
=================
Framework Exceptions
====================

The following custom exceptions are available:
The following framework exceptions are available:

PageNotFoundException
---------------------
Expand All @@ -89,7 +89,7 @@ is not the right type, etc:

.. literalinclude:: errors/008.php

This provides an HTTP status code of 500 and an exit code of 3.
This provides an exit code of 3.

DatabaseException
-----------------
Expand All @@ -99,7 +99,7 @@ or when it is temporarily lost:

.. literalinclude:: errors/009.php

This provides an HTTP status code of 500 and an exit code of 8.
This provides an exit code of 8.

RedirectException
-----------------
Expand All @@ -113,3 +113,23 @@ forcing a redirect to a specific route or URL:
redirect code to use instead of the default (``302``, "temporary redirect"):

.. literalinclude:: errors/011.php

.. _error-specify-http-status-code:

Specify HTTP Status Code in Your Exception
==========================================

Since v4.3.0, you can specify the HTTP status code for your Exception class to implement
``HTTPExceptionInterface``.

When an exception implementing ``HTTPExceptionInterface`` is caught by CodeIgniter's exception handler, the Exception code will become the HTTP status code.

.. _error-specify-exit-code:

Specify Exit Code in Your Exception
===================================

Since v4.3.0, you can specify the exit code for your Exception class to implement
``HasExitCodeInterface``.

When an exception implementing ``HasExitCodeInterface`` is caught by CodeIgniter's exception handler, the code returned from the ``getExitCode()`` method will become the exit code.
11 changes: 11 additions & 0 deletions user_guide_src/source/installation/upgrade_430.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@ Add **types** to the properties in these Config classes. You may need to fix the
Breaking Changes
****************

HTTP Status Code and Exit Code of Uncaught Exceptions
=====================================================

- If you expect *Exception code* as *HTTP status code*, the HTTP status code will be changed.
In that case, you need to implement ``HTTPExceptionInterface`` in the Exception. See :ref:`error-specify-http-status-code`.
- If you expect *Exit code* based on *Exception code*, the Exit code will be changed.
In that case, you need to implement ``HasExitCodeInterface`` in the Exception. See :ref:`error-specify-exit-code`.

Others
======

- The exception classes may be changed when database errors occur. If you catch the exceptions, you must confirm that your code can catch the exceptions. See :ref:`exceptions-when-database-errors-occur` for details.

Breaking Enhancements
Expand Down