Bug: Force File Download error in the latest version (4.2.3)

[vachzar]

PHP Version

7.4

CodeIgniter4 Version

4.2.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

No response

What happened?

when I executed return $this->response->download('some.txt', 'some text', true) it return an error below.

TypeError

Argument 1 passed to CodeIgniter\CodeIgniter::displayPerformanceMetrics() must be of the type string, null given, 
called in D:\laragon\www\iprisinsi\vendor\codeigniter4\framework\system\CodeIgniter.php on line 500 

SYSTEMPATH\CodeIgniter.php at line 740

733 
734         return md5($name);
735     }
736 
737     /**
738      * Replaces the elapsed_time tag.
739      */
740     public function displayPerformanceMetrics(string $output): string
741     {
742         return str_replace('{elapsed_time}', (string) $this->totalTime, $output);
743     }
744 
745     /**
746      * Try to Route It - As it sounds like, works with the router to
747      * match a route against the current URI. If the route is a

actually I found this when 4.2.2 released

Steps to Reproduce

just run this code on home Controller return $this->response->download('some.txt', 'some text', true)

Expected Output

file some.txt downloaded

Anything else?

since I m new to this I couldn't point what file to fix

[datamweb]

I don't see any problem with CI version 4.2.1. Exact PHP version please?

I edited the CI version.

[vachzar]

I don't see any problem with CI version 4.2.2. Exact PHP version please?

PHP 7.4.19

[datamweb]

I confirm, after updating to CI 4.2.3, I also have the error.

[paulbalandan]

Confirming this also in v4.2.3.

@kenjis in #6282 , I think this part in gatherOutput should be moved also along with the move in cache save.

if ($returned instanceof DownloadResponse) {
    // Turn off output buffering completely, even if php.ini output_buffering is not off
    while (ob_get_level() > 0) {
        ob_end_clean();
    }
    $this->response = $returned;
    return;
}

[vachzar]

this is the backtrace of error after the error above.

SYSTEMPATH\CodeIgniter.php : 500 — CodeIgniter\CodeIgniter->displayPerformanceMetrics ( [arguments]

493         // so that we can have live speed updates along the way.
494         // Must be run after filters to preserve the Response headers.
495         if (static::$cacheTTL > 0) {
496             $this->cachePage($cacheConfig);
497         }
498 
499         // Update the performance metrics
500         $output = $this->displayPerformanceMetrics($this->response->getBody());
501         $this->response->setBody($output);
502 
503         // Save our current URI as the previous URI in the session
504         // for safer, more accurate use with `previous_url()` helper function.
505         $this->storePreviousURL(current_url(true));
506 
507         unset($uri);

line 500 was not in previous version and the return of $this->response0>getBody() is string or null so I tried to type casting that to string $output = $this->displayPerformanceMetrics( (string) $this->response->getBody());

and now it runs smoothly, but I don't know wether it's the right way

[kenjis]

@paulbalandan To where?

[kenjis]

@vachzar Yes, workaroud:

--- a/system/CodeIgniter.php
+++ b/system/CodeIgniter.php
@@ -497,7 +497,7 @@ class CodeIgniter
         }
 
         // Update the performance metrics
-        $output = $this->displayPerformanceMetrics($this->response->getBody());
+        $output = $this->displayPerformanceMetrics((string) $this->response->getBody());
         $this->response->setBody($output);
 
         // Save our current URI as the previous URI in the session

[kenjis]

@paulbalandan How about this?

--- a/system/CodeIgniter.php
+++ b/system/CodeIgniter.php
@@ -485,24 +485,26 @@ class CodeIgniter
             }
         }
 
-        if ($response instanceof ResponseInterface) {
-            $this->response = $response;
-        }
+        if (($response instanceof DownloadResponse) === false) {
+            if ($response instanceof ResponseInterface) {
+                $this->response = $response;
+            }
 
-        // Cache it without the performance metrics replaced
-        // so that we can have live speed updates along the way.
-        // Must be run after filters to preserve the Response headers.
-        if (static::$cacheTTL > 0) {
-            $this->cachePage($cacheConfig);
-        }
+            // Cache it without the performance metrics replaced
+            // so that we can have live speed updates along the way.
+            // Must be run after filters to preserve the Response headers.
+            if (static::$cacheTTL > 0) {
+                $this->cachePage($cacheConfig);
+            }
 
-        // Update the performance metrics
-        $output = $this->displayPerformanceMetrics($this->response->getBody());
-        $this->response->setBody($output);
+            // Update the performance metrics
+            $output = $this->displayPerformanceMetrics($this->response->getBody());
+            $this->response->setBody($output);
 
-        // Save our current URI as the previous URI in the session
-        // for safer, more accurate use with `previous_url()` helper function.
-        $this->storePreviousURL(current_url(true));
+            // Save our current URI as the previous URI in the session
+            // for safer, more accurate use with `previous_url()` helper function.
+            $this->storePreviousURL(current_url(true));
+        }
 
         unset($uri);
 

[kenjis]

@vachzar I sent a PR #6361 Please try if you can.

[vachzar]

@kenjis have just checked and tested it on my local and runs well