Merge pull request #6228 from kenjis/fix-exceptionHandler-statusCode

fix: exceptionHandler may return invalid HTTP status code

Author: kenjis

system/Database/Exceptions/DatabaseException.php

@@ -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;
+    }
 }

system/Debug/Exceptions.php

@@ -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;
@@ -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];

system/Entity/Exceptions/CastException.php

@@ -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.
      *

system/Exceptions/CastException.php

@@ -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)
     {

system/Exceptions/ConfigException.php

@@ -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()
     {

system/Exceptions/HTTPExceptionInterface.php

@@ -0,0 +1,19 @@
+<?php
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <[email protected]>
+ *
+ * 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
+{
+}

system/Exceptions/HasExitCodeInterface.php

@@ -0,0 +1,23 @@
+<?php
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <[email protected]>
+ *
+ * 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;
+}

system/Exceptions/PageNotFoundException.php

@@ -14,7 +14,7 @@
 use Config\Services;
 use OutOfBoundsException;
 
-class PageNotFoundException extends OutOfBoundsException implements ExceptionInterface
+class PageNotFoundException extends OutOfBoundsException implements ExceptionInterface, HTTPExceptionInterface
 {
     use DebugTraceableTrait;
 

system/Router/Exceptions/RedirectException.php

@@ -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

tests/system/Debug/ExceptionsTest.php

@@ -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

user_guide_src/source/changelogs/v4.3.0.rst

@@ -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
 ------
 

user_guide_src/source/general/errors.rst

@@ -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
 ---------------------
@@ -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
 -----------------
@@ -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
 -----------------
@@ -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.

user_guide_src/source/installation/upgrade_430.rst

@@ -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