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

Bug: CodeIgniter\Debug\Exceptions::exceptionHandler() sending header causes error if headers already sent #5679

Closed
sclubricants opened this issue Feb 11, 2022 · 7 comments · Fixed by #5680
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@sclubricants
Copy link
Member

sclubricants commented Feb 11, 2022

PHP Version

8.0

CodeIgniter4 Version

4.1.8

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?

If for some reason the application has already sent headers it causes another error in the exception handler and fails to properly handle this circumstance. This allows php to output error in production environment.

Steps to Reproduce

In a controller add the following lines:

echo 'Some output<br>';
flush();
setcookie("printer", 37); 
return;

Expected Output

It is expected to receive an error that you cannot modify headers because they are already sent.

However if in production or development environments the error is dumped by php as well as an additional similar error caused in the call to header() in CodeIgniter\Debug\Exceptions::exceptionHandler()

        if (! is_cli()) {
            $this->response->setStatusCode($statusCode);
            header(sprintf('HTTP/%s %s %s', $this->request->getProtocolVersion(), $this->response->getStatusCode(), $this->response->getReasonPhrase()), true, $statusCode);

            if (strpos($this->request->getHeaderLine('accept'), 'text/html') === false) {
                $this->respond(ENVIRONMENT === 'development' ? $this->collectVars($exception, $statusCode) : '', $statusCode)->send();

                exit($exitCode);
            }
        }

The screen as well as log files show:

CRITICAL - 2022-02-11 11:32:34 --> Cannot modify header information - headers already sent
#0 [internal function]: CodeIgniter\Debug\Exceptions->errorHandler(2, 'Cannot modify h...', 'C:\\xampp\\portal...', 16)
#1 C:\xampp\portal\app\Controllers\Admin\Test.php(16): setcookie('printer', '37')
#2 C:\xampp\portal\vendor\codeigniter4\framework\system\CodeIgniter.php(825): App\Controllers\Admin\Test->index()
#3 C:\xampp\portal\vendor\codeigniter4\framework\system\CodeIgniter.php(412): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\Admin\Test))
#4 C:\xampp\portal\vendor\codeigniter4\framework\system\CodeIgniter.php(320): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
#5 C:\xampp\portal\public\index.php(79): CodeIgniter\CodeIgniter->run()
#6 {main}
CRITICAL - 2022-02-11 11:32:34 --> Uncaught ErrorException: Cannot modify header information - headers already sent in C:\xampp\portal\vendor\codeigniter4\framework\system\Debug\Exceptions.php:115
Stack trace:
#0 [internal function]: CodeIgniter\Debug\Exceptions->errorHandler(2, 'Cannot modify h...', 'C:\\xampp\\portal...', 115)
#1 C:\xampp\portal\vendor\codeigniter4\framework\system\Debug\Exceptions.php(115): header('HTTP/1.1 500 In...', true, 500)
#2 [internal function]: CodeIgniter\Debug\Exceptions->exceptionHandler(Object(ErrorException))
#3 {main}
  thrown
#0 [internal function]: CodeIgniter\Debug\Exceptions->shutdownHandler()
#1 {main}

Anything else?

Its a messy situation I know.. Not sure the best way to handle it. If we suppress the second error by using @Header() at least the page will continue to render in development and in production will render error page instead of outputting errors.

        if (! is_cli()) {
            $this->response->setStatusCode($statusCode);
            @header(sprintf('HTTP/%s %s %s', $this->request->getProtocolVersion(), $this->response->getStatusCode(), $this->response->getReasonPhrase()), true, $statusCode);

            if (strpos($this->request->getHeaderLine('accept'), 'text/html') === false) {
                $this->respond(ENVIRONMENT === 'development' ? $this->collectVars($exception, $statusCode) : '', $statusCode)->send();

                exit($exitCode);
            }
        } 

Then our log outputs only the first error:

CRITICAL - 2022-02-11 11:52:34 --> Cannot modify header information - headers already sent
#0 [internal function]: CodeIgniter\Debug\Exceptions->errorHandler(2, 'Cannot modify h...', 'C:\\xampp\\portal...', 16)
#1 C:\xampp\portal\app\Controllers\Admin\Test.php(16): setcookie('printer', '37')
#2 C:\xampp\portal\vendor\codeigniter4\framework\system\CodeIgniter.php(825): App\Controllers\Admin\Test->index()
#3 C:\xampp\portal\vendor\codeigniter4\framework\system\CodeIgniter.php(412): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\Admin\Test))
#4 C:\xampp\portal\vendor\codeigniter4\framework\system\CodeIgniter.php(320): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
#5 C:\xampp\portal\public\index.php(79): CodeIgniter\CodeIgniter->run()
#6 {main}
@sclubricants sclubricants added the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 11, 2022
@kenjis kenjis removed the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 11, 2022
@kenjis
Copy link
Member

kenjis commented Feb 12, 2022

This allows php to output error in production environment.

I tried Steps to Reproduce, but I see only "Some output". No errors in the web page.

It is expected to receive an error that you cannot modify headers because they are already sent.

Why?
What's your exact problem?

@kenjis
Copy link
Member

kenjis commented Feb 12, 2022

By the way, I recommend not use echo or setcookie().
Personally, I think it is bad old coding style.

@sclubricants
Copy link
Member Author

sclubricants commented Feb 12, 2022

Only using echo and setcookie() to demonstrate.

I tried Steps to Reproduce, but I see only "Some output". No errors in the web page.

Ok, my bad, it doesn't put out the error, however it doesn't attempt to render the error page. Maybe this is ok.. but it would be nice to render the page if possible. You would have any output before the page render though. Same with the debug page.

Also, it still sets another error: Cannot modify header information - headers already sent in /system/Debug/Exceptions.php:xxx

Its clear that in the event that headers have been sent then the exception handler is going to cause an exception. I think it makes sense to suppress this in this particular case.

It is expected to receive an error that you cannot modify headers because they are already sent.

Why?
What's your exact problem?

If you have already begun output and headers have been sent then you can't set a cookie.

I'm not having any particular issue but in the course of testing some things I've discovered that the exception handler breaks if headers have been sent and an exception occurs.

With the @header():
image

@kenjis
Copy link
Member

kenjis commented Feb 12, 2022

Only using echo and setcookie() to demonstrate.

Your example is not realistic. I think you just wrote incorrect code.

If you have already begun output and headers have been sent then you can't set a cookie.

You should not output anything by yourself. The Response object does.
You just wrote incorrect code.

And I think if there are two errors, it should be shown or logged as it is.
What's wrong with the two errors?

CRITICAL - 2022-02-11 11:32:34 --> Cannot modify header information - headers already sent
CRITICAL - 2022-02-11 11:32:34 --> Uncaught ErrorException: Cannot modify header information - headers already sent in C:\xampp\portal\vendor\codeigniter4\framework\system\Debug\Exceptions.php:115
Stack trace:

You had the error "Cannot modify header information", and you did not catch the error,
so you had "Uncaught ErrorException".

@sclubricants
Copy link
Member Author

sclubricants commented Feb 12, 2022

the only unhandled error is the one from the call to header() when headers have been sent.

@paulbalandan
Copy link
Member

@sclubricants please mind your language. As with other OSS, please abide by the code of conduct of this repo.

@kenjis
Copy link
Member

kenjis commented Feb 12, 2022

the only unhandled error is the one from the call to header()

Okay, I got your point.
See #5680

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants