bcit-ci / CodeIgniter

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
18.27k stars 7.6k forks source link

Fix PHP 8.1 E_DEPRECATED issue when the page has no output. #6217

Closed pocketarc closed 1 year ago

pocketarc commented 1 year ago

At the moment, if a page doesn't use CI for output (e.g. sending a file's contents), an E_DEPRECATED will be triggered because $output is null:

str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated [path]/system/core/Output.php 462

This is a quick fix for that.

Note: The code seems to override the original $output with $this->final_output, which initially is a string, but isn't when it actually gets to that point. I didn't investigate that further, as there was no need, but it was surprising.

// Set the output data
if ($output === NULL)
{
    $output =& $this->final_output;
}
daveherman71 commented 1 year ago

This patch has already been applied to the develop branch which is currently version 3.2.0-dev and includes many other PHP 8.1 patches.

pocketarc commented 1 year ago

@daveherman71 I am able to reproduce this in 3.2.0-dev, which is the version I'm running. I can provide a test example if necessary (an empty function in a controller -should- do it).

daveherman71 commented 1 year ago

If you have a look at the code in the repository you will see that the patch has already been applied.

https://github.com/bcit-ci/CodeIgniter/blob/63d037565dd782795021dcbd3b1ca6f8c8a509e4/system/core/Output.php#L436

        // Set the output data
        if ($output === NULL)
        {
            $output =& $this->final_output;
        }
pocketarc commented 1 year ago

I think there might be some confusion. The code I have in my comment isn't my patch. It's code that I know was already there, it's part of the discussion.

You can click the Files changed tab to see my patch.

daveherman71 commented 1 year ago

Please provide an example, I tried your suggestion and the output is an empty string (not NULL) and no error was thrown.

My code:

class Test extends CI_Controller {

    public function nothing()
    {
    }

}

The results of a cURL request are as follows:

C:\>curl -i http://localhost:8000/test/nothing
HTTP/1.1 200 OK
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache
Content-Type: text/html; charset=UTF-8
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Server: localhost
Set-Cookie: ci_session=s4djnf0a29p1pu93l9i49ijpvdfm07ch; path=/; secure; HttpOnly; SameSite=Lax
Date: Thu, 08 Jun 2023 12:01:25 GMT
Content-Length: 0
pocketarc commented 1 year ago

This is reproducible by calling $this->set_output(null), which is allowed by CI, and in my mind makes this an issue, but it can easily be resolved by telling the end user to just change their code, so I'm closing the PR.