Merge pull request #5703 from kenjis/fix-404Override

fix: 404 override controller does not output Response object body

Author: kenjis

system/CodeIgniter.php

@@ -777,6 +777,8 @@ public function setPath(string $path)
      * Now that everything has been setup, this method attempts to run the
      * controller method and make the script go. If it's not able to, will
      * show the appropriate Page Not Found error.
+     *
+     * @return ResponseInterface|string|void
      */
     protected function startController()
     {
@@ -804,7 +806,7 @@ protected function startController()
     /**
      * Instantiates the controller class.
      *
-     * @return mixed
+     * @return Controller
      */
     protected function createController()
     {
@@ -821,7 +823,7 @@ protected function createController()
      *
      * @param mixed $class
      *
-     * @return mixed
+     * @return false|ResponseInterface|string|void
      */
     protected function runController($class)
     {
@@ -847,6 +849,8 @@ protected function display404errors(PageNotFoundException $e)
     {
         // Is there a 404 Override available?
         if ($override = $this->router->get404Override()) {
+            $returned = null;
+
             if ($override instanceof Closure) {
                 echo $override($e->getMessage());
             } elseif (is_array($override)) {
@@ -857,13 +861,13 @@ protected function display404errors(PageNotFoundException $e)
                 $this->method     = $override[1];
 
                 $controller = $this->createController();
-                $this->runController($controller);
+                $returned   = $this->runController($controller);
             }
 
             unset($override);
 
             $cacheConfig = new Cache();
-            $this->gatherOutput($cacheConfig);
+            $this->gatherOutput($cacheConfig, $returned);
             $this->sendResponse();
 
             return;
@@ -891,7 +895,7 @@ protected function display404errors(PageNotFoundException $e)
      * Gathers the script output from the buffer, replaces some execution
      * time tag in the output and displays the debug toolbar, if required.
      *
-     * @param mixed|null $returned
+     * @param ResponseInterface|string|null $returned
      */
     protected function gatherOutput(?Cache $cacheConfig = null, $returned = null)
     {

tests/_support/Controllers/Popcorn.php

@@ -30,7 +30,7 @@ public function index()
 
     public function pop()
     {
-        $this->respond('Oops', 567, 'Surprise');
+        return $this->respond('Oops', 567, 'Surprise');
     }
 
     public function popper()
@@ -40,12 +40,12 @@ public function popper()
 
     public function weasel()
     {
-        $this->respond('', 200);
+        return $this->respond('', 200);
     }
 
     public function oops()
     {
-        $this->failUnauthorized();
+        return $this->failUnauthorized();
     }
 
     public function goaway()
@@ -72,12 +72,12 @@ public function cat()
 
     public function json()
     {
-        $this->respond(['answer' => 42]);
+        return $this->respond(['answer' => 42]);
     }
 
     public function xml()
     {
-        $this->respond('<my><pet>cat</pet></my>');
+        return $this->respond('<my><pet>cat</pet></my>');
     }
 
     public function toindex()

tests/system/CodeIgniterTest.php

@@ -94,15 +94,34 @@ public function testRun404Override()
         // Inject mock router.
         $routes = Services::routes();
         $routes->setAutoRoute(false);
-        $routes->set404Override('Home::index');
+        $routes->set404Override('Tests\Support\Controllers\Hello::index');
         $router = Services::router($routes, Services::request());
         Services::injectMock('router', $router);
 
         ob_start();
-        $this->codeigniter->useSafeOutput(true)->run();
+        $this->codeigniter->useSafeOutput(true)->run($routes);
         $output = ob_get_clean();
 
-        $this->assertStringContainsString('Welcome to CodeIgniter', $output);
+        $this->assertStringContainsString('Hello', $output);
+    }
+
+    public function testRun404OverrideControllerReturnsResponse()
+    {
+        $_SERVER['argv'] = ['index.php', '/'];
+        $_SERVER['argc'] = 2;
+
+        // Inject mock router.
+        $routes = Services::routes();
+        $routes->setAutoRoute(false);
+        $routes->set404Override('Tests\Support\Controllers\Popcorn::pop');
+        $router = Services::router($routes, Services::request());
+        Services::injectMock('router', $router);
+
+        ob_start();
+        $this->codeigniter->useSafeOutput(true)->run($routes);
+        $output = ob_get_clean();
+
+        $this->assertStringContainsString('Oops', $output);
     }
 
     public function testRun404OverrideByClosure()